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 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.
BenchmarkDotNet v0.13.12, Windows 11 (10.0.22635.3790)
Intel Core i9-10940X CPU 3.30GHz, 1 CPU, 28 logical and 14 physical cores
.NET SDK 8.0.301
[Host] : .NET 8.0.6 (8.0.624.26715), X64 RyuJIT AVX-512F+CD+BW+DQ+VL
Job=InProcess Toolchain=InProcessEmitToolchain InvocationCount=1
UnrollFactor=1
| Method |
Mean |
Error |
StdDev |
Ratio |
RatioSD |
Gen0 |
Gen1 |
Allocated |
Alloc Ratio |
| OldMoq1300WithDiagnostics |
181.6 ms |
4.76 ms |
13.58 ms |
1.75 |
0.14 |
7000.0000 |
2000.0000 |
68.38 MB |
1.39 |
| NewMoq1300WithDiagnostics |
159.9 ms |
3.17 ms |
8.00 ms |
1.54 |
0.11 |
6000.0000 |
2000.0000 |
62.33 MB |
1.26 |
| Moq1300Baseline |
104.3 ms |
2.06 ms |
5.02 ms |
1.00 |
0.00 |
5000.0000 |
2000.0000 |
49.28 MB |
1.00 |
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.
![dependabot[bot] avatar](https://avatars.githubusercontent.com/in/29110?v=4 "dependabot[bot]")
React
Typescript
Django
Laravel
Facebook
Microsoft
Google
Alibaba
D3
Tencent