-
Notifications
You must be signed in to change notification settings - Fork 55
Add analyzer to detect and warn about enums in generic types #156
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 4 commits
371e6d6
8a3b006
3c47390
b1f39f6
3d10750
c4499a4
e9be8ff
e9d16da
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,16 @@ | ||||||
| using Microsoft.CodeAnalysis; | ||||||
|
|
||||||
| namespace NetEscapades.EnumGenerators; | ||||||
|
|
||||||
| public static class DiagnosticHelper | ||||||
| { | ||||||
| public static readonly DiagnosticDescriptor EnumInGenericType = new( | ||||||
| #pragma warning disable RS2008 // Enable Analyzer Release Tracking | ||||||
| id: "NEEG002", | ||||||
| #pragma warning restore RS2008 | ||||||
| title: "Enum in generic type not supported", | ||||||
| messageFormat: "The enum '{0}' is nested inside a generic type, which is not supported for enum extension generation", | ||||||
|
||||||
| messageFormat: "The enum '{0}' is nested inside a generic type, which is not supported for enum extension generation", | |
| messageFormat: "The enum '{0}' is nested inside a generic type. [EnumExtension] attribute is not supported", |
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.
Updated message format as suggested in c4499a4.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,9 +40,21 @@ public void Initialize(IncrementalGeneratorInitializationContext context) | |
| .SelectMany(static (m, _) => m!.Value) | ||
| .WithTrackingName(TrackingNames.InitialExternalExtraction); | ||
|
|
||
| // Check for enums in generic types and generate diagnostics | ||
| IncrementalValuesProvider<Diagnostic> diagnostics = context.SyntaxProvider | ||
| .ForAttributeWithMetadataName(Attributes.EnumExtensionsAttribute, | ||
| predicate: static (node, _) => node is EnumDeclarationSyntax, | ||
| transform: GetDiagnosticForGenericTypeEnum) | ||
| .Where(static diagnostic => diagnostic is not null) | ||
| .Select(static (diagnostic, _) => diagnostic!) | ||
| .WithTrackingName(TrackingNames.GenericTypeDiagnostics); | ||
|
||
|
|
||
| context.RegisterSourceOutput(enumsToGenerate.Combine(csharp14IsSupported), | ||
| static (spc, enumToGenerate) => Execute(in enumToGenerate.Left, enumToGenerate.Right, spc)); | ||
|
|
||
| context.RegisterSourceOutput(diagnostics, | ||
| static (spc, diagnostic) => spc.ReportDiagnostic(diagnostic)); | ||
|
|
||
| context.RegisterSourceOutput(externalEnums.Combine(csharp14IsSupported), | ||
| static (spc, enumToGenerate) => Execute(in enumToGenerate.Left, enumToGenerate.Right, spc)); | ||
| } | ||
|
|
@@ -129,6 +141,12 @@ static void Execute(in EnumToGenerate enumToGenerate, bool csharp14IsSupported, | |
|
|
||
| ct.ThrowIfCancellationRequested(); | ||
|
|
||
| // Skip enums in generic types - these will be handled by diagnostics provider | ||
| if (IsNestedInGenericType(enumSymbol)) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| var hasFlags = false; | ||
| string? nameSpace = null; | ||
| string? name = null; | ||
|
|
@@ -169,6 +187,41 @@ static void Execute(in EnumToGenerate enumToGenerate, bool csharp14IsSupported, | |
| return TryExtractEnumSymbol(enumSymbol, name, nameSpace, hasFlags); | ||
| } | ||
|
|
||
| static Diagnostic? GetDiagnosticForGenericTypeEnum(GeneratorAttributeSyntaxContext context, CancellationToken ct) | ||
| { | ||
| INamedTypeSymbol? enumSymbol = context.TargetSymbol as INamedTypeSymbol; | ||
| if (enumSymbol is null) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| ct.ThrowIfCancellationRequested(); | ||
|
|
||
| if (IsNestedInGenericType(enumSymbol)) | ||
| { | ||
| return Diagnostic.Create( | ||
| DiagnosticHelper.EnumInGenericType, | ||
| context.TargetNode.GetLocation(), | ||
| enumSymbol.Name); | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| static bool IsNestedInGenericType(INamedTypeSymbol enumSymbol) | ||
| { | ||
| var containingType = enumSymbol.ContainingType; | ||
| while (containingType is not null) | ||
| { | ||
| if (containingType.IsGenericType) | ||
| { | ||
| return true; | ||
| } | ||
| containingType = containingType.ContainingType; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| static EnumToGenerate? TryExtractEnumSymbol(INamedTypeSymbol enumSymbol, string? name, string? nameSpace, bool hasFlags) | ||
| { | ||
| name ??= enumSymbol.Name + "Extensions"; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ | |
| <ArtifactsPath>$(MSBuildThisFileDirectory)../artifacts</ArtifactsPath> | ||
| <SuppressTfmSupportBuildWarnings>true</SuppressTfmSupportBuildWarnings> | ||
| <NoWarn>NU1901;NU1902;NU1903;NU1904</NoWarn> | ||
| <TargetFrameworks>net6.0;net7.0;net8.0;net9.0;net10.0</TargetFrameworks> | ||
| <TargetFrameworks>net6.0;net7.0;net8.0</TargetFrameworks> | ||
|
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. Don't change this
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. Reverted changes to
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. You didn't revert this. Revert it |
||
| <TargetFrameworks Condition=" '$(OS)' == 'Windows_NT' ">net48;netcoreapp2.1;netcoreapp3.1;$(TargetFrameworks)</TargetFrameworks> | ||
|
|
||
| <!-- Attempt workaround for https://github.com/dotnet/sdk/issues/37636 --> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| using System; | ||
| using System.Linq; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.CodeAnalysis; | ||
|
|
@@ -583,4 +584,179 @@ public enum MyEnum | |
| Assert.Empty(diagnostics); | ||
| return Verifier.Verify(output, Settings()); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void GeneratesWarningForEnumInGenericType() | ||
| { | ||
| const string input = | ||
| """ | ||
| using NetEscapades.EnumGenerators; | ||
|
|
||
| namespace MyTestNameSpace | ||
| { | ||
| public class GenericClass<T> | ||
| { | ||
| [EnumExtensions] | ||
| public enum MyEnum | ||
| { | ||
| First, | ||
| Second, | ||
| } | ||
| } | ||
| } | ||
| """; | ||
|
|
||
| // Create a simplified compilation and run the generator | ||
| var syntaxTree = CSharpSyntaxTree.ParseText(input); | ||
|
||
| var references = AppDomain.CurrentDomain.GetAssemblies() | ||
| .Where(assembly => !assembly.IsDynamic && !string.IsNullOrWhiteSpace(assembly.Location)) | ||
| .Select(assembly => MetadataReference.CreateFromFile(assembly.Location)) | ||
| .Concat([ | ||
| MetadataReference.CreateFromFile(typeof(NetEscapades.EnumGenerators.EnumGenerator).Assembly.Location), | ||
| MetadataReference.CreateFromFile(typeof(EnumExtensionsAttribute).Assembly.Location), | ||
| MetadataReference.CreateFromFile(typeof(System.ComponentModel.DataAnnotations.DisplayAttribute).Assembly.Location), | ||
| MetadataReference.CreateFromFile(typeof(System.CodeDom.Compiler.GeneratedCodeAttribute).Assembly.Location) | ||
| ]); | ||
|
|
||
| var compilation = CSharpCompilation.Create( | ||
| "test", | ||
| [syntaxTree], | ||
| references, | ||
| new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); | ||
|
|
||
| var generator = new EnumGenerator(); | ||
| GeneratorDriver driver = CSharpGeneratorDriver.Create(generator); | ||
|
|
||
| driver = driver.RunGeneratorsAndUpdateCompilation(compilation, out var outputCompilation, out var diagnostics); | ||
|
|
||
| Assert.Single(diagnostics); | ||
| Assert.Equal("NEEG002", diagnostics[0].Id); | ||
| Assert.Equal(DiagnosticSeverity.Warning, diagnostics[0].Severity); | ||
| Assert.Contains("MyEnum", diagnostics[0].GetMessage()); | ||
| Assert.Contains("generic type", diagnostics[0].GetMessage()); | ||
|
|
||
| // Verify no enum extension source files were generated (only the attribute) | ||
| var result = driver.GetRunResult(); | ||
| var generatedSources = result.Results[0].GeneratedSources; | ||
| Assert.Single(generatedSources); // Only the attribute should be generated | ||
| Assert.Contains("EnumExtensionsAttribute", generatedSources[0].HintName); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void GeneratesWarningForEnumInDeeplyNestedGenericType() | ||
| { | ||
| const string input = | ||
| """ | ||
| using NetEscapades.EnumGenerators; | ||
|
|
||
| namespace MyTestNameSpace | ||
| { | ||
| public class OuterClass | ||
| { | ||
| public class GenericClass<T> | ||
| { | ||
| public class InnerClass | ||
| { | ||
| [EnumExtensions] | ||
| public enum MyEnum | ||
| { | ||
| First, | ||
| Second, | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| """; | ||
|
|
||
| // Create a simplified compilation and run the generator | ||
| var syntaxTree = CSharpSyntaxTree.ParseText(input); | ||
| var references = AppDomain.CurrentDomain.GetAssemblies() | ||
| .Where(assembly => !assembly.IsDynamic && !string.IsNullOrWhiteSpace(assembly.Location)) | ||
| .Select(assembly => MetadataReference.CreateFromFile(assembly.Location)) | ||
| .Concat([ | ||
| MetadataReference.CreateFromFile(typeof(NetEscapades.EnumGenerators.EnumGenerator).Assembly.Location), | ||
| MetadataReference.CreateFromFile(typeof(EnumExtensionsAttribute).Assembly.Location), | ||
| MetadataReference.CreateFromFile(typeof(System.ComponentModel.DataAnnotations.DisplayAttribute).Assembly.Location), | ||
| MetadataReference.CreateFromFile(typeof(System.CodeDom.Compiler.GeneratedCodeAttribute).Assembly.Location) | ||
| ]); | ||
|
|
||
| var compilation = CSharpCompilation.Create( | ||
| "test", | ||
| [syntaxTree], | ||
| references, | ||
| new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); | ||
|
|
||
| var generator = new EnumGenerator(); | ||
| GeneratorDriver driver = CSharpGeneratorDriver.Create(generator); | ||
|
|
||
| driver = driver.RunGeneratorsAndUpdateCompilation(compilation, out var outputCompilation, out var diagnostics); | ||
|
|
||
| Assert.Single(diagnostics); | ||
| Assert.Equal("NEEG002", diagnostics[0].Id); | ||
| Assert.Equal(DiagnosticSeverity.Warning, diagnostics[0].Severity); | ||
| Assert.Contains("MyEnum", diagnostics[0].GetMessage()); | ||
| Assert.Contains("generic type", diagnostics[0].GetMessage()); | ||
|
|
||
| // Verify no enum extension source files were generated (only the attribute) | ||
| var result = driver.GetRunResult(); | ||
| var generatedSources = result.Results[0].GeneratedSources; | ||
| Assert.Single(generatedSources); // Only the attribute should be generated | ||
| Assert.Contains("EnumExtensionsAttribute", generatedSources[0].HintName); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void DoesNotGenerateWarningForEnumInNonGenericNestedClass() | ||
| { | ||
| const string input = | ||
| """ | ||
| using NetEscapades.EnumGenerators; | ||
|
|
||
| namespace MyTestNameSpace | ||
| { | ||
| public class NonGenericClass | ||
| { | ||
| [EnumExtensions] | ||
| public enum MyEnum | ||
| { | ||
| First, | ||
| Second, | ||
| } | ||
| } | ||
| } | ||
| """; | ||
|
|
||
| // Create a simplified compilation and run the generator | ||
| var syntaxTree = CSharpSyntaxTree.ParseText(input); | ||
| var references = AppDomain.CurrentDomain.GetAssemblies() | ||
| .Where(assembly => !assembly.IsDynamic && !string.IsNullOrWhiteSpace(assembly.Location)) | ||
| .Select(assembly => MetadataReference.CreateFromFile(assembly.Location)) | ||
| .Concat([ | ||
| MetadataReference.CreateFromFile(typeof(NetEscapades.EnumGenerators.EnumGenerator).Assembly.Location), | ||
| MetadataReference.CreateFromFile(typeof(EnumExtensionsAttribute).Assembly.Location), | ||
| MetadataReference.CreateFromFile(typeof(System.ComponentModel.DataAnnotations.DisplayAttribute).Assembly.Location), | ||
| MetadataReference.CreateFromFile(typeof(System.CodeDom.Compiler.GeneratedCodeAttribute).Assembly.Location) | ||
| ]); | ||
|
|
||
| var compilation = CSharpCompilation.Create( | ||
| "test", | ||
| [syntaxTree], | ||
| references, | ||
| new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); | ||
|
|
||
| var generator = new EnumGenerator(); | ||
| GeneratorDriver driver = CSharpGeneratorDriver.Create(generator); | ||
|
|
||
| driver = driver.RunGeneratorsAndUpdateCompilation(compilation, out var outputCompilation, out var diagnostics); | ||
|
|
||
| // Should not generate any diagnostics | ||
| Assert.Empty(diagnostics); | ||
|
|
||
| // Should generate the enum extension class (plus the attribute) | ||
| var result = driver.GetRunResult(); | ||
| var generatedSources = result.Results[0].GeneratedSources; | ||
| Assert.Equal(2, generatedSources.Length); // Attribute + enum extensions | ||
| Assert.Contains(generatedSources, s => s.HintName.Contains("EnumExtensionsAttribute")); | ||
| Assert.Contains(generatedSources, s => s.HintName.Contains("MyEnum")); | ||
| } | ||
| } | ||
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.
We should start this at
NEEG001seeing as it's the first diagnostic being addedThere 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.
Changed diagnostic ID from
NEEG002toNEEG001in 3d10750.