From 6e3b3cf2cc4f19c49eafbbf89ba000c073f3ddf2 Mon Sep 17 00:00:00 2001 From: tpodolak Date: Sun, 7 Apr 2019 00:40:36 +0200 Subject: [PATCH] [GH-87] - Better warning location for non-virtual members in When substitutions --- .../NonSubstitutableMemberReceivedAnalyzer.cs | 2 +- .../NonSubstitutableMemberWhenAnalyzer.cs | 2 +- ...ractNonSubstitutableMemberReceivedAnalyzer.cs | 13 ++++++------- ...AbstractNonSubstitutableMemberWhenAnalyzer.cs | 14 ++++++++++---- .../Extensions/SyntaxNodeExtensions.cs | 8 ++++++++ .../NonSubstitutableMemberReceivedAnalyzer.cs | 2 +- .../NonSubstitutableMemberWhenAnalyzer.cs | 2 +- ...nSubstitutableMemberWhenDiagnosticVerifier.cs | 16 ++++++++-------- .../WhenAsExtensionMethodTests.cs | 8 ++++---- .../WhenAsOrdinaryMethodTests.cs | 8 ++++---- 10 files changed, 44 insertions(+), 31 deletions(-) diff --git a/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/NonSubstitutableMemberReceivedAnalyzer.cs b/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/NonSubstitutableMemberReceivedAnalyzer.cs index 1f2f02b2..96c9b45a 100644 --- a/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/NonSubstitutableMemberReceivedAnalyzer.cs +++ b/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/NonSubstitutableMemberReceivedAnalyzer.cs @@ -9,7 +9,7 @@ namespace NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers { [DiagnosticAnalyzer(LanguageNames.CSharp)] - internal class NonSubstitutableMemberReceivedAnalyzer : AbstractNonSubstitutableMemberReceivedAnalyzer + internal class NonSubstitutableMemberReceivedAnalyzer : AbstractNonSubstitutableMemberReceivedAnalyzer { protected override ImmutableArray PossibleParents { get; } = ImmutableArray.Create( Parent.Create(), diff --git a/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/NonSubstitutableMemberWhenAnalyzer.cs b/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/NonSubstitutableMemberWhenAnalyzer.cs index 16aaf349..021c10b2 100644 --- a/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/NonSubstitutableMemberWhenAnalyzer.cs +++ b/src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/NonSubstitutableMemberWhenAnalyzer.cs @@ -9,7 +9,7 @@ namespace NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers { [DiagnosticAnalyzer(LanguageNames.CSharp)] - internal class NonSubstitutableMemberWhenAnalyzer : AbstractNonSubstitutableMemberWhenAnalyzer + internal class NonSubstitutableMemberWhenAnalyzer : AbstractNonSubstitutableMemberWhenAnalyzer { public NonSubstitutableMemberWhenAnalyzer() : base(new DiagnosticDescriptorsProvider()) diff --git a/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/AbstractNonSubstitutableMemberReceivedAnalyzer.cs b/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/AbstractNonSubstitutableMemberReceivedAnalyzer.cs index 0514ff1f..08c9cd78 100644 --- a/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/AbstractNonSubstitutableMemberReceivedAnalyzer.cs +++ b/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/AbstractNonSubstitutableMemberReceivedAnalyzer.cs @@ -8,8 +8,9 @@ namespace NSubstitute.Analyzers.Shared.DiagnosticAnalyzers { - internal abstract class AbstractNonSubstitutableMemberReceivedAnalyzer : AbstractDiagnosticAnalyzer + internal abstract class AbstractNonSubstitutableMemberReceivedAnalyzer : AbstractDiagnosticAnalyzer where TSyntaxKind : struct + where TMemberAccessExpressionSyntax : SyntaxNode { private static readonly ImmutableHashSet MethodNames = ImmutableHashSet.Create( MetadataNames.NSubstituteReceivedMethod, @@ -76,7 +77,7 @@ private void AnalyzeInvocation(SyntaxNodeAnalysisContext syntaxNodeContext) { var diagnostic = Diagnostic.Create( DiagnosticDescriptorsProvider.NonVirtualReceivedSetupSpecification, - GetSymbolLocation(parentNode, symbolInfo.Symbol), + GetSubstitutionNodeActualLocation(parentNode, symbolInfo.Symbol), symbolInfo.Symbol.Name); syntaxNodeContext.ReportDiagnostic(diagnostic); @@ -86,7 +87,7 @@ private void AnalyzeInvocation(SyntaxNodeAnalysisContext syntaxNodeContext) { var diagnostic = Diagnostic.Create( DiagnosticDescriptorsProvider.InternalSetupSpecification, - GetSymbolLocation(parentNode, symbolInfo.Symbol), + GetSubstitutionNodeActualLocation(parentNode, symbolInfo.Symbol), symbolInfo.Symbol.Name); syntaxNodeContext.ReportDiagnostic(diagnostic); @@ -118,11 +119,9 @@ private SyntaxNode GetKnownParent(SyntaxNode receivedSyntaxNode) return null; } - private Location GetSymbolLocation(SyntaxNode syntaxNode, ISymbol symbol) + private Location GetSubstitutionNodeActualLocation(SyntaxNode syntaxNode, ISymbol symbol) { - var actualNode = symbol is IMethodSymbol _ ? syntaxNode.Parent : syntaxNode; - - return actualNode.GetLocation(); + return syntaxNode.GetSubstitutionNodeActualLocation(symbol); } } } \ No newline at end of file diff --git a/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/AbstractNonSubstitutableMemberWhenAnalyzer.cs b/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/AbstractNonSubstitutableMemberWhenAnalyzer.cs index 696f3bf3..6eaa0ab9 100644 --- a/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/AbstractNonSubstitutableMemberWhenAnalyzer.cs +++ b/src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/AbstractNonSubstitutableMemberWhenAnalyzer.cs @@ -8,9 +8,10 @@ namespace NSubstitute.Analyzers.Shared.DiagnosticAnalyzers { - internal abstract class AbstractNonSubstitutableMemberWhenAnalyzer : AbstractDiagnosticAnalyzer + internal abstract class AbstractNonSubstitutableMemberWhenAnalyzer : AbstractDiagnosticAnalyzer where TInvocationExpressionSyntax : SyntaxNode - where TSyntaxKind : struct + where TMemberAccessExpressionSyntax : SyntaxNode + where TSyntaxKind : struct, Enum { public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create( DiagnosticDescriptorsProvider.NonVirtualWhenSetupSpecification, @@ -67,7 +68,7 @@ private void AnalyzeInvocation(SyntaxNodeAnalysisContext syntaxNodeContext) { var diagnostic = Diagnostic.Create( DiagnosticDescriptorsProvider.NonVirtualWhenSetupSpecification, - analysedSyntax.GetLocation(), + GetSubstitutionNodeActualLocation(analysedSyntax, symbolInfo), symbolInfo.Symbol.Name); syntaxNodeContext.ReportDiagnostic(diagnostic); @@ -77,7 +78,7 @@ private void AnalyzeInvocation(SyntaxNodeAnalysisContext syntaxNodeContext) { var diagnostic = Diagnostic.Create( DiagnosticDescriptorsProvider.InternalSetupSpecification, - analysedSyntax.GetLocation(), + GetSubstitutionNodeActualLocation(analysedSyntax, symbolInfo), symbolInfo.Symbol.Name); syntaxNodeContext.ReportDiagnostic(diagnostic); @@ -98,5 +99,10 @@ private bool IsWhenLikeMethod(SyntaxNodeAnalysisContext syntaxNodeContext, Synta return symbol.Symbol?.ContainingAssembly?.Name.Equals(MetadataNames.NSubstituteAssemblyName, StringComparison.OrdinalIgnoreCase) == true && symbol.Symbol?.ContainingType?.ToString().Equals(MetadataNames.NSubstituteSubstituteExtensionsFullTypeName, StringComparison.OrdinalIgnoreCase) == true; } + + private static Location GetSubstitutionNodeActualLocation(SyntaxNode analysedSyntax, SymbolInfo symbolInfo) + { + return analysedSyntax.GetSubstitutionNodeActualLocation(symbolInfo.Symbol); + } } } \ No newline at end of file diff --git a/src/NSubstitute.Analyzers.Shared/Extensions/SyntaxNodeExtensions.cs b/src/NSubstitute.Analyzers.Shared/Extensions/SyntaxNodeExtensions.cs index 2571b312..8ce69a7e 100644 --- a/src/NSubstitute.Analyzers.Shared/Extensions/SyntaxNodeExtensions.cs +++ b/src/NSubstitute.Analyzers.Shared/Extensions/SyntaxNodeExtensions.cs @@ -28,5 +28,13 @@ public static SyntaxNode GetParentNode(this SyntaxNode syntaxNode, IEnumerable(this SyntaxNode node, ISymbol symbol) + where TMemberAccessExpression : SyntaxNode + { + var actualNode = node is TMemberAccessExpression && symbol is IMethodSymbol _ ? node.Parent : node; + + return actualNode.GetLocation(); + } } } \ No newline at end of file diff --git a/src/NSubstitute.Analyzers.VisualBasic/DiagnosticAnalyzers/NonSubstitutableMemberReceivedAnalyzer.cs b/src/NSubstitute.Analyzers.VisualBasic/DiagnosticAnalyzers/NonSubstitutableMemberReceivedAnalyzer.cs index ca0f0cab..da02b91f 100644 --- a/src/NSubstitute.Analyzers.VisualBasic/DiagnosticAnalyzers/NonSubstitutableMemberReceivedAnalyzer.cs +++ b/src/NSubstitute.Analyzers.VisualBasic/DiagnosticAnalyzers/NonSubstitutableMemberReceivedAnalyzer.cs @@ -8,7 +8,7 @@ namespace NSubstitute.Analyzers.VisualBasic.DiagnosticAnalyzers { [DiagnosticAnalyzer(LanguageNames.VisualBasic)] - internal class NonSubstitutableMemberReceivedAnalyzer : AbstractNonSubstitutableMemberReceivedAnalyzer + internal class NonSubstitutableMemberReceivedAnalyzer : AbstractNonSubstitutableMemberReceivedAnalyzer { protected override ImmutableArray PossibleParents { get; } = ImmutableArray.Create( Parent.Create(), diff --git a/src/NSubstitute.Analyzers.VisualBasic/DiagnosticAnalyzers/NonSubstitutableMemberWhenAnalyzer.cs b/src/NSubstitute.Analyzers.VisualBasic/DiagnosticAnalyzers/NonSubstitutableMemberWhenAnalyzer.cs index ed38b502..9c907663 100644 --- a/src/NSubstitute.Analyzers.VisualBasic/DiagnosticAnalyzers/NonSubstitutableMemberWhenAnalyzer.cs +++ b/src/NSubstitute.Analyzers.VisualBasic/DiagnosticAnalyzers/NonSubstitutableMemberWhenAnalyzer.cs @@ -9,7 +9,7 @@ namespace NSubstitute.Analyzers.VisualBasic.DiagnosticAnalyzers { [DiagnosticAnalyzer(LanguageNames.VisualBasic)] - internal class NonSubstitutableMemberWhenAnalyzer : AbstractNonSubstitutableMemberWhenAnalyzer + internal class NonSubstitutableMemberWhenAnalyzer : AbstractNonSubstitutableMemberWhenAnalyzer { public NonSubstitutableMemberWhenAnalyzer() : base(new DiagnosticDescriptorsProvider()) diff --git a/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/NonSubstitutableMemberWhenAnalyzerTests/NonSubstitutableMemberWhenDiagnosticVerifier.cs b/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/NonSubstitutableMemberWhenAnalyzerTests/NonSubstitutableMemberWhenDiagnosticVerifier.cs index 8a83072a..d3152e7a 100644 --- a/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/NonSubstitutableMemberWhenAnalyzerTests/NonSubstitutableMemberWhenDiagnosticVerifier.cs +++ b/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/NonSubstitutableMemberWhenAnalyzerTests/NonSubstitutableMemberWhenDiagnosticVerifier.cs @@ -17,9 +17,9 @@ public abstract class NonSubstitutableMemberWhenDiagnosticVerifier : CSharpDiagn protected DiagnosticDescriptor InternalSetupSpecificationDescriptor { get; } = DiagnosticDescriptors.InternalSetupSpecification; [CombinatoryTheory] - [InlineData("sub => [|sub.Bar|]()")] - [InlineData("delegate(Foo sub) { [|sub.Bar|](); }")] - [InlineData("sub => { [|sub.Bar|](); }")] + [InlineData("sub => [|sub.Bar()|]")] + [InlineData("delegate(Foo sub) { [|sub.Bar()|]; }")] + [InlineData("sub => { [|sub.Bar()|]; }")] public abstract Task ReportsDiagnostics_WhenSettingValueForNonVirtualMethod(string method, string whenAction); [CombinatoryTheory] @@ -41,9 +41,9 @@ public abstract class NonSubstitutableMemberWhenDiagnosticVerifier : CSharpDiagn public abstract Task ReportsNoDiagnostics_WhenSettingValueForDelegate(string method, string whenAction); [CombinatoryTheory] - [InlineData("sub => [|sub.Bar|]()")] - [InlineData("delegate(Foo sub) { [|sub.Bar|](); }")] - [InlineData("sub => { [|sub.Bar|](); }")] + [InlineData("sub => [|sub.Bar()|]")] + [InlineData("delegate(Foo sub) { [|sub.Bar()|]; }")] + [InlineData("sub => { [|sub.Bar()|]; }")] public abstract Task ReportsDiagnostics_WhenSettingValueForSealedOverrideMethod(string method, string whenAction); [CombinatoryTheory] @@ -138,7 +138,7 @@ public abstract class NonSubstitutableMemberWhenDiagnosticVerifier : CSharpDiagn [CombinatoryTheory] [InlineData("sub => { var x = [|sub.Bar|]; }", "Internal member Bar can not be intercepted without InternalsVisibleToAttribute.")] - [InlineData("sub => [|sub.FooBar|]()", "Internal member FooBar can not be intercepted without InternalsVisibleToAttribute.")] + [InlineData("sub => [|sub.FooBar()|]", "Internal member FooBar can not be intercepted without InternalsVisibleToAttribute.")] [InlineData("sub => { var x = [|sub[0]|]; }", "Internal member this[] can not be intercepted without InternalsVisibleToAttribute.")] public abstract Task ReportsDiagnostics_WhenSettingValueForInternalVirtualMember_AndInternalsVisibleToNotApplied(string method, string call, string message); @@ -150,7 +150,7 @@ public abstract class NonSubstitutableMemberWhenDiagnosticVerifier : CSharpDiagn [CombinatoryTheory] [InlineData("sub => { var x = [|sub.Bar|]; }", "Internal member Bar can not be intercepted without InternalsVisibleToAttribute.")] - [InlineData("sub => [|sub.FooBar|]()", "Internal member FooBar can not be intercepted without InternalsVisibleToAttribute.")] + [InlineData("sub => [|sub.FooBar()|]", "Internal member FooBar can not be intercepted without InternalsVisibleToAttribute.")] [InlineData("sub => { var x = [|sub[0]|]; }", "Internal member this[] can not be intercepted without InternalsVisibleToAttribute.")] public abstract Task ReportsDiagnostics_WhenSettingValueForInternalVirtualMember_AndInternalsVisibleToAppliedToWrongAssembly(string method, string call, string message); diff --git a/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/NonSubstitutableMemberWhenAnalyzerTests/WhenAsExtensionMethodTests.cs b/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/NonSubstitutableMemberWhenAnalyzerTests/WhenAsExtensionMethodTests.cs index b329c2c1..6985e767 100644 --- a/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/NonSubstitutableMemberWhenAnalyzerTests/WhenAsExtensionMethodTests.cs +++ b/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/NonSubstitutableMemberWhenAnalyzerTests/WhenAsExtensionMethodTests.cs @@ -432,7 +432,7 @@ public void Test() void SubstituteCall(Foo sub) {{ - [|sub.Bar|](); + [|sub.Bar()|]; }} substitute.{method}(SubstituteCall).Do(callInfo => i++); @@ -465,7 +465,7 @@ public void Test() int i = 0; var substitute = NSubstitute.Substitute.For(); - void SubstituteCall(Foo sub) => [|sub.Bar|](); + void SubstituteCall(Foo sub) => [|sub.Bar()|]; substitute.{method}(SubstituteCall).Do(callInfo => i++); substitute.Bar(); @@ -503,7 +503,7 @@ public void Test() private void SubstituteCall(Foo sub) {{ - var objBarr = [|sub.Bar|](); + var objBarr = [|sub.Bar()|]; }} }} }}"; @@ -536,7 +536,7 @@ public void Test() substitute.Bar(); }} - private void SubstituteCall(Foo sub) => [|sub.Bar|](); + private void SubstituteCall(Foo sub) => [|sub.Bar()|]; }} }}"; diff --git a/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/NonSubstitutableMemberWhenAnalyzerTests/WhenAsOrdinaryMethodTests.cs b/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/NonSubstitutableMemberWhenAnalyzerTests/WhenAsOrdinaryMethodTests.cs index 72117b53..093ce29f 100644 --- a/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/NonSubstitutableMemberWhenAnalyzerTests/WhenAsOrdinaryMethodTests.cs +++ b/tests/NSubstitute.Analyzers.Tests.CSharp/DiagnosticAnalyzerTests/NonSubstitutableMemberWhenAnalyzerTests/WhenAsOrdinaryMethodTests.cs @@ -432,7 +432,7 @@ public void Test() void SubstituteCall(Foo sub) {{ - [|sub.Bar|](); + [|sub.Bar()|]; }} {method}(substitute, SubstituteCall).Do(callInfo => i++); @@ -464,7 +464,7 @@ public void Test() int i = 0; var substitute = NSubstitute.Substitute.For(); - void SubstituteCall(Foo sub) => [|sub.Bar|](); + void SubstituteCall(Foo sub) => [|sub.Bar()|]; {method}(substitute, SubstituteCall).Do(callInfo => i++); substitute.Bar(); @@ -502,7 +502,7 @@ public void Test() private void SubstituteCall(Foo sub) {{ - var objBarr = [|sub.Bar|](); + var objBarr = [|sub.Bar()|]; }} }} }}"; @@ -534,7 +534,7 @@ public void Test() substitute.Bar(); }} - private void SubstituteCall(Foo sub) => [|sub.Bar|](); + private void SubstituteCall(Foo sub) => [|sub.Bar()|]; }} }}"; await VerifyDiagnostic(source, NonVirtualWhenSetupSpecificationDescriptor, "Member Bar can not be intercepted. Only interface members and virtual, overriding, and abstract members can be intercepted.");