-
-
Notifications
You must be signed in to change notification settings - Fork 4
Add support for non-static Handle method
#223
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
Conversation
e175fc3 to
782dd69
Compare
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.
Pull Request Overview
This PR adds support for non-static Handle method implementations to the Immediate.Handlers library. The library previously only supported static handler methods, but now allows instance methods that can utilize dependency injection through constructor parameters.
Key changes:
- Extended the generator to detect and process both static and non-static handler methods
- Updated template generation to handle instance-based handlers with constructor injection
- Added comprehensive test coverage for the new instance method functionality
Reviewed Changes
Copilot reviewed 24 out of 28 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Immediate.Handlers.Generators/TransformHandler.cs | Core generator logic updated to support both static and instance handler methods |
| src/Immediate.Handlers.Generators/Templates/Handler.sbntxt | Template modified to generate appropriate code for static vs instance handlers |
| src/Immediate.Handlers.Generators/Models.cs | Added IsStatic property to track handler method type |
| tests/Immediate.Handlers.Tests/GeneratorTests/HandlerTests.cs | Updated existing tests to cover both static and instance scenarios |
| tests/Immediate.Handlers.Tests/GeneratorTests/InvalidHandlerTests.cs | Added test for invalid instance handler with too many parameters |
| benchmarks/Benchmark.*/Benchmark.cs | Updated benchmarks to compare static vs instance handler performance |
Comments suppressed due to low confidence (3)
src/Immediate.Handlers.Generators/TransformHandler.cs:26
- [nitpick] The variable name 'handleMethod' could be more descriptive. Consider renaming to 'candidateHandleMethod' or 'foundHandleMethod' to better reflect that this may be null.
var handleMethod = symbol.GetHandleMethod();
src/Immediate.Handlers.Generators/TransformHandler.cs:161
- [nitpick] The lambda parameter 'ims' is unclear. Consider using a more descriptive name like 'method' or 'methodSymbol'.
_ = candidates.RemoveAll(ims => !ims.IsStatic);
src/Immediate.Handlers.Generators/ImmediateHandlersGenerator.cs:126
- [nitpick] The variable name 'so' is unclear. Consider using a more descriptive name like 'scriptObject' or 'templateData'.
var so = new ScriptObject(StringComparer.Ordinal);
Codecov ReportAll modified and coverable lines are covered by tests ✅
🚀 New features to boost your workflow:
|
Fixes #224
Fixes #225
Fixes #226