feat: source generator for Brighter handler/mapper/transform registration#4138
feat: source generator for Brighter handler/mapper/transform registration#4138slang25 wants to merge 8 commits into
Conversation
…ransform registration Adds a Roslyn incremental source generator that emits a partial method body registering handlers, message mappers and transforms discovered in the current compilation, replacing runtime AutoFromAssemblies reflection scanning. Includes: - [BrighterRegistrations] marker attribute on a partial static method - [ExcludeFromBrighterRegistration] opt-out attribute - IBrighterBuilder.Transforms callback for explicit transform registration - HelloWorld sample wired up via AddFromThisAssembly() Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Extract IsRegistrationCandidate / ClassifyType / TryClassifyGenericInterface / IsTransformInterface from DiscoverRegistrations - Extract IsOpenGeneric helper from EmitHandlers conditional - Replace 7-arg MarkerSymbols constructor with object initializer in Resolve Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Following the Kathleen Dollard incremental-generator pattern: keep semantic model reads in one place, project to a Roslyn-free intermediate model, and write source from that model as a pure function. The writer becomes trivially unit-testable and the model is structurally equatable so the incremental pipeline can cache it. Structure: - Model/RegistrationModel.cs — pure-data records describing the emit - Model/EquatableArray.cs — value-equality wrapper for caching - SemanticModelReader.cs — single point that touches Roslyn symbols - RegistrationWriter.cs — pure model -> source text - MarkerSymbols.cs / Diagnostics.cs — extracted concerns - BrighterRegistrationsGenerator.cs — thin orchestrator/pipeline only - IsExternalInit.cs — polyfill for records on netstandard2.0 Tests (new tests/Paramore.Brighter.SourceGenerators.Tests project): - RegistrationWriterTests — 8 unit tests on the pure writer - BrighterRegistrationsGeneratorTests — 4 validation tests using Microsoft.CodeAnalysis.CSharp.SourceGenerators.Testing, asserting full generated source and BRGEN001 diagnostic. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…n-auto-assemblies # Conflicts: # Brighter.slnx
- TryBuildModel returns a BuildResult struct instead of two out params (5 args -> 3 args) - TryClassifyGenericInterface delegates mapper classification to a TryAddMapper helper, lowering cyclomatic complexity - Introduce a small Same(ISymbol?, ISymbol?) wrapper around SymbolEqualityComparer.Default.Equals to tidy the call sites Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Rework the generator so no ISymbol, Compilation, SemanticModel, or SyntaxNode ever escapes a transform. Every value flowing through the incremental graph is now a value-equatable record, which lets Roslyn skip transforms and the source-output step when an edit doesn't change the semantically relevant shape of the compilation. Pipeline: - ForAttributeWithMetadataName.transform projects IMethodSymbol -> MethodCandidate (record holding either a MethodTarget or a DiagnosticInfo). - CreateSyntaxProvider for `class ... : Base` transforms to EquatableArray<DiscoveredEntry> (zero, one, or many records per class). - The discovery batches are Collect()ed and Select()ed into a single flattened, sorted EquatableArray for stable, cache-friendly output. - methodCandidates.Combine(discovered) -> RegisterSourceOutput builds a RegistrationModel from pure data and emits via the unchanged writer. DiagnosticInfo + LocationInfo carry the (path, TextSpan, LinePositionSpan) needed to reconstruct a real Diagnostic at source-output time without holding non-cacheable Roslyn objects in the cache. Tests: - IncrementalCachingTests drives CSharpGeneratorDriver with trackIncrementalGeneratorSteps and asserts on IncrementalStepRunReason: trailing-comment edits and unrelated class additions yield only Cached / Unchanged outputs; adding a real handler yields Modified plus the new handler in the generated source. 15/15 tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ference A second pipeline synthesises an `internal static class BrighterAssemblyRegistrations` with an `AddFromThisAssembly()` extension on IBrighterBuilder, populated from the same DiscoveredEntry stream as the attribute-based path. Consumers no longer need to hand-write a partial class. Gating: - The generator only emits the auto class when the MSBuild property BrighterAutoRegistration=true is visible via AnalyzerConfigOptionsProvider. - The new build/Paramore.Brighter.SourceGenerators.props sets that property and declares it CompilerVisible. Because NuGet only applies build/ to *direct* PackageReferences, transitive consumers won't see the property and the auto class won't be generated for them. - Users opt out per project with <BrighterAutoRegistration>false</BrighterAutoRegistration>. Writer: - RegistrationModel/MethodTarget gain an IsPartial flag (default true). The writer omits `partial` on both the class and the method when IsPartial=false, so the auto class is a normal static class. Sample: - HelloWorld drops the hand-written BrighterRegistrations.cs and just calls builder.Services.AddBrighter().AddFromThisAssembly(). It opts in via <BrighterAutoRegistration>true</BrighterAutoRegistration> + CompilerVisibleProperty, because ProjectReference scenarios don't pick up the package's build/ props automatically. Tests: - AutoRegistrationTests verifies property=true emits the class with discovered handlers, property=false suppresses emission, and property-missing (the transitive-consumer scenario) also suppresses emission. 18/18 tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Review: Source generator for Brighter handler/mapper/transform registrationOverall this is a well-structured generator — the strict separation of 1. Breaking change to
|
Generator surface: - IBrighterBuilder.Transforms is now a BrighterBuilderExtensions extension method, not an interface member, so the original PR no longer makes a binary-breaking change to the public IBrighterBuilder contract. - Closed-generic handlers emit r.Register<TRequest, TImpl>() via the public IAm(Async)SubscriberRegistry interfaces, so the cast to the concrete ServiceCollectionSubscriberRegistry only appears when at least one open generic is present (and then exactly once). - Generated source calls Transforms statically via the fully-qualified BrighterBuilderExtensions, so it doesn't depend on the consumer adding a `using`. - Auto-generated AddFromThisAssembly is `internal` (was `public` on an internal class, which was noisy). Reader / model hardening: - ExcludeFromBrighterRegistrationAttribute is folded into MarkerSymbols so the symbol is resolved once per compilation instead of per class. - IsPrimaryDeclaration sorts DeclaringSyntaxReferences by (FilePath, Start) explicitly rather than relying on undocumented Roslyn ordering. - ReadClass returns a DiscoveryBatch carrying both entries and diagnostics, so a discovery-time warning (BRGEN005) can travel through the cached pipeline alongside the entries. - New BRGEN005 warning fires when a generic class implements a Brighter mapper or transform interface (previously silently dropped). - BuildHintName appends an FNV-1a hash of the original display string so types differing only in non-identifier characters (e.g. Foo.Bar vs Foo_Bar) can't collide. - EquatableArray<T>.Equals/GetHashCode use EqualityComparer<T>.Default, so the public type doesn't NRE if a future caller passes a null element. - DescriptorFor throws on an unknown diagnostic id instead of defaulting to MustBePartial, catching missed updates at test time. Tests (27 passing): - End-to-end tests added for BRGEN002 / BRGEN003 / BRGEN004 / BRGEN005. - End-to-end test for mapper + async mapper + transform discovery. - End-to-end test for partial-class dedup (a handler split across two syntax trees registers exactly once). - End-to-end test that IsClassifiable filters out abstract and private nested handlers. - AutoRegistrationTests asserts Empty diagnostics on the happy path. - RegistrationWriter unit tests updated for the new emit shape and a new case verifies the implementation cast is emitted exactly once when both closed and open-generic handlers are present. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Gates Failed
(1 new file with code health below 9.00)
(2 files with Bumpy Road Ahead)
(3 files with Excess Number of Function Arguments, Complex Method, Complex Conditional, Overall Code Complexity)
Gates Passed
See analysis details in CodeScene
Reason for failure
| New code is healthy | Violations | Code Health Impact | |
|---|---|---|---|
| SemanticModelReader.cs | 5 rules | 7.96 | Suppress |
| Enforce critical code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| SemanticModelReader.cs | 1 critical rule | 7.96 | Suppress |
| RegistrationWriter.cs | 1 critical rule | 9.49 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| SemanticModelReader.cs | 4 advisory rules | 7.96 | Suppress |
| RegistrationWriter.cs | 1 advisory rule | 9.49 | Suppress |
| RegistrationModel.cs | 1 advisory rule | 9.69 | Suppress |
Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
| private static void WriteHandlers( | ||
| StringBuilder sb, | ||
| string paramName, | ||
| EquatableArray<HandlerEntry> entries, | ||
| bool isAsync) | ||
| { | ||
| if (entries.Count == 0) | ||
| return; | ||
|
|
||
| var callbackMethod = isAsync ? "AsyncHandlers" : "Handlers"; | ||
| var registerMethod = isAsync ? "RegisterAsync" : "Register"; | ||
| var hasOpenGeneric = false; | ||
| foreach (var entry in entries) | ||
| { | ||
| if (entry.IsOpenGeneric) { hasOpenGeneric = true; break; } | ||
| } | ||
|
|
||
| sb.Append(" ").Append(paramName).Append('.').Append(callbackMethod).AppendLine("(r =>"); | ||
| sb.AppendLine(" {"); | ||
|
|
||
| // For closed-generic handlers, use the strongly-typed Register<TRequest, TImpl>() method | ||
| // available on the public interface — no implementation cast needed. | ||
| foreach (var entry in entries) | ||
| { | ||
| if (entry.IsOpenGeneric) continue; | ||
| sb.Append(" r.").Append(registerMethod).Append('<') | ||
| .Append(entry.RequestTypeFullyQualified).Append(", ") | ||
| .Append(entry.HandlerTypeFullyQualified) | ||
| .AppendLine(">();"); | ||
| } | ||
|
|
||
| // Open-generic handlers need EnsureHandlerIsRegistered, which only exists on the DI | ||
| // extension's concrete ServiceCollectionSubscriberRegistry. Emit the cast only when at | ||
| // least one open generic is present, so the common case stays interface-only. | ||
| if (hasOpenGeneric) | ||
| { | ||
| sb.AppendLine(" var registry = (global::Paramore.Brighter.Extensions.DependencyInjection.ServiceCollectionSubscriberRegistry)r;"); | ||
| foreach (var entry in entries) | ||
| { | ||
| if (!entry.IsOpenGeneric) continue; | ||
| sb.Append(" registry.EnsureHandlerIsRegistered(typeof(") | ||
| .Append(entry.HandlerTypeFullyQualified) | ||
| .AppendLine("));"); | ||
| } | ||
| } | ||
|
|
||
| sb.AppendLine(" });"); | ||
| } |
There was a problem hiding this comment.
❌ New issue: Complex Method
WriteHandlers has a cyclomatic complexity of 13, threshold = 9
| private static void WriteHandlers( | ||
| StringBuilder sb, | ||
| string paramName, | ||
| EquatableArray<HandlerEntry> entries, | ||
| bool isAsync) | ||
| { | ||
| if (entries.Count == 0) | ||
| return; | ||
|
|
||
| var callbackMethod = isAsync ? "AsyncHandlers" : "Handlers"; | ||
| var registerMethod = isAsync ? "RegisterAsync" : "Register"; | ||
| var hasOpenGeneric = false; | ||
| foreach (var entry in entries) | ||
| { | ||
| if (entry.IsOpenGeneric) { hasOpenGeneric = true; break; } | ||
| } | ||
|
|
||
| sb.Append(" ").Append(paramName).Append('.').Append(callbackMethod).AppendLine("(r =>"); | ||
| sb.AppendLine(" {"); | ||
|
|
||
| // For closed-generic handlers, use the strongly-typed Register<TRequest, TImpl>() method | ||
| // available on the public interface — no implementation cast needed. | ||
| foreach (var entry in entries) | ||
| { | ||
| if (entry.IsOpenGeneric) continue; | ||
| sb.Append(" r.").Append(registerMethod).Append('<') | ||
| .Append(entry.RequestTypeFullyQualified).Append(", ") | ||
| .Append(entry.HandlerTypeFullyQualified) | ||
| .AppendLine(">();"); | ||
| } | ||
|
|
||
| // Open-generic handlers need EnsureHandlerIsRegistered, which only exists on the DI | ||
| // extension's concrete ServiceCollectionSubscriberRegistry. Emit the cast only when at | ||
| // least one open generic is present, so the common case stays interface-only. | ||
| if (hasOpenGeneric) | ||
| { | ||
| sb.AppendLine(" var registry = (global::Paramore.Brighter.Extensions.DependencyInjection.ServiceCollectionSubscriberRegistry)r;"); | ||
| foreach (var entry in entries) | ||
| { | ||
| if (!entry.IsOpenGeneric) continue; | ||
| sb.Append(" registry.EnsureHandlerIsRegistered(typeof(") | ||
| .Append(entry.HandlerTypeFullyQualified) | ||
| .AppendLine("));"); | ||
| } | ||
| } | ||
|
|
||
| sb.AppendLine(" });"); | ||
| } |
There was a problem hiding this comment.
❌ New issue: Bumpy Road Ahead
WriteHandlers has 3 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is 2 blocks per function
| private static DiscoveredEntry? TryClassifyInterface( | ||
| INamedTypeSymbol type, | ||
| INamedTypeSymbol iface, | ||
| MarkerSymbols markers, | ||
| ref bool seenTransform, | ||
| ref bool unsupportedGenericMapperOrTransform) | ||
| { | ||
| if (Same(iface, markers.MessageTransform) || Same(iface, markers.MessageTransformAsync)) | ||
| { | ||
| if (type.IsGenericType) | ||
| unsupportedGenericMapperOrTransform = true; | ||
| else | ||
| seenTransform = true; | ||
| return null; | ||
| } | ||
|
|
||
| if (!iface.IsGenericType || iface.TypeArguments.Length != 1) | ||
| return null; | ||
|
|
||
| var def = iface.OriginalDefinition; | ||
| var requestType = iface.TypeArguments[0]; | ||
|
|
||
| if (Same(def, markers.HandleRequests)) | ||
| return MakeHandlerEntry(DiscoveredKind.SyncHandler, type, requestType); | ||
| if (Same(def, markers.HandleRequestsAsync)) | ||
| return MakeHandlerEntry(DiscoveredKind.AsyncHandler, type, requestType); | ||
| if (Same(def, markers.MessageMapper)) | ||
| { | ||
| if (type.IsGenericType) { unsupportedGenericMapperOrTransform = true; return null; } | ||
| return new DiscoveredEntry(DiscoveredKind.Mapper, FullyQualified(requestType), FullyQualified(type), IsOpenGeneric: false); | ||
| } | ||
| if (Same(def, markers.MessageMapperAsync)) | ||
| { | ||
| if (type.IsGenericType) { unsupportedGenericMapperOrTransform = true; return null; } | ||
| return new DiscoveredEntry(DiscoveredKind.AsyncMapper, FullyQualified(requestType), FullyQualified(type), IsOpenGeneric: false); | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
❌ New issue: Complex Method
TryClassifyInterface has a cyclomatic complexity of 12, threshold = 9
| public static DiscoveryBatch ReadClass(GeneratorSyntaxContext ctx, CancellationToken cancellationToken) | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
|
|
||
| if (ctx.Node is not ClassDeclarationSyntax cls) | ||
| return DiscoveryBatch.Empty; | ||
|
|
||
| if (ctx.SemanticModel.GetDeclaredSymbol(cls, cancellationToken) is not INamedTypeSymbol type) | ||
| return DiscoveryBatch.Empty; | ||
|
|
||
| if (!IsClassifiable(type)) | ||
| return DiscoveryBatch.Empty; | ||
|
|
||
| // Only emit from the "primary" partial declaration so partial classes don't get | ||
| // discovered N times. Order explicitly so the choice is self-evidently stable. | ||
| if (!IsPrimaryDeclaration(type, cls)) | ||
| return DiscoveryBatch.Empty; | ||
|
|
||
| var markers = MarkerSymbols.Resolve(ctx.SemanticModel.Compilation); | ||
| if (!markers.IsValid) | ||
| return DiscoveryBatch.Empty; | ||
|
|
||
| if (markers.ExcludeAttribute is not null && HasAttribute(type, markers.ExcludeAttribute)) | ||
| return DiscoveryBatch.Empty; | ||
|
|
||
| var entries = new List<DiscoveredEntry>(); | ||
| var diagnostics = new List<DiagnosticInfo>(); | ||
| ClassifyEntries(type, markers, entries, diagnostics); | ||
| if (entries.Count == 0 && diagnostics.Count == 0) | ||
| return DiscoveryBatch.Empty; | ||
| return new DiscoveryBatch( | ||
| new EquatableArray<DiscoveredEntry>(entries), | ||
| new EquatableArray<DiagnosticInfo>(diagnostics)); | ||
| } |
There was a problem hiding this comment.
❌ New issue: Complex Method
ReadClass has a cyclomatic complexity of 10, threshold = 9
| private static DiscoveredEntry? TryClassifyInterface( | ||
| INamedTypeSymbol type, | ||
| INamedTypeSymbol iface, | ||
| MarkerSymbols markers, | ||
| ref bool seenTransform, | ||
| ref bool unsupportedGenericMapperOrTransform) | ||
| { | ||
| if (Same(iface, markers.MessageTransform) || Same(iface, markers.MessageTransformAsync)) | ||
| { | ||
| if (type.IsGenericType) | ||
| unsupportedGenericMapperOrTransform = true; | ||
| else | ||
| seenTransform = true; | ||
| return null; | ||
| } | ||
|
|
||
| if (!iface.IsGenericType || iface.TypeArguments.Length != 1) | ||
| return null; | ||
|
|
||
| var def = iface.OriginalDefinition; | ||
| var requestType = iface.TypeArguments[0]; | ||
|
|
||
| if (Same(def, markers.HandleRequests)) | ||
| return MakeHandlerEntry(DiscoveredKind.SyncHandler, type, requestType); | ||
| if (Same(def, markers.HandleRequestsAsync)) | ||
| return MakeHandlerEntry(DiscoveredKind.AsyncHandler, type, requestType); | ||
| if (Same(def, markers.MessageMapper)) | ||
| { | ||
| if (type.IsGenericType) { unsupportedGenericMapperOrTransform = true; return null; } | ||
| return new DiscoveredEntry(DiscoveredKind.Mapper, FullyQualified(requestType), FullyQualified(type), IsOpenGeneric: false); | ||
| } | ||
| if (Same(def, markers.MessageMapperAsync)) | ||
| { | ||
| if (type.IsGenericType) { unsupportedGenericMapperOrTransform = true; return null; } | ||
| return new DiscoveredEntry(DiscoveredKind.AsyncMapper, FullyQualified(requestType), FullyQualified(type), IsOpenGeneric: false); | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
❌ New issue: Bumpy Road Ahead
TryClassifyInterface has 3 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is 2 blocks per function
| @@ -0,0 +1,308 @@ | |||
| #region Licence | |||
There was a problem hiding this comment.
❌ New issue: Overall Code Complexity
This module has a mean cyclomatic complexity of 4.22 across 18 functions. The mean complexity threshold is 4
| private static DiscoveredEntry? TryClassifyInterface( | ||
| INamedTypeSymbol type, | ||
| INamedTypeSymbol iface, | ||
| MarkerSymbols markers, | ||
| ref bool seenTransform, | ||
| ref bool unsupportedGenericMapperOrTransform) | ||
| { | ||
| if (Same(iface, markers.MessageTransform) || Same(iface, markers.MessageTransformAsync)) | ||
| { | ||
| if (type.IsGenericType) | ||
| unsupportedGenericMapperOrTransform = true; | ||
| else | ||
| seenTransform = true; | ||
| return null; | ||
| } | ||
|
|
||
| if (!iface.IsGenericType || iface.TypeArguments.Length != 1) | ||
| return null; | ||
|
|
||
| var def = iface.OriginalDefinition; | ||
| var requestType = iface.TypeArguments[0]; | ||
|
|
||
| if (Same(def, markers.HandleRequests)) | ||
| return MakeHandlerEntry(DiscoveredKind.SyncHandler, type, requestType); | ||
| if (Same(def, markers.HandleRequestsAsync)) | ||
| return MakeHandlerEntry(DiscoveredKind.AsyncHandler, type, requestType); | ||
| if (Same(def, markers.MessageMapper)) | ||
| { | ||
| if (type.IsGenericType) { unsupportedGenericMapperOrTransform = true; return null; } | ||
| return new DiscoveredEntry(DiscoveredKind.Mapper, FullyQualified(requestType), FullyQualified(type), IsOpenGeneric: false); | ||
| } | ||
| if (Same(def, markers.MessageMapperAsync)) | ||
| { | ||
| if (type.IsGenericType) { unsupportedGenericMapperOrTransform = true; return null; } | ||
| return new DiscoveredEntry(DiscoveredKind.AsyncMapper, FullyQualified(requestType), FullyQualified(type), IsOpenGeneric: false); | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
❌ New issue: Excess Number of Function Arguments
TryClassifyInterface has 5 arguments, max arguments = 4
Code Review: Source generator for Brighter handler/mapper/transform registrationSolid contribution overall — the architecture is exactly what you want from an incremental generator (Roslyn types confined to Correctness / behavioural concerns
Code quality
Tests
SecurityNo notable concerns. The generator reads only the user's compilation; emitted code uses fully-qualified PerformanceThe incremental pipeline is well-structured. The Nitpicks
Nice work overall. The "verified incrementality" via 🤖 Generated with Claude Code |
1 participant
Description
Adds
Paramore.Brighter.SourceGenerators: a Roslyn incremental source generator that emits handler / message-mapper / transform registrations at compile time, as an alternative to runtimeAutoFromAssembliesreflection scanning. The generator follows the recommended "read → intermediate model → write" structure and runs as a properly incremental pipeline (verified via tests onIncrementalStepRunReason).Consumers can either:
internal static class BrighterAssemblyRegistrationsis auto-generated with anAddFromThisAssembly()extension onIBrighterBuilder. The opt-in flows via abuild/props file so it applies only to direct PackageReferences, not transitive ones. Override per-project with<BrighterAutoRegistration>false</BrighterAutoRegistration>.static partialmethod marked with[BrighterRegistrations]and the generator fills in the body.[ExcludeFromBrighterRegistration]opts a single type out either way. A newIBrighterBuilder.Transforms(...)callback onServiceCollectionBrighterBuilderis added so transforms can be registered explicitly (symmetric withHandlers/MapperRegistry). The HelloWorld sample exercises both: the auto-generated path plus aNoOpTransformerfor transform discovery.Related Issues
Type of Change
Checklist
Additional Notes
Replaces #4127 (which was opened from my fork).
Architecture follows the Kathleen Dollard incremental-generator pattern:
SemanticModelReaderis the only place that touches Roslyn symbols and projects everything to Roslyn-free records;RegistrationWriteris a pureRegistrationModel → stringfunction and is exhaustively unit-tested without aCompilation. Diagnostics are carried through the pipeline asDiagnosticInfo+LocationInfo(value-equatable) and rebuilt at source-output time.Pipeline incrementality is verified, not just structural:
IncrementalCachingTestsdrivesCSharpGeneratorDriverwithtrackIncrementalGeneratorSteps: trueand asserts onIncrementalStepRunReason— trailing-comment edits and unrelated class additions yield onlyCached/Unchangedoutputs; adding a real handler yields exactly oneModifiedsource output containing the new handler.