Skip to content

Commit

Permalink
[GH-62] - conflicting ref/out arguments - draft implementation for cs…
Browse files Browse the repository at this point in the history
…harp
  • Loading branch information
tpodolak committed Mar 9, 2019
1 parent 31970dd commit e93cfc0
Show file tree
Hide file tree
Showing 10 changed files with 703 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
using System.Collections.Generic;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using NSubstitute.Analyzers.CSharp.Extensions;
using NSubstitute.Analyzers.Shared;
using NSubstitute.Analyzers.Shared.DiagnosticAnalyzers;

namespace NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
internal class ConflictingRefOutAnalyzer : AbstractConflictingRefOutAnalyzer<SyntaxKind, InvocationExpressionSyntax, ExpressionSyntax, ElementAccessExpressionSyntax>
{
public ConflictingRefOutAnalyzer()
: base(new DiagnosticDescriptorsProvider())
{
}

protected override SyntaxKind InvocationExpressionKind { get; } = SyntaxKind.InvocationExpression;

protected override IEnumerable<ExpressionSyntax> GetArgumentExpressions(InvocationExpressionSyntax invocationExpressionSyntax)
{
return invocationExpressionSyntax.ArgumentList.Arguments.Select(arg => arg.Expression);
}

protected override SyntaxNode GetSubstituteCall(SyntaxNodeAnalysisContext syntaxNodeContext, IMethodSymbol methodSymbol, InvocationExpressionSyntax invocationExpressionSyntax)
{
if (methodSymbol.IsExtensionMethod)
{
switch (methodSymbol.MethodKind)
{
case MethodKind.ReducedExtension:
return invocationExpressionSyntax.Expression.DescendantNodes().First();
case MethodKind.Ordinary:
return invocationExpressionSyntax.ArgumentList.Arguments.First().Expression;
default:
return null;
}
}

var parentInvocation = invocationExpressionSyntax.GetParentInvocationExpression();

if (parentInvocation == null)
{
return null;
}

var symbol = syntaxNodeContext.SemanticModel.GetSymbolInfo(parentInvocation);

if (symbol.Symbol is IMethodSymbol mSymbol && mSymbol.ReducedFrom == null)
{
return parentInvocation.ArgumentList.Arguments.First().Expression;
}

return parentInvocation;
}

protected override AbstractCallInfoFinder<InvocationExpressionSyntax, ElementAccessExpressionSyntax> GetCallInfoFinder()
{
return new CallInfoCallFinder();
}

protected override int? GetIndexerPosition(SyntaxNodeAnalysisContext syntaxNodeAnalysisContext, ElementAccessExpressionSyntax indexerExpressionSyntax)
{
var position = syntaxNodeAnalysisContext.SemanticModel.GetConstantValue(indexerExpressionSyntax.ArgumentList.Arguments.First().Expression);
return (int?)(position.HasValue ? position.Value : null);
}

protected override ISymbol GetIndexerSymbol(SyntaxNodeAnalysisContext syntaxNodeAnalysisContext, ElementAccessExpressionSyntax indexerExpressionSyntax)
{
return syntaxNodeAnalysisContext.SemanticModel.GetSymbolInfo(indexerExpressionSyntax).Symbol ??
syntaxNodeAnalysisContext.SemanticModel.GetSymbolInfo(indexerExpressionSyntax.Expression).Symbol;
}

protected override SyntaxNode GetAssignmentExpression(ElementAccessExpressionSyntax indexerExpressionSyntax)
{
if (indexerExpressionSyntax.Parent is AssignmentExpressionSyntax assignmentExpressionSyntax)
{
return assignmentExpressionSyntax.Right;
}

return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,7 @@ internal class AbstractDiagnosticDescriptorsProvider<T> : IDiagnosticDescriptors
public DiagnosticDescriptor CallInfoArgumentSetWithIncompatibleValue { get; } = DiagnosticDescriptors<T>.CallInfoArgumentSetWithIncompatibleValue;

public DiagnosticDescriptor CallInfoArgumentIsNotOutOrRef { get; } = DiagnosticDescriptors<T>.CallInfoArgumentIsNotOutOrRef;

public DiagnosticDescriptor ConflictingAssignmentsToOutRefArgument { get; } = DiagnosticDescriptors<T>.ConflictingAssignmentsToOutRefArgument;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using NSubstitute.Analyzers.Shared.Extensions;

namespace NSubstitute.Analyzers.Shared.DiagnosticAnalyzers
{
internal abstract class AbstractConflictingRefOutAnalyzer<TSyntaxKind, TInvocationExpressionSyntax, TExpressionSyntax, TIndexerExpressionSyntax> : AbstractDiagnosticAnalyzer
where TInvocationExpressionSyntax : SyntaxNode
where TExpressionSyntax : SyntaxNode
where TIndexerExpressionSyntax : SyntaxNode
where TSyntaxKind : struct
{
protected AbstractConflictingRefOutAnalyzer(IDiagnosticDescriptorsProvider diagnosticDescriptorsProvider)
: base(diagnosticDescriptorsProvider)
{
}

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
ImmutableArray.Create(DiagnosticDescriptorsProvider.ConflictingAssignmentsToOutRefArgument);

protected abstract TSyntaxKind InvocationExpressionKind { get; }

private static readonly ImmutableDictionary<string, string> MethodNames = new Dictionary<string, string>()
{
[MetadataNames.NSubstituteAndDoesMethod] = MetadataNames.NSubstituteConfiguredCallFullTypeName
}.ToImmutableDictionary();

public override void Initialize(AnalysisContext context)
{
context.RegisterSyntaxNodeAction(AnalyzeInvocation, InvocationExpressionKind);
}

protected abstract IEnumerable<TExpressionSyntax> GetArgumentExpressions(TInvocationExpressionSyntax invocationExpressionSyntax);

protected abstract SyntaxNode GetSubstituteCall(SyntaxNodeAnalysisContext syntaxNodeContext, IMethodSymbol methodSymbol, TInvocationExpressionSyntax invocationExpressionSyntax);

protected abstract ISymbol GetIndexerSymbol(SyntaxNodeAnalysisContext syntaxNodeAnalysisContext, TIndexerExpressionSyntax indexerExpressionSyntax);

protected abstract AbstractCallInfoFinder<TInvocationExpressionSyntax, TIndexerExpressionSyntax> GetCallInfoFinder();

protected abstract int? GetIndexerPosition(SyntaxNodeAnalysisContext syntaxNodeAnalysisContext, TIndexerExpressionSyntax indexerExpressionSyntax);

protected abstract SyntaxNode GetAssignmentExpression(TIndexerExpressionSyntax indexerExpressionSyntax);

private void AnalyzeInvocation(SyntaxNodeAnalysisContext syntaxNodeContext)
{
var invocationExpression = (TInvocationExpressionSyntax)syntaxNodeContext.Node;
var methodSymbolInfo = syntaxNodeContext.SemanticModel.GetSymbolInfo(invocationExpression);

if (methodSymbolInfo.Symbol?.Kind != SymbolKind.Method)
{
return;
}

var methodSymbol = (IMethodSymbol)methodSymbolInfo.Symbol;

if (MethodNames.TryGetValue(methodSymbol.Name, out var typeName) == false)
{
return;
}

if (SupportsCallInfo(syntaxNodeContext, invocationExpression, methodSymbol) == false)
{
return;
}

var previousCall = GetSubstituteCall(syntaxNodeContext, methodSymbol, invocationExpression) as TInvocationExpressionSyntax;

var expressionSyntax = GetArgumentExpressions(invocationExpression).First();

// analyze assignments
var andDoesIndexers = GetCallInfoFinder().GetCallInfoContext(syntaxNodeContext.SemanticModel, expressionSyntax);
var previousCallIndexer = GetArgumentExpressions(previousCall).Select(arg => GetCallInfoFinder().GetCallInfoContext(syntaxNodeContext.SemanticModel, arg));

var immutableHashSet = previousCallIndexer.SelectMany(x => x.IndexerAccesses).Where(acc => GetIndexerInfo(syntaxNodeContext, acc).VerifyAssignment && GetAssignmentExpression(acc) != null)
.Select(x => GetIndexerPosition(syntaxNodeContext, x)).ToImmutableHashSet();

foreach (var indexerExpressionSyntax in andDoesIndexers.IndexerAccesses)
{
// TODO
var info = GetIndexerInfo(syntaxNodeContext, indexerExpressionSyntax);
if (info.VerifyAssignment && GetAssignmentExpression(indexerExpressionSyntax) != null)
{
var position = GetIndexerPosition(syntaxNodeContext, indexerExpressionSyntax);
if (position.HasValue && immutableHashSet.Contains(position.Value))
{
syntaxNodeContext.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptorsProvider.ConflictingAssignmentsToOutRefArgument, indexerExpressionSyntax.GetLocation()));
}
}
}
}

private bool SupportsCallInfo(SyntaxNodeAnalysisContext syntaxNodeContext, TInvocationExpressionSyntax syntax, IMethodSymbol methodSymbol)
{
var allArguments = GetArgumentExpressions(syntax);
IEnumerable<TExpressionSyntax> argumentsForAnalysis;
if (methodSymbol.MethodKind == MethodKind.ReducedExtension)
argumentsForAnalysis = allArguments;
else if (methodSymbol.IsExtensionMethod)
argumentsForAnalysis = allArguments.Skip(1);
else
argumentsForAnalysis = allArguments;

var symbol = syntaxNodeContext.SemanticModel.GetSymbolInfo(syntax);

// TODO
// var supportsCallInfo =
// symbol.Symbol?.ContainingAssembly?.Name.Equals(MetadataNames.NSubstituteAssemblyName, StringComparison.OrdinalIgnoreCase) == true &&
// symbol.Symbol?.ContainingType?.ToString().Equals(typeName, StringComparison.OrdinalIgnoreCase) == true;
return argumentsForAnalysis.Any(arg => syntaxNodeContext.SemanticModel.GetTypeInfo(arg).IsCallInfoDelegate(syntaxNodeContext.SemanticModel));
}

private IndexerInfo GetIndexerInfo(SyntaxNodeAnalysisContext syntaxNodeAnalysisContext, TIndexerExpressionSyntax indexerExpressionSyntax)
{
var info = GetIndexerSymbol(syntaxNodeAnalysisContext, indexerExpressionSyntax);
var symbol = info as IMethodSymbol;
var verifyIndexerCast = symbol == null || symbol.Name != MetadataNames.CallInfoArgTypesMethod;
var verifyAssignment = symbol == null;

var indexerInfo = new IndexerInfo(verifyIndexerCast, verifyAssignment);
return indexerInfo;
}

private struct IndexerInfo
{
public bool VerifyIndexerCast { get; }

public bool VerifyAssignment { get; }

public IndexerInfo(bool verifyIndexerCast, bool verifyAssignment)
{
VerifyIndexerCast = verifyIndexerCast;
VerifyAssignment = verifyAssignment;
}
}
}
}
8 changes: 8 additions & 0 deletions src/NSubstitute.Analyzers.Shared/DiagnosticDescriptors.cs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,14 @@ internal class DiagnosticDescriptors<T>
defaultSeverity: DiagnosticSeverity.Warning,
isEnabledByDefault: true);

public static DiagnosticDescriptor ConflictingAssignmentsToOutRefArgument { get; } =
CreateDiagnosticDescriptor(
name: nameof(ConflictingAssignmentsToOutRefArgument),
id: DiagnosticIdentifiers.UnusedReceived,
category: DiagnosticCategory.Usage.GetDisplayName(),
defaultSeverity: DiagnosticSeverity.Warning,
isEnabledByDefault: true);

private static DiagnosticDescriptor CreateDiagnosticDescriptor(
string name, string id, string category, DiagnosticSeverity defaultSeverity, bool isEnabledByDefault)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,7 @@ internal interface IDiagnosticDescriptorsProvider
DiagnosticDescriptor CallInfoArgumentSetWithIncompatibleValue { get; }

DiagnosticDescriptor CallInfoArgumentIsNotOutOrRef { get; }

DiagnosticDescriptor ConflictingAssignmentsToOutRefArgument { get; }
}
}
27 changes: 27 additions & 0 deletions src/NSubstitute.Analyzers.Shared/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 13 additions & 1 deletion src/NSubstitute.Analyzers.Shared/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -350,4 +350,16 @@
<value>Could not set argument.</value>
<comment>The title of the diagnostic.</comment>
</data>
</root>
<data name="ConflictingAssignmentsToOutRefArgumentDescription" xml:space="preserve">
<value>Conflicting assignments to out/ref arguments.</value>
<comment>An optional longer localizable description of the diagnostic.</comment>
</data>
<data name="ConflictingAssignmentsToOutRefArgumentMessageFormat" xml:space="preserve">
<value>Conflicting assignments to out/ref arguments.</value>
<comment>The format-able message the diagnostic displays.</comment>
</data>
<data name="ConflictingAssignmentsToOutRefArgumentsTitle" xml:space="preserve">
<value>Conflicting assignments to out/ref arguments.</value>
<comment>The title of the diagnostic.</comment>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using NSubstitute.Analyzers.CSharp;
using NSubstitute.Analyzers.CSharp.DiagnosticAnalyzers;
using NSubstitute.Analyzers.Shared;

namespace NSubstitute.Analyzers.Tests.CSharp.DiagnosticAnalyzerTests.ConflictingRefOutAnalyzerTests
{
public class ConflictingRefOutDiagnosticVerifier : CSharpDiagnosticVerifier
{
public DiagnosticDescriptor Descriptor { get; } = DiagnosticDescriptors<DiagnosticDescriptorsProvider>.ConflictingAssignmentsToOutRefArgument;

protected override DiagnosticAnalyzer GetDiagnosticAnalyzer()
{
return new ConflictingRefOutAnalyzer();
}
}
}
Loading

0 comments on commit e93cfc0

Please sign in to comment.