Skip to content

Commit

Permalink
Merge pull request #92 from nsubstitute/GH-91-erroneous-NS1002
Browse files Browse the repository at this point in the history 
[GH-91] - better checks for when symbols usage
  • Loading branch information
@tpodolak
tpodolak authored Apr 11, 2019
2 parents 7bad880 + fbd8573 commit 81549f2
Show file tree
Hide file tree
Showing 9 changed files with 256 additions and 241 deletions.
30 changes: 9 additions & 21 deletions src/NSubstitute.Analyzers.CSharp/DiagnosticAnalyzers/SubstitutionNodeFinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,27 +15,6 @@ namespace NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers
/// </summary>
internal class SubstitutionNodeFinder : AbstractSubstitutionNodeFinder<InvocationExpressionSyntax>
{
public override IEnumerable<SyntaxNode> FindForWhenExpression(SyntaxNodeAnalysisContext syntaxNodeContext, InvocationExpressionSyntax whenInvocationExpression, IMethodSymbol whenInvocationSymbol = null)
{
if (whenInvocationExpression == null)
{
return Enumerable.Empty<SyntaxNode>();
}

whenInvocationSymbol = whenInvocationSymbol ?? syntaxNodeContext.SemanticModel.GetSymbolInfo(whenInvocationExpression).Symbol as IMethodSymbol;

if (whenInvocationSymbol == null)
{
return Enumerable.Empty<SyntaxNode>();
}

var argumentExpression = whenInvocationSymbol.MethodKind == MethodKind.ReducedExtension
? whenInvocationExpression.ArgumentList.Arguments.First().Expression
: whenInvocationExpression.ArgumentList.Arguments.Skip(1).First().Expression;

return FindForWhenExpression(syntaxNodeContext, argumentExpression);
}

public override SyntaxNode FindForAndDoesExpression(SyntaxNodeAnalysisContext syntaxNodeContext, InvocationExpressionSyntax invocationExpression, IMethodSymbol invocationExpressionSymbol)
{
var parentInvocationExpression = invocationExpression?.GetParentInvocationExpression();
Expand Down Expand Up @@ -69,6 +48,15 @@ protected override InvocationExpressionSyntax GetParentInvocationExpression(Invo
return invocationExpressionSyntax.GetParentInvocationExpression();
}

protected override IEnumerable<SyntaxNode> FindForWhenExpressionInternal(SyntaxNodeAnalysisContext syntaxNodeContext, InvocationExpressionSyntax whenInvocationExpression, IMethodSymbol whenInvocationSymbol)
{
var argumentExpression = whenInvocationSymbol.MethodKind == MethodKind.ReducedExtension
? whenInvocationExpression.ArgumentList.Arguments.First().Expression
: whenInvocationExpression.ArgumentList.Arguments.Skip(1).First().Expression;

return FindForWhenExpression(syntaxNodeContext, argumentExpression);
}

private IEnumerable<SyntaxNode> FindForWhenExpression(SyntaxNodeAnalysisContext syntaxNodeContext, SyntaxNode argumentSyntax)
{
SyntaxNode body = null;
Expand Down
42 changes: 41 additions & 1 deletion src/NSubstitute.Analyzers.Shared/DiagnosticAnalyzers/AbstractSubstitutionNodeFinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,52 @@ public IEnumerable<SyntaxNode> Find(SyntaxNodeAnalysisContext syntaxNodeContext,
return standardSubstitution != null ? new[] { standardSubstitution } : Enumerable.Empty<SyntaxNode>();
}

public abstract IEnumerable<SyntaxNode> FindForWhenExpression(SyntaxNodeAnalysisContext syntaxNodeContext, TInvocationExpressionSyntax whenInvocationExpression, IMethodSymbol whenInvocationSymbol = null);
public IEnumerable<SyntaxNode> FindForWhenExpression(SyntaxNodeAnalysisContext syntaxNodeContext, TInvocationExpressionSyntax whenInvocationExpression, IMethodSymbol whenInvocationSymbol = null)
{
if (whenInvocationExpression == null)
{
yield break;
}

whenInvocationSymbol = whenInvocationSymbol ?? syntaxNodeContext.SemanticModel.GetSymbolInfo(whenInvocationExpression).Symbol as IMethodSymbol;

if (whenInvocationSymbol == null)
{
yield break;
}

var typeSymbol = whenInvocationSymbol.TypeArguments.FirstOrDefault() ?? whenInvocationSymbol.ReceiverType;
foreach (var syntaxNode in FindForWhenExpressionInternal(syntaxNodeContext, whenInvocationExpression, whenInvocationSymbol))
{
var symbol = syntaxNodeContext.SemanticModel.GetSymbolInfo(syntaxNode).Symbol;
if (symbol != null && typeSymbol != null && ContainsSymbol(typeSymbol, symbol))
{
yield return syntaxNode;
}
}
}

public abstract SyntaxNode FindForAndDoesExpression(SyntaxNodeAnalysisContext syntaxNodeContext, TInvocationExpressionSyntax invocationExpression, IMethodSymbol invocationExpressionSymbol = null);

public abstract SyntaxNode FindForStandardExpression(TInvocationExpressionSyntax invocationExpressionSyntax, IMethodSymbol invocationExpressionSymbol = null);

protected abstract TInvocationExpressionSyntax GetParentInvocationExpression(TInvocationExpressionSyntax invocationExpressionSyntax);

protected abstract IEnumerable<SyntaxNode> FindForWhenExpressionInternal(SyntaxNodeAnalysisContext syntaxNodeContext, TInvocationExpressionSyntax whenInvocationExpression, IMethodSymbol whenInvocationSymbol);

private bool ContainsSymbol(ITypeSymbol containerSymbol, ISymbol symbol)
{
return GetBaseTypesAndThis(containerSymbol).Any(typeSymbol => typeSymbol == symbol.ContainingType);
}

private static IEnumerable<ITypeSymbol> GetBaseTypesAndThis(ITypeSymbol type)
{
var current = type;
while (current != null)
{
yield return current;
current = current.BaseType;
}
}
}
}
30 changes: 9 additions & 21 deletions src/NSubstitute.Analyzers.VisualBasic/DiagnosticAnalyzers/SubstitutionNodeFinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,6 @@ namespace NSubstitute.Analyzers.VisualBasic.DiagnosticAnalyzers
{
internal class SubstitutionNodeFinder : AbstractSubstitutionNodeFinder<InvocationExpressionSyntax>
{
public override IEnumerable<SyntaxNode> FindForWhenExpression(SyntaxNodeAnalysisContext syntaxNodeContext, InvocationExpressionSyntax whenInvocationExpression, IMethodSymbol whenInvocationSymbol = null)
{
if (whenInvocationExpression == null)
{
return Enumerable.Empty<SyntaxNode>();
}

whenInvocationSymbol = whenInvocationSymbol ?? syntaxNodeContext.SemanticModel.GetSymbolInfo(whenInvocationExpression).Symbol as IMethodSymbol;

if (whenInvocationSymbol == null)
{
return Enumerable.Empty<SyntaxNode>();
}

var argumentExpression = whenInvocationSymbol.MethodKind == MethodKind.ReducedExtension
? whenInvocationExpression.ArgumentList.Arguments.First().GetExpression()
: whenInvocationExpression.ArgumentList.Arguments.Skip(1).First().GetExpression();

return FindForWhenExpression(syntaxNodeContext, argumentExpression);
}

public override SyntaxNode FindForAndDoesExpression(SyntaxNodeAnalysisContext syntaxNodeContext, InvocationExpressionSyntax invocationExpression, IMethodSymbol invocationExpressionSymbol)
{
var parentInvocationExpression = invocationExpression?.GetParentInvocationExpression();
Expand Down Expand Up @@ -65,6 +44,15 @@ protected override InvocationExpressionSyntax GetParentInvocationExpression(Invo
return invocationExpressionSyntax.GetParentInvocationExpression();
}

protected override IEnumerable<SyntaxNode> FindForWhenExpressionInternal(SyntaxNodeAnalysisContext syntaxNodeContext, InvocationExpressionSyntax whenInvocationExpression, IMethodSymbol whenInvocationSymbol)
{
var argumentExpression = whenInvocationSymbol.MethodKind == MethodKind.ReducedExtension
? whenInvocationExpression.ArgumentList.Arguments.First().GetExpression()
: whenInvocationExpression.ArgumentList.Arguments.Skip(1).First().GetExpression();

return FindForWhenExpression(syntaxNodeContext, argumentExpression);
}

private IEnumerable<SyntaxNode> FindForWhenExpression(SyntaxNodeAnalysisContext syntaxNodeContext, SyntaxNode argumentSyntax)
{
SyntaxNode body = null;
Expand Down
78 changes: 39 additions & 39 deletions ...s/NonSubstitutableMemberWhenAnalyzerTests/NonSubstitutableMemberWhenDiagnosticVerifier.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,27 @@ 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(Arg.Any<int>())|]")]
[InlineData("delegate(Foo sub) { [|sub.Bar(Arg.Any<int>())|]; }")]
[InlineData("sub => { [|sub.Bar(Arg.Any<int>())|]; }")]
public abstract Task ReportsDiagnostics_WhenSettingValueForNonVirtualMethod(string method, string whenAction);

[CombinatoryTheory]
[InlineData("sub => [|sub.Bar()|]")]
[InlineData("delegate(Foo sub) { [|sub.Bar()|]; }")]
[InlineData("sub => { [|sub.Bar()|]; }")]
[InlineData("sub => [|sub.Bar(Arg.Any<int>())|]")]
[InlineData("delegate(Foo sub) { [|sub.Bar(Arg.Any<int>())|]; }")]
[InlineData("sub => { [|sub.Bar(Arg.Any<int>())|]; }")]
public abstract Task ReportsDiagnostics_WhenSettingValueForNonVirtualMemberFromBaseClass(string method, string whenAction);

[CombinatoryTheory]
[InlineData("sub => sub.Bar()")]
[InlineData("delegate(Foo sub) { sub.Bar(); }")]
[InlineData("sub => { sub.Bar(); }")]
[InlineData("sub => sub.Bar(Arg.Any<int>())")]
[InlineData("delegate(Foo sub) { sub.Bar(Arg.Any<int>()); }")]
[InlineData("sub => { sub.Bar(Arg.Any<int>()); }")]
public abstract Task ReportsNoDiagnostics_WhenSettingValueForVirtualMethod(string method, string whenAction);

[CombinatoryTheory]
[InlineData("sub => sub.Bar()")]
[InlineData("delegate(Foo sub) { sub.Bar(); }")]
[InlineData("sub => { sub.Bar(); }")]
[InlineData("sub => sub.Bar(Arg.Any<int>())")]
[InlineData("delegate(Foo sub) { sub.Bar(Arg.Any<int>()); }")]
[InlineData("sub => { sub.Bar(Arg.Any<int>()); }")]
public abstract Task ReportsNoDiagnostics_WhenSettingValueForNonSealedOverrideMethod(string method, string whenAction);

[CombinatoryTheory]
Expand All @@ -47,21 +47,21 @@ 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(Arg.Any<int>())|]")]
[InlineData("delegate(Foo sub) { [|sub.Bar(Arg.Any<int>())|]; }")]
[InlineData("sub => { [|sub.Bar(Arg.Any<int>())|]; }")]
public abstract Task ReportsDiagnostics_WhenSettingValueForSealedOverrideMethod(string method, string whenAction);

[CombinatoryTheory]
[InlineData("sub => sub.Bar()")]
[InlineData("delegate(Foo sub) { sub.Bar(); }")]
[InlineData("sub => { sub.Bar(); }")]
[InlineData("sub => sub.Bar(Arg.Any<int>())")]
[InlineData("delegate(Foo sub) { sub.Bar(Arg.Any<int>()); }")]
[InlineData("sub => { sub.Bar(Arg.Any<int>()); }")]
public abstract Task ReportsNoDiagnostics_WhenSettingValueForAbstractMethod(string method, string whenAction);

[CombinatoryTheory]
[InlineData("sub => sub.Bar()")]
[InlineData("delegate(Foo sub) { sub.Bar(); }")]
[InlineData("sub => { sub.Bar(); }")]
[InlineData("sub => sub.Bar(Arg.Any<int>())")]
[InlineData("delegate(Foo sub) { sub.Bar(Arg.Any<int>()); }")]
[InlineData("sub => { sub.Bar(Arg.Any<int>()); }")]
public abstract Task ReportsNoDiagnostics_WhenSettingValueForInterfaceMethod(string method, string whenAction);

[CombinatoryTheory]
Expand All @@ -71,9 +71,9 @@ public abstract class NonSubstitutableMemberWhenDiagnosticVerifier : CSharpDiagn
public abstract Task ReportsNoDiagnostics_WhenSettingValueForInterfaceProperty(string method, string whenAction);

[CombinatoryTheory]
[InlineData("sub => sub.Bar<int>()")]
[InlineData("delegate(Foo<int> sub) { sub.Bar<int>(); }")]
[InlineData("sub => { sub.Bar<int>(); }")]
[InlineData("sub => sub.Bar<int>(Arg.Any<int>())")]
[InlineData("delegate(Foo<int> sub) { sub.Bar<int>(Arg.Any<int>()); }")]
[InlineData("sub => { sub.Bar<int>(Arg.Any<int>()); }")]
public abstract Task ReportsNoDiagnostics_WhenSettingValueForGenericInterfaceMethod(string method, string whenAction);

[CombinatoryTheory]
Expand All @@ -83,14 +83,14 @@ public abstract class NonSubstitutableMemberWhenDiagnosticVerifier : CSharpDiagn
public abstract Task ReportsNoDiagnostics_WhenSettingValueForAbstractProperty(string method, string whenAction);

[CombinatoryTheory]
[InlineData("delegate(Foo sub) { var x = sub[1]; }")]
[InlineData("sub => { var x = sub[1]; }")]
[InlineData("delegate(Foo sub) { var x = sub[Arg.Any<int>()]; }")]
[InlineData("sub => { var x = sub[Arg.Any<int>()]; }")]
public abstract Task ReportsNoDiagnostics_WhenSettingValueForInterfaceIndexer(string method, string whenAction);

[CombinatoryTheory]
[InlineData("sub => sub.Bar()")]
[InlineData("delegate(Foo sub) { sub.Bar(); }")]
[InlineData("sub => { sub.Bar(); }")]
[InlineData("sub => sub.Bar(Arg.Any<int>())")]
[InlineData("delegate(Foo sub) { sub.Bar(Arg.Any<int>()); }")]
[InlineData("sub => { sub.Bar(Arg.Any<int>()); }")]
public abstract Task ReportsNoDiagnostics_WhenUsingUnfortunatelyNamedMethod(string method, string whenAction);

[CombinatoryTheory]
Expand All @@ -106,8 +106,8 @@ public abstract class NonSubstitutableMemberWhenDiagnosticVerifier : CSharpDiagn
public abstract Task ReportsNoDiagnostics_WhenSettingValueForVirtualProperty(string method, string whenAction);

[CombinatoryTheory]
[InlineData("sub => { var x = [|sub[1]|]; }")]
[InlineData("delegate(Foo sub) { var x = [|sub[1]|]; }")]
[InlineData("sub => { var x = [|sub[Arg.Any<int>()]|]; }")]
[InlineData("delegate(Foo sub) { var x = [|sub[Arg.Any<int>()]|]; }")]
public abstract Task ReportsDiagnostics_WhenSettingValueForNonVirtualIndexer(string method, string whenAction);

[CombinatoryTheory]
Expand Down Expand Up @@ -144,26 +144,26 @@ 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 => { var x = [|sub[0]|]; }", "Internal member this[] can not be intercepted without InternalsVisibleToAttribute.")]
[InlineData("sub => [|sub.FooBar(Arg.Any<int>())|]", "Internal member FooBar can not be intercepted without InternalsVisibleToAttribute.")]
[InlineData("sub => { var x = [|sub[Arg.Any<int>()]|]; }", "Internal member this[] can not be intercepted without InternalsVisibleToAttribute.")]
public abstract Task ReportsDiagnostics_WhenSettingValueForInternalVirtualMember_AndInternalsVisibleToNotApplied(string method, string call, string message);

[CombinatoryTheory]
[InlineData("sub => { var x = sub.Bar; }")]
[InlineData("sub => sub.FooBar()")]
[InlineData("sub => { var x = sub[0]; }")]
[InlineData("sub => sub.FooBar(Arg.Any<int>())")]
[InlineData("sub => { var x = sub[Arg.Any<int>()]; }")]
public abstract Task ReportsNoDiagnostics_WhenSettingValueForInternalVirtualMember_AndInternalsVisibleToApplied(string method, string call);

[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 => { var x = [|sub[0]|]; }", "Internal member this[] can not be intercepted without InternalsVisibleToAttribute.")]
[InlineData("sub => [|sub.FooBar(Arg.Any<int>())|]", "Internal member FooBar can not be intercepted without InternalsVisibleToAttribute.")]
[InlineData("sub => { var x = [|sub[Arg.Any<int>()]|]; }", "Internal member this[] can not be intercepted without InternalsVisibleToAttribute.")]
public abstract Task ReportsDiagnostics_WhenSettingValueForInternalVirtualMember_AndInternalsVisibleToAppliedToWrongAssembly(string method, string call, string message);

[CombinatoryTheory]
[InlineData("sub => { var x = sub.Bar; }")]
[InlineData("sub => sub.FooBar()")]
[InlineData("sub => { var x = sub[0]; }")]
[InlineData("sub => sub.FooBar(Arg.Any<int>())")]
[InlineData("sub => { var x = sub[Arg.Any<int>()]; }")]
public abstract Task ReportsNoDiagnostics_WhenSettingValueForProtectedInternalVirtualMember(string method, string call);

protected override DiagnosticAnalyzer GetDiagnosticAnalyzer()
Expand Down
Loading

0 comments on commit 81549f2

Please sign in to comment.