feat: Add support for pure open-generic handler registration#405
Merged
Conversation
samtrion
added
state:ready for merge
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 20 minutes and 49 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (39)
WalkthroughAdds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/NetEvolve.Pulse.SourceGeneration/Generators/PulseHandlerGenerator.cs (1)
423-444:⚠️ Potential issue | 🟠 MajorDon't suppress diagnostics for closed types annotated with
[PulseGenericHandler].
ExtractPureGenericHandlerInforeturnsnullfor non-generic classes at Lines 363-365. With this unconditional skip, a closed handler annotated by mistake with[PulseGenericHandler]is neither registered nor diagnosed. Please only treat[PulseGenericHandler]as "handled" when the target is actually open generic, or emit a dedicated invalid-usage diagnostic.Suggested fix
- || string.Equals( - GetFullMetadataName(attr.AttributeClass), - PulseGenericHandlerAttributeFullName, - StringComparison.Ordinal - ) + || ( + classSymbol.TypeParameters.Length > 0 + && string.Equals( + GetFullMetadataName(attr.AttributeClass), + PulseGenericHandlerAttributeFullName, + StringComparison.Ordinal + ) + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/NetEvolve.Pulse.SourceGeneration/Generators/PulseHandlerGenerator.cs` around lines 423 - 444, The skip logic in PulseHandlerGenerator.cs currently treats any class annotated with PulseGenericHandlerAttributeFullName as "handled" even for closed/non-generic classes, which hides misuse; update the attribute-check in the classSymbol.GetAttributes().Any(...) branch so that PulseGenericHandlerAttributeFullName only causes a skip if ExtractPureGenericHandlerInfo(classSymbol) returns non-null (i.e., the class is an open generic), and if ExtractPureGenericHandlerInfo returns null instead emit the dedicated invalid-usage diagnostic (use the existing diagnostic reporting flow used elsewhere in PulseHandlerGenerator), while leaving the existing handling for PulseHandlerAttributeFullName and PulseHandlerGenericAttributeFullName unchanged.
🧹 Nitpick comments (1)
tests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/PulseHandlerGeneratorTests.cs (1)
1140-1228: Consider adding pure-generic query/stream-query coverage for parity.You now validate pure generic command/event paths; adding
IQueryHandler<TQuery, TResult>andIStreamQueryHandler<TQuery, TResult>pure-generic cases would close the remaining handler-kind gap and reduce regression risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/PulseHandlerGeneratorTests.cs` around lines 1140 - 1228, Add unit tests mirroring the existing pure-generic command/event cases to cover query and stream-query handlers: create tests like WhenPureGenericQueryHandlerAnnotatedThenOpenGenericRegistrationIsGenerated and WhenPureGenericStreamQueryHandlerAnnotatedThenOpenGenericRegistrationIsGenerated that use source snippets defining GenericQueryHandler<TQuery, TResult> : IQueryHandler<TQuery, TResult> and GenericStreamQueryHandler<TQuery, TResult> : IStreamQueryHandler<TQuery, TResult> (with and without [PulseGenericHandler], and a variant using [PulseGenericHandler(Lifetime = PulseServiceLifetime.Singleton)]), then call RunGenerator(...) and Await VerifySources(...) just like the existing tests for GenericCommandHandler and GenericEventHandler to ensure parity and close the handler-kind gap.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/NetEvolve.Pulse.SourceGeneration/Generators/PulseHandlerGenerator.cs`:
- Around line 1015-1037: BuildOpenGenericHandlerRegistrations is currently
emitting open-generic DI registrations for every original interface without
checking that the handler's type parameters align exactly with the interface's
type arguments; add an IsPureOpenGenericMapping(INamedTypeSymbol handler,
INamedTypeSymbol iface) helper that returns true only when
handler.TypeParameters.Length == iface.TypeArguments.Length and each
iface.TypeArguments[i] is the corresponding handler.TypeParameters[i] (use
SymbolEqualityComparer.Default for comparison), then call this helper before
adding a HandlerRegistration (when using GetOpenGenericTypeName,
TryGetHandlerKind and creating HandlerRegistration) and skip/ignore interfaces
that do not pass the check so mismatched arity/position cases are not emitted.
In
`@tests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerWithSingletonLifetimeThenSingletonOpenGenericRegistrationIsGenerated.verified.txt`:
- Around line 13-21: The XML documentation generated for
PulseRegistrationExtensions incorrectly references only [PulseHandler]; update
the generator template producing the summary text (used in the GeneratedCode
class PulseRegistrationExtensions and the method-level summary) to either
mention both attributes ([PulseHandler] and [PulseGenericHandler]) or use
neutral wording like "Pulse handler attributes" or "Pulse handlers" so the docs
accurately reflect handlers created from PulseGenericHandler as well.
---
Outside diff comments:
In `@src/NetEvolve.Pulse.SourceGeneration/Generators/PulseHandlerGenerator.cs`:
- Around line 423-444: The skip logic in PulseHandlerGenerator.cs currently
treats any class annotated with PulseGenericHandlerAttributeFullName as
"handled" even for closed/non-generic classes, which hides misuse; update the
attribute-check in the classSymbol.GetAttributes().Any(...) branch so that
PulseGenericHandlerAttributeFullName only causes a skip if
ExtractPureGenericHandlerInfo(classSymbol) returns non-null (i.e., the class is
an open generic), and if ExtractPureGenericHandlerInfo returns null instead emit
the dedicated invalid-usage diagnostic (use the existing diagnostic reporting
flow used elsewhere in PulseHandlerGenerator), while leaving the existing
handling for PulseHandlerAttributeFullName and
PulseHandlerGenericAttributeFullName unchanged.
---
Nitpick comments:
In
`@tests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/PulseHandlerGeneratorTests.cs`:
- Around line 1140-1228: Add unit tests mirroring the existing pure-generic
command/event cases to cover query and stream-query handlers: create tests like
WhenPureGenericQueryHandlerAnnotatedThenOpenGenericRegistrationIsGenerated and
WhenPureGenericStreamQueryHandlerAnnotatedThenOpenGenericRegistrationIsGenerated
that use source snippets defining GenericQueryHandler<TQuery, TResult> :
IQueryHandler<TQuery, TResult> and GenericStreamQueryHandler<TQuery, TResult> :
IStreamQueryHandler<TQuery, TResult> (with and without [PulseGenericHandler],
and a variant using [PulseGenericHandler(Lifetime =
PulseServiceLifetime.Singleton)]), then call RunGenerator(...) and Await
VerifySources(...) just like the existing tests for GenericCommandHandler and
GenericEventHandler to ensure parity and close the handler-kind gap.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dd4abef5-3ba1-4095-a4bb-7368ac990dd7
📒 Files selected for processing (11)
src/NetEvolve.Pulse.Extensibility/Attributes/PulseGenericHandlerAttribute.cssrc/NetEvolve.Pulse.Extensibility/README.mdsrc/NetEvolve.Pulse.SourceGeneration/Generators/PulseHandlerGenerator.cssrc/NetEvolve.Pulse.SourceGeneration/Models/HandlerRegistration.cssrc/NetEvolve.Pulse.SourceGeneration/README.mdsrc/NetEvolve.Pulse.SourceGeneration/WellKnownTypeNames.cstests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/PulseHandlerGeneratorTests.cstests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericEventHandlerAnnotatedThenOpenGenericRegistrationIsGenerated.verified.txttests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerAnnotatedThenOpenGenericRegistrationIsGenerated.verified.txttests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerNotAnnotatedThenNoPulse003.verified.txttests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerWithSingletonLifetimeThenSingletonOpenGenericRegistrationIsGenerated.verified.txt
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #405 +/- ##
==========================================
- Coverage 91.98% 91.76% -0.23%
==========================================
Files 139 140 +1
Lines 5266 5330 +64
Branches 484 498 +14
==========================================
+ Hits 4844 4891 +47
- Misses 282 294 +12
- Partials 140 145 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
samtrion
force-pushed
the
feature/sourcegenerator-open-generic-handlers
branch
from
April 16, 2026 11:48
cb4d0ec to
8819321
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerWithSingletonLifetimeThenSingletonOpenGenericRegistrationIsGenerated.verified.txt (1)
13-21:⚠️ Potential issue | 🟡 MinorXML docs still reference the wrong attribute for this scenario.
Line 14 and Line 20 still mention only
<c>[PulseHandler]</c>, but this snapshot is produced from[PulseGenericHandler]. Please fix the generator template wording to mention both attributes or use neutral wording (e.g., “Pulse handlers”).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerWithSingletonLifetimeThenSingletonOpenGenericRegistrationIsGenerated.verified.txt` around lines 13 - 21, The XML documentation in the generated PulseRegistrationExtensions class incorrectly references only "<c>[PulseHandler]</c>" even when generation was driven by "[PulseGenericHandler]"; update the source generator template that emits the summary (the text around the GeneratedCode("NetEvolve.Pulse.SourceGeneration", "{version}") and the class-level docs for PulseRegistrationExtensions) to either mention both attributes (e.g., "<c>[PulseHandler]</c> and <c>[PulseGenericHandler]</c>") or use neutral wording like "Pulse handlers" so the docs match all originating attributes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@tests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerWithSingletonLifetimeThenSingletonOpenGenericRegistrationIsGenerated.verified.txt`:
- Around line 13-21: The XML documentation in the generated
PulseRegistrationExtensions class incorrectly references only
"<c>[PulseHandler]</c>" even when generation was driven by
"[PulseGenericHandler]"; update the source generator template that emits the
summary (the text around the GeneratedCode("NetEvolve.Pulse.SourceGeneration",
"{version}") and the class-level docs for PulseRegistrationExtensions) to either
mention both attributes (e.g., "<c>[PulseHandler]</c> and
<c>[PulseGenericHandler]</c>") or use neutral wording like "Pulse handlers" so
the docs match all originating attributes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 830633d7-9d4b-49d6-a665-d6b395469904
📒 Files selected for processing (11)
src/NetEvolve.Pulse.Extensibility/Attributes/PulseGenericHandlerAttribute.cssrc/NetEvolve.Pulse.Extensibility/README.mdsrc/NetEvolve.Pulse.SourceGeneration/Generators/PulseHandlerGenerator.cssrc/NetEvolve.Pulse.SourceGeneration/Models/HandlerRegistration.cssrc/NetEvolve.Pulse.SourceGeneration/README.mdsrc/NetEvolve.Pulse.SourceGeneration/WellKnownTypeNames.cstests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/PulseHandlerGeneratorTests.cstests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericEventHandlerAnnotatedThenOpenGenericRegistrationIsGenerated.verified.txttests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerAnnotatedThenOpenGenericRegistrationIsGenerated.verified.txttests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerNotAnnotatedThenNoPulse003.verified.txttests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerWithSingletonLifetimeThenSingletonOpenGenericRegistrationIsGenerated.verified.txt
✅ Files skipped from review due to trivial changes (6)
- tests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerNotAnnotatedThenNoPulse003.verified.txt
- src/NetEvolve.Pulse.SourceGeneration/WellKnownTypeNames.cs
- tests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/PulseHandlerGeneratorTests.cs
- src/NetEvolve.Pulse.Extensibility/Attributes/PulseGenericHandlerAttribute.cs
- src/NetEvolve.Pulse.SourceGeneration/README.md
- src/NetEvolve.Pulse.Extensibility/README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/NetEvolve.Pulse.SourceGeneration/Models/HandlerRegistration.cs
- src/NetEvolve.Pulse.SourceGeneration/Generators/PulseHandlerGenerator.cs
Introduce [PulseGenericHandler] for open-generic DI registration via typeof()-based syntax. Update generator logic, HandlerRegistration, and WellKnownTypeNames to support and track open-generic handlers. Extend documentation and tests to cover new attribute usage and ensure correct diagnostics. Add PulseGenericHandlerAttribute.cs.
samtrion
force-pushed
the
feature/sourcegenerator-open-generic-handlers
branch
from
April 16, 2026 12:53
8819321 to
10aef38
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerAnnotatedThenOpenGenericRegistrationIsGenerated.verified.txt (1)
13-15:⚠️ Potential issue | 🟡 MinorGenerated XML docs still exclude
[PulseGenericHandler]scenarios.Line 14 and Line 20 mention only
<c>[PulseHandler]</c>, but this snapshot is produced from[PulseGenericHandler]. Update the generator template wording (neutral “Pulse handlers” or both attributes) and regenerate snapshots.Suggested wording update
-/// Auto-generated extension method to register Pulse handlers annotated -/// with <c>[PulseHandler]</c> inside the Assembly <c>TestAssembly</c>. +/// Auto-generated extension method to register Pulse handlers discovered +/// inside the Assembly <c>TestAssembly</c>. ... -/// Registers all <c>[PulseHandler]</c>-annotated handlers from the assembly <c>TestAssembly</c> into the DI container. +/// Registers all discovered Pulse handlers from the assembly <c>TestAssembly</c> into the DI container.Also applies to: 20-21
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerAnnotatedThenOpenGenericRegistrationIsGenerated.verified.txt` around lines 13 - 15, The XML doc template used to generate the snapshot incorrectly references only <c>[PulseHandler]</c> even when the test (PulseHandlerGeneratorTests.WhenPureGenericHandlerAnnotatedThenOpenGenericRegistrationIsGenerated) is driven by <c>[PulseGenericHandler]</c>; update the generator template text to use neutral wording like "Pulse handlers" or explicitly include both <c>[PulseHandler]</c> and <c>[PulseGenericHandler]</c> (look for the generator/template method that emits the summary for handler registration), then regenerate the snapshots so the verified snapshot _snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerAnnotatedThenOpenGenericRegistrationIsGenerated.verified.txt reflects the corrected wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@tests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerAnnotatedThenOpenGenericRegistrationIsGenerated.verified.txt`:
- Around line 13-15: The XML doc template used to generate the snapshot
incorrectly references only <c>[PulseHandler]</c> even when the test
(PulseHandlerGeneratorTests.WhenPureGenericHandlerAnnotatedThenOpenGenericRegistrationIsGenerated)
is driven by <c>[PulseGenericHandler]</c>; update the generator template text to
use neutral wording like "Pulse handlers" or explicitly include both
<c>[PulseHandler]</c> and <c>[PulseGenericHandler]</c> (look for the
generator/template method that emits the summary for handler registration), then
regenerate the snapshots so the verified snapshot
_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerAnnotatedThenOpenGenericRegistrationIsGenerated.verified.txt
reflects the corrected wording.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ceb2bdc4-c52d-4886-92f0-cb587173e1dc
📒 Files selected for processing (11)
src/NetEvolve.Pulse.Extensibility/Attributes/PulseGenericHandlerAttribute.cssrc/NetEvolve.Pulse.Extensibility/README.mdsrc/NetEvolve.Pulse.SourceGeneration/Generators/PulseHandlerGenerator.cssrc/NetEvolve.Pulse.SourceGeneration/Models/HandlerRegistration.cssrc/NetEvolve.Pulse.SourceGeneration/README.mdsrc/NetEvolve.Pulse.SourceGeneration/WellKnownTypeNames.cstests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/PulseHandlerGeneratorTests.cstests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericEventHandlerAnnotatedThenOpenGenericRegistrationIsGenerated.verified.txttests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerAnnotatedThenOpenGenericRegistrationIsGenerated.verified.txttests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerNotAnnotatedThenNoPulse003.verified.txttests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerWithSingletonLifetimeThenSingletonOpenGenericRegistrationIsGenerated.verified.txt
✅ Files skipped from review due to trivial changes (6)
- src/NetEvolve.Pulse.SourceGeneration/WellKnownTypeNames.cs
- tests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/_snapshots/PulseHandlerGeneratorTests.WhenPureGenericHandlerNotAnnotatedThenNoPulse003.verified.txt
- src/NetEvolve.Pulse.Extensibility/README.md
- src/NetEvolve.Pulse.SourceGeneration/README.md
- src/NetEvolve.Pulse.Extensibility/Attributes/PulseGenericHandlerAttribute.cs
- tests/NetEvolve.Pulse.SourceGeneration.Tests.Unit/PulseHandlerGeneratorTests.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/NetEvolve.Pulse.SourceGeneration/Generators/PulseHandlerGenerator.cs
Update PulseHandler generator to better support [PulseGenericHandler] for open generics. Only skip [PulseHandler] and [PulseHandler<T>] for closed types, ensuring correct diagnostics for [PulseGenericHandler] misuse. Add IsPureOpenGenericMapping to restrict open generic registrations to pure positional mappings. Update generated docs to mention both handler attributes. Add and update tests and verified outputs for new registration logic.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Introduce [PulseGenericHandler] for open-generic DI registration via typeof()-based syntax. Update generator logic, HandlerRegistration, and WellKnownTypeNames to support and track open-generic handlers. Extend documentation and tests to cover new attribute usage and ensure correct diagnostics. Add PulseGenericHandlerAttribute.cs.
Summary by CodeRabbit
New Features
Documentation
Tests
Bug Fixes