Skip to content

Commit

Permalink
[GH-87] - Better warning location for non-virtual members in When sub…
Browse files Browse the repository at this point in the history
…stitutions
  • Loading branch information
tpodolak committed Apr 7, 2019
1 parent 7aac44f commit 6e3b3cf
Show file tree
Hide file tree
Showing 10 changed files with 44 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
namespace NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
internal class NonSubstitutableMemberReceivedAnalyzer : AbstractNonSubstitutableMemberReceivedAnalyzer<SyntaxKind>
internal class NonSubstitutableMemberReceivedAnalyzer : AbstractNonSubstitutableMemberReceivedAnalyzer<SyntaxKind, MemberAccessExpressionSyntax>
{
protected override ImmutableArray<Parent> PossibleParents { get; } = ImmutableArray.Create(
Parent.Create<MemberAccessExpressionSyntax>(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
namespace NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
internal class NonSubstitutableMemberWhenAnalyzer : AbstractNonSubstitutableMemberWhenAnalyzer<SyntaxKind, InvocationExpressionSyntax>
internal class NonSubstitutableMemberWhenAnalyzer : AbstractNonSubstitutableMemberWhenAnalyzer<SyntaxKind, InvocationExpressionSyntax, MemberAccessExpressionSyntax>
{
public NonSubstitutableMemberWhenAnalyzer()
: base(new DiagnosticDescriptorsProvider())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@

namespace NSubstitute.Analyzers.Shared.DiagnosticAnalyzers
{
internal abstract class AbstractNonSubstitutableMemberReceivedAnalyzer<TSyntaxKind> : AbstractDiagnosticAnalyzer
internal abstract class AbstractNonSubstitutableMemberReceivedAnalyzer<TSyntaxKind, TMemberAccessExpressionSyntax> : AbstractDiagnosticAnalyzer
where TSyntaxKind : struct
where TMemberAccessExpressionSyntax : SyntaxNode
{
private static readonly ImmutableHashSet<string> MethodNames = ImmutableHashSet.Create(
MetadataNames.NSubstituteReceivedMethod,
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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<TMemberAccessExpressionSyntax>(symbol);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@

namespace NSubstitute.Analyzers.Shared.DiagnosticAnalyzers
{
internal abstract class AbstractNonSubstitutableMemberWhenAnalyzer<TSyntaxKind, TInvocationExpressionSyntax> : AbstractDiagnosticAnalyzer
internal abstract class AbstractNonSubstitutableMemberWhenAnalyzer<TSyntaxKind, TInvocationExpressionSyntax, TMemberAccessExpressionSyntax> : AbstractDiagnosticAnalyzer
where TInvocationExpressionSyntax : SyntaxNode
where TSyntaxKind : struct
where TMemberAccessExpressionSyntax : SyntaxNode
where TSyntaxKind : struct, Enum
{
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(
DiagnosticDescriptorsProvider.NonVirtualWhenSetupSpecification,
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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<TMemberAccessExpressionSyntax>(symbolInfo.Symbol);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,13 @@ public static SyntaxNode GetParentNode(this SyntaxNode syntaxNode, IEnumerable<i

return null;
}

public static Location GetSubstitutionNodeActualLocation<TMemberAccessExpression>(this SyntaxNode node, ISymbol symbol)
where TMemberAccessExpression : SyntaxNode
{
var actualNode = node is TMemberAccessExpression && symbol is IMethodSymbol _ ? node.Parent : node;

return actualNode.GetLocation();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
namespace NSubstitute.Analyzers.VisualBasic.DiagnosticAnalyzers
{
[DiagnosticAnalyzer(LanguageNames.VisualBasic)]
internal class NonSubstitutableMemberReceivedAnalyzer : AbstractNonSubstitutableMemberReceivedAnalyzer<SyntaxKind>
internal class NonSubstitutableMemberReceivedAnalyzer : AbstractNonSubstitutableMemberReceivedAnalyzer<SyntaxKind, MemberAccessExpressionSyntax>
{
protected override ImmutableArray<Parent> PossibleParents { get; } = ImmutableArray.Create(
Parent.Create<MemberAccessExpressionSyntax>(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
namespace NSubstitute.Analyzers.VisualBasic.DiagnosticAnalyzers
{
[DiagnosticAnalyzer(LanguageNames.VisualBasic)]
internal class NonSubstitutableMemberWhenAnalyzer : AbstractNonSubstitutableMemberWhenAnalyzer<SyntaxKind, InvocationExpressionSyntax>
internal class NonSubstitutableMemberWhenAnalyzer : AbstractNonSubstitutableMemberWhenAnalyzer<SyntaxKind, InvocationExpressionSyntax, MemberAccessExpressionSyntax>
{
public NonSubstitutableMemberWhenAnalyzer()
: base(new DiagnosticDescriptorsProvider())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ public abstract class NonSubstitutableMemberWhenDiagnosticVerifier : CSharpDiagn
protected DiagnosticDescriptor InternalSetupSpecificationDescriptor { get; } = DiagnosticDescriptors<DiagnosticDescriptorsProvider>.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]
Expand All @@ -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]
Expand Down Expand Up @@ -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);

Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ public void Test()
void SubstituteCall(Foo sub)
{{
[|sub.Bar|]();
[|sub.Bar()|];
}}
substitute.{method}(SubstituteCall).Do(callInfo => i++);
Expand Down Expand Up @@ -465,7 +465,7 @@ public void Test()
int i = 0;
var substitute = NSubstitute.Substitute.For<Foo>();
void SubstituteCall(Foo sub) => [|sub.Bar|]();
void SubstituteCall(Foo sub) => [|sub.Bar()|];
substitute.{method}(SubstituteCall).Do(callInfo => i++);
substitute.Bar();
Expand Down Expand Up @@ -503,7 +503,7 @@ public void Test()
private void SubstituteCall(Foo sub)
{{
var objBarr = [|sub.Bar|]();
var objBarr = [|sub.Bar()|];
}}
}}
}}";
Expand Down Expand Up @@ -536,7 +536,7 @@ public void Test()
substitute.Bar();
}}
private void SubstituteCall(Foo sub) => [|sub.Bar|]();
private void SubstituteCall(Foo sub) => [|sub.Bar()|];
}}
}}";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ public void Test()
void SubstituteCall(Foo sub)
{{
[|sub.Bar|]();
[|sub.Bar()|];
}}
{method}(substitute, SubstituteCall).Do(callInfo => i++);
Expand Down Expand Up @@ -464,7 +464,7 @@ public void Test()
int i = 0;
var substitute = NSubstitute.Substitute.For<Foo>();
void SubstituteCall(Foo sub) => [|sub.Bar|]();
void SubstituteCall(Foo sub) => [|sub.Bar()|];
{method}(substitute, SubstituteCall).Do(callInfo => i++);
substitute.Bar();
Expand Down Expand Up @@ -502,7 +502,7 @@ public void Test()
private void SubstituteCall(Foo sub)
{{
var objBarr = [|sub.Bar|]();
var objBarr = [|sub.Bar()|];
}}
}}
}}";
Expand Down Expand Up @@ -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.");
Expand Down

0 comments on commit 6e3b3cf

Please sign in to comment.