[GH-135] - Detecting received like methods used in Received.InOrder c…

…allback block(#136)

NSubstitute.Analyzers.sln

@@ -43,6 +43,7 @@ ProjectSection(SolutionItems) = preProject
 	documentation\rules\NS3006.md = documentation\rules\NS3006.md
 	documentation\rules\NS4000.md = documentation\rules\NS4000.md
 	documentation\rules\NS5000.md = documentation\rules\NS5000.md
+	documentation\rules\NS5001.md = documentation\rules\NS5001.md
 EndProjectSection
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "benchmarks", "benchmarks", "{1629DF5F-9BC0-49C0-975E-E45C3E58EB3A}"

benchmarks/NSubstitute.Analyzers.Benchmarks.Source.CSharp/DiagnosticsSources/ReceivedLikeUsedInReceivedInOrderDiagnosticSource.cs

@@ -0,0 +1,22 @@
+using NSubstitute.Analyzers.Benchmarks.Source.CSharp.Models;
+
+namespace NSubstitute.Analyzers.Benchmarks.Source.CSharp.DiagnosticsSources
+{
+    public class ReceivedLikeUsedInReceivedInOrderDiagnosticSource
+    {
+        public void NS5001_ReceivedLikeUsedInReceivedInOrderCallback()
+        {
+            var substitute = Substitute.For<Foo>();
+            Received.InOrder(() =>
+            {
+                substitute.Received().ObjectReturningMethod();
+                _ = substitute.Received().InternalObjectReturningProperty;
+                _ = substitute.Received()[0];
+
+                SubstituteExtensions.Received(substitute).ObjectReturningMethod();
+                _ = SubstituteExtensions.Received(substitute).InternalObjectReturningProperty;
+                _ = SubstituteExtensions.Received(substitute)[0];
+            });
+        }
+    }
+}

benchmarks/NSubstitute.Analyzers.Benchmarks.Source.VisualBasic/DiagnosticsSources/ReceivedLikeUsedInReceivedInOrderDiagnosticSource.vb

@@ -0,0 +1,18 @@
+Imports NSubstitute.Analyzers.Benchmarks.Source.VisualBasic.Models
+
+Namespace DiagnosticsSources
+    Public Class ReceivedLikeUsedInReceivedInOrderDiagnosticSource
+        Public Sub NS5001_ReceivedLikeUsedInReceivedInOrderCallback()
+            Dim substitute = NSubstitute.Substitute.[For](Of Foo)()
+            NSubstitute.Received.InOrder(Function()
+                substitute.Received().ObjectReturningMethod()
+                Dim x = substitute.Received().InternalObjectReturningProperty
+                Dim y = substitute.Received()(0)
+                SubstituteExtensions.Received(substitute).ObjectReturningMethod()
+                Dim a = SubstituteExtensions.Received(substitute).InternalObjectReturningProperty
+                Dim b = SubstituteExtensions.Received(substitute)(0)
+                Throw New System.Exception()
+            End Function)
+        End Sub
+    End Class
+End Namespace

benchmarks/NSubstitute.Analyzers.Benchmarks/CSharp/CSharpDiagnosticAnalyzersBenchmarks.cs

@@ -25,6 +25,8 @@ public class CSharpDiagnosticAnalyzersBenchmarks : AbstractDiagnosticAnalyzersBe
 
         protected override AnalyzerBenchmark ArgumentMatcherAnalyzerBenchmark { get; }
 
+        protected override AnalyzerBenchmark ReceivedInReceivedInOrderAnalyzerBenchmark { get; }
+
         protected override AbstractSolutionLoader SolutionLoader { get; }
 
         protected override string SourceProjectFolderName { get; }
@@ -46,6 +48,7 @@ public CSharpDiagnosticAnalyzersBenchmarks()
             SubstituteAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new SubstituteAnalyzer());
             UnusedReceivedAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new UnusedReceivedAnalyzer());
             ArgumentMatcherAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new NonSubstitutableMemberArgumentMatcherAnalyzer());
+            ReceivedInReceivedInOrderAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new ReceivedInReceivedInOrderAnalyzer());
         }
     }
 }

benchmarks/NSubstitute.Analyzers.Benchmarks/Shared/AbstractDiagnosticAnalyzersBenchmarks.cs

@@ -33,6 +33,8 @@ public abstract class AbstractDiagnosticAnalyzersBenchmarks
 
         protected abstract AnalyzerBenchmark ArgumentMatcherAnalyzerBenchmark { get; }
 
+        protected abstract AnalyzerBenchmark ReceivedInReceivedInOrderAnalyzerBenchmark { get; }
+
         protected abstract AbstractSolutionLoader SolutionLoader { get; }
 
         protected abstract string SourceProjectFolderName { get; }
@@ -100,6 +102,12 @@ public void ArgumentMatcherAnalyzer()
             ArgumentMatcherAnalyzerBenchmark.Run();
         }
 
+        [Benchmark]
+        public void ReceivedInReceivedInOrderAnalyzer()
+        {
+            ReceivedInReceivedInOrderAnalyzerBenchmark.Run();
+        }
+
         [IterationCleanup(Target = nameof(ArgumentMatcherAnalyzer))]
         public void CleanUp()
         {

benchmarks/NSubstitute.Analyzers.Benchmarks/VisualBasic/VisualBasicDiagnosticAnalyzersBenchmarks.cs

@@ -25,6 +25,8 @@ public class VisualBasicDiagnosticAnalyzersBenchmarks : AbstractDiagnosticAnalyz
 
         protected override AnalyzerBenchmark ArgumentMatcherAnalyzerBenchmark { get; }
 
+        protected override AnalyzerBenchmark ReceivedInReceivedInOrderAnalyzerBenchmark { get; }
+
         protected override AbstractSolutionLoader SolutionLoader { get; }
 
         protected override string SourceProjectFolderName { get; }
@@ -46,6 +48,7 @@ public VisualBasicDiagnosticAnalyzersBenchmarks()
             SubstituteAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new SubstituteAnalyzer());
             UnusedReceivedAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new UnusedReceivedAnalyzer());
             ArgumentMatcherAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new NonSubstitutableMemberArgumentMatcherAnalyzer());
+            ReceivedInReceivedInOrderAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new ReceivedInReceivedInOrderAnalyzer());
         }
     }
 }

documentation/rules/NS5001.md

@@ -0,0 +1,83 @@
+# NS5001
+
+<table>
+<tr>
+  <td>CheckId</td>
+  <td>NS5001</td>
+</tr>
+<tr>
+  <td>Category</td>
+  <td>Usage</td>
+</tr>
+</table>
+
+## Cause
+
+Usage of received-like method in `Received.InOrder` callback.
+
+## Rule description
+
+A violation of this rule occurs when any of the following are used inside a `Received.InOrder` callback:
+
+- `Received()`
+- `ReceivedWithAnyArgs()`
+- `DidNotReceive()`
+- `DidNotReceiveWithAnyArgs()`
+
+Calls within `Received.InOrder` are already checked to ensure they were received in the expected order. Individual received-like assertions should be moved outside the `Received.InOrder` callback.
+
+## How to fix violations
+
+To fix a violation of this rule, remove received-like method calls from `Received.InOrder` callback.
+
+For example:
+
+````c#
+// Incorrect:
+Received.InOrder(() =>
+{
+    sub.Received().Baz();
+    sub.Received().Bar();
+})
+
+// Correct:
+Received.InOrder(() =>
+{
+    sub.Baz();
+    sub.Bar();
+})
+````
+
+Alternatively, move any received-like methods outside of `Received.InOrder` block if ordering is not important:
+
+````c#
+// Incorrect:
+Received.InOrder(() =>
+{
+    sub.Baz();
+    sub.Zap();
+    sub.DidNotReceive().Bar();
+})
+
+// Correct:
+Received.InOrder(() =>
+{
+    sub.Baz();
+    sub.Zap();
+})
+sub.DidNotReceive().Bar();
+````
+
+## 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", "NS5001:Received-like method used in Received.InOrder block.", Justification = "Reviewed")]
+````
+
+Or for a specific code block:
+````c#
+#pragma warning disable NS5001 // Received-like method used in Received.InOrder block.
+// the code which produces warning
+#pragma warning restore NS5001 // Received-like method used in Received.InOrder block.

documentation/rules/README.md

@@ -24,4 +24,5 @@
 | [NS3005](NS3005.md) | Argument specification | Assigning call argument which is not ref nor out argument. |
 | [NS3006](NS3006.md) | Argument specification | Conflicting assignments to out/ref arguments. |
 | [NS4000](NS4000.md) | Call configuration | Calling substitute from within `Returns` block. |
-| [NS5000](NS5000.md) | Usage | Checking received calls without specifying member. |
+| [NS5000](NS5000.md) | Usage | Checking received calls without specifying member. |
+| [NS5001](NS5001.md) | Usage | Usage of received-like method in `Received.InOrder` callback. |

src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/ReceivedInReceivedInOrderAnalyzer.cs

@@ -0,0 +1,19 @@
+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 ReceivedInReceivedInOrderAnalyzer : AbstractReceivedInReceivedInOrderAnalyzer<SyntaxKind, InvocationExpressionSyntax>
+    {
+        public ReceivedInReceivedInOrderAnalyzer()
+            : base(SubstitutionNodeFinder.Instance, CSharp.DiagnosticDescriptorsProvider.Instance)
+        {
+        }
+
+        protected override SyntaxKind InvocationExpressionKind { get; } = SyntaxKind.InvocationExpression;
+    }
+}

src/NSubstitute.Analyzers.Shared/AbstractDiagnosticDescriptorsProvider.cs

@@ -49,5 +49,7 @@ internal class AbstractDiagnosticDescriptorsProvider<T> : IDiagnosticDescriptors
         public DiagnosticDescriptor ConflictingArgumentAssignments { get; } = DiagnosticDescriptors<T>.ConflictingArgumentAssignments;
 
         public DiagnosticDescriptor NonSubstitutableMemberArgumentMatcherUsage { get; } = DiagnosticDescriptors<T>.NonSubstitutableMemberArgumentMatcherUsage;
+
+        public DiagnosticDescriptor ReceivedUsedInReceivedInOrder { get; } = DiagnosticDescriptors<T>.ReceivedUsedInReceivedInOrder;
     }
 }

src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/AbstractReceivedInReceivedInOrderAnalyzer.cs

@@ -0,0 +1,71 @@
+using System;
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using NSubstitute.Analyzers.Shared.Extensions;
+
+namespace NSubstitute.Analyzers.Shared.DiagnosticAnalyzers
+{
+    internal abstract class AbstractReceivedInReceivedInOrderAnalyzer<TSyntaxKind, TInvocationExpressionSyntax> : AbstractDiagnosticAnalyzer
+        where TSyntaxKind : struct
+        where TInvocationExpressionSyntax : SyntaxNode
+    {
+        private readonly ISubstitutionNodeFinder<TInvocationExpressionSyntax> _substitutionNodeFinder;
+        private readonly Action<SyntaxNodeAnalysisContext> _analyzeInvocationAction;
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }
+
+        protected AbstractReceivedInReceivedInOrderAnalyzer(
+            ISubstitutionNodeFinder<TInvocationExpressionSyntax> substitutionNodeFinder,
+            IDiagnosticDescriptorsProvider diagnosticDescriptorsProvider)
+            : base(diagnosticDescriptorsProvider)
+        {
+            _substitutionNodeFinder = substitutionNodeFinder;
+            _analyzeInvocationAction = AnalyzeInvocation;
+            SupportedDiagnostics = ImmutableArray.Create(diagnosticDescriptorsProvider.ReceivedUsedInReceivedInOrder);
+        }
+
+        protected abstract TSyntaxKind InvocationExpressionKind { get; }
+
+        protected override void InitializeAnalyzer(AnalysisContext context)
+        {
+            context.RegisterSyntaxNodeAction(_analyzeInvocationAction, InvocationExpressionKind);
+        }
+
+        private void AnalyzeInvocation(SyntaxNodeAnalysisContext syntaxNodeContext)
+        {
+            var invocationExpression = (TInvocationExpressionSyntax)syntaxNodeContext.Node;
+            var methodSymbolInfo = syntaxNodeContext.SemanticModel.GetSymbolInfo(invocationExpression);
+
+            if (methodSymbolInfo.Symbol?.Kind != SymbolKind.Method)
+            {
+                return;
+            }
+
+            if (methodSymbolInfo.Symbol.IsReceivedInOrderMethod() == false)
+            {
+                return;
+            }
+
+            foreach (var syntaxNode in _substitutionNodeFinder.FindForReceivedInOrderExpression(
+                syntaxNodeContext,
+                invocationExpression,
+                (IMethodSymbol)methodSymbolInfo.Symbol))
+            {
+                var symbolInfo = syntaxNodeContext.SemanticModel.GetSymbolInfo(syntaxNode);
+
+                if (symbolInfo.Symbol.IsReceivedLikeMethod() == false)
+                {
+                    continue;
+                }
+
+                var diagnostic = Diagnostic.Create(
+                    DiagnosticDescriptorsProvider.ReceivedUsedInReceivedInOrder,
+                    syntaxNode.GetLocation(),
+                    symbolInfo.Symbol.Name);
+
+                syntaxNodeContext.ReportDiagnostic(diagnostic);
+            }
+        }
+    }
+}

src/NSubstitute.Analyzers.Shared/DiagnosticDescriptors.cs

@@ -197,6 +197,13 @@ internal class DiagnosticDescriptors<T>
                 defaultSeverity: DiagnosticSeverity.Warning,
                 isEnabledByDefault: true);
 
+        public static DiagnosticDescriptor ReceivedUsedInReceivedInOrder { get; } = CreateDiagnosticDescriptor(
+            name: nameof(ReceivedUsedInReceivedInOrder),
+            id: DiagnosticIdentifiers.ReceivedUsedInReceivedInOrder,
+            category: DiagnosticCategory.Usage.GetDisplayName(),
+            defaultSeverity: DiagnosticSeverity.Warning,
+            isEnabledByDefault: true);
+
         private static DiagnosticDescriptor CreateDiagnosticDescriptor(
             string name, string id, string category, DiagnosticSeverity defaultSeverity, bool isEnabledByDefault)
         {

src/NSubstitute.Analyzers.Shared/DiagnosticIdentifiers.cs

@@ -28,5 +28,6 @@ internal class DiagnosticIdentifiers
         public const string ReEntrantSubstituteCall = "NS4000";
 
         public const string UnusedReceived = "NS5000";
+        public const string ReceivedUsedInReceivedInOrder = "NS5001";
     }
 }

src/NSubstitute.Analyzers.Shared/Extensions/SubstituteSymbolExtensions.cs

@@ -48,6 +48,11 @@ public static bool IsReceivedLikeMethod(this ISymbol symbol)
                    IsMember(symbol, MetadataNames.ReceivedWithQuantityMethodNames);
         }
 
+        public static bool IsReceivedInOrderMethod(this ISymbol symbol)
+        {
+            return IsMember(symbol, MetadataNames.NSubstituteInOrderMethod, MetadataNames.NSubstituteReceivedFullTypeName);
+        }
+
         public static bool IsWhenLikeMethod(this ISymbol symbol)
         {
             return IsMember(symbol, MetadataNames.WhenMethodNames);

src/NSubstitute.Analyzers.Shared/IDiagnosticDescriptorsProvider.cs

@@ -49,5 +49,7 @@ internal interface IDiagnosticDescriptorsProvider
         DiagnosticDescriptor ConflictingArgumentAssignments { get; }
 
         DiagnosticDescriptor NonSubstitutableMemberArgumentMatcherUsage { get; }
+
+        DiagnosticDescriptor ReceivedUsedInReceivedInOrder { get; }
     }
 }

src/NSubstitute.Analyzers.Shared/MetadataNames.cs

@@ -27,6 +27,8 @@ internal class MetadataNames
         public const string NSubstituteReceivedWithAnyArgsMethod = "ReceivedWithAnyArgs";
         public const string NSubstituteDidNotReceiveMethod = "DidNotReceive";
         public const string NSubstituteDidNotReceiveWithAnyArgsMethod = "DidNotReceiveWithAnyArgs";
+        public const string NSubstituteInOrderMethod = "InOrder";
+        public const string NSubstituteReceivedFullTypeName = "NSubstitute.Received";
         public const string NSubstituteForMethod = "For";
         public const string NSubstituteForPartsOfMethod = "ForPartsOf";
         public const string SubstituteFactoryCreate = "Create";

src/NSubstitute.Analyzers.Shared/Resources.resx

@@ -375,4 +375,17 @@
     <value>Argument matcher used with a non-virtual member of a class.</value>
         <comment>The title of the diagnostic.</comment>
   </data>
+
+    <data name="ReceivedUsedInReceivedInOrderDescription" xml:space="preserve">
+    <value>Received-like method used in Received.InOrder block.</value>
+        <comment>An optional longer localizable description of the diagnostic.</comment>
+  </data>
+    <data name="ReceivedUsedInReceivedInOrderMessageFormat" xml:space="preserve">
+    <value>{0} method used in Received.InOrder block.</value>
+        <comment>The format-able message the diagnostic displays.</comment>
+  </data>
+    <data name="ReceivedUsedInReceivedInOrderTitle" xml:space="preserve">
+    <value>Received-like method used in Received.InOrder block.</value>
+        <comment>The title of the diagnostic.</comment>
+  </data>
 </root>