From b71c04f027efa543268b0e7ad716b0d96704d564 Mon Sep 17 00:00:00 2001 From: Gilles TOURREAU Date: Thu, 24 Oct 2024 17:17:17 +0200 Subject: [PATCH] Updates the PosInfoMoq2001 to check property in the SetupSet(). --- README.md | 2 +- docs/Compilation/PosInfoMoq2001.md | 27 ++++--- ...BeUsedOnlyForOverridableMembersAnalyzer.cs | 6 +- src/Moq.Analyzers/Moq.Analyzers.csproj | 2 + src/Moq.Analyzers/MoqExpressionAnalyzer.cs | 7 ++ ...edOnlyForOverridableMembersAnalyzerTest.cs | 78 +++++++++++++++---- 6 files changed, 91 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 322835c..eb45187 100644 --- a/README.md +++ b/README.md @@ -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` 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` class can be used only to mock non-sealed class](docs/Compilation/PosInfoMoq2002.md) | The `Mock` 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. | diff --git a/docs/Compilation/PosInfoMoq2001.md b/docs/Compilation/PosInfoMoq2001.md index f851c66..9f98a05 100644 --- a/docs/Compilation/PosInfoMoq2001.md +++ b/docs/Compilation/PosInfoMoq2001.md @@ -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: @@ -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 @@ -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 diff --git a/src/Moq.Analyzers/Analyzers/SetupMustBeUsedOnlyForOverridableMembersAnalyzer.cs b/src/Moq.Analyzers/Analyzers/SetupMustBeUsedOnlyForOverridableMembersAnalyzer.cs index d565f8a..2d6927c 100644 --- a/src/Moq.Analyzers/Analyzers/SetupMustBeUsedOnlyForOverridableMembersAnalyzer.cs +++ b/src/Moq.Analyzers/Analyzers/SetupMustBeUsedOnlyForOverridableMembersAnalyzer.cs @@ -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, @@ -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; } diff --git a/src/Moq.Analyzers/Moq.Analyzers.csproj b/src/Moq.Analyzers/Moq.Analyzers.csproj index 73b5f61..99b7406 100644 --- a/src/Moq.Analyzers/Moq.Analyzers.csproj +++ b/src/Moq.Analyzers/Moq.Analyzers.csproj @@ -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<T> 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<T> instantiation with the lambda expression to create mock instances. - Fix the PosInfoMoq2004 rule for the Mock<T> instantiation with the lambda expression to create mock instances. - Fix the PosInfoMoq1001 fixer when using the Mock.Of<T> as argument in the constructor call. diff --git a/src/Moq.Analyzers/MoqExpressionAnalyzer.cs b/src/Moq.Analyzers/MoqExpressionAnalyzer.cs index c588345..7183f3f 100644 --- a/src/Moq.Analyzers/MoqExpressionAnalyzer.cs +++ b/src/Moq.Analyzers/MoqExpressionAnalyzer.cs @@ -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(); diff --git a/tests/Moq.Analyzers.Tests/Analyzers/SetupMustBeUsedOnlyForOverridableMembersAnalyzerTest.cs b/tests/Moq.Analyzers.Tests/Analyzers/SetupMustBeUsedOnlyForOverridableMembersAnalyzerTest.cs index fe6bfe0..5216328 100644 --- a/tests/Moq.Analyzers.Tests/Analyzers/SetupMustBeUsedOnlyForOverridableMembersAnalyzerTest.cs +++ b/tests/Moq.Analyzers.Tests/Analyzers/SetupMustBeUsedOnlyForOverridableMembersAnalyzerTest.cs @@ -26,48 +26,84 @@ public void TestMethod() var mock1 = new Mock(); mock1.Setup(i => i.TestMethod()); mock1.Setup(i => i.TestProperty); + mock1.SetupSet(i => i.TestProperty = ""Foobard""); + mock1.SetupSet(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(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(i => i.InnerObject.AbstractProperty = ""Foobard""); var mock2 = new Mock(); mock2.Setup(i => i.VirtualMethod()); mock2.Setup(i => i.VirtualProperty); + mock2.SetupSet(i => i.VirtualProperty = ""Foobard""); + mock2.SetupSet(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(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(i => i.InnerObject.AbstractProperty = ""Foobard""); var mock3 = new Mock(); mock3.Setup(i => i.VirtualMethod()); mock3.Setup(i => i.VirtualProperty); + mock3.SetupSet(i => i.VirtualProperty = ""Foobard""); + mock3.SetupSet(i => i.VirtualProperty = ""Foobard""); mock3.Setup(i => i.AbstractMethod()); mock3.Setup(i => i.AbstractProperty); + mock3.SetupSet(i => i.InnerObject.AbstractProperty = ""Foobard""); + mock3.SetupSet(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(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(i => i.InnerObject.AbstractProperty = ""Foobard""); var mock4 = new Mock(); mock4.Setup(i => i.VirtualMethod()); mock4.Setup(i => i.VirtualProperty); + mock4.SetupSet(i => i.VirtualProperty = ""Foobard""); + mock4.SetupSet(i => i.VirtualProperty = ""Foobard""); mock4.Setup(i => i.AbstractMethod()); mock4.Setup(i => i.AbstractProperty); + mock4.SetupSet(i => i.InnerObject.AbstractProperty = ""Foobard""); + mock4.SetupSet(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(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(i => i.InnerObject.AbstractProperty = ""Foobard""); var mock5 = new Mock(); mock5.Setup(i => i.VirtualMethod()); mock5.Setup(i => i.VirtualProperty); + mock5.SetupSet(i => i.VirtualProperty = ""Foobard""); + mock5.SetupSet(i => i.VirtualProperty = ""Foobard""); mock5.Setup(i => i.AbstractMethod()); mock5.Setup(i => i.AbstractProperty); + mock5.SetupSet(i => i.InnerObject.AbstractProperty = ""Foobard""); + mock5.SetupSet(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(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(i => i.InnerObject.AbstractProperty = ""Foobard""); } } @@ -75,7 +111,7 @@ public interface I { int TestMethod(); - string TestProperty { get; } + string TestProperty { get; set; } InnerObject InnerObject { get; } } @@ -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; } } @@ -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; } @@ -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; } @@ -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; } } }"; @@ -155,12 +191,20 @@ public void TestMethod() var mock1 = new Mock(); mock1.Setup(i => i.[|Method|]()); mock1.Setup(i => i.[|Property|]); + mock1.SetupSet(i => i.[|Property|] = ""1234""); + mock1.SetupSet(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(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(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(i => i.[|SealedInnerObject|].VirtualProperty = ""1234""); var mock2 = new Mock(); mock2.Setup(i => StandardClass.[|StaticMethod|]()); @@ -168,6 +212,8 @@ public void TestMethod() var mock3 = new Mock(); mock3.Setup(i => i.[|Method|]()); mock3.Setup(i => i.[|Property|]); + mock3.SetupSet(i => i.[|Property|] = ""1234""); + mock3.SetupSet(i => i.[|Property|] = ""1234""); var mock4 = new Mock(); mock4.Setup(i => i.[|ExtensionMethod|]()); @@ -178,7 +224,7 @@ public class StandardClass { public void Method() { } - public string Property => null; + public string Property { get => null; set { } } public static void StaticMethod() { } @@ -193,7 +239,7 @@ public abstract class AbstractClass { public void Method() { } - public string Property => null; + public string Property { get => null; set { } } } public interface I @@ -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 { } } } }";