-
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
Merged
andrewlock
merged 11 commits into
main
from
copilot/add-analyzer-for-tostring-replacement
Dec 17, 2025
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
36f1d5c
Initial plan
Copilot 9f41c76
Add ToStringAnalyzer (NEEG004) and ToStringCodeFixProvider
Copilot 56f5a2f
Add fixer to tests
andrewlock b790890
Add editor config to check it works properly
andrewlock 7daba34
minor optimizations
andrewlock 2208218
Confirm the analyzer works in integration tests
andrewlock 5b1a131
Add test for format strings
andrewlock ffc5fae
Handle format specifiers in ToStringAnalyzer - only suggest replaceme…
Copilot 20c8610
Tweaks and updates
andrewlock b291228
Add support for external enum types in ToStringAnalyzer - detect Enum…
Copilot 48573cd
minor tweaks
andrewlock File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
168 changes: 168 additions & 0 deletions
168
src/NetEscapades.EnumGenerators/Diagnostics/ToStringAnalyzer.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,168 @@ | ||
| using System.Collections.Generic; | ||
| 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); | ||
| var externalEnumExtensionsAttr = | ||
| ctx.Compilation.GetTypeByMetadataName(Attributes.ExternalEnumExtensionsAttribute); | ||
|
|
||
| if (enumExtensionsAttr is null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Collect all enum types that have EnumExtensions<T> attributes | ||
| var externalEnumTypes = new HashSet<INamedTypeSymbol>(SymbolEqualityComparer.Default); | ||
| if (externalEnumExtensionsAttr is not null) | ||
| { | ||
| foreach (var attribute in ctx.Compilation.Assembly.GetAttributes()) | ||
| { | ||
| if (attribute.AttributeClass is { IsGenericType: true } attrClass && | ||
| SymbolEqualityComparer.Default.Equals(attrClass.ConstructedFrom, externalEnumExtensionsAttr) && | ||
| attrClass.TypeArguments is [INamedTypeSymbol { TypeKind: TypeKind.Enum } enumType]) | ||
| { | ||
| externalEnumTypes.Add(enumType); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| ctx.RegisterSyntaxNodeAction( | ||
| c => AnalyzeInvocation(c, enumExtensionsAttr, externalEnumTypes), | ||
| SyntaxKind.InvocationExpression); | ||
| }); | ||
| } | ||
|
|
||
| private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context, INamedTypeSymbol enumExtensionsAttr, HashSet<INamedTypeSymbol> externalEnumTypes) | ||
| { | ||
| 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 there are too many arguments) | ||
| if (invocation.ArgumentList.Arguments.Count > 1) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Check if the method name is "ToString" | ||
| if (memberAccess.Name.Identifier.Text != "ToString") | ||
| { | ||
| 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 | ||
| // We handle format specifiers, so accept ToString() with 0 or 1 parameters | ||
| if (methodSymbol.Name != "ToString" || | ||
| methodSymbol.Parameters.Length > 1 || | ||
| (methodSymbol.ContainingType.SpecialType != SpecialType.System_Object && | ||
| methodSymbol.ContainingType.SpecialType != SpecialType.System_Enum)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // If there's a format parameter, check if it's compatible with ToStringFast() | ||
| // ToStringFast() is equivalent to ToString() with no args or ToString("G")/ToString("g")/ToString("") | ||
| if (invocation.ArgumentList.Arguments.Count > 0) | ||
| { | ||
| var argument = invocation.ArgumentList.Arguments[0]; | ||
| var constantValue = context.SemanticModel.GetConstantValue(argument.Expression); | ||
|
|
||
| // If we can't determine the value at compile time, don't suggest replacement | ||
| // If it's not a string (e.g., it's an IFormatProvider), don't suggest replacement | ||
| if (!constantValue.HasValue || constantValue.Value is not string formatString) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Check if the format string is compatible with ToStringFast() | ||
| // Only "", "G", and "g" are compatible | ||
| if (!string.IsNullOrEmpty(formatString) && | ||
| !string.Equals(formatString, "G", StringComparison.Ordinal) && | ||
| !string.Equals(formatString, "g", StringComparison.Ordinal)) | ||
| { | ||
| 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 or is referenced in EnumExtensions<T> | ||
| bool hasEnumExtensionsAttribute = false; | ||
|
|
||
| // First check if the enum itself has the attribute | ||
| foreach (var attributeData in receiverType.GetAttributes()) | ||
| { | ||
| if (SymbolEqualityComparer.Default.Equals( | ||
| attributeData.AttributeClass, | ||
| enumExtensionsAttr)) | ||
| { | ||
| hasEnumExtensionsAttribute = true; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // If not, check if it's in the external enum types (EnumExtensions<T>) | ||
| if (!hasEnumExtensionsAttribute && receiverType is INamedTypeSymbol namedType) | ||
| { | ||
| hasEnumExtensionsAttribute = externalEnumTypes.Contains(namedType); | ||
| } | ||
|
|
||
| if (!hasEnumExtensionsAttribute) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Report the diagnostic | ||
| var diagnostic = Diagnostic.Create( | ||
| Rule, | ||
| memberAccess.Name.GetLocation(), | ||
| receiverType.Name); | ||
|
|
||
| context.ReportDiagnostic(diagnostic); | ||
| } | ||
| } |
93 changes: 93 additions & 0 deletions
93
src/NetEscapades.EnumGenerators/Diagnostics/ToStringCodeFixProvider.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| 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; | ||
| } | ||
|
|
||
| // Find the invocation expression to replace arguments as well | ||
| var invocationExpression = identifierName.Parent?.Parent as InvocationExpressionSyntax; | ||
| if (invocationExpression is null) | ||
| { | ||
| return document; | ||
| } | ||
|
|
||
| // Create the new identifier with "ToStringFast" | ||
| var newIdentifier = SyntaxFactory.IdentifierName("ToStringFast") | ||
| .WithTriviaFrom(identifierName); | ||
|
|
||
| // Create new member access with the new identifier | ||
| var memberAccess = identifierName.Parent as MemberAccessExpressionSyntax; | ||
| if (memberAccess is null) | ||
| { | ||
| return document; | ||
| } | ||
|
|
||
| var newMemberAccess = memberAccess.WithName(newIdentifier); | ||
|
|
||
| // Create new invocation with empty argument list | ||
| var newArgumentList = SyntaxFactory.ArgumentList() | ||
| .WithTrailingTrivia(invocationExpression.ArgumentList.GetTrailingTrivia()); | ||
|
|
||
| var newInvocation = invocationExpression | ||
| .WithExpression(newMemberAccess) | ||
| .WithArgumentList(newArgumentList); | ||
|
|
||
| // Replace the old invocation with the new one | ||
| var newRoot = root.ReplaceNode(invocationExpression, newInvocation); | ||
|
|
||
| return document.WithSyntaxRoot(newRoot); | ||
| } | ||
| } |
7 changes: 7 additions & 0 deletions
7
tests/NetEscapades.EnumGenerators.IntegrationTests/.editorconfig
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 |
23 changes: 23 additions & 0 deletions
23
tests/NetEscapades.EnumGenerators.IntegrationTests/AnalyzerTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| 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(); | ||
| _ = EnumInSystem.First.ToString("G"); | ||
| _ = EnumInSystem.First.ToString("x"); // no error | ||
| _ = EnumInSystem.First.ToString(format: "g"); | ||
| _ = EnumInSystem.First.ToString(format: null); // no error | ||
| _ = DateTimeKind.Local.ToString(); | ||
| #pragma warning restore NEEG004 | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The 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
EnumExtensions<T>attribute as well. Add tests and update the implementationThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit b291228. The analyzer now checks for
EnumExtensions<T>assembly attributes and collects all external enum types during compilation start. Added 8 new tests covering external enum scenarios (DateTimeKind, FileShare) with various format specifiers. All 198 tests pass.