diff --git a/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/ReEntrantCallFinder.cs b/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/ReEntrantCallFinder.cs index b1cc0e77..e67fbbc7 100644 --- 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 { - public static ReEntrantCallFinder Instance { get; } = new ReEntrantCallFinder(); + public static ReEntrantCallFinder Instance { get; } = new ReEntrantCallFinder(SubstitutionNodeFinder.Instance); - private ReEntrantCallFinder() + private ReEntrantCallFinder(ISubstitutionNodeFinder substitutionNodeFinder) + : base(substitutionNodeFinder) { } @@ -23,6 +24,32 @@ protected override ImmutableList GetReEntrantSymbols(Compilation compil return visitor.InvocationSymbols; } + protected override IEnumerable GetPotentialPreviousReturnsLikeInvocations(IEnumerable 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 index 0218cc94..50a5a095 100644 --- 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 : IReEntrantCallFinder + where TInvocationExpressionSyntax : SyntaxNode + where TIdentifierExpressionSyntax : SyntaxNode { + private readonly ISubstitutionNodeFinder _substitutionNodeFinder; + + protected AbstractReEntrantCallFinder(ISubstitutionNodeFinder substitutionNodeFinder) + { + _substitutionNodeFinder = substitutionNodeFinder; + } + public ImmutableList 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.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 GetReEntrantSymbols(Compilation compilation, SemanticModel semanticModel, SyntaxNode originatingExpression, SyntaxNode rootNode); + protected abstract IEnumerable GetPotentialPreviousReturnsLikeInvocations(IEnumerable nodes, SemanticModel semanticModel); + protected IEnumerable GetRelatedNodes(Compilation compilation, SemanticModel semanticModel, SyntaxNode syntaxNode) { if (compilation.ContainsSyntaxTree(syntaxNode.SyntaxTree) == false) @@ -31,7 +45,7 @@ protected IEnumerable 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 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; } } } \ No newline at end of file diff --git a/src/NSubstitute.Analyzers.Shared/Extensions/ISymbolExtensions.cs b/src/NSubstitute.Analyzers.Shared/Extensions/ISymbolExtensions.cs index 0ab337cd..831db435 100644 --- 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 index 8b929825..4fc4e9ea 100644 --- 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 { - public static ReEntrantCallFinder Instance { get; } = new ReEntrantCallFinder(); + public static ReEntrantCallFinder Instance { get; } = new ReEntrantCallFinder(SubstitutionNodeFinder.Instance); - private ReEntrantCallFinder() + private ReEntrantCallFinder(ISubstitutionNodeFinder substitutionNodeFinder) + : base(substitutionNodeFinder) { } @@ -23,6 +24,31 @@ protected override ImmutableList GetReEntrantSymbols(Compilation compil return visitor.InvocationSymbols; } + protected override IEnumerable GetPotentialPreviousReturnsLikeInvocations(IEnumerable 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/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReEntrantReturnsSetupDiagnosticVerifier.cs b/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReEntrantReturnsSetupDiagnosticVerifier.cs index 357cf394..ec17a27f 100644 --- a/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReEntrantReturnsSetupDiagnosticVerifier.cs +++ b/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/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(); diff --git a/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsExtensionMethodTests.cs b/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsExtensionMethodTests.cs index d7957d3f..7432efac 100644 --- a/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsExtensionMethodTests.cs +++ b/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsExtensionMethodTests.cs @@ -12,19 +12,16 @@ public override async Task ReportsDiagnostic_WhenUsingReEntrantReturnsViaMethodC { var plainMethodName = method.Replace("", string.Empty); var source = $@"using NSubstitute; - namespace MyNamespace {{ public interface Foo {{ int Bar(); }} - public interface IBar {{ int Foo(); }} - public class FooTests {{ public void Test() @@ -32,13 +29,10 @@ public void Test() var substitute = Substitute.For(); substitute.Bar().{method}([|ReturnThis()|], [|OtherReturn()|]); }} - - private int ReturnThis() {{ return OtherReturn(); }} - private int OtherReturn() {{ var substitute = Substitute.For(); @@ -634,6 +628,129 @@ public void Test() }} }} }} +}}"; + await VerifyNoDiagnostic(source); + } + + public override async Task ReportsDiagnostics_WhenReturnValueIsCalledWhileBeingConfigured(string method) + { + var plainMethodName = method.Replace("", string.Empty); + var source = $@"using NSubstitute; + +namespace MyNamespace +{{ + public class FooTests + {{ + public interface IFoo + {{ + int Id {{ get; }} + }} + + public void Test() + {{ + var firstSubstitute = Substitute.For(); + firstSubstitute.Id.Returns(45); + + var secondSubstitute = Substitute.For(); + secondSubstitute.Id.{method}([|firstSubstitute.Id|]); + }} + }} +}}"; + await VerifyDiagnostic(source, Descriptor, $"{plainMethodName}() is set with a method that itself calls Id. This can cause problems with NSubstitute. Consider replacing with a lambda: {plainMethodName}(x => firstSubstitute.Id)."); + } + + public override async Task ReportsDiagnostics_WhenReturnValueIsCalledWhileBeingConfiguredInConstructorBody(string method) + { + var plainMethodName = method.Replace("SubstituteExtensions.", string.Empty).Replace("", string.Empty); + + var source = $@"using NSubstitute; + +namespace MyNamespace +{{ + public class FooTests + {{ + private IFoo firstSubstitute = Substitute.For(); + + public FooTests() + {{ + firstSubstitute.Id.Returns(45); + }} + + public interface IFoo + {{ + int Id {{ get; }} + }} + + public void Test() + {{ + var secondSubstitute = Substitute.For(); + secondSubstitute.Id.{method}([|firstSubstitute.Id|]); + }} + }} +}}"; + await VerifyDiagnostic(source, Descriptor, $"{plainMethodName}() is set with a method that itself calls Id. This can cause problems with NSubstitute. Consider replacing with a lambda: {plainMethodName}(x => firstSubstitute.Id)."); + } + + public override async Task ReportsDiagnostics_WhenReturnValueIsCalledWhileBeingConfiguredInConstructorExpressionBody(string method) + { + var plainMethodName = method.Replace("SubstituteExtensions.", string.Empty).Replace("", string.Empty); + + var source = $@"using NSubstitute; + +namespace MyNamespace +{{ + public class FooTests + {{ + private IFoo firstSubstitute = Substitute.For(); + + public FooTests() => firstSubstitute.Id.Returns(45); + + public interface IFoo + {{ + int Id {{ get; }} + }} + + public void Test() + {{ + var secondSubstitute = Substitute.For(); + secondSubstitute.Id.{method}([|firstSubstitute.Id|]); + }} + }} +}}"; + await VerifyDiagnostic(source, Descriptor, $"{plainMethodName}() is set with a method that itself calls Id. This can cause problems with NSubstitute. Consider replacing with a lambda: {plainMethodName}(x => firstSubstitute.Id)."); + } + + public override async Task ReportsNoDiagnostics_WhenReturnValueIsCalledAfterIsConfigured(string method) + { + var source = $@"using NSubstitute; + +namespace MyNamespace +{{ + public class FooTests + {{ + public interface IFoo + {{ + int Id {{ get; }} + + int OtherId {{ get; }} + }} + + public void Test() + {{ + var firstSubstitute = Substitute.For(); + var secondSubstitute = Substitute.For(); + var thirdSubstitute = Substitute.For(); + var fourthSubstitute = Substitute.For(); + + firstSubstitute.OtherId.Returns(45); + thirdSubstitute.Id.Returns(45); + fourthSubstitute.Id.Returns(45); + var value = fourthSubstitute.Id; + + secondSubstitute.Id.{method}(firstSubstitute.Id); + secondSubstitute.Id.{method}(value); + }} + }} }}"; await VerifyNoDiagnostic(source); } diff --git a/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsOrdinaryMethodTests.cs b/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsOrdinaryMethodTests.cs index 5670cb52..64d8fcf8 100644 --- a/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsOrdinaryMethodTests.cs +++ b/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsOrdinaryMethodTests.cs @@ -633,6 +633,130 @@ public void Test() }} }} }} +}}"; + await VerifyNoDiagnostic(source); + } + + public override async Task ReportsDiagnostics_WhenReturnValueIsCalledWhileBeingConfigured(string method) + { + var plainMethodName = method.Replace("SubstituteExtensions.", string.Empty).Replace("", string.Empty); + + var source = $@"using NSubstitute; + +namespace MyNamespace +{{ + public class FooTests + {{ + public interface IFoo + {{ + int Id {{ get; }} + }} + + public void Test() + {{ + var firstSubstitute = Substitute.For(); + firstSubstitute.Id.Returns(45); + + var secondSubstitute = Substitute.For(); + {method}(secondSubstitute.Id, [|firstSubstitute.Id|]); + }} + }} +}}"; + await VerifyDiagnostic(source, Descriptor, $"{plainMethodName}() is set with a method that itself calls Id. This can cause problems with NSubstitute. Consider replacing with a lambda: {plainMethodName}(x => firstSubstitute.Id)."); + } + + public override async Task ReportsDiagnostics_WhenReturnValueIsCalledWhileBeingConfiguredInConstructorBody(string method) + { + var plainMethodName = method.Replace("SubstituteExtensions.", string.Empty).Replace("", string.Empty); + + var source = $@"using NSubstitute; + +namespace MyNamespace +{{ + public class FooTests + {{ + private IFoo firstSubstitute = Substitute.For(); + + public FooTests() + {{ + firstSubstitute.Id.Returns(45); + }} + + public interface IFoo + {{ + int Id {{ get; }} + }} + + public void Test() + {{ + var secondSubstitute = Substitute.For(); + {method}(secondSubstitute.Id, [|firstSubstitute.Id|]); + }} + }} +}}"; + await VerifyDiagnostic(source, Descriptor, $"{plainMethodName}() is set with a method that itself calls Id. This can cause problems with NSubstitute. Consider replacing with a lambda: {plainMethodName}(x => firstSubstitute.Id)."); + } + + public override async Task ReportsDiagnostics_WhenReturnValueIsCalledWhileBeingConfiguredInConstructorExpressionBody(string method) + { + var plainMethodName = method.Replace("SubstituteExtensions.", string.Empty).Replace("", string.Empty); + + var source = $@"using NSubstitute; + +namespace MyNamespace +{{ + public class FooTests + {{ + private IFoo firstSubstitute = Substitute.For(); + + public FooTests() => firstSubstitute.Id.Returns(45); + + public interface IFoo + {{ + int Id {{ get; }} + }} + + public void Test() + {{ + var secondSubstitute = Substitute.For(); + {method}(secondSubstitute.Id, [|firstSubstitute.Id|]); + }} + }} +}}"; + await VerifyDiagnostic(source, Descriptor, $"{plainMethodName}() is set with a method that itself calls Id. This can cause problems with NSubstitute. Consider replacing with a lambda: {plainMethodName}(x => firstSubstitute.Id)."); + } + + public override async Task ReportsNoDiagnostics_WhenReturnValueIsCalledAfterIsConfigured(string method) + { + var source = $@"using NSubstitute; + +namespace MyNamespace +{{ + public class FooTests + {{ + public interface IFoo + {{ + int Id {{ get; }} + + int OtherId {{ get; }} + }} + + public void Test() + {{ + var firstSubstitute = Substitute.For(); + var secondSubstitute = Substitute.For(); + var thirdSubstitute = Substitute.For(); + var fourthSubstitute = Substitute.For(); + + firstSubstitute.OtherId.Returns(45); + thirdSubstitute.Id.Returns(45); + fourthSubstitute.Id.Returns(45); + var value = fourthSubstitute.Id; + + {method}(secondSubstitute.Id, firstSubstitute.Id); + {method}(secondSubstitute.Id, value); + }} + }} }}"; await VerifyNoDiagnostic(source); } diff --git a/tests/NSubstitute.Analyzers.Tests.Shared/DiagnosticAnalyzers/IReEntrantReturnsSetupDiagnosticVerifier.cs b/tests/NSubstitute.Analyzers.Tests.Shared/DiagnosticAnalyzers/IReEntrantReturnsSetupDiagnosticVerifier.cs index f8dcdfea..4ac5d6ef 100644 --- a/tests/NSubstitute.Analyzers.Tests.Shared/DiagnosticAnalyzers/IReEntrantReturnsSetupDiagnosticVerifier.cs +++ b/tests/NSubstitute.Analyzers.Tests.Shared/DiagnosticAnalyzers/IReEntrantReturnsSetupDiagnosticVerifier.cs @@ -29,5 +29,11 @@ public interface IReEntrantReturnsSetupDiagnosticVerifier Task ReportsNoDiagnostics_WhenReturnsValueIsSet_InForEachLoop(string method); Task ReportsNoDiagnostics_WhenElementUsedTwice_InForEachLoop(string method); + + Task ReportsDiagnostics_WhenReturnValueIsCalledWhileBeingConfigured(string method); + + Task ReportsDiagnostics_WhenReturnValueIsCalledWhileBeingConfiguredInConstructorBody(string method); + + Task ReportsNoDiagnostics_WhenReturnValueIsCalledAfterIsConfigured(string method); } } \ No newline at end of file diff --git a/tests/NSubstitute.Analyzers.Tests.VisualBasic/DiagnosticAnalyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReEntrantReturnsSetupDiagnosticVerifier.cs b/tests/NSubstitute.Analyzers.Tests.VisualBasic/DiagnosticAnalyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReEntrantReturnsSetupDiagnosticVerifier.cs index b908933d..f10720a6 100644 --- a/tests/NSubstitute.Analyzers.Tests.VisualBasic/DiagnosticAnalyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReEntrantReturnsSetupDiagnosticVerifier.cs +++ b/tests/NSubstitute.Analyzers.Tests.VisualBasic/DiagnosticAnalyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReEntrantReturnsSetupDiagnosticVerifier.cs @@ -102,6 +102,18 @@ Dim barr As IBar [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 ReportsNoDiagnostics_WhenReturnValueIsCalledAfterIsConfigured(string method); + protected override DiagnosticAnalyzer GetDiagnosticAnalyzer() { return new ReEntrantSetupAnalyzer(); diff --git a/tests/NSubstitute.Analyzers.Tests.VisualBasic/DiagnosticAnalyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsExtensionMethodTests.cs b/tests/NSubstitute.Analyzers.Tests.VisualBasic/DiagnosticAnalyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsExtensionMethodTests.cs index b6234059..be6e60d0 100644 --- a/tests/NSubstitute.Analyzers.Tests.VisualBasic/DiagnosticAnalyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsExtensionMethodTests.cs +++ b/tests/NSubstitute.Analyzers.Tests.VisualBasic/DiagnosticAnalyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsExtensionMethodTests.cs @@ -543,5 +543,86 @@ End Class await VerifyNoDiagnostic(source); } + + public override async Task ReportsDiagnostics_WhenReturnValueIsCalledWhileBeingConfigured(string method) + { + var plainMethodName = method.Replace("", string.Empty); + var source = $@"Imports NSubstitute + +Namespace MyNamespace + Public Class FooTests + Public Interface IFoo + ReadOnly Property Id As Integer + End Interface + + Public Sub Test() + Dim firstSubstitute = Substitute.[For](Of IFoo)() + firstSubstitute.Id.Returns(45) + + Dim secondSubstitute = Substitute.[For](Of IFoo)() + secondSubstitute.Id.{method}([|firstSubstitute.Id|]) + End Sub + End Class +End Namespace"; + + await VerifyDiagnostic(source, Descriptor, $"{plainMethodName}() is set with a method that itself calls Id. This can cause problems with NSubstitute. Consider replacing with a lambda: {plainMethodName}(Function(x) firstSubstitute.Id)."); + } + + public override async Task ReportsDiagnostics_WhenReturnValueIsCalledWhileBeingConfiguredInConstructorBody(string method) + { + var plainMethodName = method.Replace("SubstituteExtensions.", string.Empty).Replace("", string.Empty); + + var source = $@"Imports NSubstitute + +Namespace MyNamespace + Public Class FooTests + Private firstSubstitute As IFoo = Substitute.[For](Of IFoo)() + + Public Sub New() + firstSubstitute.Id.Returns(45) + End Sub + + Public Interface IFoo + ReadOnly Property Id As Integer + End Interface + + Public Sub Test() + Dim secondSubstitute = Substitute.[For](Of IFoo)() + secondSubstitute.Id.{method}([|firstSubstitute.Id|]) + End Sub + End Class +End Namespace +"; + await VerifyDiagnostic(source, Descriptor, $"{plainMethodName}() is set with a method that itself calls Id. This can cause problems with NSubstitute. Consider replacing with a lambda: {plainMethodName}(Function(x) firstSubstitute.Id)."); + } + + public override async Task ReportsNoDiagnostics_WhenReturnValueIsCalledAfterIsConfigured(string method) + { + var source = $@"Imports NSubstitute + +Namespace MyNamespace + Public Class FooTests + Interface IFoo + ReadOnly Property Id As Integer + ReadOnly Property OtherId As Integer + End Interface + + Public Sub Test() + Dim firstSubstitute = Substitute.[For](Of IFoo)() + Dim secondSubstitute = Substitute.[For](Of IFoo)() + Dim thirdSubstitute = Substitute.[For](Of IFoo)() + Dim fourthSubstitute = Substitute.[For](Of IFoo)() + firstSubstitute.OtherId.Returns(45) + thirdSubstitute.Id.Returns(45) + fourthSubstitute.Id.Returns(45) + Dim value = fourthSubstitute.Id + secondSubstitute.Id.{method}(firstSubstitute.Id) + secondSubstitute.Id.{method}(value) + End Sub + End Class +End Namespace"; + + await VerifyNoDiagnostic(source); + } } } \ No newline at end of file diff --git a/tests/NSubstitute.Analyzers.Tests.VisualBasic/DiagnosticAnalyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsOrdinaryMethodTests.cs b/tests/NSubstitute.Analyzers.Tests.VisualBasic/DiagnosticAnalyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsOrdinaryMethodTests.cs index 50b32660..ff7452b7 100644 --- a/tests/NSubstitute.Analyzers.Tests.VisualBasic/DiagnosticAnalyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsOrdinaryMethodTests.cs +++ b/tests/NSubstitute.Analyzers.Tests.VisualBasic/DiagnosticAnalyzersTests/ReEntrantReturnsSetupAnalyzerTests/ReturnsAsOrdinaryMethodTests.cs @@ -552,5 +552,63 @@ End Class await VerifyNoDiagnostic(source); } + + public override async Task ReportsDiagnostics_WhenReturnValueIsCalledWhileBeingConfigured(string method) + { + var plainMethodName = method.Replace("SubstituteExtensions.", string.Empty).Replace("(Of Integer)", string.Empty); + + var source = $@"Imports NSubstitute + +Namespace MyNamespace + Public Class FooTests + Public Interface IFoo + ReadOnly Property Id As Integer + End Interface + + Public Sub Test() + Dim firstSubstitute = Substitute.[For](Of IFoo)() + firstSubstitute.Id.Returns(45) + Dim secondSubstitute = Substitute.[For](Of IFoo)() + {method}(secondSubstitute.Id, [|firstSubstitute.Id|]) + End Sub + End Class +End Namespace"; + + await VerifyDiagnostic(source, Descriptor, $"{plainMethodName}() is set with a method that itself calls Id. This can cause problems with NSubstitute. Consider replacing with a lambda: {plainMethodName}(Function(x) firstSubstitute.Id)."); + } + + public override Task ReportsDiagnostics_WhenReturnValueIsCalledWhileBeingConfiguredInConstructorBody(string method) + { + throw new System.NotImplementedException(); + } + + public override async Task ReportsNoDiagnostics_WhenReturnValueIsCalledAfterIsConfigured(string method) + { + var source = $@"Imports NSubstitute + +Namespace MyNamespace + Public Class FooTests + Interface IFoo + ReadOnly Property Id As Integer + ReadOnly Property OtherId As Integer + End Interface + + Public Sub Test() + Dim firstSubstitute = Substitute.[For](Of IFoo)() + Dim secondSubstitute = Substitute.[For](Of IFoo)() + Dim thirdSubstitute = Substitute.[For](Of IFoo)() + Dim fourthSubstitute = Substitute.[For](Of IFoo)() + firstSubstitute.OtherId.Returns(45) + thirdSubstitute.Id.Returns(45) + fourthSubstitute.Id.Returns(45) + Dim value = fourthSubstitute.Id + {method}(secondSubstitute.Id, firstSubstitute.Id) + {method}(secondSubstitute.Id, value) + End Sub + End Class +End Namespace"; + + await VerifyNoDiagnostic(source); + } } } \ No newline at end of file