[GH-134] - detecting async Received.InOrder callbacks (#137)

NSubstitute.Analyzers.sln

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

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

@@ -0,0 +1,12 @@
+using System.Threading.Tasks;
+
+namespace NSubstitute.Analyzers.Benchmarks.Source.CSharp.DiagnosticsSources
+{
+    public class AsyncReceivedInOrderCallbackDiagnosticsSource
+    {
+        public void NS5002_AsyncCallbackUsedInReceivedInOrderMethod()
+        {
+            Received.InOrder(async () => { await Task.CompletedTask; });
+        }
+    }
+}

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

@@ -0,0 +1,11 @@
+Imports System.Threading.Tasks
+
+Namespace DiagnosticsSources
+    Public Class AsyncReceivedInOrderCallbackDiagnosticsSource
+        Public Sub NS5002_AsyncCallbackUsedInReceivedInOrderMethod()
+            NSubstitute.Received.InOrder(Async Sub()
+                Await Task.CompletedTask
+            End Sub)
+        End Sub
+    End Class
+End Namespace

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

@@ -27,6 +27,8 @@ public class CSharpDiagnosticAnalyzersBenchmarks : AbstractDiagnosticAnalyzersBe
 
         protected override AnalyzerBenchmark ReceivedInReceivedInOrderAnalyzerBenchmark { get; }
 
+        protected override AnalyzerBenchmark AsyncReceivedInOrderCallbackAnalyzerBenchmark { get; }
+
         protected override AbstractSolutionLoader SolutionLoader { get; }
 
         protected override string SourceProjectFolderName { get; }
@@ -49,6 +51,7 @@ public CSharpDiagnosticAnalyzersBenchmarks()
             UnusedReceivedAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new UnusedReceivedAnalyzer());
             ArgumentMatcherAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new NonSubstitutableMemberArgumentMatcherAnalyzer());
             ReceivedInReceivedInOrderAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new ReceivedInReceivedInOrderAnalyzer());
+            AsyncReceivedInOrderCallbackAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new AsyncReceivedInOrderCallbackAnalyzer());
         }
     }
 }

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

@@ -4,7 +4,6 @@
 using System.Linq;
 using System.Reflection;
 using BenchmarkDotNet.Attributes;
-using BenchmarkDotNet.Engines;
 using Microsoft.CodeAnalysis;
 
 namespace NSubstitute.Analyzers.Benchmarks.Shared
@@ -35,6 +34,8 @@ public abstract class AbstractDiagnosticAnalyzersBenchmarks
 
         protected abstract AnalyzerBenchmark ReceivedInReceivedInOrderAnalyzerBenchmark { get; }
 
+        protected abstract AnalyzerBenchmark AsyncReceivedInOrderCallbackAnalyzerBenchmark { get; }
+
         protected abstract AbstractSolutionLoader SolutionLoader { get; }
 
         protected abstract string SourceProjectFolderName { get; }
@@ -108,6 +109,12 @@ public void ReceivedInReceivedInOrderAnalyzer()
             ReceivedInReceivedInOrderAnalyzerBenchmark.Run();
         }
 
+        [Benchmark]
+        public void AsyncReceivedInOrderCallbackAnalyzer()
+        {
+            AsyncReceivedInOrderCallbackAnalyzerBenchmark.Run();
+        }
+
         [IterationCleanup(Target = nameof(ArgumentMatcherAnalyzer))]
         public void CleanUp()
         {

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

@@ -27,6 +27,8 @@ public class VisualBasicDiagnosticAnalyzersBenchmarks : AbstractDiagnosticAnalyz
 
         protected override AnalyzerBenchmark ReceivedInReceivedInOrderAnalyzerBenchmark { get; }
 
+        protected override AnalyzerBenchmark AsyncReceivedInOrderCallbackAnalyzerBenchmark { get; }
+
         protected override AbstractSolutionLoader SolutionLoader { get; }
 
         protected override string SourceProjectFolderName { get; }
@@ -49,6 +51,7 @@ public VisualBasicDiagnosticAnalyzersBenchmarks()
             UnusedReceivedAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new UnusedReceivedAnalyzer());
             ArgumentMatcherAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new NonSubstitutableMemberArgumentMatcherAnalyzer());
             ReceivedInReceivedInOrderAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new ReceivedInReceivedInOrderAnalyzer());
+            AsyncReceivedInOrderCallbackAnalyzerBenchmark = AnalyzerBenchmark.CreateBenchmark(Solution, new AsyncReceivedInOrderCallbackAnalyzer());
         }
     }
 }

documentation/rules/NS5002.md

@@ -0,0 +1,54 @@
+# NS5002
+
+<table>
+<tr>
+  <td>CheckId</td>
+  <td>NS5002</td>
+</tr>
+<tr>
+  <td>Category</td>
+  <td>Usage</td>
+</tr>
+</table>
+
+## Cause
+
+Usage of async callback in `Received.InOrder` method.
+
+## Rule description
+
+A violation of this rule occurs when async callback is used in `Received.InOrder` method.  `Received.InOrder` is used to specify calls which should be received by one or more substitutes. Running/awaiting these calls is not required for this, and can cause some problems as described in issue [#604](https://github.com/nsubstitute/NSubstitute/issues/604).
+
+## How to fix violations
+
+To fix a violation of this rule, remove async modifier from `Received.InOrder` callback.
+
+For example:
+
+````c#
+// Incorrect:
+Received.InOrder(async () =>
+{
+    await sub.Received().Bar();
+})
+
+// Correct:
+Received.InOrder(() =>
+{
+    sub.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", "NS5002:Async callback used in Received.InOrder method.", Justification = "Reviewed")]
+````
+
+Or for a specific code block:
+````c#
+#pragma warning disable NS5002 // Async callback used in Received.InOrder method.
+// the code which produces warning
+#pragma warning restore NS5002 // Async callback used in Received.InOrder method.

documentation/rules/README.md

@@ -26,3 +26,4 @@
 | [NS4000](NS4000.md) | Call configuration | Calling substitute from within `Returns` block. |
 | [NS5000](NS5000.md) | Usage | Checking received calls without specifying member. |
 | [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. |

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

@@ -0,0 +1,33 @@
+using System.Collections.Generic;
+using System.Linq;
+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 AsyncReceivedInOrderCallbackAnalyzer : AbstractAsyncReceivedInOrderCallbackAnalyzer<SyntaxKind, InvocationExpressionSyntax>
+    {
+        public AsyncReceivedInOrderCallbackAnalyzer()
+            : base(CSharp.DiagnosticDescriptorsProvider.Instance)
+        {
+        }
+
+        protected override int AsyncExpressionRawKind { get; } = (int)SyntaxKind.AsyncKeyword;
+
+        protected override SyntaxKind InvocationExpressionKind { get; } = SyntaxKind.InvocationExpression;
+
+        protected override IEnumerable<SyntaxNode> GetArgumentExpressions(InvocationExpressionSyntax invocationExpressionSyntax)
+        {
+            return invocationExpressionSyntax.ArgumentList.Arguments.Select<ArgumentSyntax, SyntaxNode>(arg => arg.Expression);
+        }
+
+        protected override IEnumerable<SyntaxToken?> GetCallbackArgumentSyntaxTokens(SyntaxNode node)
+        {
+            return node.ChildTokens().Select<SyntaxToken, SyntaxToken?>(token => token);
+        }
+    }
+}

src/NSubstitute.Analyzers.Shared/AbstractDiagnosticDescriptorsProvider.cs

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

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

@@ -0,0 +1,73 @@
+using System;
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using NSubstitute.Analyzers.Shared.Extensions;
+
+namespace NSubstitute.Analyzers.Shared.DiagnosticAnalyzers
+{
+    internal abstract class AbstractAsyncReceivedInOrderCallbackAnalyzer<TSyntaxKind, TInvocationExpressionSyntax> : AbstractDiagnosticAnalyzer
+        where TSyntaxKind : struct
+        where TInvocationExpressionSyntax : SyntaxNode
+    {
+        private readonly Action<SyntaxNodeAnalysisContext> _analyzeInvocationAction;
+
+        protected AbstractAsyncReceivedInOrderCallbackAnalyzer(
+            IDiagnosticDescriptorsProvider diagnosticDescriptorsProvider)
+            : base(diagnosticDescriptorsProvider)
+        {
+            _analyzeInvocationAction = AnalyzeInvocation;
+            SupportedDiagnostics = ImmutableArray.Create(diagnosticDescriptorsProvider.AsyncCallbackUsedInReceivedInOrder);
+        }
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }
+
+        protected abstract int AsyncExpressionRawKind { get; }
+
+        protected abstract TSyntaxKind InvocationExpressionKind { get; }
+
+        protected abstract IEnumerable<SyntaxNode> GetArgumentExpressions(TInvocationExpressionSyntax invocationExpressionSyntax);
+
+        protected abstract IEnumerable<SyntaxToken?> GetCallbackArgumentSyntaxTokens(SyntaxNode node);
+
+        protected sealed 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 expression in GetArgumentExpressions(invocationExpression))
+            {
+                var asyncToken = GetCallbackArgumentSyntaxTokens(expression)
+                    .FirstOrDefault(token => token.HasValue && token.Value.RawKind == AsyncExpressionRawKind);
+
+                if (asyncToken.HasValue == false)
+                {
+                   continue;
+                }
+
+                var diagnostic = Diagnostic.Create(
+                    DiagnosticDescriptorsProvider.AsyncCallbackUsedInReceivedInOrder,
+                    asyncToken.Value.GetLocation());
+
+                syntaxNodeContext.ReportDiagnostic(diagnostic);
+            }
+        }
+    }
+}

src/NSubstitute.Analyzers.Shared/DiagnosticDescriptors.cs

@@ -204,6 +204,13 @@ internal class DiagnosticDescriptors<T>
             defaultSeverity: DiagnosticSeverity.Warning,
             isEnabledByDefault: true);
 
+        public static DiagnosticDescriptor AsyncCallbackUsedInReceivedInOrder { get; } = CreateDiagnosticDescriptor(
+            name: nameof(AsyncCallbackUsedInReceivedInOrder),
+            id: DiagnosticIdentifiers.AsyncCallbackUsedInReceivedInOrder,
+            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

@@ -29,5 +29,6 @@ internal class DiagnosticIdentifiers
 
         public const string UnusedReceived = "NS5000";
         public const string ReceivedUsedInReceivedInOrder = "NS5001";
+        public const string AsyncCallbackUsedInReceivedInOrder = "NS5002";
     }
 }

src/NSubstitute.Analyzers.Shared/IDiagnosticDescriptorsProvider.cs

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

src/NSubstitute.Analyzers.Shared/Resources.resx

@@ -388,4 +388,17 @@
     <value>Received-like method used in Received.InOrder block.</value>
         <comment>The title of the diagnostic.</comment>
   </data>
+
+    <data name="AsyncCallbackUsedInReceivedInOrderDescription" xml:space="preserve">
+    <value>Async callback used in Received.InOrder method.</value>
+        <comment>An optional longer localizable description of the diagnostic.</comment>
+  </data>
+    <data name="AsyncCallbackUsedInReceivedInOrderMessageFormat" xml:space="preserve">
+    <value>Async callback used in Received.InOrder method.</value>
+        <comment>The format-able message the diagnostic displays.</comment>
+  </data>
+    <data name="AsyncCallbackUsedInReceivedInOrderTitle" xml:space="preserve">
+    <value>Async callback used in Received.InOrder method.</value>
+        <comment>The title of the diagnostic.</comment>
+  </data>
 </root>

src/NSubstitute.Analyzers.VisualBasic/DiagnosticAnalyzers/AsyncReceivedInOrderCallbackAnalyzer.cs

@@ -0,0 +1,39 @@
+using System.Collections.Generic;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.VisualBasic;
+using Microsoft.CodeAnalysis.VisualBasic.Syntax;
+using NSubstitute.Analyzers.Shared.DiagnosticAnalyzers;
+
+namespace NSubstitute.Analyzers.VisualBasic.DiagnosticAnalyzers
+{
+    [DiagnosticAnalyzer(LanguageNames.VisualBasic)]
+    internal sealed class AsyncReceivedInOrderCallbackAnalyzer : AbstractAsyncReceivedInOrderCallbackAnalyzer<SyntaxKind, InvocationExpressionSyntax>
+    {
+        public AsyncReceivedInOrderCallbackAnalyzer()
+            : base(VisualBasic.DiagnosticDescriptorsProvider.Instance)
+        {
+        }
+
+        protected override int AsyncExpressionRawKind { get; } = (int)SyntaxKind.AsyncKeyword;
+
+        protected override SyntaxKind InvocationExpressionKind { get; } = SyntaxKind.InvocationExpression;
+
+        protected override IEnumerable<SyntaxNode> GetArgumentExpressions(InvocationExpressionSyntax invocationExpressionSyntax)
+        {
+            return invocationExpressionSyntax.ArgumentList.Arguments.Select<ArgumentSyntax, SyntaxNode>(arg => arg.GetExpression());
+        }
+
+        protected override IEnumerable<SyntaxToken?> GetCallbackArgumentSyntaxTokens(SyntaxNode node)
+        {
+            switch (node)
+            {
+                case LambdaExpressionSyntax lambdaExpressionSyntax:
+                    return lambdaExpressionSyntax.SubOrFunctionHeader.ChildTokens().Select<SyntaxToken, SyntaxToken?>(token => token);
+            }
+
+            return Enumerable.Empty<SyntaxToken?>();
+        }
+    }
+}