Skip to content

Commit

Permalink
Updates the PosInfoMoq2001 to check property in the SetupSet().
Browse files Browse the repository at this point in the history
  • Loading branch information
@GillesTourreau
GillesTourreau committed Oct 24, 2024
1 parent ded1fb1 commit b71c04f
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 31 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ All the rules of this category should not be disabled (or changed their severity
| Rule | Description |
| - | - |
| [PosInfoMoq2000: The `Returns()` or `ReturnsAsync()` methods must be call for Strict mocks](docs/Compilation/PosInfoMoq2000.md) | When a `Mock<T>` has been defined with the `Strict` behavior, the `Returns()` or `ReturnsAsync()` method must be called when setup a method to mock which returns a value. |
| [PosInfoMoq2001: The `Setup()` method must be used only on overridable members](docs/Compilation/PosInfoMoq2001.md)) | The `Setup()` method must be applied only for overridable members. |
| [PosInfoMoq2001: The `Setup()`/`SetupSet()` method must be used only on overridable members](docs/Compilation/PosInfoMoq2001.md)) | The `Setup()` method must be applied only for overridable members. |
| [PosInfoMoq2002: `Mock<T>` class can be used only to mock non-sealed class](docs/Compilation/PosInfoMoq2002.md) | The `Mock<T>` class can mock only interfaces or non-`sealed` classes. |
| [PosInfoMoq2003: The `Callback()` delegate expression must match the signature of the mocked method](docs/Compilation/PosInfoMoq2003.md) | The delegate in the argument of the `Callback()` method must match the signature of the mocked method. |
| [PosInfoMoq2004: Constructor arguments cannot be passed for interface mocks](docs/Compilation/PosInfoMoq2004.md) | No arguments can be passed to a mocked interface. |
Expand Down
27 changes: 16 additions & 11 deletions docs/Compilation/PosInfoMoq2001.md
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
# PosInfoMoq2001: The `Setup()` method must be used only on overridable members
# PosInfoMoq2001: The `Setup()`/`SetupSet()` method must be used only on overridable members

| Property | Value |
|-------------------------------------|---------------------------------------------------------------|
| **Rule ID** | PosInfoMoq2001 |
| **Title** | The `Setup()` method must be used only on overridable members |
| **Title** | The `Setup()`/`SetupSet()` method must be used only on overridable members |
| **Category** | Compilation |
| **Default severity** | Error |

## Cause

The `Setup()` method must be applied only for overridable members.
The `Setup()` or `SetupSet()` methods must be applied only for overridable members.
An overridable member is a **method** or **property** which is in:
- An `interface`.
- A non-`sealed` `class`. In this case, the member must be:
Expand All @@ -20,13 +20,18 @@ An overridable member is a **method** or **property** which is in:

The `Setup()` method must be applied only for overridable members.

For example, the following methods and properties can be mock and used in the `Setup()` method:
- `IService.MethodCanBeMocked()`
- `IService.PropertyCanBeMocked`
- `Service.VirtualMethodCanBeMocked`
- `Service.VirtualPropertyCanBeMocked`
- `Service.AbstractMethodCanBeMocked`
- `Service.AbstractPropertyCanBeMocked`
For example:
- The following methods and properties can be mock and used in the `Setup()` method:
- `IService.MethodCanBeMocked()`
- `IService.PropertyCanBeMocked`
- `Service.VirtualMethodCanBeMocked`
- `Service.VirtualPropertyCanBeMocked`
- `Service.AbstractMethodCanBeMocked`
- `Service.AbstractPropertyCanBeMocked`
- The following properties can be mock and used in the `SetupSet()` method:
- `IService.PropertyCanBeMocked`
- `Service.VirtualPropertyCanBeMocked`
- `Service.AbstractPropertyCanBeMocked`

```csharp
public interface IService
Expand All @@ -53,7 +58,7 @@ static methods which can not be overriden.

## How to fix violations

To fix a violation of this rule, be sure to mock a member in the `Setup()` method which can be overriden.
To fix a violation of this rule, be sure to mock a member in the `Setup()` or `SetupSet()` method which can be overriden.

## When to suppress warnings

Expand Down
6 changes: 3 additions & 3 deletions src/Moq.Analyzers/Analyzers/SetupMustBeUsedOnlyForOverridableMembersAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ public class SetupMustBeUsedOnlyForOverridableMembersAnalyzer : DiagnosticAnalyz
{
private static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(
"PosInfoMoq2001",
"The Setup() method must be used only on overridable members",
"The Setup() method must be used only on overridable members",
"The Setup()/SetupSet() method must be used only on overridable members",
"The Setup()/SetupSet() method must be used only on overridable members",
"Compilation",
DiagnosticSeverity.Error,
isEnabledByDefault: true,
Expand Down Expand Up @@ -51,7 +51,7 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
// Check is Setup() method.
var methodSymbol = context.SemanticModel.GetSymbolInfo(invocationExpression, context.CancellationToken);

if (!moqSymbols.IsSetupMethod(methodSymbol.Symbol))
if (!moqSymbols.IsSetupMethod(methodSymbol.Symbol) && !moqSymbols.IsSetupSetMethod(methodSymbol.Symbol))
{
return;
}
Expand Down
2 changes: 2 additions & 0 deletions src/Moq.Analyzers/Moq.Analyzers.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
- PosInfoMoq2014: The delegate in the argument of the Returns() method must return a value with same type of the mocked method.
- PosInfoMoq2015: The Protected().Setup() method must match the return type of the mocked method.
- PosInfoMoq2016: Mock&lt;T&gt; constructor with factory lambda expression can be used only with classes.
- Improvements of the rules:
- PosInfoMoq2001: Check that property setup with the SetupSet() method are overridable.
- Fix the PosInfoMoq1001 rule for the Mock&lt;T&gt; instantiation with the lambda expression to create mock instances.
- Fix the PosInfoMoq2004 rule for the Mock&lt;T&gt; instantiation with the lambda expression to create mock instances.
- Fix the PosInfoMoq1001 fixer when using the Mock.Of&lt;T&gt; as argument in the constructor call.
Expand Down
7 changes: 7 additions & 0 deletions src/Moq.Analyzers/MoqExpressionAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,13 @@ public bool IsStrictBehavior(IdentifierNameSyntax localVariableExpression, Cance
}

bodyExpression = lambdaExpression.ExpressionBody;

// Special case for the SetupSet(), if the body expression is an assignment syntax
// "mock => mock.Property = xxx", so we get the left part of the assignment.
if (bodyExpression is AssignmentExpressionSyntax assignmentExpressionSyntax)
{
bodyExpression = assignmentExpressionSyntax.Left;
}
}

var members = new List<ChainMember>();
Expand Down
78 changes: 62 additions & 16 deletions tests/Moq.Analyzers.Tests/Analyzers/SetupMustBeUsedOnlyForOverridableMembersAnalyzerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,56 +26,92 @@ public void TestMethod()
var mock1 = new Mock<I>();
mock1.Setup(i => i.TestMethod());
mock1.Setup(i => i.TestProperty);
mock1.SetupSet(i => i.TestProperty = ""Foobard"");
mock1.SetupSet<string>(i => i.TestProperty = ""Foobard"");
mock1.Setup(i => i.InnerObject.VirtualMethod());
mock1.Setup(i => i.InnerObject.VirtualProperty);
mock1.SetupSet(i => i.InnerObject.VirtualProperty = ""Foobard"");
mock1.SetupSet<string>(i => i.InnerObject.VirtualProperty = ""Foobard"");
mock1.Setup(i => i.InnerObject.AbstractMethod());
mock1.Setup(i => i.InnerObject.AbstractProperty);
mock1.SetupSet(i => i.InnerObject.AbstractProperty = ""Foobard"");
mock1.SetupSet<string>(i => i.InnerObject.AbstractProperty = ""Foobard"");
var mock2 = new Mock<StandardClass>();
mock2.Setup(i => i.VirtualMethod());
mock2.Setup(i => i.VirtualProperty);
mock2.SetupSet(i => i.VirtualProperty = ""Foobard"");
mock2.SetupSet<string>(i => i.VirtualProperty = ""Foobard"");
mock2.Setup(i => i.InnerObject.VirtualMethod());
mock2.Setup(i => i.InnerObject.VirtualProperty);
mock2.SetupSet(i => i.InnerObject.VirtualProperty = ""Foobard"");
mock2.SetupSet<string>(i => i.InnerObject.VirtualProperty = ""Foobard"");
mock2.Setup(i => i.InnerObject.AbstractMethod());
mock2.Setup(i => i.InnerObject.AbstractProperty);
mock2.SetupSet(i => i.InnerObject.AbstractProperty = ""Foobard"");
mock2.SetupSet<string>(i => i.InnerObject.AbstractProperty = ""Foobard"");
var mock3 = new Mock<AbstractClass>();
mock3.Setup(i => i.VirtualMethod());
mock3.Setup(i => i.VirtualProperty);
mock3.SetupSet(i => i.VirtualProperty = ""Foobard"");
mock3.SetupSet<string>(i => i.VirtualProperty = ""Foobard"");
mock3.Setup(i => i.AbstractMethod());
mock3.Setup(i => i.AbstractProperty);
mock3.SetupSet(i => i.InnerObject.AbstractProperty = ""Foobard"");
mock3.SetupSet<string>(i => i.InnerObject.AbstractProperty = ""Foobard"");
mock3.Setup(i => i.InnerObject.VirtualMethod());
mock3.Setup(i => i.InnerObject.VirtualProperty);
mock3.SetupSet(i => i.InnerObject.VirtualProperty = ""Foobard"");
mock3.SetupSet<string>(i => i.InnerObject.VirtualProperty = ""Foobard"");
mock3.Setup(i => i.InnerObject.AbstractMethod());
mock3.Setup(i => i.InnerObject.AbstractProperty);
mock3.SetupSet(i => i.InnerObject.AbstractProperty = ""Foobard"");
mock3.SetupSet<string>(i => i.InnerObject.AbstractProperty = ""Foobard"");
var mock4 = new Mock<InheritedFromAbstractClass>();
mock4.Setup(i => i.VirtualMethod());
mock4.Setup(i => i.VirtualProperty);
mock4.SetupSet(i => i.VirtualProperty = ""Foobard"");
mock4.SetupSet<string>(i => i.VirtualProperty = ""Foobard"");
mock4.Setup(i => i.AbstractMethod());
mock4.Setup(i => i.AbstractProperty);
mock4.SetupSet(i => i.InnerObject.AbstractProperty = ""Foobard"");
mock4.SetupSet<string>(i => i.InnerObject.AbstractProperty = ""Foobard"");
mock4.Setup(i => i.InnerObject.VirtualMethod());
mock4.Setup(i => i.InnerObject.VirtualProperty);
mock4.SetupSet(i => i.InnerObject.VirtualProperty = ""Foobard"");
mock4.SetupSet<string>(i => i.InnerObject.VirtualProperty = ""Foobard"");
mock4.Setup(i => i.InnerObject.AbstractMethod());
mock4.Setup(i => i.InnerObject.AbstractProperty);
mock4.SetupSet(i => i.InnerObject.AbstractProperty = ""Foobard"");
mock4.SetupSet<string>(i => i.InnerObject.AbstractProperty = ""Foobard"");
var mock5 = new Mock<InheritedFromAbstractClassDontOverrideVirtual>();
mock5.Setup(i => i.VirtualMethod());
mock5.Setup(i => i.VirtualProperty);
mock5.SetupSet(i => i.VirtualProperty = ""Foobard"");
mock5.SetupSet<string>(i => i.VirtualProperty = ""Foobard"");
mock5.Setup(i => i.AbstractMethod());
mock5.Setup(i => i.AbstractProperty);
mock5.SetupSet(i => i.InnerObject.AbstractProperty = ""Foobard"");
mock5.SetupSet<string>(i => i.InnerObject.AbstractProperty = ""Foobard"");
mock5.Setup(i => i.InnerObject.VirtualMethod());
mock5.Setup(i => i.InnerObject.VirtualProperty);
mock5.SetupSet(i => i.InnerObject.VirtualProperty = ""Foobard"");
mock5.SetupSet<string>(i => i.InnerObject.VirtualProperty = ""Foobard"");
mock5.Setup(i => i.InnerObject.AbstractMethod());
mock5.Setup(i => i.InnerObject.AbstractProperty);
mock5.SetupSet(i => i.InnerObject.AbstractProperty = ""Foobard"");
mock5.SetupSet<string>(i => i.InnerObject.AbstractProperty = ""Foobard"");
}
}
public interface I
{
int TestMethod();
string TestProperty { get; }
string TestProperty { get; set; }
InnerObject InnerObject { get; }
}
Expand All @@ -84,20 +120,20 @@ public class StandardClass
{
public virtual void VirtualMethod() { }
public virtual string VirtualProperty => null;
public virtual string VirtualProperty { get => null; set { } }
public virtual InnerObject InnerObject { get => null; }
public virtual InnerObject InnerObject { get => null; set { } }
}
public abstract class AbstractClass
{
public virtual void VirtualMethod() { }
public virtual string VirtualProperty => null;
public virtual string VirtualProperty { get => null; set { } }
public abstract void AbstractMethod();
public abstract string AbstractProperty { get; }
public abstract string AbstractProperty { get; set; }
public abstract InnerObject InnerObject { get; }
}
Expand All @@ -106,11 +142,11 @@ public class InheritedFromAbstractClass : AbstractClass
{
public override void AbstractMethod() { }
public override string AbstractProperty => null;
public override string AbstractProperty { get => null; set { } }
public override void VirtualMethod() { }
public override string VirtualProperty => null;
public override string VirtualProperty { get => null; set { } }
public override InnerObject InnerObject => null;
}
Expand All @@ -119,7 +155,7 @@ public class InheritedFromAbstractClassDontOverrideVirtual : AbstractClass
{
public override void AbstractMethod() { }
public override string AbstractProperty => null;
public override string AbstractProperty { get => null; set { } }
public override InnerObject InnerObject => null;
}
Expand All @@ -128,11 +164,11 @@ public abstract class InnerObject
{
public virtual void VirtualMethod() { }
public virtual string VirtualProperty => null;
public virtual string VirtualProperty { get => null; set { } }
public abstract void AbstractMethod();
public abstract string AbstractProperty { get; }
public abstract string AbstractProperty { get; set; }
}
}";

Expand All @@ -155,19 +191,29 @@ public void TestMethod()
var mock1 = new Mock<StandardClass>();
mock1.Setup(i => i.[|Method|]());
mock1.Setup(i => i.[|Property|]);
mock1.SetupSet(i => i.[|Property|] = ""1234"");
mock1.SetupSet<string>(i => i.[|Property|] = ""1234"");
mock1.Setup(i => i.[|InnerObject|].VirtualMethod());
mock1.Setup(i => i.[|InnerObject|].VirtualProperty);
mock1.SetupSet(i => i.[|InnerObject|].VirtualProperty = ""1234"");
mock1.SetupSet<string>(i => i.[|InnerObject|].VirtualProperty = ""1234"");
mock1.Setup(i => i.AbstractInnerObject.[|Method|]());
mock1.Setup(i => i.AbstractInnerObject.[|Property|]);
mock1.SetupSet(i => i.AbstractInnerObject.[|Property|] = ""1234"");
mock1.SetupSet<string>(i => i.AbstractInnerObject.[|Property|] = ""1234"");
mock1.Setup(i => i.[|SealedInnerObject|].VirtualMethod());
mock1.Setup(i => i.[|SealedInnerObject|].VirtualProperty);
mock1.SetupSet(i => i.[|SealedInnerObject|].VirtualProperty = ""1234"");
mock1.SetupSet<string>(i => i.[|SealedInnerObject|].VirtualProperty = ""1234"");
var mock2 = new Mock<StandardClass>();
mock2.Setup(i => StandardClass.[|StaticMethod|]());
var mock3 = new Mock<AbstractClass>();
mock3.Setup(i => i.[|Method|]());
mock3.Setup(i => i.[|Property|]);
mock3.SetupSet(i => i.[|Property|] = ""1234"");
mock3.SetupSet<string>(i => i.[|Property|] = ""1234"");
var mock4 = new Mock<I>();
mock4.Setup(i => i.[|ExtensionMethod|]());
Expand All @@ -178,7 +224,7 @@ public class StandardClass
{
public void Method() { }
public string Property => null;
public string Property { get => null; set { } }
public static void StaticMethod() { }
Expand All @@ -193,7 +239,7 @@ public abstract class AbstractClass
{
public void Method() { }
public string Property => null;
public string Property { get => null; set { } }
}
public interface I
Expand All @@ -210,25 +256,25 @@ public class InnerObject
{
public virtual void VirtualMethod() { }
public virtual string VirtualProperty => null;
public virtual string VirtualProperty { get => null; set { } }
}
public abstract class AbstractInnerObject
{
public void Method() { }
public string Property { get => null; }
public string Property { get => null; set { } }
public abstract void VirtualMethod();
public abstract string VirtualProperty { get; }
public abstract string VirtualProperty { get; set; }
}
public sealed class SealedInnerObject : AbstractInnerObject
{
public override void VirtualMethod() { }
public override string VirtualProperty => null;
public override string VirtualProperty { get => null; set { } }
}
}";

Expand Down

0 comments on commit b71c04f

Please sign in to comment.