The code analyzers and fixes in this repo directly use the C# syntax tree and the semantic model to detect some patterns and report warnings. This means the analyzers cannot be used for other .NET languages, as the syntax tree is different. This means for each language to be supported, a new analyzer must be duplicated, which increases complexity and maintenance costs. Additionally, the computational cost of traversing the syntax tree is expensive. Moving to IOperation reduces the complexity and makes the analyzers easier to understand, operationally faster, and can now be reused for other .NET languages. Neat!
Approach
Pros
easier code to understand
multiple language support
faster
Cons
Squiggles are less precise
A subset of C# and VB.NET is representable using IOperation. Some operations may have to remain on SyntaxNode.
The change moves most of the logic that is now contained in the void Analyze(SyntaxNodeAnalysisContext) method to the callback registered during void Initialize(AnalysisContext), only invoking code to analyze when preconditions are met. This has the benefit of both simplifying to reduce issues identified in #90 as well as separating concerns about filtering and analyzing symbols.
Strategy
Convert to IOperation where possible
Where less precise squiggles are observed, improve analyzer messaging to guide user to correct action (and/or provide Code Fixes)
Improve analyzer messaging #85
Add back in capabilities for precision with benchmarks to track performance
Since IOperation does not represent the full syntax tree, there may be loss of precision in the squiggle shown when writing code. This effort may require driving changes upstream with the Roslyn team.
Proposal
This may be best understood through an example. We will use the existing Mock.As<T>() analyzer as an example.
Existing
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class AsShouldBeUsedOnlyForInterfaceAnalyzer : DiagnosticAnalyzer
{
internal const string RuleId = "Moq1300";
private const string Title = "Moq: Invalid As type parameter";
private const string Message = "Mock.As() should take interfaces only";
private static readonly MoqMethodDescriptorBase MoqAsMethodDescriptor = new MoqAsMethodDescriptor();
private static readonly DiagnosticDescriptor Rule = new(
RuleId,
Title,
Message,
DiagnosticCategory.Moq,
DiagnosticSeverity.Error,
isEnabledByDefault: true,
helpLinkUri: $"https://github.com/rjmurillo/moq.analyzers/blob/main/docs/rules/{RuleId}.md");
/// <inheritdoc />
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression);
}
[System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "AV1500:Member or local function contains too many statements", Justification = "Tracked in https://github.com/rjmurillo/moq.analyzers/issues/90")]
private static void Analyze(SyntaxNodeAnalysisContext context)
{
if (context.Node is not InvocationExpressionSyntax invocationExpression)
{
return;
}
if (invocationExpression.Expression is not MemberAccessExpressionSyntax memberAccessSyntax)
{
return;
}
if (!MoqAsMethodDescriptor.IsMatch(context.SemanticModel, memberAccessSyntax, context.CancellationToken))
{
return;
}
if (!memberAccessSyntax.Name.TryGetGenericArguments(out SeparatedSyntaxList<TypeSyntax> typeArguments))
{
return;
}
if (typeArguments.Count != 1)
{
return;
}
TypeSyntax typeArgument = typeArguments[0];
SymbolInfo symbolInfo = context.SemanticModel.GetSymbolInfo(typeArgument, context.CancellationToken);
if (symbolInfo.Symbol is ITypeSymbol { TypeKind: not TypeKind.Interface })
{
context.ReportDiagnostic(Diagnostic.Create(Rule, typeArgument.GetLocation()));
}
}
}
Using IOperation
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class AsShouldBeUsedOnlyForInterfaceAnalyzer2 : DiagnosticAnalyzer
{
internal const string RuleId = "Moq1300";
private const string Title = "Moq: Invalid As type parameter";
private const string Message = "Mock.As() should take interfaces only";
private static readonly DiagnosticDescriptor Rule = new(
RuleId,
Title,
Message,
DiagnosticCategory.Moq,
DiagnosticSeverity.Error,
isEnabledByDefault: true,
helpLinkUri: $"https://github.com/rjmurillo/moq.analyzers/blob/main/docs/rules/{RuleId}.md");
/// <inheritdoc />
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
/// <inheritdoc />
public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterCompilationStartAction(static context =>
{
ImmutableArray<INamedTypeSymbol> mockTypes = context.Compilation.GetTypesByMetadataNames([WellKnownTypeNames.MoqMock, WellKnownTypeNames.MoqMock1]);
if (mockTypes.IsEmpty)
{
return;
}
ImmutableArray<IMethodSymbol> asMethods = mockTypes
.SelectMany(mockType => mockType.GetMembers("As"))
.OfType<IMethodSymbol>()
.Where(method => method.IsGenericMethod)
.ToImmutableArray();
if (asMethods.IsEmpty)
{
return;
}
context.RegisterOperationAction(context => Analyze(context, asMethods), OperationKind.Invocation);
});
}
private static void Analyze(OperationAnalysisContext context, ImmutableArray<IMethodSymbol> wellKnownAsMethods)
{
if (context.Operation is not IInvocationOperation invocationOperation)
{
return;
}
IMethodSymbol targetMethod = invocationOperation.TargetMethod;
if (!wellKnownAsMethods.Any(asMethod => asMethod.Equals(targetMethod.OriginalDefinition, SymbolEqualityComparer.Default)))
{
return;
}
ImmutableArray<ITypeSymbol> typeArguments = targetMethod.TypeArguments;
if (typeArguments.Length != 1)
{
return;
}
if (typeArguments[0] is ITypeSymbol { TypeKind: not TypeKind.Interface })
{
context.ReportDiagnostic(Diagnostic.Create(Rule, invocationOperation.Syntax.GetLocation()));
}
}
}
The benchmark shows an improvement between the old and new methods.
Refactor to IOperation explained
What's All This About?
The code analyzers and fixes in this repo directly use the C# syntax tree and the semantic model to detect some patterns and report warnings. This means the analyzers cannot be used for other .NET languages, as the syntax tree is different. This means for each language to be supported, a new analyzer must be duplicated, which increases complexity and maintenance costs. Additionally, the computational cost of traversing the syntax tree is expensive. Moving to
IOperationreduces the complexity and makes the analyzers easier to understand, operationally faster, and can now be reused for other .NET languages. Neat!Approach
Pros
Cons
IOperation. Some operations may have to remain onSyntaxNode.The change moves most of the logic that is now contained in the
void Analyze(SyntaxNodeAnalysisContext)method to the callback registered duringvoid Initialize(AnalysisContext), only invoking code to analyze when preconditions are met. This has the benefit of both simplifying to reduce issues identified in #90 as well as separating concerns about filtering and analyzing symbols.Strategy
IOperationwhere possibleSince
IOperationdoes not represent the full syntax tree, there may be loss of precision in the squiggle shown when writing code. This effort may require driving changes upstream with the Roslyn team.Proposal
This may be best understood through an example. We will use the existing
Mock.As<T>()analyzer as an example.Existing
Using
IOperationThe benchmark shows an improvement between the old and new methods.
Open Questions
This is a work in progress! We are patterning the initial design after Roslyn Analyzers.
The specific timing of the change is TBD. Pre-release packages will be submitted to NuGet.org to gather community feedback.