diff --git a/PosInformatique.Moq.Analyzers.sln b/PosInformatique.Moq.Analyzers.sln index c4ca56b..5d9a6d4 100644 --- a/PosInformatique.Moq.Analyzers.sln +++ b/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}" diff --git a/README.md b/README.md index f84a756..12ab764 100644 --- a/README.md +++ b/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. | diff --git a/docs/Compilation/PosInfoMoq2014.md b/docs/Compilation/PosInfoMoq2014.md new file mode 100644 index 0000000..b935a48 --- /dev/null +++ b/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.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. diff --git a/src/Moq.Analyzers/AnalyzerReleases.Shipped.md b/src/Moq.Analyzers/AnalyzerReleases.Shipped.md index 2111b71..31e25bd 100644 --- a/src/Moq.Analyzers/AnalyzerReleases.Shipped.md +++ b/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 diff --git a/src/Moq.Analyzers/Analyzers/CallBackDelegateMustMatchMockedMethodAnalyzer.cs b/src/Moq.Analyzers/Analyzers/CallBackDelegateMustMatchMockedMethodAnalyzer.cs index 6205c65..59c34d1 100644 --- a/src/Moq.Analyzers/Analyzers/CallBackDelegateMustMatchMockedMethodAnalyzer.cs +++ b/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 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 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,7 +102,7 @@ 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; @@ -91,12 +110,38 @@ private static void Analyze(SyntaxNodeAnalysisContext context) } 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 returnStatements; + + private ReturnStatementSyntaxFinder() + { + this.returnStatements = new List(); + } + + public static IList 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); + } + } } } diff --git a/src/Moq.Analyzers/Moq.Analyzers.csproj b/src/Moq.Analyzers/Moq.Analyzers.csproj index a2d0d4d..d6fcc5c 100644 --- a/src/Moq.Analyzers/Moq.Analyzers.csproj +++ b/src/Moq.Analyzers/Moq.Analyzers.csproj @@ -17,6 +17,10 @@ https://github.com/PosInformatique/PosInformatique.Moq.Analyzers README.md + 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. diff --git a/src/Moq.Analyzers/SyntaxNodeAnalysisContextExtensions.cs b/src/Moq.Analyzers/SyntaxNodeAnalysisContextExtensions.cs index 76d28fa..3acc7ef 100644 --- a/src/Moq.Analyzers/SyntaxNodeAnalysisContextExtensions.cs +++ b/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 locations, params object[]? messageArgs) + { + var diagnostic = Diagnostic.Create(descriptor, locations.First(), locations.Skip(1), messageArgs); + context.ReportDiagnostic(diagnostic); + } } } diff --git a/tests/Moq.Analyzers.Tests/Analyzers/CallBackDelegateMustMatchMockedMethodAnalyzerTest.cs b/tests/Moq.Analyzers.Tests/Analyzers/CallBackDelegateMustMatchMockedMethodAnalyzerTest.cs index e57bf06..7316795 100644 --- a/tests/Moq.Analyzers.Tests/Analyzers/CallBackDelegateMustMatchMockedMethodAnalyzerTest.cs +++ b/tests/Moq.Analyzers.Tests/Analyzers/CallBackDelegateMustMatchMockedMethodAnalyzerTest.cs @@ -6,6 +6,7 @@ namespace PosInformatique.Moq.Analyzers.Tests { + using Microsoft.CodeAnalysis.Testing; using Verifier = MoqCSharpAnalyzerVerifier; public class CallBackDelegateMustMatchMockedMethodAnalyzerTest @@ -104,35 +105,57 @@ public void TestMethod() var mock1 = new Mock(); 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())) - .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]