GH-174 - analyzing usages of WithAnyArgs like methods

nsubstitute · Aug 26, 2022 · 3957171 · 3957171
1 parent 6053d65
commit 3957171
Show file tree

Hide file tree

Showing 62 changed files with 5,654 additions and 47 deletions.
diff --git a/Directory.Build.props b/Directory.Build.props
@@ -8,7 +8,7 @@
         <AdditionalFiles Include="$(SolutionDir)/StyleCop.json" />
     </ItemGroup>
     <ItemGroup>
-        <PackageReference Include="StyleCop.Analyzers" Version="1.1.0-beta009" PrivateAssets="all" />
+        <PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.435" PrivateAssets="all" />
     </ItemGroup>
     <PropertyGroup>
         <SignAssembly>true</SignAssembly>

diff --git a/NSubstitute.Analyzers.sln b/NSubstitute.Analyzers.sln
@@ -47,6 +47,7 @@ ProjectSection(SolutionItems) = preProject
 	documentation\rules\NS5001.md = documentation\rules\NS5001.md
 	documentation\rules\NS5001.md = documentation\rules\NS5002.md
 	documentation\rules\NS5001.md = documentation\rules\NS5003.md
+	documentation\rules\NS5001.md = documentation\rules\NS5004.md
 EndProjectSection
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "benchmarks", "benchmarks", "{1629DF5F-9BC0-49C0-975E-E45C3E58EB3A}"

diff --git a/...ute.Analyzers.Benchmarks.Source.CSharp/DiagnosticsSources/WithAnyArgsDiagnosticsSource.cs b/...ute.Analyzers.Benchmarks.Source.CSharp/DiagnosticsSources/WithAnyArgsDiagnosticsSource.cs
@@ -0,0 +1,40 @@
+using NSubstitute.Analyzers.Benchmarks.Source.CSharp.Models;
+
+namespace NSubstitute.Analyzers.Benchmarks.Source.CSharp.DiagnosticsSources
+{
+    public class WithAnyArgsDiagnosticsSource
+    {
+        public void NS5004_InvalidArgumentMatcherUsedWithAnyArgs()
+        {
+            var substitute = Substitute.For<IFoo>();
+
+            _ = substitute.DidNotReceiveWithAnyArgs()[Arg.Is(1)];
+            _ = substitute.DidNotReceiveWithAnyArgs()[Arg.Do<int>(_ => { })];
+            substitute.DidNotReceiveWithAnyArgs().IntReturningProperty = Arg.Is(1);
+            substitute.DidNotReceiveWithAnyArgs().IntReturningProperty = Arg.Do<int>(_ => { });
+            substitute.DidNotReceiveWithAnyArgs()
+                .ObjectReturningMethodWithArguments(Arg.Is(1), Arg.Is(1), Arg.Do<int>(_ => { }));
+
+            _ = substitute.DidNotReceiveWithAnyArgs()[Arg.Is(1)];
+            _ = substitute.DidNotReceiveWithAnyArgs()[Arg.Do<int>(_ => { })];
+            substitute.DidNotReceiveWithAnyArgs().IntReturningProperty = Arg.Is(1);
+            substitute.DidNotReceiveWithAnyArgs().IntReturningProperty = Arg.Do<int>(_ => { });
+            substitute.DidNotReceiveWithAnyArgs()
+                .ObjectReturningMethodWithArguments(Arg.Is(1), Arg.Is(1), Arg.Do<int>(_ => { }));
+
+            _ = SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute)[Arg.Is(1)];
+            _ = SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute)[Arg.Do<int>(_ => { })];
+            SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute).IntReturningProperty = Arg.Is(1);
+            SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute).IntReturningProperty = Arg.Do<int>(_ => { });
+            SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute)
+                .ObjectReturningMethodWithArguments(Arg.Is(1), Arg.Is(1), Arg.Do<int>(_ => { }));
+
+            _ = SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute)[Arg.Is(1)];
+            _ = SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute)[Arg.Do<int>(_ => { })];
+            SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute).IntReturningProperty = Arg.Is(1);
+            SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute).IntReturningProperty = Arg.Do<int>(_ => { });
+            SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute)
+                .ObjectReturningMethodWithArguments(Arg.Is(1), Arg.Is(1), Arg.Do<int>(_ => { }));
+        }
+    }
+}
diff --git a/benchmarks/NSubstitute.Analyzers.Benchmarks.Source.CSharp/Models/Foo.cs b/benchmarks/NSubstitute.Analyzers.Benchmarks.Source.CSharp/Models/Foo.cs
@@ -42,7 +42,7 @@ public Task<object> GenericTaskReturningAsyncMethod()
 
         public ConfiguredCall ConfiguredCallReturningProperty { get; }
 
-        public int IntReturningProperty { get; }
+        public int IntReturningProperty { get; set; }
 
         public IFoo ObjectReturningProperty { get; }
 

diff --git a/benchmarks/NSubstitute.Analyzers.Benchmarks.Source.CSharp/Models/IFoo.cs b/benchmarks/NSubstitute.Analyzers.Benchmarks.Source.CSharp/Models/IFoo.cs
@@ -18,7 +18,7 @@ public interface IFoo
 
         ConfiguredCall ConfiguredCallReturningProperty { get; }
 
-        int IntReturningProperty { get; }
+        int IntReturningProperty { get; set; }
 
         IFoo ObjectReturningProperty { get; }
 

diff --git a/...nalyzers.Benchmarks.Source.VisualBasic/DiagnosticsSources/WithAnyArgsDiagnosticsSource.vb b/...nalyzers.Benchmarks.Source.VisualBasic/DiagnosticsSources/WithAnyArgsDiagnosticsSource.vb
@@ -0,0 +1,54 @@
+Imports System
+Imports NSubstitute.Analyzers.Benchmarks.Source.VisualBasic.Models
+
+Namespace DiagnosticsSources
+    Public Class WithAnyArgsDiagnosticsSource
+        Public Sub NS5004_InvalidArgumentMatcherUsedWithAnyArgs()
+            Dim substitute = NSubstitute.Substitute.[For](Of IFoo)()
+            Dim fist = substitute.DidNotReceiveWithAnyArgs()(Arg.[Is](1))
+            Dim second = substitute.DidNotReceiveWithAnyArgs()(Arg.[Do](Of Integer)(Function(x)
+                                                                                        Throw New Exception
+                                                                                    End Function))
+            substitute.DidNotReceiveWithAnyArgs().IntReturningProperty = Arg.[Is](1)
+            substitute.DidNotReceiveWithAnyArgs().IntReturningProperty = Arg.[Do](Of Integer)(Function(x)
+                                                                                                    Throw New Exception
+                                                                                              End Function)
+            substitute.DidNotReceiveWithAnyArgs().ObjectReturningMethodWithArguments(Arg.[Is](1), Arg.[Is](1), Arg.[Do](Of Integer)(Function(x)
+                                                                                                                                            Throw New Exception
+                                                                                                                                    End Function))
+            Dim third = substitute.DidNotReceiveWithAnyArgs()(Arg.[Is](1))
+            Dim fourth = substitute.DidNotReceiveWithAnyArgs()(Arg.[Do](Of Integer)(Function(x)
+                                                                                        Throw New Exception()
+                                                                                    End Function))
+            substitute.DidNotReceiveWithAnyArgs().IntReturningProperty = Arg.[Is](1)
+            substitute.DidNotReceiveWithAnyArgs().IntReturningProperty = Arg.[Do](Of Integer)(Function(x)
+                                                                                                Throw New Exception()
+                                                                                              End Function)
+            substitute.DidNotReceiveWithAnyArgs().ObjectReturningMethodWithArguments(Arg.[Is](1), Arg.[Is](1), Arg.[Do](Of Integer)(Function(x)
+                                                                                                                                            Throw New Exception()
+                                                                                                                                    End Function))
+            Dim fifth = SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute)(Arg.[Is](1))
+            Dim sixth = SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute)(Arg.[Do](Of Integer)(Function(x)
+                                                                                                        Throw New Exception()
+                                                                                                       End Function))
+            SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute).IntReturningProperty = Arg.[Is](1)
+            SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute).IntReturningProperty = Arg.[Do](Of Integer)(Function(x)
+                                                                                                                    Throw New Exception()
+                                                                                                                  End Function)
+            SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute).ObjectReturningMethodWithArguments(Arg.[Is](1), Arg.[Is](1), Arg.[Do](Of Integer)(Function(x)
+                                                                                                                                                                Throw New Exception()
+                                                                                                                                                        End Function))
+            Dim seventh = SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute)(Arg.[Is](1))
+            Dim eigth = SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute)(Arg.[Do](Of Integer)(Function(x)
+                                                                                                        Throw New Exception()
+                                                                                                       End Function))
+            SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute).IntReturningProperty = Arg.[Is](1)
+            SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute).IntReturningProperty = Arg.[Do](Of Integer)(Function(x)
+                                                                                                                    Throw New Exception()
+                                                                                                                  End Function)
+            SubstituteExtensions.DidNotReceiveWithAnyArgs(substitute).ObjectReturningMethodWithArguments(Arg.[Is](1), Arg.[Is](1), Arg.[Do](Of Integer)(Function(x)
+                                                                                                                                                                Throw New Exception()
+                                                                                                                                                        End Function))
+        End Sub
+    End Class
+End Namespace
diff --git a/benchmarks/NSubstitute.Analyzers.Benchmarks.Source.VisualBasic/Models/Foo.vb b/benchmarks/NSubstitute.Analyzers.Benchmarks.Source.VisualBasic/Models/Foo.vb
@@ -62,7 +62,7 @@ Namespace Models
             End Get
         End Property
 
-        Public ReadOnly Property IntReturningProperty As Integer Implements IFoo.IntReturningProperty
+        Public Property IntReturningProperty As Integer Implements IFoo.IntReturningProperty
         Public ReadOnly Property ObjectReturningProperty As IFoo Implements IFoo.ObjectReturningProperty
 
         Default Public ReadOnly Property Item(a As Integer) As Integer Implements IFoo.Item

diff --git a/benchmarks/NSubstitute.Analyzers.Benchmarks.Source.VisualBasic/Models/IFoo.vb b/benchmarks/NSubstitute.Analyzers.Benchmarks.Source.VisualBasic/Models/IFoo.vb
@@ -16,7 +16,7 @@ Namespace Models
 
         ReadOnly Property ConfiguredCallReturningProperty As ConfiguredCall
 
-        ReadOnly Property IntReturningProperty As Integer
+        Property IntReturningProperty As Integer
 
         ReadOnly Property ObjectReturningProperty As IFoo
 

diff --git a/benchmarks/NSubstitute.Analyzers.Benchmarks/CSharp/CSharpDiagnosticAnalyzersBenchmarks.cs b/benchmarks/NSubstitute.Analyzers.Benchmarks/CSharp/CSharpDiagnosticAnalyzersBenchmarks.cs
@@ -33,6 +33,8 @@ public class CSharpDiagnosticAnalyzersBenchmarks : AbstractDiagnosticAnalyzersBe
 
     protected override AnalyzerBenchmark SyncOverAsyncThrowsAnalyzerBenchmark { get; }
 
+    protected override AnalyzerBenchmark WithAnyArgsAnalyzerBenchmark { get; }
+
     protected override AbstractSolutionLoader SolutionLoader { get; }
 
     protected override string SourceProjectFolderName { get; }
@@ -58,5 +60,6 @@ public CSharpDiagnosticAnalyzersBenchmarks()
         ReceivedInReceivedInOrderAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new ReceivedInReceivedInOrderAnalyzer());
         AsyncReceivedInOrderCallbackAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new AsyncReceivedInOrderCallbackAnalyzer());
         SyncOverAsyncThrowsAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new SyncOverAsyncThrowsAnalyzer());
-        }
+        WithAnyArgsAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new WithAnyArgsArgumentMatcherAnalyzer());
+    }
 }
diff --git a/benchmarks/NSubstitute.Analyzers.Benchmarks/Shared/AbstractDiagnosticAnalyzersBenchmarks.cs b/benchmarks/NSubstitute.Analyzers.Benchmarks/Shared/AbstractDiagnosticAnalyzersBenchmarks.cs
@@ -40,6 +40,8 @@ public abstract class AbstractDiagnosticAnalyzersBenchmarks
 
     protected abstract AnalyzerBenchmark SyncOverAsyncThrowsAnalyzerBenchmark { get; }
 
+    protected abstract AnalyzerBenchmark WithAnyArgsAnalyzerBenchmark { get; }
+
     protected abstract AbstractSolutionLoader SolutionLoader { get; }
 
     protected abstract string SourceProjectFolderName { get; }

diff --git a/.../NSubstitute.Analyzers.Benchmarks/VisualBasic/VisualBasicDiagnosticAnalyzersBenchmarks.cs b/.../NSubstitute.Analyzers.Benchmarks/VisualBasic/VisualBasicDiagnosticAnalyzersBenchmarks.cs
@@ -33,6 +33,8 @@ public class VisualBasicDiagnosticAnalyzersBenchmarks : AbstractDiagnosticAnalyz
 
     protected override AnalyzerBenchmark SyncOverAsyncThrowsAnalyzerBenchmark { get; }
 
+    protected override AnalyzerBenchmark WithAnyArgsAnalyzerBenchmark { get; }
+
     protected override AbstractSolutionLoader SolutionLoader { get; }
 
     protected override string SourceProjectFolderName { get; }
@@ -58,5 +60,6 @@ public VisualBasicDiagnosticAnalyzersBenchmarks()
         ReceivedInReceivedInOrderAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new ReceivedInReceivedInOrderAnalyzer());
         AsyncReceivedInOrderCallbackAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new AsyncReceivedInOrderCallbackAnalyzer());
         SyncOverAsyncThrowsAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new SyncOverAsyncThrowsAnalyzer());
+        WithAnyArgsAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new WithAnyArgsArgumentMatcherAnalyzer());
     }
 }
diff --git a/documentation/rules/NS5004.md b/documentation/rules/NS5004.md
@@ -0,0 +1,52 @@
+# NS5004
+
+<table>
+<tr>
+  <td>CheckId</td>
+  <td>NS5004</td>
+</tr>
+<tr>
+  <td>Category</td>
+  <td>Usage</td>
+</tr>
+</table>
+
+## Cause
+
+Argument matcher used with WithAnyArgs. This matcher may not be executed.
+
+## Rule description
+
+A violation of this rule occurs when `ReturnsForAnyArgs`,`ReturnsNullForAnyArgs`,`ThrowsForAnyArgs`,`ThrowsAsyncForAnyArgs`, `WhenForAnyArgs`, `DidNotReceiveWithAnyArgs` or `ReceivedWithAnyArgs` is used in combination with arg matchers other than `Arg.Any`.
+
+## How to fix violations
+
+To fix a violation of this rule, replace arg matchers used in aforementioned methods with `Arg.Any`
+
+For example:
+
+````c#
+// Incorrect:
+sub.DidNotReceiveWithAnyArgs().Bar(Arg.Is(1));
+
+// Correct:
+sub.DidNotReceiveWithAnyArgs().Bar(Arg.Any<int>());
+
+// The following are also correct. The exact arguments will not be checked due to *WithAnyArgs.
+sub.DidNotReceiveWithAnyArgs().Bar(default(int));
+sub.DidNotReceiveWithAnyArgs().Bar(123);
+````
+
+## How to suppress violations
+
+This warning can be suppressed by disabling the warning in the **ruleset** file for the project.
+The warning can also be suppressed programmatically for an assembly:
+````c#
+[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "NS5004:Argument matcher used with WithAnyArgs. This matcher may not be executed..", Justification = "Reviewed")]
+````
+
+Or for a specific code block:
+````c#
+#pragma warning disable NS5004 // Argument matcher used with WithAnyArgs. This matcher may not be executed..
+// the code which produces warning
+#pragma warning restore NS5004 // Argument matcher used with WithAnyArgs. This matcher may not be executed..
diff --git a/documentation/rules/README.md b/documentation/rules/README.md
@@ -29,3 +29,4 @@
 | [NS5001](NS5001.md) | Usage | Usage of received-like method in `Received.InOrder` callback. |
 | [NS5002](NS5002.md) | Usage | Usage of async callback in `Received.InOrder` method. |
 | [NS5003](NS5003.md) | Usage | Sync `Throws` used in async method. |
+| [NS5004](NS5004.md) | Usage | Argument matcher used with WithAnyArgs method. |
diff --git a/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/SubstitutionNodeFinder.cs b/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/SubstitutionNodeFinder.cs
@@ -25,6 +25,7 @@ protected override SyntaxNode GetSubstitutionActualNode(SyntaxNodeAnalysisContex
         return syntaxNode.GetSubstitutionActualNode(node => syntaxNodeContext.SemanticModel.GetSymbolInfo(node).Symbol);
     }
 
+    // TODO replace with SyntaxVisitor or OperationVisitor
     protected override IEnumerable<SyntaxNode> FindInvocations(SyntaxNodeAnalysisContext syntaxNodeContext, SyntaxNode argumentSyntax)
     {
         SyntaxNode body = null;

diff --git a/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/WithAnyArgsArgumentMatcherAnalyzer.cs b/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/WithAnyArgsArgumentMatcherAnalyzer.cs
@@ -0,0 +1,26 @@
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using NSubstitute.Analyzers.Shared.DiagnosticAnalyzers;
+
+namespace NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+internal sealed class WithAnyArgsArgumentMatcherAnalyzer : AbstractWithAnyArgsArgumentMatcherAnalyzer<SyntaxKind, InvocationExpressionSyntax>
+{
+    internal static ImmutableHashSet<int> MaybeAllowedAncestors { get; } = ImmutableHashSet.Create(
+        (int)SyntaxKind.InvocationExpression,
+        (int)SyntaxKind.ElementAccessExpression,
+        (int)SyntaxKind.SimpleAssignmentExpression);
+
+    public WithAnyArgsArgumentMatcherAnalyzer()
+        : base(CSharp.DiagnosticDescriptorsProvider.Instance, SubstitutionNodeFinder.Instance)
+    {
+    }
+
+    protected override ImmutableHashSet<int> MaybeAllowedArgMatcherAncestors { get; } = MaybeAllowedAncestors;
+
+    protected override SyntaxKind InvocationExpressionKind { get; } = SyntaxKind.InvocationExpression;
+}
diff --git a/src/NSubstitute.Analyzers.Shared/AbstractDiagnosticDescriptorsProvider.cs b/src/NSubstitute.Analyzers.Shared/AbstractDiagnosticDescriptorsProvider.cs
@@ -52,6 +52,8 @@ internal class AbstractDiagnosticDescriptorsProvider<T> : IDiagnosticDescriptors
 
     public DiagnosticDescriptor NonSubstitutableMemberArgumentMatcherUsage { get; } = DiagnosticDescriptors<T>.NonSubstitutableMemberArgumentMatcherUsage;
 
+    public DiagnosticDescriptor WithAnyArgsArgumentMatcherUsage { get; } = DiagnosticDescriptors<T>.WithAnyArgsArgumentMatcherUsage;
+
     public DiagnosticDescriptor ReceivedUsedInReceivedInOrder { get; } = DiagnosticDescriptors<T>.ReceivedUsedInReceivedInOrder;
 
     public DiagnosticDescriptor AsyncCallbackUsedInReceivedInOrder { get; } = DiagnosticDescriptors<T>.AsyncCallbackUsedInReceivedInOrder;

diff --git a/...ubstitute.Analyzers.Shared/CodeFixProviders/AbstractSyncOverAsyncThrowsCodeFixProvider.cs b/...ubstitute.Analyzers.Shared/CodeFixProviders/AbstractSyncOverAsyncThrowsCodeFixProvider.cs
@@ -93,7 +93,7 @@ private async Task<SyntaxNode> CreateThrowsAsyncInvocationExpression(
         CodeFixContext context)
     {
         var updatedMethodName =
-            invocationSymbol.IsThrowsSyncForAnyArgsMethod()
+            invocationSymbol.IsThrowsSyncMethod()
                 ? MetadataNames.NSubstituteThrowsAsyncMethod
                 : MetadataNames.NSubstituteThrowsAsyncForAnyArgsMethod;
 
@@ -120,7 +120,7 @@ private async Task<SyntaxNode> CreateReturnInvocationExpression(
             CreateFromExceptionInvocationExpression(syntaxGenerator, invocationOperation);
 
         var returnsMethodName =
-            invocationSymbol.IsThrowsSyncForAnyArgsMethod() ? "Returns" : "ReturnsForAnyArgs";
+            invocationSymbol.IsThrowsSyncMethod() ? "Returns" : "ReturnsForAnyArgs";
 
         if (invocationSymbol.MethodKind == MethodKind.Ordinary)
         {
@@ -213,12 +213,12 @@ private static string GetReplacementMethodName(IMethodSymbol methodSymbol, bool
     {
         if (useModernSyntax)
         {
-            return methodSymbol.IsThrowsSyncForAnyArgsMethod()
+            return methodSymbol.IsThrowsSyncMethod()
                 ? MetadataNames.NSubstituteThrowsAsyncMethod
                 : MetadataNames.NSubstituteThrowsAsyncForAnyArgsMethod;
         }
 
-        return methodSymbol.IsThrowsSyncForAnyArgsMethod()
+        return methodSymbol.IsThrowsSyncMethod()
             ? MetadataNames.NSubstituteReturnsMethod
             : MetadataNames.NSubstituteReturnsForAnyArgsMethod;
     }