-
Notifications
You must be signed in to change notification settings - Fork 57
Add NEEG004 analyzer to detect ToString() on enums with [EnumExtensions] or EnumExtensions<T> #196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
36f1d5c
9f41c76
56f5a2f
b790890
7daba34
2208218
5b1a131
ffc5fae
20c8610
b291228
48573cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| using System.Collections.Immutable; | ||
| using Microsoft.CodeAnalysis; | ||
| using Microsoft.CodeAnalysis.CSharp; | ||
| using Microsoft.CodeAnalysis.CSharp.Syntax; | ||
| using Microsoft.CodeAnalysis.Diagnostics; | ||
|
|
||
| namespace NetEscapades.EnumGenerators.Diagnostics; | ||
|
|
||
| [DiagnosticAnalyzer(LanguageNames.CSharp)] | ||
| public class ToStringAnalyzer : DiagnosticAnalyzer | ||
| { | ||
| public const string DiagnosticId = "NEEG004"; | ||
| public static readonly DiagnosticDescriptor Rule = new( | ||
| #pragma warning disable RS2008 // Enable Analyzer Release Tracking | ||
| id: DiagnosticId, | ||
| #pragma warning restore RS2008 | ||
| title: "Use ToStringFast() instead of ToString()", | ||
| messageFormat: "Use ToStringFast() instead of ToString() for better performance on enum '{0}'", | ||
| category: "Usage", | ||
| defaultSeverity: DiagnosticSeverity.Info, | ||
| isEnabledByDefault: true); | ||
|
|
||
| public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics | ||
| => ImmutableArray.Create(Rule); | ||
|
|
||
| public override void Initialize(AnalysisContext context) | ||
| { | ||
| context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); | ||
| context.EnableConcurrentExecution(); | ||
| context.RegisterCompilationStartAction(ctx => | ||
| { | ||
| var enumExtensionsAttr = | ||
| ctx.Compilation.GetTypeByMetadataName(Attributes.EnumExtensionsAttribute); | ||
|
|
||
| if (enumExtensionsAttr is null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| ctx.RegisterSyntaxNodeAction( | ||
| c => AnalyzeInvocation(c, enumExtensionsAttr), | ||
| SyntaxKind.InvocationExpression); | ||
| }); | ||
| } | ||
|
|
||
| private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context, INamedTypeSymbol enumExtensionsAttr) | ||
| { | ||
| var invocation = (InvocationExpressionSyntax)context.Node; | ||
|
|
||
| // Check if this is a member access expression (e.g., value.ToString()) | ||
| if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Check if the method name is "ToString" | ||
| if (memberAccess.Name.Identifier.Text != "ToString") | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Check if there are too many arguments) | ||
| if (invocation.ArgumentList.Arguments.Count > 0) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Get the symbol information for the invocation | ||
| var symbolInfo = context.SemanticModel.GetSymbolInfo(invocation); | ||
| if (symbolInfo.Symbol is not IMethodSymbol methodSymbol) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Verify this is the ToString() method from System.Object or System.Enum | ||
| if (methodSymbol.Name != "ToString" || | ||
| methodSymbol.Parameters.Length != 0 || | ||
| (methodSymbol.ContainingType.SpecialType != SpecialType.System_Object && | ||
| methodSymbol.ContainingType.SpecialType != SpecialType.System_Enum)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Get the type of the receiver (the thing before .ToString()) | ||
| var receiverType = context.SemanticModel.GetTypeInfo(memberAccess.Expression).Type; | ||
| if (receiverType is null || receiverType.TypeKind != TypeKind.Enum) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Check if the enum has the [EnumExtensions] attribute | ||
| bool hasEnumExtensionsAttribute = false; | ||
| foreach (var attributeData in receiverType.GetAttributes()) | ||
| { | ||
| if (SymbolEqualityComparer.Default.Equals( | ||
| attributeData.AttributeClass, | ||
| enumExtensionsAttr)) | ||
| { | ||
| hasEnumExtensionsAttribute = true; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (!hasEnumExtensionsAttribute) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Report the diagnostic | ||
| var diagnostic = Diagnostic.Create( | ||
| Rule, | ||
| memberAccess.Name.GetLocation(), | ||
| receiverType.Name); | ||
|
|
||
| context.ReportDiagnostic(diagnostic); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| using System.Collections.Immutable; | ||
| using System.Composition; | ||
| using Microsoft.CodeAnalysis; | ||
| using Microsoft.CodeAnalysis.CodeActions; | ||
| using Microsoft.CodeAnalysis.CodeFixes; | ||
| using Microsoft.CodeAnalysis.CSharp; | ||
| using Microsoft.CodeAnalysis.CSharp.Syntax; | ||
|
|
||
| namespace NetEscapades.EnumGenerators.Diagnostics; | ||
|
|
||
| [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ToStringCodeFixProvider)), Shared] | ||
| public class ToStringCodeFixProvider : CodeFixProvider | ||
| { | ||
| private const string Title = "Replace with ToStringFast()"; | ||
|
|
||
| public sealed override ImmutableArray<string> FixableDiagnosticIds | ||
| => ImmutableArray.Create(ToStringAnalyzer.DiagnosticId); | ||
|
|
||
| public sealed override FixAllProvider GetFixAllProvider() | ||
| => WellKnownFixAllProviders.BatchFixer; | ||
|
|
||
| public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) | ||
| { | ||
| var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); | ||
| if (root is null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var diagnostic = context.Diagnostics.First(); | ||
| var diagnosticSpan = diagnostic.Location.SourceSpan; | ||
|
|
||
| // Find the ToString identifier | ||
| var identifierNode = root.FindToken(diagnosticSpan.Start).Parent; | ||
| if (identifierNode is not IdentifierNameSyntax identifierName) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Register a code action that will invoke the fix | ||
| context.RegisterCodeFix( | ||
| CodeAction.Create( | ||
| title: Title, | ||
| createChangedDocument: c => ReplaceToStringWithToStringFast(context.Document, identifierName, c), | ||
| equivalenceKey: Title), | ||
| context.Diagnostics); | ||
| } | ||
|
|
||
| private static async Task<Document> ReplaceToStringWithToStringFast( | ||
| Document document, | ||
| IdentifierNameSyntax identifierName, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); | ||
| if (root is null) | ||
| { | ||
| return document; | ||
| } | ||
|
|
||
| // Create the new identifier with "ToStringFast" | ||
| var newIdentifier = SyntaxFactory.IdentifierName("ToStringFast") | ||
| .WithTriviaFrom(identifierName); | ||
|
|
||
| // Replace the old identifier with the new one | ||
| var newRoot = root.ReplaceNode(identifierName, newIdentifier); | ||
|
|
||
| return document.WithSyntaxRoot(newRoot); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| #### .NET Coding Conventions #### | ||
| [*.{cs,vb}] | ||
|
|
||
| dotnet_diagnostic.NEEG001.severity = error | ||
| dotnet_diagnostic.NEEG002.severity = error | ||
| dotnet_diagnostic.NEEG003.severity = error | ||
| dotnet_diagnostic.NEEG004.severity = error |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| using System; | ||
| using Xunit; | ||
|
|
||
| namespace NetEscapades.EnumGenerators.Benchmarks; | ||
|
|
||
| public class AnalyzerTests | ||
| { | ||
| // These method calls should all be flagged by the analyzer | ||
| [Fact] | ||
| public void Neeg004Testing() | ||
| { | ||
| #pragma warning disable NEEG004 | ||
| var test = EnumInSystem.First; | ||
| _ = test.ToString(); | ||
| _ = EnumInSystem.First.ToString(); | ||
| #pragma warning restore NEEG004 | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,8 @@ | |
| using System.ComponentModel.DataAnnotations; | ||
| using System.Runtime.Serialization; | ||
|
|
||
| #pragma warning disable NEEG003 | ||
|
|
||
| [assembly:NetEscapades.EnumGenerators.EnumExtensions<DateTimeKind>()] | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot when checking if an enum is valid to be replaced with ToStringFast(), we need to check if it's referenced in an
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in commit b291228. The analyzer now checks for |
||
| [assembly:NetEscapades.EnumGenerators.EnumExtensions<System.IO.FileShare>()] | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.