[GH-126] - Reporting NS400 when substituting variable while configuring

other
nsubstitute · Jan 12, 2020 · 99d894a · 99d894a
1 parent f51aeb2
commit 99d894a
Show file tree

Hide file tree

Showing 11 changed files with 567 additions and 19 deletions.
diff --git a/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/ReEntrantCallFinder.cs b/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/ReEntrantCallFinder.cs
@@ -8,11 +8,12 @@
 
 namespace NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers
 {
-    internal class ReEntrantCallFinder : AbstractReEntrantCallFinder
+    internal class ReEntrantCallFinder : AbstractReEntrantCallFinder<InvocationExpressionSyntax, IdentifierNameSyntax>
     {
-        public static ReEntrantCallFinder Instance { get; } = new ReEntrantCallFinder();
+        public static ReEntrantCallFinder Instance { get; } = new ReEntrantCallFinder(SubstitutionNodeFinder.Instance);
 
-        private ReEntrantCallFinder()
+        private ReEntrantCallFinder(ISubstitutionNodeFinder<InvocationExpressionSyntax> substitutionNodeFinder)
+            : base(substitutionNodeFinder)
         {
         }
 
@@ -23,6 +24,32 @@ protected override ImmutableList<ISymbol> GetReEntrantSymbols(Compilation compil
             return visitor.InvocationSymbols;
         }
 
+        protected override IEnumerable<InvocationExpressionSyntax> GetPotentialPreviousReturnsLikeInvocations(IEnumerable<SyntaxNode> nodes, SemanticModel semanticModel)
+        {
+            foreach (var node in nodes)
+            {
+                switch (node)
+                {
+                    case InvocationExpressionSyntax invocationExpressionSyntax:
+                        yield return invocationExpressionSyntax;
+                        break;
+                    case ExpressionStatementSyntax expressionStatementSyntax when expressionStatementSyntax.Expression is InvocationExpressionSyntax invocationExpressionSyntax:
+                        yield return invocationExpressionSyntax;
+                        break;
+                    case ConstructorDeclarationSyntax constructorDeclarationSyntax:
+                        foreach (var potentialPreviousReturnsLikeInvocation in
+                            GetPotentialPreviousReturnsLikeInvocations(
+                                constructorDeclarationSyntax.Body?.ChildNodes() ??
+                                constructorDeclarationSyntax.ExpressionBody?.ChildNodes(), semanticModel))
+                        {
+                            yield return potentialPreviousReturnsLikeInvocation;
+                        }
+
+                        break;
+                }
+            }
+        }
+
         private class ReEntrantCallVisitor : CSharpSyntaxWalker
         {
             private readonly ReEntrantCallFinder _reEntrantCallFinder;

diff --git a/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/AbstractReEntrantCallFinder.cs b/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/AbstractReEntrantCallFinder.cs
@@ -1,4 +1,3 @@
-using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
@@ -7,22 +6,37 @@
 
 namespace NSubstitute.Analyzers.Shared.DiagnosticAnalyzers
 {
-    internal abstract class AbstractReEntrantCallFinder : IReEntrantCallFinder
+    internal abstract class AbstractReEntrantCallFinder<TInvocationExpressionSyntax, TIdentifierExpressionSyntax> : IReEntrantCallFinder
+        where TInvocationExpressionSyntax : SyntaxNode
+        where TIdentifierExpressionSyntax : SyntaxNode
     {
+        private readonly ISubstitutionNodeFinder<TInvocationExpressionSyntax> _substitutionNodeFinder;
+
+        protected AbstractReEntrantCallFinder(ISubstitutionNodeFinder<TInvocationExpressionSyntax> substitutionNodeFinder)
+        {
+            _substitutionNodeFinder = substitutionNodeFinder;
+        }
+
         public ImmutableList<ISymbol> GetReEntrantCalls(Compilation compilation, SemanticModel semanticModel, SyntaxNode originatingExpression, SyntaxNode rootNode)
         {
             var symbolInfo = semanticModel.GetSymbolInfo(rootNode);
 
-            if (IsLocalSymbol(symbolInfo.Symbol) || semanticModel.GetTypeInfo(rootNode).IsCallInfoDelegate(semanticModel))
+            if (symbolInfo.Symbol == null || symbolInfo.Symbol.IsLocal() ||
+                semanticModel.GetTypeInfo(rootNode).IsCallInfoDelegate(semanticModel))
             {
                 return ImmutableList<ISymbol>.Empty;
             }
 
-            return GetReEntrantSymbols(compilation, semanticModel, originatingExpression, rootNode);
+            var reEntrantSymbols = GetReEntrantSymbols(compilation, semanticModel, originatingExpression, rootNode);
+            var previouslySubstitutedSymbols = GetPreviouslySubstitutedSymbols(semanticModel, rootNode, symbolInfo.Symbol);
+
+            return reEntrantSymbols.AddRange(previouslySubstitutedSymbols);
         }
 
         protected abstract ImmutableList<ISymbol> GetReEntrantSymbols(Compilation compilation, SemanticModel semanticModel, SyntaxNode originatingExpression, SyntaxNode rootNode);
 
+        protected abstract IEnumerable<TInvocationExpressionSyntax> GetPotentialPreviousReturnsLikeInvocations(IEnumerable<SyntaxNode> nodes, SemanticModel semanticModel);
+
         protected IEnumerable<SyntaxNode> GetRelatedNodes(Compilation compilation, SemanticModel semanticModel, SyntaxNode syntaxNode)
         {
             if (compilation.ContainsSyntaxTree(syntaxNode.SyntaxTree) == false)
@@ -31,7 +45,7 @@ protected IEnumerable<SyntaxNode> GetRelatedNodes(Compilation compilation, Seman
             }
 
             var symbol = GetSemanticModel(compilation, semanticModel, syntaxNode).GetSymbolInfo(syntaxNode);
-            if (symbol.Symbol != null && IsLocalSymbol(symbol.Symbol) == false && symbol.Symbol.Locations.Any())
+            if (symbol.Symbol != null && symbol.Symbol.IsLocal() == false && symbol.Symbol.Locations.Any())
             {
                 foreach (var symbolLocation in symbol.Symbol.Locations.Where(location => location.SourceTree != null))
                 {
@@ -62,9 +76,51 @@ protected bool IsInnerReEntryLikeMethod(SemanticModel semanticModel, ISymbol sym
             return symbol.IsInnerReEntryLikeMethod();
         }
 
-        private bool IsLocalSymbol(ISymbol symbol)
+        private IEnumerable<ISymbol> GetPreviouslySubstitutedSymbols(SemanticModel semanticModel, SyntaxNode rootNode, ISymbol rootNodeSymbol)
+        {
+            var rootIdentifierNode = GetIdentifierExpressionSyntax(rootNode);
+            if (rootIdentifierNode == null)
+            {
+                yield break;
+            }
+
+            var rootIdentifierSymbol = semanticModel.GetSymbolInfo(rootIdentifierNode);
+
+            if (rootIdentifierSymbol.Symbol == null)
+            {
+                yield break;
+            }
+
+            var ancestorChildNodes = rootNode.Ancestors().SelectMany(ancestor => ancestor.ChildNodes());
+            foreach (var syntaxNode in GetPotentialPreviousReturnsLikeInvocations(ancestorChildNodes, semanticModel))
+            {
+                var symbol = semanticModel.GetSymbolInfo(syntaxNode).Symbol;
+                if (symbol.IsReturnLikeMethod() == false)
+                {
+                    continue;
+                }
+
+                var substitutedNode = _substitutionNodeFinder.FindForStandardExpression(syntaxNode, symbol as IMethodSymbol);
+
+                var substituteNodeSymbol = semanticModel.GetSymbolInfo(substitutedNode).Symbol;
+                if (substituteNodeSymbol != rootNodeSymbol)
+                {
+                    continue;
+                }
+
+                var substituteNodeIdentifier = GetIdentifierExpressionSyntax(substitutedNode);
+
+                var substituteIdentifierSymbol = semanticModel.GetSymbolInfo(substituteNodeIdentifier);
+                if (rootIdentifierSymbol.Symbol == substituteIdentifierSymbol.Symbol)
+                {
+                    yield return substituteNodeSymbol;
+                }
+            }
+        }
+
+        private TIdentifierExpressionSyntax GetIdentifierExpressionSyntax(SyntaxNode node)
         {
-            return symbol != null && symbol.Kind == SymbolKind.Local;
+            return node.ChildNodes().FirstOrDefault() as TIdentifierExpressionSyntax;
         }
     }
 }
diff --git a/src/NSubstitute.Analyzers.Shared/Extensions/ISymbolExtensions.cs b/src/NSubstitute.Analyzers.Shared/Extensions/ISymbolExtensions.cs
@@ -37,6 +37,11 @@ public static string ToMinimalMethodString(this ISymbol symbol, SemanticModel se
             return $"{symbol.ContainingType}.{minimumDisplayString}";
         }
 
+        public static bool IsLocal(this ISymbol symbol)
+        {
+            return symbol != null && symbol.Kind == SymbolKind.Local;
+        }
+
         private static bool IsInterfaceMember(ISymbol symbol)
         {
             return symbol?.ContainingType?.TypeKind == TypeKind.Interface;

diff --git a/src/NSubstitute.Analyzers.VisualBasic/DiagnosticAnalyzers/ReEntrantCallFinder.cs b/src/NSubstitute.Analyzers.VisualBasic/DiagnosticAnalyzers/ReEntrantCallFinder.cs
@@ -8,11 +8,12 @@
 
 namespace NSubstitute.Analyzers.VisualBasic.DiagnosticAnalyzers
 {
-    internal class ReEntrantCallFinder : AbstractReEntrantCallFinder
+    internal class ReEntrantCallFinder : AbstractReEntrantCallFinder<InvocationExpressionSyntax, IdentifierNameSyntax>
     {
-        public static ReEntrantCallFinder Instance { get; } = new ReEntrantCallFinder();
+        public static ReEntrantCallFinder Instance { get; } = new ReEntrantCallFinder(SubstitutionNodeFinder.Instance);
 
-        private ReEntrantCallFinder()
+        private ReEntrantCallFinder(ISubstitutionNodeFinder<InvocationExpressionSyntax> substitutionNodeFinder)
+            : base(substitutionNodeFinder)
         {
         }
 
@@ -23,6 +24,31 @@ protected override ImmutableList<ISymbol> GetReEntrantSymbols(Compilation compil
             return visitor.InvocationSymbols;
         }
 
+        protected override IEnumerable<InvocationExpressionSyntax> GetPotentialPreviousReturnsLikeInvocations(IEnumerable<SyntaxNode> nodes, SemanticModel semanticModel)
+        {
+            foreach (var node in nodes)
+            {
+                switch (node)
+                {
+                    case InvocationExpressionSyntax invocationExpressionSyntax:
+                        yield return invocationExpressionSyntax;
+                        break;
+                    case ExpressionStatementSyntax expressionStatementSyntax when expressionStatementSyntax.Expression is InvocationExpressionSyntax invocationExpressionSyntax:
+                        yield return invocationExpressionSyntax;
+                        break;
+                    case ConstructorBlockSyntax constructorDeclarationSyntax:
+                        foreach (var potentialPreviousReturnsLikeInvocation in
+                            GetPotentialPreviousReturnsLikeInvocations(
+                                constructorDeclarationSyntax.ChildNodes(), semanticModel))
+                        {
+                            yield return potentialPreviousReturnsLikeInvocation;
+                        }
+
+                        break;
+                }
+            }
+        }
+
         private class ReEntrantCallVisitor : VisualBasicSyntaxWalker
         {
             private readonly ReEntrantCallFinder _reEntrantCallFinder;

diff --git a/...alyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReEntrantReturnsSetupDiagnosticVerifier.cs b/...alyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReEntrantReturnsSetupDiagnosticVerifier.cs
@@ -113,6 +113,22 @@ public abstract class ReEntrantReturnsSetupDiagnosticVerifier : CSharpDiagnostic
         [InlineData]
         public abstract Task ReportsNoDiagnostics_WhenElementUsedTwice_InForEachLoop(string method);
 
+        [CombinatoryTheory]
+        [InlineData]
+        public abstract Task ReportsDiagnostics_WhenReturnValueIsCalledWhileBeingConfigured(string method);
+
+        [CombinatoryTheory]
+        [InlineData]
+        public abstract Task ReportsDiagnostics_WhenReturnValueIsCalledWhileBeingConfiguredInConstructorBody(string method);
+
+        [CombinatoryTheory]
+        [InlineData]
+        public abstract Task ReportsDiagnostics_WhenReturnValueIsCalledWhileBeingConfiguredInConstructorExpressionBody(string method);
+
+        [CombinatoryTheory]
+        [InlineData]
+        public abstract Task ReportsNoDiagnostics_WhenReturnValueIsCalledAfterIsConfigured(string method);
+
         protected override DiagnosticAnalyzer GetDiagnosticAnalyzer()
         {
             return new ReEntrantSetupAnalyzer();