diff --git a/PosInformatique.Moq.Analyzers.sln b/PosInformatique.Moq.Analyzers.sln index 5d9a6d4..5bf8b7a 100644 --- a/PosInformatique.Moq.Analyzers.sln +++ b/PosInformatique.Moq.Analyzers.sln @@ -52,6 +52,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Compilation", "Compilation" docs\Compilation\PosInfoMoq2012.md = docs\Compilation\PosInfoMoq2012.md docs\Compilation\PosInfoMoq2013.md = docs\Compilation\PosInfoMoq2013.md docs\Compilation\PosInfoMoq2014.md = docs\Compilation\PosInfoMoq2014.md + docs\Compilation\PosInfoMoq2015.md = docs\Compilation\PosInfoMoq2015.md EndProjectSection EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Moq.Analyzers.Sandbox", "tests\Moq.Analyzers.Sandbox\Moq.Analyzers.Sandbox.csproj", "{07F970A1-1477-4D4C-B233-C9B4DA6E3AD6}" diff --git a/README.md b/README.md index 12ab764..4221644 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,7 @@ All the rules of this category should not be disabled (or changed their severity | [PosInfoMoq2012: The delegate in the argument of the `Returns()` method must return a value with same type of the mocked method.](docs/Compilation/PosInfoMoq2012.md) | The lambda expression, anonymous method or method in the argument of the `Returns()` must return return a value of the same type as the mocked method or property. | | [PosInfoMoq2013: The delegate in the argument of the `Returns()`/`ReturnsAsync()` method must have the same parameter types of the mocked method/property.](docs/Compilation/PosInfoMoq2013.md) | The lambda expression, anonymous method or method in the argument of the `Returns()`/`ReturnsAsync()` must have the same arguments type of the mocked method or property. | | [PosInfoMoq2014: The `Callback()` delegate expression must not return a value.](docs/Compilation/PosInfoMoq2014.md) | The `Callback()` delegate expression must not return a value. | +| [PosInfoMoq2014: PosInfoMoq2015: The `Protected().Setup()` method must match the return type of the mocked method](docs/Compilation/PosInfoMoq2015.md) | The method setup with `Protected().Setup()` must match the return type of the mocked method. | diff --git a/docs/Compilation/PosInfoMoq2006.md b/docs/Compilation/PosInfoMoq2006.md index c479384..253ff40 100644 --- a/docs/Compilation/PosInfoMoq2006.md +++ b/docs/Compilation/PosInfoMoq2006.md @@ -24,7 +24,7 @@ and must be overridable (`virtual`, `abstract` and `override`, but not `sealed`) public void Test() { var service = new Mock(); - service.Protected().Setup("GetData") // The GetData() is public and can be mocked with Protected() feature. + service.Protected().Setup("GetData") // The GetData() is public and can't be mocked with Protected() feature. .Returns(10); service.Protected().Setup("NotExists") // The NotExists() method does not exist. .Returns(10); diff --git a/docs/Compilation/PosInfoMoq2015.md b/docs/Compilation/PosInfoMoq2015.md new file mode 100644 index 0000000..26ef987 --- /dev/null +++ b/docs/Compilation/PosInfoMoq2015.md @@ -0,0 +1,50 @@ +# PosInfoMoq2015: The `Protected().Setup()` method must match the return type of the mocked method + +| Property | Value | +|-------------------------------------|----------------------------------------------------------------------------------------------| +| **Rule ID** | PosInfoMoq2015 | +| **Title** | The `Protected().Setup()` method must match the return type of the mocked method. | +| **Category** | Compilation | +| **Default severity** | Error | + +## Cause + +The method setup with `Protected().Setup()` must match the return type of the mocked method. + +## Rule description + +When using the `Protected().Setup()`, the return type of mocked method must match of the generic +argument specified in the `Setup()` method. + +```csharp +[Fact] +public void Test() +{ + var service = new Mock(); + service.Protected().Setup("GetData") // OK. + .Returns(10); + service.Protected().Setup("GetData") // Error: The GetData() return an int, Setup() must be use. + service.Protected().Setup("SendEmail") // Error: The SendEmail() method does not return a value, the `int` generic argument must be remove.0 + .Returns(10); + service.Protected().Setup("GetData") // Error: The GetData() return an int, Setup() must be use. + .Returns("The data"); +} + +public abstract class Service +{ + protected abstract int GetData(); + + protected abstract void SendEmail(); +} +``` + +## How to fix violations + +To fix a violation of this rule, use the generic parameter of the `Setup()` method if the protected mocked +method return a value. Else do not specify a generic parameter for the `Setup()` method of the protected mocked +method does not return a value (`void`). + +## When to suppress warnings + +Do not suppress an error from this rule. If bypassed, the execution of the unit test will be failed with a `ArgumentException` +thrown with the *"Can't set return value for void method xxx."* message. diff --git a/src/Moq.Analyzers/AnalyzerReleases.Shipped.md b/src/Moq.Analyzers/AnalyzerReleases.Shipped.md index 31e25bd..a40b80c 100644 --- a/src/Moq.Analyzers/AnalyzerReleases.Shipped.md +++ b/src/Moq.Analyzers/AnalyzerReleases.Shipped.md @@ -3,7 +3,8 @@ ### New Rules Rule ID | Category | Severity | Notes --------|----------|----------|------- -PosInfoMoq2014 | Compilation | Error | CallBackDelegateMustMatchMockedMethodAnalyzer, [Documentation](https://posinformatique.github.io/PosInformatique.Moq.Analyzers/docs/Compilation/PosInfoMoq2014.html)PosInfoMoq2013 | Compilation | Error | The delegate in the argument of the `Returns()`/`ReturnsAsync()` method must have the same parameter types of the mocked method/property. +PosInfoMoq2014 | Compilation | Error | The `Callback()` delegate expression must not return a value. +PosInfoMoq2015 | Compilation | Error | The `Protected().Setup()` method must match the return type of the mocked method. ## Release 1.10.0 diff --git a/src/Moq.Analyzers/Analyzers/SetupProtectedMustBeUsedWithProtectedOrInternalMembersAnalyzer.cs b/src/Moq.Analyzers/Analyzers/SetupProtectedMustBeUsedWithProtectedOrInternalMembersAnalyzer.cs index 7c93bdd..a6424c9 100644 --- a/src/Moq.Analyzers/Analyzers/SetupProtectedMustBeUsedWithProtectedOrInternalMembersAnalyzer.cs +++ b/src/Moq.Analyzers/Analyzers/SetupProtectedMustBeUsedWithProtectedOrInternalMembersAnalyzer.cs @@ -15,7 +15,7 @@ namespace PosInformatique.Moq.Analyzers [DiagnosticAnalyzer(LanguageNames.CSharp)] public class SetupProtectedMustBeUsedWithProtectedOrInternalMembersAnalyzer : DiagnosticAnalyzer { - private static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor( + private static readonly DiagnosticDescriptor SetupMustBeOnOverridableMethods = new DiagnosticDescriptor( "PosInfoMoq2006", "The Protected().Setup() method must be use with overridable protected or internal methods", "The Protected().Setup() method must be use with overridable protected or internal methods", @@ -25,7 +25,17 @@ public class SetupProtectedMustBeUsedWithProtectedOrInternalMembersAnalyzer : Di description: "The Protected().Setup() method must be use with overridable protected or internal methods.", helpLinkUri: "https://posinformatique.github.io/PosInformatique.Moq.Analyzers/docs/Compilation/PosInfoMoq2006.html"); - public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + private static readonly DiagnosticDescriptor SetupReturnTypeMustMatch = new DiagnosticDescriptor( + "PosInfoMoq2015", + "The Protected().Setup() method must match the return type of the mocked method", + "The Protected().Setup() method must match the return type of the mocked method", + "Compilation", + DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "The Protected().Setup() method must match the return type of the mocked method.", + helpLinkUri: "https://posinformatique.github.io/PosInformatique.Moq.Analyzers/docs/Compilation/PosInfoMoq2015.html"); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(SetupMustBeOnOverridableMethods, SetupReturnTypeMustMatch); public override void Initialize(AnalysisContext context) { @@ -81,6 +91,8 @@ private static void Analyze(SyntaxNodeAnalysisContext context) } // Check if a method exists with the specified name + IMethodSymbol? methodMatch = null; + foreach (var method in mockedType.GetAllMembers(methodName).OfType()) { if (!method.IsAbstract && !method.IsVirtual && !method.IsOverride) @@ -95,23 +107,78 @@ private static void Analyze(SyntaxNodeAnalysisContext context) if (method.DeclaredAccessibility == Accessibility.Protected) { - return; + methodMatch = method; + break; } if (method.DeclaredAccessibility == Accessibility.Internal) { - return; + methodMatch = method; + break; } if (method.DeclaredAccessibility == Accessibility.ProtectedOrInternal) { - return; + methodMatch = method; + break; + } + } + + if (methodMatch is null) + { + // No method match, raise an error. + context.ReportDiagnostic(SetupMustBeOnOverridableMethods, literalExpression.GetLocation()); + return; + } + + // Else check the argument type of the Setup() method and the method found. + var setupMethodSymbol = context.SemanticModel.GetSymbolInfo(invocationExpression, context.CancellationToken); + + if (setupMethodSymbol.Symbol is not IMethodSymbol setupMethod) + { + return; + } + + if (invocationExpression.Expression is not MemberAccessExpressionSyntax memberAccessExpressionSyntax) + { + return; + } + + if (methodMatch.ReturnsVoid) + { + if (setupMethod.TypeArguments.Length > 0) + { + if (memberAccessExpressionSyntax.Name is not GenericNameSyntax genericNameSyntax) + { + return; + } + + // The method mocked is void and no generic arguments has been specified in the Setup() method. + context.ReportDiagnostic(SetupReturnTypeMustMatch, genericNameSyntax.TypeArgumentList.Arguments[0].GetLocation()); } + + return; + } + + if (setupMethod.TypeArguments.Length != 1) + { + // No generic type has been specified in the Setup(). + context.ReportDiagnostic(SetupReturnTypeMustMatch, memberAccessExpressionSyntax.Name.GetLocation()); + return; } - // No returns method has been specified with Strict mode. Report the diagnostic issue. - var diagnostic = Diagnostic.Create(Rule, literalExpression.GetLocation()); - context.ReportDiagnostic(diagnostic); + if (!SymbolEqualityComparer.Default.Equals(setupMethod.TypeArguments[0], methodMatch.ReturnType)) + { + if (memberAccessExpressionSyntax.Name is not GenericNameSyntax genericNameSyntax) + { + return; + } + + // The method mocked return a type which does not match the argument type of the Setup() method. + context.ReportDiagnostic(SetupReturnTypeMustMatch, genericNameSyntax.TypeArgumentList.Arguments[0].GetLocation()); + + return; + } } } } diff --git a/src/Moq.Analyzers/Moq.Analyzers.csproj b/src/Moq.Analyzers/Moq.Analyzers.csproj index d6fcc5c..8cd3932 100644 --- a/src/Moq.Analyzers/Moq.Analyzers.csproj +++ b/src/Moq.Analyzers/Moq.Analyzers.csproj @@ -20,6 +20,7 @@ 1.10.0 - Add new rules: - 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. 1.10.0 - Add new rules: diff --git a/tests/Moq.Analyzers.Tests/Analyzers/SetupProtectedMustBeUsedWithProtectedOrInternalMembersAnalyzerTest.cs b/tests/Moq.Analyzers.Tests/Analyzers/SetupProtectedMustBeUsedWithProtectedOrInternalMembersAnalyzerTest.cs index 1652c4e..bbd918c 100644 --- a/tests/Moq.Analyzers.Tests/Analyzers/SetupProtectedMustBeUsedWithProtectedOrInternalMembersAnalyzerTest.cs +++ b/tests/Moq.Analyzers.Tests/Analyzers/SetupProtectedMustBeUsedWithProtectedOrInternalMembersAnalyzerTest.cs @@ -38,15 +38,16 @@ public void TestMethod() mock1.Protected().Setup(""InternalMethod""); mock1.Protected().Setup(""InternalOverrideMethod""); - mock1.Protected().Setup(""ProtectedVirtualMethod""); - mock1.Protected().Setup(""ProtectedInternalVirtualMethod""); - mock1.Protected().Setup(""InternalVirtualMethod""); - mock1.Protected().Setup(""ProtectedAbstractMethod""); - mock1.Protected().Setup(""ProtectedInternalAbstractMethod""); - mock1.Protected().Setup(""InternalAbstractMethod""); - mock1.Protected().Setup(""ProtectedOverrideMethod""); - mock1.Protected().Setup(""ProtectedInternalOverrideMethod""); - mock1.Protected().Setup(""InternalOverrideMethod""); + mock1.Protected().Setup(""ProtectedVirtualMethodReturnValue""); + mock1.Protected().Setup(""ProtectedInternalVirtualMethodReturnValue""); + mock1.Protected().Setup(""InternalVirtualMethodReturnValue""); + mock1.Protected().Setup(""ProtectedAbstractMethodReturnValue""); + mock1.Protected().Setup(""ProtectedInternalAbstractMethodReturnValue""); + mock1.Protected().Setup(""InternalAbstractMethodReturnValue""); + mock1.Protected().Setup(""ProtectedOverrideMethodReturnValue""); + mock1.Protected().Setup(""ProtectedInternalOverrideMethodReturnValue""); + mock1.Protected().Setup(""InternalOverrideMethodReturnValue""); + mock1.Protected().Setup(""InternalMethodReturnValue""); } } @@ -55,38 +56,64 @@ public abstract class C : BaseClass // Virtual protected virtual void ProtectedVirtualMethod() { } + protected virtual int ProtectedVirtualMethodReturnValue() { return 10;} + protected internal virtual void ProtectedInternalVirtualMethod() { } + protected internal virtual int ProtectedInternalVirtualMethodReturnValue() { return 10; } + internal virtual void InternalVirtualMethod() { } + internal virtual int InternalVirtualMethodReturnValue() { return 10; } + // Abstract protected abstract void ProtectedAbstractMethod(); + protected abstract int ProtectedAbstractMethodReturnValue(); + protected internal abstract void ProtectedInternalAbstractMethod(); + protected internal abstract int ProtectedInternalAbstractMethodReturnValue(); + internal abstract void InternalAbstractMethod(); + internal abstract int InternalAbstractMethodReturnValue(); + // Override protected override void ProtectedOverrideMethod() { } + protected override int ProtectedOverrideMethodReturnValue() { return 10; } + protected internal override void ProtectedInternalOverrideMethod() { } + protected internal override int ProtectedInternalOverrideMethodReturnValue() { return 10; } + internal override void InternalOverrideMethod() { } + + internal override int InternalOverrideMethodReturnValue() { return 10; } } public abstract class BaseClass { protected virtual void ProtectedOverrideMethod() { } + protected virtual int ProtectedOverrideMethodReturnValue() { return 10; } + protected virtual void ProtectedMethod() { } protected internal virtual void ProtectedInternalOverrideMethod() { } + protected internal virtual int ProtectedInternalOverrideMethodReturnValue() { return 10; } + protected internal virtual void ProtectedInternalMethod() { } internal virtual void InternalOverrideMethod() { } + internal virtual int InternalOverrideMethodReturnValue() { return 10; } + internal virtual void InternalMethod() { } + + internal virtual int InternalMethodReturnValue() { return 10; } } }"; @@ -108,8 +135,8 @@ public class TestClass public void TestMethod() { var mock1 = new Mock(); - mock1.Protected().Setup([|""TestMethodPublic""|]); - mock1.Protected().Setup([|""TestMethodPublic""|]); + mock1.Protected().Setup({|PosInfoMoq2006:""TestMethodPublic""|}); + mock1.Protected().Setup({|PosInfoMoq2006:""TestMethodPublic""|}); } } @@ -137,8 +164,8 @@ public class TestClass public void TestMethod() { var mock1 = new Mock(); - mock1.Protected().Setup([|""TestMethod""|]); - mock1.Protected().Setup([|""TestMethod""|]); + mock1.Protected().Setup({|PosInfoMoq2006:""TestMethod""|}); + mock1.Protected().Setup({|PosInfoMoq2006:""TestMethod""|}); } } @@ -171,8 +198,8 @@ public class TestClass public void TestMethod() { var mock1 = new Mock(); - mock1.Protected().Setup([|""TestMethodPublic""|]); - mock1.Protected().Setup([|""TestMethodPublic""|]); + mock1.Protected().Setup({|PosInfoMoq2006:""TestMethodPublic""|}); + mock1.Protected().Setup({|PosInfoMoq2006:""TestMethodPublic""|}); } } @@ -200,13 +227,13 @@ public class TestClass public void TestMethod() { var mock1 = new Mock(); - mock1.Protected().Setup([|""NonOverridableProtectedMethod""|]); - mock1.Protected().Setup([|""NonOverridableProtectedInternalMethod""|]); - mock1.Protected().Setup([|""NonOverridableInternalMethod""|]); + mock1.Protected().Setup({|PosInfoMoq2006:""NonOverridableProtectedMethod""|}); + mock1.Protected().Setup({|PosInfoMoq2006:""NonOverridableProtectedInternalMethod""|}); + mock1.Protected().Setup({|PosInfoMoq2006:""NonOverridableInternalMethod""|}); - mock1.Protected().Setup([|""NonOverridableProtectedMethod""|]); - mock1.Protected().Setup([|""NonOverridableProtectedInternalMethod""|]); - mock1.Protected().Setup([|""NonOverridableInternalMethod""|]); + mock1.Protected().Setup({|PosInfoMoq2006:""NonOverridableProtectedMethod""|}); + mock1.Protected().Setup({|PosInfoMoq2006:""NonOverridableProtectedInternalMethod""|}); + mock1.Protected().Setup({|PosInfoMoq2006:""NonOverridableInternalMethod""|}); } } @@ -221,6 +248,38 @@ internal void NonOverridableInternalMethod() { } await Verifier.VerifyAnalyzerAsync(source); } + [Fact] + public async Task ReturnMethodTypeNotMatching_DiagnosticReported() + { + var source = @" + namespace ConsoleApplication1 + { + using Moq; + using Moq.Protected; + using System; + + public class TestClass + { + public void TestMethod() + { + var mock1 = new Mock(); + mock1.Protected().Setup<{|PosInfoMoq2015:long|}>(""TestMethodProtectedReturnValue""); + mock1.Protected().Setup<{|PosInfoMoq2015:long|}>(""TestMethodProtected""); + mock1.Protected().{|PosInfoMoq2015:Setup|}(""TestMethodProtectedReturnValue""); + } + } + + public class C + { + protected virtual int TestMethodProtectedReturnValue() { return 0; } + + protected virtual void TestMethodProtected() { } + } + }"; + + await Verifier.VerifyAnalyzerAsync(source); + } + [Fact] public async Task NoMoqLibrary() {