[GH-93] - excluding root symbol from reentrant analysys

nsubstitute · Apr 15, 2019 · f2c1e1a · f2c1e1a
1 parent 81549f2
commit f2c1e1a
Show file tree

Hide file tree

Showing 12 changed files with 147 additions and 13 deletions.
diff --git a/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/ReEntrantCallFinder.cs b/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/ReEntrantCallFinder.cs
@@ -10,9 +10,9 @@ namespace NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers
 {
     internal class ReEntrantCallFinder : AbstractReEntrantCallFinder
     {
-        protected override ImmutableList<ISymbol> GetReEntrantSymbols(Compilation compilation, SyntaxNode rootNode)
+        protected override ImmutableList<ISymbol> GetReEntrantSymbols(Compilation compilation, SyntaxNode originatingExpression, SyntaxNode rootNode)
         {
-            var visitor = new ReEntrantCallVisitor(this, compilation);
+            var visitor = new ReEntrantCallVisitor(this, compilation, originatingExpression);
             visitor.Visit(rootNode);
             return visitor.InvocationSymbols;
         }
@@ -26,15 +26,16 @@ private class ReEntrantCallVisitor : CSharpSyntaxWalker
 
             public ImmutableList<ISymbol> InvocationSymbols => _invocationSymbols.ToImmutableList();
 
-            public ReEntrantCallVisitor(ReEntrantCallFinder reEntrantCallFinder, Compilation compilation)
+            public ReEntrantCallVisitor(ReEntrantCallFinder reEntrantCallFinder, Compilation compilation, SyntaxNode originatingExpression)
             {
                 _reEntrantCallFinder = reEntrantCallFinder;
                 _compilation = compilation;
+                _visitedNodes.Add(originatingExpression);
             }
 
             public override void VisitInvocationExpression(InvocationExpressionSyntax node)
             {
-                if (_compilation.ContainsSyntaxTree(node.SyntaxTree))
+                if (_visitedNodes.Contains(node) == false && _compilation.ContainsSyntaxTree(node.SyntaxTree))
                 {
                     var semanticModel = _compilation.GetSemanticModel(node.SyntaxTree);
                     var symbolInfo = semanticModel.GetSymbolInfo(node);
@@ -69,7 +70,7 @@ private void VisitRelatedSymbols(SyntaxNode syntaxNode)
                     syntaxNode.IsKind(SyntaxKind.SimpleMemberAccessExpression)))
                 {
                     _visitedNodes.Add(syntaxNode);
-                    foreach (var relatedNode in _reEntrantCallFinder.GetRelatedNodes(_compilation, syntaxNode))
+                    foreach (var relatedNode in _reEntrantCallFinder.GetRelatedNodes(_compilation, syntaxNode).Where(node => _visitedNodes.Contains(node) == false))
                     {
                         Visit(relatedNode);
                     }

diff --git a/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/AbstractReEntrantCallFinder.cs b/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/AbstractReEntrantCallFinder.cs
@@ -16,7 +16,7 @@ internal abstract class AbstractReEntrantCallFinder : IReEntrantCallFinder
             [MetadataNames.NSubstituteDoMethod] = MetadataNames.NSubstituteWhenCalledType
         }.ToImmutableDictionary();
 
-        public ImmutableList<ISymbol> GetReEntrantCalls(Compilation compilation, SyntaxNode rootNode)
+        public ImmutableList<ISymbol> GetReEntrantCalls(Compilation compilation, SyntaxNode originatingExpression, SyntaxNode rootNode)
         {
             var semanticModel = compilation.GetSemanticModel(rootNode.SyntaxTree);
             var symbolInfo = semanticModel.GetSymbolInfo(rootNode);
@@ -26,10 +26,10 @@ public ImmutableList<ISymbol> GetReEntrantCalls(Compilation compilation, SyntaxN
                 return ImmutableList<ISymbol>.Empty;
             }
 
-            return GetReEntrantSymbols(compilation, rootNode);
+            return GetReEntrantSymbols(compilation, originatingExpression, rootNode);
         }
 
-        protected abstract ImmutableList<ISymbol> GetReEntrantSymbols(Compilation compilation, SyntaxNode rootNode);
+        protected abstract ImmutableList<ISymbol> GetReEntrantSymbols(Compilation compilation, SyntaxNode originatingExpression, SyntaxNode rootNode);
 
         protected IEnumerable<SyntaxNode> GetRelatedNodes(Compilation compilation, SyntaxNode syntaxNode)
         {

diff --git a/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/AbstractReEntrantSetupAnalyzer.cs b/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/AbstractReEntrantSetupAnalyzer.cs
@@ -57,7 +57,7 @@ private void AnalyzeInvocation(SyntaxNodeAnalysisContext syntaxNodeContext)
 
             foreach (var argument in argumentsForAnalysis)
             {
-                var reentrantSymbol = _reEntrantCallFinder.GetReEntrantCalls(syntaxNodeContext.Compilation, argument).FirstOrDefault();
+                var reentrantSymbol = _reEntrantCallFinder.GetReEntrantCalls(syntaxNodeContext.Compilation, invocationExpression, argument).FirstOrDefault();
                 if (reentrantSymbol != null)
                 {
                     var diagnostic = Diagnostic.Create(

diff --git a/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/IReEntrantCallFinder.cs b/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/IReEntrantCallFinder.cs
@@ -5,6 +5,6 @@ namespace NSubstitute.Analyzers.Shared.DiagnosticAnalyzers
 {
     internal interface IReEntrantCallFinder
     {
-        ImmutableList<ISymbol> GetReEntrantCalls(Compilation compilation, SyntaxNode rootNode);
+        ImmutableList<ISymbol> GetReEntrantCalls(Compilation compilation, SyntaxNode originatingExpression, SyntaxNode rootNode);
     }
 }
diff --git a/src/NSubstitute.Analyzers.VisualBasic/DiagnosticAnalyzers/ReEntrantCallFinder.cs b/src/NSubstitute.Analyzers.VisualBasic/DiagnosticAnalyzers/ReEntrantCallFinder.cs
@@ -1,5 +1,6 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Linq;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.VisualBasic;
 using Microsoft.CodeAnalysis.VisualBasic.Syntax;
@@ -9,7 +10,7 @@ namespace NSubstitute.Analyzers.VisualBasic.DiagnosticAnalyzers
 {
     internal class ReEntrantCallFinder : AbstractReEntrantCallFinder
     {
-        protected override ImmutableList<ISymbol> GetReEntrantSymbols(Compilation compilation, SyntaxNode rootNode)
+        protected override ImmutableList<ISymbol> GetReEntrantSymbols(Compilation compilation, SyntaxNode originatingExpression, SyntaxNode rootNode)
         {
             var visitor = new ReEntrantCallVisitor(this, compilation);
             visitor.Visit(rootNode);
@@ -33,7 +34,7 @@ public ReEntrantCallVisitor(ReEntrantCallFinder reEntrantCallFinder, Compilation
 
             public override void VisitInvocationExpression(InvocationExpressionSyntax node)
             {
-                if (_compilation.ContainsSyntaxTree(node.SyntaxTree))
+                if (_visitedNodes.Contains(node) == false && _compilation.ContainsSyntaxTree(node.SyntaxTree))
                 {
                     var semanticModel = _compilation.GetSemanticModel(node.SyntaxTree);
                     var symbolInfo = semanticModel.GetSymbolInfo(node);
@@ -67,7 +68,7 @@ private void VisitRelatedSymbols(SyntaxNode syntaxNode)
                     syntaxNode.IsKind(SyntaxKind.SimpleMemberAccessExpression)))
                 {
                     _visitedNodes.Add(syntaxNode);
-                    foreach (var relatedNode in _reEntrantCallFinder.GetRelatedNodes(_compilation, syntaxNode))
+                    foreach (var relatedNode in _reEntrantCallFinder.GetRelatedNodes(_compilation, syntaxNode).Where(node => _visitedNodes.Contains(node) == false))
                     {
                         var currentNode = relatedNode;
 

diff --git a/...alyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReEntrantReturnsSetupDiagnosticVerifier.cs b/...alyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReEntrantReturnsSetupDiagnosticVerifier.cs
@@ -1,4 +1,5 @@
 using System.Threading.Tasks;
+using Markdig.Syntax.Inlines;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Diagnostics;
 using NSubstitute.Analyzers.CSharp;
@@ -104,6 +105,10 @@ public abstract class ReEntrantReturnsSetupDiagnosticVerifier : CSharpDiagnostic
         [InlineData("FooBar")]
         public abstract Task ReportsNoDiagnostic_WhenUsed_WithTypeofExpression(string method, string type);
 
+        [CombinatoryTheory]
+        [InlineData]
+        public abstract Task ReportsNoDiagnostics_WhenReturnsValueIsSet_InForEachLoop(string method);
+
         protected override DiagnosticAnalyzer GetDiagnosticAnalyzer()
         {
             return new ReEntrantSetupAnalyzer();

diff --git a/...agnosticAnalyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsExtensionMethodTests.cs b/...agnosticAnalyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsExtensionMethodTests.cs
@@ -2,6 +2,7 @@
 using System.Threading.Tasks;
 using NSubstitute.Analyzers.Tests.Shared.Extensibility;
 using NSubstitute.Analyzers.Tests.Shared.Extensions;
+using Xunit;
 
 namespace NSubstitute.Analyzers.Tests.CSharp.DiagnosticAnalyzerTests.ReEntrantReturnsSetupAnalyzerTests
 {
@@ -571,6 +572,38 @@ public void Test()
             substitute.FooBar().{method}(typeof({type}));
         }}
     }}
+}}";
+            await VerifyNoDiagnostic(source);
+        }
+
+        public override async Task ReportsNoDiagnostics_WhenReturnsValueIsSet_InForEachLoop(string method)
+        {
+            var source = $@"using NSubstitute;
+using NSubstitute.Core;
+
+namespace MyNamespace
+{{
+    public interface IFoo
+    {{
+        int Bar();
+    }}
+
+    public class FooBar
+    {{
+        public int Value {{ get; set; }}
+    }}
+
+    public class FooTests
+    {{
+        public void Test()
+        {{
+            var substitute = Substitute.For<IFoo>();
+            foreach (var fooBar in new FooBar[0])
+            {{
+                substitute.Bar().{method}(fooBar.Value);
+            }}
+        }}
+    }}
 }}";
             await VerifyNoDiagnostic(source);
         }

diff --git a/...iagnosticAnalyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsOrdinaryMethodTests.cs b/...iagnosticAnalyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsOrdinaryMethodTests.cs
@@ -570,6 +570,38 @@ public void Test()
             {method}(substitute.FooBar(), typeof({type}));
         }}
     }}
+}}";
+            await VerifyNoDiagnostic(source);
+        }
+
+        public override async Task ReportsNoDiagnostics_WhenReturnsValueIsSet_InForEachLoop(string method)
+        {
+            var source = $@"using NSubstitute;
+using NSubstitute.Core;
+
+namespace MyNamespace
+{{
+    public interface IFoo
+    {{
+        int Bar();
+    }}
+
+    public class FooBar
+    {{
+        public int Value {{ get; set; }}
+    }}
+
+    public class FooTests
+    {{
+        public void Test()
+        {{
+            var substitute = Substitute.For<IFoo>();
+            foreach (var fooBar in new FooBar[0])
+            {{
+                {method}(substitute.Bar(), fooBar.Value);
+            }}
+        }}
+    }}
 }}";
             await VerifyNoDiagnostic(source);
         }

diff --git a/...te.Analyzers.Tests.Shared/DiagnosticAnalyzers/IReEntrantReturnsSetupDiagnosticVerifier.cs b/...te.Analyzers.Tests.Shared/DiagnosticAnalyzers/IReEntrantReturnsSetupDiagnosticVerifier.cs
@@ -25,5 +25,7 @@ public interface IReEntrantReturnsSetupDiagnosticVerifier
         Task ReportsDiagnostic_WhenUsingReEntrantReturns_AcrossMultipleFiles(string method);
 
         Task ReportsNoDiagnostic_WhenUsed_WithTypeofExpression(string method, string type);
+
+        Task ReportsNoDiagnostics_WhenReturnsValueIsSet_InForEachLoop(string method);
     }
 }
diff --git a/...lyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReEntrantReturnsSetupDiagnosticVerifier.cs b/...lyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReEntrantReturnsSetupDiagnosticVerifier.cs
@@ -94,6 +94,10 @@ Dim barr As IBar
         [InlineData("FooBar")]
         public abstract Task ReportsNoDiagnostic_WhenUsed_WithTypeofExpression(string method, string type);
 
+        [CombinatoryTheory]
+        [InlineData]
+        public abstract Task ReportsNoDiagnostics_WhenReturnsValueIsSet_InForEachLoop(string method);
+
         protected override DiagnosticAnalyzer GetDiagnosticAnalyzer()
         {
             return new ReEntrantSetupAnalyzer();

diff --git a/...gnosticAnalyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsExtensionMethodTests.cs b/...gnosticAnalyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsExtensionMethodTests.cs
@@ -486,5 +486,33 @@ End Class
 
             await VerifyNoDiagnostic(source);
         }
+
+        public override async Task ReportsNoDiagnostics_WhenReturnsValueIsSet_InForEachLoop(string method)
+        {
+            var source = $@"Imports NSubstitute
+Imports NSubstitute.Core
+
+Namespace MyNamespace
+    Public Interface IFoo
+        Function Bar() As Integer
+    End Interface
+
+    Public Class FooBar
+        Public Property Value As Integer
+    End Class
+
+    Public Class FooTests
+        Public Sub Test()
+            Dim substitute = NSubstitute.Substitute.[For](Of IFoo)()
+
+            For Each fooBar In New FooBar(-1) {{}}
+                substitute.Bar().{method}(fooBar.Value)
+            Next
+        End Sub
+    End Class
+End Namespace";
+
+            await VerifyNoDiagnostic(source);
+        }
     }
 }
diff --git a/...agnosticAnalyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsOrdinaryMethodTests.cs b/...agnosticAnalyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsOrdinaryMethodTests.cs
@@ -495,5 +495,33 @@ End Class
 
             await VerifyNoDiagnostic(source);
         }
+
+        public override async Task ReportsNoDiagnostics_WhenReturnsValueIsSet_InForEachLoop(string method)
+        {
+            var source = $@"Imports NSubstitute
+Imports NSubstitute.Core
+
+Namespace MyNamespace
+    Public Interface IFoo
+        Function Bar() As Integer
+    End Interface
+
+    Public Class FooBar
+        Public Property Value As Integer
+    End Class
+
+    Public Class FooTests
+        Public Sub Test()
+            Dim substitute = NSubstitute.Substitute.[For](Of IFoo)()
+
+            For Each fooBar In New FooBar(-1) {{}}
+                {method}(substitute.Bar(), fooBar.Value)
+            Next
+        End Sub
+    End Class
+End Namespace";
+
+            await VerifyNoDiagnostic(source);
+        }
     }
 }