-
Notifications
You must be signed in to change notification settings - Fork 0
Add DP024 analyzer for unregistered handler implementations #8
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 all commits
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,7 @@ | ||
| namespace DesignPatterns.Analyzers; | ||
|
|
||
| internal static class HandlerAnalysisConstants | ||
| { | ||
| internal const string HandlerOrderMetadataName = "DesignPatterns.Behavioral.HandlerOrderAttribute"; | ||
| internal const string HandlerOrderGenericMetadataName = "DesignPatterns.Behavioral.HandlerOrderAttribute`1"; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,198 @@ | ||
| using System.Collections.Generic; | ||
| using System.Collections.Immutable; | ||
| using System.Linq; | ||
| using DesignPatterns.Diagnostics; | ||
| using Microsoft.CodeAnalysis; | ||
| using Microsoft.CodeAnalysis.Diagnostics; | ||
|
|
||
| namespace DesignPatterns.Analyzers; | ||
|
|
||
| /// <summary> | ||
| /// Reports concrete handler implementations that implement <c>IHandler<TContext></c> for a registered context but lack <c>[HandlerOrder]</c>. | ||
| /// </summary> | ||
| [DiagnosticAnalyzer(LanguageNames.CSharp)] | ||
| public sealed class UnregisteredHandlerAnalyzer : DiagnosticAnalyzer | ||
| { | ||
| private static readonly DiagnosticDescriptor Rule = new( | ||
| DiagnosticIds.HandlerOrderUnregisteredImplementation, | ||
| title: "Handler implementation is not registered", | ||
| messageFormat: "Type '{0}' implements IHandler<{1}> but is missing [HandlerOrder]", | ||
| category: "DesignPatterns.Analyzers", | ||
| defaultSeverity: DiagnosticSeverity.Info, | ||
| isEnabledByDefault: true); | ||
|
|
||
| /// <inheritdoc /> | ||
| public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => | ||
| ImmutableArray.Create(Rule); | ||
|
|
||
| /// <inheritdoc /> | ||
| public override void Initialize(AnalysisContext context) | ||
| { | ||
| context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); | ||
| context.EnableConcurrentExecution(); | ||
| context.RegisterCompilationStartAction(OnCompilationStart); | ||
| } | ||
|
|
||
| private static void OnCompilationStart(CompilationStartAnalysisContext context) | ||
| { | ||
| var registeredContexts = CollectRegisteredContexts(context.Compilation); | ||
| if (registeredContexts.IsEmpty) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| context.RegisterSymbolAction( | ||
| symbolContext => AnalyzeNamedType(symbolContext, registeredContexts), | ||
| SymbolKind.NamedType); | ||
| } | ||
|
|
||
| private static void AnalyzeNamedType(SymbolAnalysisContext context, ImmutableHashSet<INamedTypeSymbol> registeredContexts) | ||
| { | ||
| if (context.Symbol is not INamedTypeSymbol typeSymbol) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (typeSymbol.TypeKind != TypeKind.Class || typeSymbol.IsAbstract) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (typeSymbol.DeclaredAccessibility == Accessibility.Private && typeSymbol.ContainingType is not null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| foreach (var contextType in GetHandlerContextTypes(typeSymbol)) | ||
| { | ||
| if (!registeredContexts.Contains(contextType)) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| if (HasHandlerOrderForContext(typeSymbol, contextType)) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| var location = typeSymbol.Locations.FirstOrDefault() ?? Location.None; | ||
| context.ReportDiagnostic(Diagnostic.Create( | ||
| Rule, | ||
| location, | ||
| typeSymbol.Name, | ||
| contextType.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat))); | ||
| } | ||
| } | ||
|
|
||
| private static ImmutableHashSet<INamedTypeSymbol> CollectRegisteredContexts(Compilation compilation) | ||
| { | ||
| var builder = ImmutableHashSet.CreateBuilder<INamedTypeSymbol>(SymbolEqualityComparer.Default); | ||
|
|
||
| foreach (var typeSymbol in GetAllTypes(compilation.Assembly.GlobalNamespace)) | ||
| { | ||
| foreach (var contextType in GetHandlerOrderContextTypes(typeSymbol)) | ||
| { | ||
| builder.Add(contextType); | ||
| } | ||
| } | ||
|
|
||
| return builder.ToImmutable(); | ||
| } | ||
|
|
||
| private static IEnumerable<INamedTypeSymbol> GetHandlerContextTypes(INamedTypeSymbol typeSymbol) | ||
| { | ||
| foreach (var iface in typeSymbol.AllInterfaces) | ||
| { | ||
| if (iface.Name != "IHandler" || iface.TypeArguments.Length != 1) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| if (iface.TypeArguments[0] is INamedTypeSymbol contextType) | ||
| { | ||
| yield return contextType; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static bool HasHandlerOrderForContext(INamedTypeSymbol typeSymbol, INamedTypeSymbol contextType) => | ||
| GetHandlerOrderContextTypes(typeSymbol).Any( | ||
| registeredContext => SymbolEqualityComparer.Default.Equals(registeredContext, contextType)); | ||
|
|
||
| private static IEnumerable<INamedTypeSymbol> GetHandlerOrderContextTypes(INamedTypeSymbol typeSymbol) | ||
| { | ||
| foreach (var attribute in typeSymbol.GetAttributes()) | ||
| { | ||
| if (!IsHandlerOrderAttribute(attribute.AttributeClass)) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| var context = TryGetContextFromAttribute(attribute); | ||
| if (context is not null) | ||
| { | ||
| yield return context; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static bool IsHandlerOrderAttribute(INamedTypeSymbol? attributeClass) | ||
| { | ||
| if (attributeClass is null) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| var metadataName = attributeClass.MetadataName; | ||
| if (metadataName == "HandlerOrderAttribute") | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| return attributeClass.OriginalDefinition.MetadataName == "HandlerOrderAttribute`1"; | ||
| } | ||
|
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. Unqualified HandlerOrder attribute matchingLow Severity
Reviewed by Cursor Bugbot for commit 30fe19e. Configure here. |
||
|
|
||
| private static INamedTypeSymbol? TryGetContextFromAttribute(AttributeData attribute) | ||
| { | ||
| if (attribute.AttributeClass?.IsGenericType == true) | ||
| { | ||
| return attribute.AttributeClass.TypeArguments.Length == 1 | ||
| ? attribute.AttributeClass.TypeArguments[0] as INamedTypeSymbol | ||
| : null; | ||
| } | ||
|
|
||
| if (attribute.ConstructorArguments.Length >= 2 && | ||
| attribute.ConstructorArguments[1].Kind == TypedConstantKind.Type && | ||
| attribute.ConstructorArguments[1].Value is INamedTypeSymbol nonGenericContext) | ||
| { | ||
| return nonGenericContext; | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| private static IEnumerable<INamedTypeSymbol> GetAllTypes(INamespaceSymbol namespaceSymbol) | ||
| { | ||
| foreach (var member in namespaceSymbol.GetMembers()) | ||
| { | ||
| switch (member) | ||
| { | ||
| case INamespaceSymbol nestedNamespace: | ||
| foreach (var nested in GetAllTypes(nestedNamespace)) | ||
| { | ||
| yield return nested; | ||
| } | ||
|
|
||
| break; | ||
| case INamedTypeSymbol typeSymbol: | ||
| yield return typeSymbol; | ||
| foreach (var nestedType in typeSymbol.GetTypeMembers()) | ||
| { | ||
| yield return nestedType; | ||
| } | ||
|
|
||
| break; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| using DesignPatterns.Analyzers; | ||
| using Microsoft.CodeAnalysis; | ||
|
|
||
| namespace DesignPatterns.Analyzers.Tests; | ||
|
|
||
| public sealed class UnregisteredHandlerAnalyzerTests | ||
| { | ||
| [Fact] | ||
| public async Task ReportsDp024WhenImplementationMissingHandlerOrder() | ||
| { | ||
| const string source = """ | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using DesignPatterns.Behavioral; | ||
|
|
||
| namespace TestAssembly; | ||
|
|
||
| public sealed class RequestContext | ||
| { | ||
| } | ||
|
|
||
| [HandlerOrder<RequestContext>(10)] | ||
| public sealed class LoggingHandler : IHandler<RequestContext> | ||
| { | ||
| public ValueTask InvokeAsync( | ||
| RequestContext context, | ||
| HandlerDelegate<RequestContext> next, | ||
| CancellationToken cancellationToken = default) => | ||
| default; | ||
| } | ||
|
|
||
| public sealed class AuditHandler : IHandler<RequestContext> | ||
| { | ||
| public ValueTask InvokeAsync( | ||
| RequestContext context, | ||
| HandlerDelegate<RequestContext> next, | ||
| CancellationToken cancellationToken = default) => | ||
| default; | ||
| } | ||
| """; | ||
|
|
||
| var diagnostics = await AnalyzerTestContext.RunAnalyzersAsync( | ||
| source, | ||
| new UnregisteredHandlerAnalyzer()); | ||
|
|
||
| Assert.Contains( | ||
| diagnostics, | ||
| diagnostic => | ||
| diagnostic.Id == "DP024" && | ||
| diagnostic.GetMessage().Contains("AuditHandler", StringComparison.Ordinal)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task DoesNotReportWhenHandlerOrderPresent() | ||
| { | ||
| const string source = """ | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using DesignPatterns.Behavioral; | ||
|
|
||
| namespace TestAssembly; | ||
|
|
||
| public sealed class RequestContext | ||
| { | ||
| } | ||
|
|
||
| [HandlerOrder<RequestContext>(10)] | ||
| public sealed class LoggingHandler : IHandler<RequestContext> | ||
| { | ||
| public ValueTask InvokeAsync( | ||
| RequestContext context, | ||
| HandlerDelegate<RequestContext> next, | ||
| CancellationToken cancellationToken = default) => | ||
| default; | ||
| } | ||
| """; | ||
|
|
||
| var diagnostics = await AnalyzerTestContext.RunAnalyzersAsync( | ||
| source, | ||
| new UnregisteredHandlerAnalyzer()); | ||
|
|
||
| Assert.DoesNotContain(diagnostics, diagnostic => diagnostic.Id == "DP024"); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task DoesNotReportWhenNoHandlerOrderInCompilation() | ||
| { | ||
| const string source = """ | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using DesignPatterns.Behavioral; | ||
|
|
||
| namespace TestAssembly; | ||
|
|
||
| public sealed class RequestContext | ||
| { | ||
| } | ||
|
|
||
| public sealed class OrphanHandler : IHandler<RequestContext> | ||
| { | ||
| public ValueTask InvokeAsync( | ||
| RequestContext context, | ||
| HandlerDelegate<RequestContext> next, | ||
| CancellationToken cancellationToken = default) => | ||
| default; | ||
| } | ||
| """; | ||
|
|
||
| var diagnostics = await AnalyzerTestContext.RunAnalyzersAsync( | ||
| source, | ||
| new UnregisteredHandlerAnalyzer()); | ||
|
|
||
| Assert.DoesNotContain(diagnostics, diagnostic => diagnostic.Id == "DP024"); | ||
| } | ||
| } |


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.
HandlerAnalysisConstants never referenced
Low Severity
HandlerAnalysisConstantsdefines fully qualifiedHandlerOrdermetadata name strings but nothing in the analyzers project references them. The new analyzer duplicates the generator’s naming approach in a dead file instead of sharing or using those constants.Reviewed by Cursor Bugbot for commit 30fe19e. Configure here.