v1.8.0 (#26)

* Add the PosInfoMoq1003 and PosInfoMoq1004 rules implementation.

.github/workflows/github-actions-release.yml

@@ -7,7 +7,7 @@ on:
         type: string
         description: The version of the library
         required: true
-        default: 1.7.0
+        default: 1.8.0
       VersionSuffix:
         type: string
         description: The version suffix of the library (for example rc.1)

PosInformatique.Moq.Analyzers.sln

@@ -31,6 +31,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Design", "Design", "{815BE8
 		docs\design\PosInfoMoq1000.md = docs\design\PosInfoMoq1000.md
 		docs\design\PosInfoMoq1001.md = docs\design\PosInfoMoq1001.md
 		docs\Design\PosInfoMoq1002.md = docs\Design\PosInfoMoq1002.md
+		docs\Design\PosInfoMoq1003.md = docs\Design\PosInfoMoq1003.md
+		docs\Design\PosInfoMoq1004.md = docs\Design\PosInfoMoq1004.md
 	EndProjectSection
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Compilation", "Compilation", "{D9C84D36-7F9C-4EFB-BE6F-9F7A05FE957D}"

README.md

@@ -29,6 +29,8 @@ Design rules used to make your unit tests more strongly strict.
 | [PosInfoMoq1000: `VerifyAll()` methods should be called when instantiate a `Mock<T>` instances](docs/Design/PosInfoMoq1000.md) | When instantiating a `Mock<T>` in the *Arrange* phase of an unit test, `VerifyAll()` method should be called in the *Assert* phase to check the setup methods has been called. |
 | [PosInfoMoq1001: The `Mock<T>` instance behavior should be defined to Strict mode](docs/Design/PosInfoMoq1001.md) | When instantiating a `Mock<T>` instance, the `MockBehavior` of the `Mock` instance should be defined to `Strict`. |
 | [PosInfoMoq1002: `Verify()` methods should be called when `Verifiable()` has been setup](docs/Design/PosInfoMoq1002.md) | When a mocked member has been setup with the `Verifiable()` method, the `Verify()` method must be called at the end of the unit test. |
+| [PosInfoMoq1003: The `Callback()` method should be used to check the parameters when mocking a method with `It.IsAny<T>()` arguments](docs/Design/PosInfoMoq1003.md) | When a mocked method contains a `It.IsAny<T>()` argument, the related parameter should be checked in the `Callback()` method. |
+| [PosInfoMoq1004: The `Callback()` parameter should not be ignored if it has been setup as an `It.IsAny<T>()` argument](docs/Design/PosInfoMoq1004.md) | When a mocked method contains a `It.IsAny<T>()` argument, the related parameter should not be ignored in the `Callback()` method. |
 
 ### Compilation
 

docs/Design/PosInfoMoq1000.md

@@ -19,13 +19,13 @@ should be called in the *Assert* phase to check the setup methods has been calle
 
 ```csharp
 [Fact]
-public void GetCustomer_ShouldCallRepository()
+public void SendMail_ShouldCallSmtpService()
 {
 	// Arrange
 	var smtpService = new Mock<ISmtpService>();
 	smtpService.Setup(s => s.SendMail("[email protected]", "Gilles"));
 
-	var service = new CustomerService(repository.Object);
+	var service = new CustomerService(smtpService.Object);
 
 	// Act
 	service.SendMail("Gilles");

docs/Design/PosInfoMoq1002.md

@@ -19,14 +19,14 @@ In the *Arrange* phase of an unit test, when `Verifiable()` method has been setu
 
 ```csharp
 [Fact]
-public void GetCustomer_ShouldCallRepository()
+public void SendMail_ShouldCallSmtpService()
 {
 	// Arrange
 	var smtpService = new Mock<ISmtpService>();
 	smtpService.Setup(s => s.SendMail("[email protected]", "Gilles"))
 		.Verifiable();
 
-	var service = new CustomerService(repository.Object);
+	var service = new CustomerService(smtpService.Object);
 
 	// Act
 	service.SendMail("Gilles");

docs/Design/PosInfoMoq1003.md

@@ -0,0 +1,93 @@
+# PosInfoMoq1003: The `Callback()` method should be used to check the parameters when mocking a method with `It.IsAny<T>()` arguments
+
+| Property                            | Value                                                                  												|
+|-------------------------------------|---------------------------------------------------------------------------------------------------------------------|
+| **Rule ID**                         | PosInfoMoq1003                                                         												|
+| **Title**                           | The `Callback()` method should be used to check the parameters when mocking a method with `It.IsAny<T>()` arguments	|
+| **Category**                        | Design																												|
+| **Default severity**				  | Warning																												|
+
+## Cause
+
+A method has been setup with `It.IsAny<T>()` arguments without checking the parameters in a `Callback()`
+method.
+
+## Rule description
+
+When setup a method using `It.IsAny<T>()` arguments, the parameters should be check in the `Callback()` method.
+
+For example if we have the following code to test:
+
+```csharp
+[Fact]
+public class CustomerService
+{
+	private readonly ISmtpService smtpService;
+
+	public CustomerService(ISmtpService smtpService)
+	{
+		this.smtpService = smtpService;
+	}
+
+	public void SendMail(string emailAddress)
+	{
+		this.smtpService.SendMail("[email protected]", emailAddress);
+	}
+}
+```
+
+If we mock the `ISmtpService.SendMail()` with a `It.IsAny<string>()` for the `emailAddress` argument,
+we can not check if the `CustomerService.SendMail()` has propagate correctly the value of the argument to the
+parameter of the `ISmtpService.SendMail()` method.
+
+```csharp
+[Fact]
+public void SendMail_ShouldCallSmtpService()
+{
+	var smtpService = new Mock<ISmtpService>();
+	smtpService.Setup(s => s.SendMail("[email protected]", It.IsAny<string>()));	// With It.IsAny<string>() we can not check that emailAddress has been correctly passed in the CustomerService.SendMail() method.
+
+	var service = new CustomerService(smtpService.Object);
+
+	service.SendMail("Gilles");
+}
+```
+
+The `emailAddress` parameter passed to the `ISmtpService.SendMail()` method should be tested
+with the `Callback()` method, when mocking the `ISmtpService.SendMail()` method with a `It.IsAny<T>()` argument.
+
+```csharp
+[Fact]
+public void SendMail_ShouldCallSmtpService()
+{
+	var smtpService = new Mock<ISmtpService>();
+	smtpService.Setup(s => s.SendMail("[email protected]", It.IsAny<string>()))		// With It.IsAny() we should test the arguments if correctly propagated in the Callback() method.
+		.Callback((string _, string emailAddress) =>
+		{
+			Assert.AreEqual("Gilles", em);	// Check the emailAddress parameter.
+		});
+
+	var service = new CustomerService(smtpService.Object);
+
+	service.SendMail("Gilles");
+}
+```
+
+### Remarks
+- If the parameters of mocked methods are very simple (primitive values for example), pass directly the expected value in the arguments of the method. For example
+  ```csharp
+  smtpService.Setup(s => s.SendMail("[email protected]", "Gilles"))
+  ```
+  Instead of
+  ```csharp
+  smtpService.Setup(s => s.SendMail("[email protected]", It.IsAny<string>()))
+  ```
+- Use the `Callback()` to assert complex parameters.
+
+## How to fix violations
+
+To fix a violation of this rule, use the `Callback()` method to check the `It.IsAny<T>()` arguments.
+
+## When to suppress warnings
+
+Do not suppress a warning from this rule. Normally `It.IsAny<T>()` arguments should be check and asserted in the `Callback()` methods.

docs/Design/PosInfoMoq1004.md

@@ -0,0 +1,93 @@
+# PosInfoMoq1004: The `Callback()` parameter should not be ignored if it has been setup as an `It.IsAny<T>()` argument.
+
+| Property                            | Value                                                                                                |
+|-------------------------------------|------------------------------------------------------------------------------------------------------|
+| **Rule ID**                         | PosInfoMoq1004                                                                                       |
+| **Title**                           | The `Callback()` parameter should not be ignored if it has been setup as an `It.IsAny<T>()` argument. |
+| **Category**                        | Design																                                 |
+| **Default severity**				  | Warning																                                 |
+
+## Cause
+
+A method has been setup with `It.IsAny<T>()` arguments and the parameter in the `Callback()` has not be used.
+
+## Rule description
+
+When setup a method using `It.IsAny<T>()` arguments, the parameters should be check in the `Callback()` method.
+
+For example if we have the following code to test:
+
+```csharp
+[Fact]
+public class CustomerService
+{
+	private readonly ISmtpService smtpService;
+
+	public CustomerService(ISmtpService smtpService)
+	{
+		this.smtpService = smtpService;
+	}
+
+	public void SendMail(string emailAddress)
+	{
+		this.smtpService.SendMail("[email protected]", emailAddress);
+	}
+}
+```
+
+If we mock the `ISmtpService.SendMail()` with a `It.IsAny<string>()` for the `emailAddress` argument,
+we can not check if the `CustomerService.SendMail()` has propagate correctly the value of the argument to the
+parameter of the `ISmtpService.SendMail()` method.
+
+```csharp
+[Fact]
+public void SendMail_ShouldCallSmtpService()
+{
+	var smtpService = new Mock<ISmtpService>();
+	smtpService.Setup(s => s.SendMail("[email protected]", It.IsAny<string>()))
+		.Callback((string _, string _) => { ... });		// The second parameter (emailAddress) should not be ignored and should be tested. 
+
+	var service = new CustomerService(smtpService.Object);
+
+	service.SendMail("Gilles");
+}
+```
+
+The `emailAddress` parameter passed to the `ISmtpService.SendMail()` method should be tested
+with the `Callback()` method, when mocking the `ISmtpService.SendMail()` method with a `It.IsAny<T>()` argument.
+
+```csharp
+[Fact]
+public void SendMail_ShouldCallSmtpService()
+{
+	var smtpService = new Mock<ISmtpService>();
+	smtpService.Setup(s => s.SendMail("[email protected]", It.IsAny<string>()))		// With It.IsAny() we should test the arguments if correctly propagated in the Callback() method.
+		.Callback((string _, string emailAddress) =>
+		{
+			Assert.AreEqual("Gilles", em);	// Check the emailAddress parameter.
+		});
+
+	var service = new CustomerService(smtpService.Object);
+
+	service.SendMail("Gilles");
+}
+```
+
+### Remarks
+- If the parameters of mocked methods are very simple (primitive values for example), pass directly the expected value in the arguments of the method. For example
+  ```csharp
+  smtpService.Setup(s => s.SendMail("[email protected]", "Gilles"))
+  ```
+  Instead of
+  ```csharp
+  smtpService.Setup(s => s.SendMail("[email protected]", It.IsAny<string>()))
+  ```
+- Use the `Callback()` to assert complex parameters.
+
+## How to fix violations
+
+To fix a violation of this rule, use the `Callback()` method to check the `It.IsAny<T>()` arguments.
+
+## When to suppress warnings
+
+Do not suppress a warning from this rule. Normally `It.IsAny<T>()` arguments should be check and asserted in the `Callback()` methods.

src/Moq.Analyzers/AnalyzerReleases.Shipped.md

@@ -1,4 +1,13 @@
-## Release 1.7.0
+## Release 1.8.0
+
+### New Rules
+
+Rule ID | Category | Severity | Notes
+--------|----------|----------|--------------------
+PosInfoMoq1003 | Design | Warning | The `Callback()` method should be used to check the parameters when mocking a method with `It.IsAny<T>()` arguments.
+PosInfoMoq1004 | Design | Warning | The `Callback()` parameter should not be ignored if it has been setup as an `It.IsAny<T>()` argument.
+
+## Release 1.7.0
 
 ### New Rules
 

src/Moq.Analyzers/Analyzers/CallBackDelegateMustMatchMockedMethodAnalyzer.cs

@@ -46,15 +46,7 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
                 return;
             }
 
-            // Try to determine if the invocation expression is a Callback() expression.
-            var methodSymbol = context.SemanticModel.GetSymbolInfo(invocationExpression, context.CancellationToken);
-
-            if (!moqSymbols.IsCallback(methodSymbol.Symbol))
-            {
-                return;
-            }
-
-            // If yes, we extract the lambda expression of it.
+            // Extract the lambda expression.
             var moqExpressionAnalyzer = new MoqExpressionAnalyzer(moqSymbols, context.SemanticModel);
 
             var callBackLambdaExpressionSymbol = moqExpressionAnalyzer.ExtractCallBackLambdaExpressionMethod(invocationExpression, out var lambdaExpression, context.CancellationToken);

src/Moq.Analyzers/Analyzers/CallBackDelegateParametersShouldNotBeIgnoredAnalyzer.cs

@@ -0,0 +1,104 @@
+//-----------------------------------------------------------------------
+// <copyright file="CallBackDelegateParametersShouldNotBeIgnoredAnalyzer.cs" company="P.O.S Informatique">
+//     Copyright (c) P.O.S Informatique. All rights reserved.
+// </copyright>
+//-----------------------------------------------------------------------
+
+namespace PosInformatique.Moq.Analyzers
+{
+    using System.Collections.Immutable;
+    using Microsoft.CodeAnalysis;
+    using Microsoft.CodeAnalysis.CSharp;
+    using Microsoft.CodeAnalysis.CSharp.Syntax;
+    using Microsoft.CodeAnalysis.Diagnostics;
+
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public class CallBackDelegateParametersShouldNotBeIgnoredAnalyzer : DiagnosticAnalyzer
+    {
+        internal static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(
+            "PosInfoMoq1004",
+            "The Callback() parameter should not be ignored if it has been setup as an It.IsAny<T>() argument",
+            "The '{0}' parameter should not be ignored if it has been setup as an It.IsAny<T>() argument",
+            "Design",
+            DiagnosticSeverity.Warning,
+            isEnabledByDefault: true,
+            description: "The Callback() parameter should not be ignored if it has been setup as an It.IsAny<T>() argument.",
+            helpLinkUri: "https://posinformatique.github.io/PosInformatique.Moq.Analyzers/docs/Design/PosInfoMoq1004.html");
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+
+        public override void Initialize(AnalysisContext context)
+        {
+            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+            context.EnableConcurrentExecution();
+
+            context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression);
+        }
+
+        private static void Analyze(SyntaxNodeAnalysisContext context)
+        {
+            var invocationExpression = (InvocationExpressionSyntax)context.Node;
+
+            var moqSymbols = MoqSymbols.FromCompilation(context.Compilation);
+
+            if (moqSymbols is null)
+            {
+                return;
+            }
+
+            var moqExpressionAnalyzer = new MoqExpressionAnalyzer(moqSymbols, context.SemanticModel);
+
+            // Extracts the Setup() linked method.
+            var setupMethod = moqExpressionAnalyzer.ExtractSetupMethod(invocationExpression, context.CancellationToken);
+
+            if (setupMethod is null)
+            {
+                return;
+            }
+
+            // Check there is It.Any<T>() parameters.
+            var itIsAnyArguments = new Dictionary<int, ChainInvocationArgument>();
+
+            for (var i = 0; i < setupMethod.InvocationArguments.Count; i++)
+            {
+                var argument = setupMethod.InvocationArguments[i];
+
+                if (moqSymbols.IsItIsAny(argument.Symbol))
+                {
+                    // The Callback() method is required for the argument, add in the list.
+                    itIsAnyArguments.Add(i, argument);
+                }
+            }
+
+            if (itIsAnyArguments.Any())
+            {
+                // Here, it is mean Setup() method has been defined with some It.IsAny<T>() parameters.
+                // Extracts the Callback() method (if exists).
+                var callbackMethod = moqExpressionAnalyzer.ExtractCallBackLambdaExpressionMethod(invocationExpression, out var lambdaExpression, context.CancellationToken);
+
+                if (callbackMethod is null)
+                {
+                    return;
+                }
+
+                // Check each parameter of the Callback() method.
+                for (var i = 0; i < callbackMethod.Parameters.Length; i++)
+                {
+                    if (!itIsAnyArguments.TryGetValue(i, out var itIsAnyArgument))
+                    {
+                        // The parameter in the Callback() method is not related to a It.IsAny<T>() expression.
+                        continue;
+                    }
+
+                    if (callbackMethod.Parameters[i].Name == "_")
+                    {
+                        // Raise warning for the parameter which is not used.
+                        var parameterName = setupMethod.InvocationArguments[i].ParameterSymbol.Name;
+
+                        context.ReportDiagnostic(Rule, lambdaExpression!.ParameterList.Parameters[i].GetLocation(), parameterName);
+                    }
+                }
+            }
+        }
+    }
+}