Add a new PosInfoMoq2014 rule to check if a delegate in the Callback(…

…) method does not a return a value (fixes: #35).

PosInformatique.Moq.Analyzers.sln

@@ -51,6 +51,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Compilation", "Compilation"
 		docs\Compilation\PosInfoMoq2011.md = docs\Compilation\PosInfoMoq2011.md
 		docs\Compilation\PosInfoMoq2012.md = docs\Compilation\PosInfoMoq2012.md
 		docs\Compilation\PosInfoMoq2013.md = docs\Compilation\PosInfoMoq2013.md
+		docs\Compilation\PosInfoMoq2014.md = docs\Compilation\PosInfoMoq2014.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}"

README.md

@@ -53,6 +53,7 @@ All the rules of this category should not be disabled (or changed their severity
 | [PosInfoMoq2011: Constructor of the mocked class must be accessible.](docs/Compilation/PosInfoMoq2011.md) | The constructor of the instantiate mocked class must non-private. |
 | [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. |
 
 
 

docs/Compilation/PosInfoMoq2014.md

@@ -0,0 +1,45 @@
+# PosInfoMoq2014: The `Callback()` delegate expression must not return a value.
+
+| Property                            | Value																					|
+|-------------------------------------|-----------------------------------------------------------------------------------------|
+| **Rule ID**                         | PosInfoMoq2014																			|
+| **Title**                           | The `Callback()` delegate expression must not return a value.                		    |
+| **Category**                        | Compilation																				|
+| **Default severity**				  | Error																					|
+
+## Cause
+
+The delegate in the argument of the `Callback()` method must not return a value.
+
+## Rule description
+
+The lambda expression in the argument of the `Callback()` method must not return a value.
+
+```csharp
+[Fact]
+public void Test()
+{
+    var service = new Mock<Service>();
+    service.Setup(s => s.GetData("TOURREAU", 1234))
+        .Callback((string n, int age) =>
+        {
+        	// ...
+            return 1234;        // The delegate in the Callback() method must not return a value.
+        })
+        .Returns(10);
+}
+
+public interface IService
+{
+	public int GetData(string name, int age) { }
+}
+```
+
+## How to fix violations
+
+To fix a violation of this rule, be sure that the delegate method in the `Callback()` method does not return a value.
+
+## 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 *"Invalid callback. This overload of the "Callback" method only accepts "void" (C#) or "Sub" (VB.NET) delegates with parameter types matching those of the set up method. (Parameter 'callback')"* message.

src/Moq.Analyzers/AnalyzerReleases.Shipped.md

@@ -1,4 +1,11 @@
-## Release 1.10.0
+## Release 1.11.0
+
+### 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.
+
+## Release 1.10.0
 
 ### New Rules
 Rule ID | Category | Severity | Notes

src/Moq.Analyzers/Analyzers/CallBackDelegateMustMatchMockedMethodAnalyzer.cs

@@ -15,7 +15,7 @@ namespace PosInformatique.Moq.Analyzers
     [DiagnosticAnalyzer(LanguageNames.CSharp)]
     public class CallBackDelegateMustMatchMockedMethodAnalyzer : DiagnosticAnalyzer
     {
-        private static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(
+        internal static readonly DiagnosticDescriptor CallbackMustMatchSignature = new DiagnosticDescriptor(
             "PosInfoMoq2003",
             "The Callback() delegate expression must match the signature of the mocked method",
             "The Callback() delegate expression must match the signature of the mocked method",
@@ -25,7 +25,17 @@ public class CallBackDelegateMustMatchMockedMethodAnalyzer : DiagnosticAnalyzer
             description: "The Callback() delegate expression must match the signature of the mocked method.",
             helpLinkUri: "https://posinformatique.github.io/PosInformatique.Moq.Analyzers/docs/Compilation/PosInfoMoq2003.html");
 
-        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+        internal static readonly DiagnosticDescriptor CallbackMustNotReturnValue = new DiagnosticDescriptor(
+            "PosInfoMoq2014",
+            "The Callback() delegate expression must not return a value",
+            "The Callback() delegate expression must not return a value",
+            "Compilation",
+            DiagnosticSeverity.Error,
+            isEnabledByDefault: true,
+            description: "The Callback() delegate expression must not return a value.",
+            helpLinkUri: "https://posinformatique.github.io/PosInformatique.Moq.Analyzers/docs/Compilation/PosInfoMoq2014.html");
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(CallbackMustMatchSignature, CallbackMustNotReturnValue);
 
         public override void Initialize(AnalysisContext context)
         {
@@ -56,6 +66,15 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
                 return;
             }
 
+            if (!callBackLambdaExpressionSymbol.ReturnsVoid)
+            {
+                var allReturnsStatements = ReturnStatementSyntaxFinder.GetAllReturnStatements(lambdaExpression!.Block!);
+
+                var allReturnsStatementsLocations = allReturnsStatements.Select(statement => statement.GetLocation());
+
+                context.ReportDiagnostic(CallbackMustNotReturnValue, allReturnsStatementsLocations);
+            }
+
             // Extracts the setup method from the Callback() method call.
             var setupMethod = moqExpressionAnalyzer.ExtractSetupMethod(invocationExpression, context.CancellationToken);
 
@@ -68,7 +87,7 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
             // 1- Compare the number of the parameters
             if (callBackLambdaExpressionSymbol.Parameters.Length != setupMethod.InvocationArguments.Count)
             {
-                var diagnostic = Diagnostic.Create(Rule, lambdaExpression!.ParameterList.GetLocation());
+                var diagnostic = Diagnostic.Create(CallbackMustMatchSignature, lambdaExpression!.ParameterList.GetLocation());
                 context.ReportDiagnostic(diagnostic);
 
                 return;
@@ -83,20 +102,46 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
                     // The callback parameter associated must be an object.
                     if (callBackLambdaExpressionSymbol.Parameters[i].Type.SpecialType != SpecialType.System_Object)
                     {
-                        var diagnostic = Diagnostic.Create(Rule, lambdaExpression!.ParameterList.Parameters[i].GetLocation());
+                        var diagnostic = Diagnostic.Create(CallbackMustMatchSignature, lambdaExpression!.ParameterList.Parameters[i].GetLocation());
                         context.ReportDiagnostic(diagnostic);
 
                         continue;
                     }
                 }
                 else if (!SymbolEqualityComparer.Default.Equals(callBackLambdaExpressionSymbol.Parameters[i].Type, setupMethod.InvocationArguments[i].ParameterSymbol.Type))
                 {
-                    var diagnostic = Diagnostic.Create(Rule, lambdaExpression!.ParameterList.Parameters[i].GetLocation());
+                    var diagnostic = Diagnostic.Create(CallbackMustMatchSignature, lambdaExpression!.ParameterList.Parameters[i].GetLocation());
                     context.ReportDiagnostic(diagnostic);
 
                     continue;
                 }
             }
         }
+
+        private sealed class ReturnStatementSyntaxFinder : CSharpSyntaxWalker
+        {
+            private readonly List<ReturnStatementSyntax> returnStatements;
+
+            private ReturnStatementSyntaxFinder()
+            {
+                this.returnStatements = new List<ReturnStatementSyntax>();
+            }
+
+            public static IList<ReturnStatementSyntax> GetAllReturnStatements(BlockSyntax blockSyntax)
+            {
+                var finder = new ReturnStatementSyntaxFinder();
+
+                finder.Visit(blockSyntax);
+
+                return finder.returnStatements;
+            }
+
+            public override void VisitReturnStatement(ReturnStatementSyntax node)
+            {
+                this.returnStatements.Add(node);
+
+                base.VisitReturnStatement(node);
+            }
+        }
     }
 }

src/Moq.Analyzers/Moq.Analyzers.csproj

@@ -17,6 +17,10 @@
     <PackageProjectUrl>https://github.com/PosInformatique/PosInformatique.Moq.Analyzers</PackageProjectUrl>
     <PackageReadmeFile>README.md</PackageReadmeFile>
     <PackageReleaseNotes>
+      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.
+
       1.10.0
       - Add new rules:
         - PosInfoMoq2012: The delegate in the argument of the Returns() method must return a value with same type of the mocked method.

src/Moq.Analyzers/SyntaxNodeAnalysisContextExtensions.cs

@@ -16,5 +16,11 @@ public static void ReportDiagnostic(this SyntaxNodeAnalysisContext context, Diag
             var diagnostic = Diagnostic.Create(descriptor, location, messageArgs);
             context.ReportDiagnostic(diagnostic);
         }
+
+        public static void ReportDiagnostic(this SyntaxNodeAnalysisContext context, DiagnosticDescriptor descriptor, IEnumerable<Location> locations, params object[]? messageArgs)
+        {
+            var diagnostic = Diagnostic.Create(descriptor, locations.First(), locations.Skip(1), messageArgs);
+            context.ReportDiagnostic(diagnostic);
+        }
     }
 }

tests/Moq.Analyzers.Tests/Analyzers/CallBackDelegateMustMatchMockedMethodAnalyzerTest.cs

@@ -6,6 +6,7 @@
 
 namespace PosInformatique.Moq.Analyzers.Tests
 {
+    using Microsoft.CodeAnalysis.Testing;
     using Verifier = MoqCSharpAnalyzerVerifier<CallBackDelegateMustMatchMockedMethodAnalyzer>;
 
     public class CallBackDelegateMustMatchMockedMethodAnalyzerTest
@@ -104,35 +105,57 @@ public void TestMethod()
                             var mock1 = new Mock<I>();
 
                             mock1" + sequence + @".Setup(m => m.TestMethod())
-                                .Callback([|(int too, int much, int parameters)|] => { })
+                                .Callback({|PosInfoMoq2003:(int too, int much, int parameters)|} => { })
                                 .Throws(new Exception());
                             mock1" + sequence + @".Setup(m => m.TestMethod(default))
-                                .Callback([|()|] => { })
+                                .Callback({|PosInfoMoq2003:()|} => { })
                                 .Throws(new Exception());
                             mock1" + sequence + @".Setup(m => m.TestMethod(default))
-                                .Callback(([|int otherType|]) => { })
+                                .Callback(({|PosInfoMoq2003:int otherType|}) => { })
                                 .Throws(new Exception());
                             mock1" + sequence + @".Setup(m => m.TestMethod(default))
-                                .Callback([|(int too, int much, int parameters)|] => { })
+                                .Callback({|PosInfoMoq2003:(int too, int much, int parameters)|} => { })
                                 .Throws(new Exception());
                             mock1" + sequence + @".Setup(m => m.TestGenericMethod(1234))
-                                .Callback(([|string x|]) => { })
+                                .Callback(({|PosInfoMoq2003:string x|}) => { })
                                 .Throws(new Exception());
                             mock1" + sequence + @".Setup(m => m.TestGenericMethod(It.IsAny<It.IsAnyType>()))
-                                .Callback(([|string x|]) => { })
+                                .Callback(({|PosInfoMoq2003:string x|}) => { })
                                 .Throws(new Exception());
 
                             mock1" + sequence + @".Setup(m => m.TestMethodReturn())
-                                .Callback([|(int too, int much, int parameters)|] => { })
+                                .Callback({|PosInfoMoq2003:(int too, int much, int parameters)|} => { })
                                 .Returns(1234);
                             mock1" + sequence + @".Setup(m => m.TestMethodReturn(default))
-                                .Callback([|()|] => { })
+                                .Callback({|PosInfoMoq2003:()|} => { })
                                 .Returns(1234);
                             mock1" + sequence + @".Setup(m => m.TestMethodReturn(default))
-                                .Callback(([|int otherType|]) => { })
+                                .Callback(({|PosInfoMoq2003:int otherType|}) => { })
                                 .Returns(1234);
                             mock1" + sequence + @".Setup(m => m.TestMethodReturn(default))
-                                .Callback([|(int too, int much, int parameters)|] => { })
+                                .Callback({|PosInfoMoq2003:(int too, int much, int parameters)|} => { })
+                                .Returns(1234);
+
+                            // Callback with return value
+                            mock1" + sequence + @".Setup(m => m.TestMethodReturn())
+                                .Callback(() => { {|PosInfoMoq2014:return false;|} })
+                                .Returns(1234);
+                            mock1" + sequence + @".Setup(m => m.TestMethodReturn())
+                                .Callback(() =>
+                                {
+                                    var b = false;
+                                    if (b)
+                                    {
+                                        for (int i=0; i<10; i++)
+                                        {
+                                            if (i == 0) {|#0:return false;|};
+                                        }
+
+                                        {|#1:return false;|}
+                                    }
+
+                                    {|#2:return true;|}
+                                })
                                 .Returns(1234);
                         }
                     }
@@ -155,7 +178,12 @@ public interface I
                     }
                 }";
 
-            await Verifier.VerifyAnalyzerAsync(source);
+            await Verifier.VerifyAnalyzerAsync(
+                source,
+                [
+                    new DiagnosticResult(CallBackDelegateMustMatchMockedMethodAnalyzer.CallbackMustNotReturnValue)
+                        .WithSpan(59, 57, 59, 70).WithSpan(62, 41, 62, 54).WithSpan(65, 37, 65, 49),
+                ]);
         }
 
         [Fact]