-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Add factory delegate support for decorator registration #9
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
Implements Phase 1 of #3: Factory Delegate Support This change adds support for factory delegate registrations, allowing decorators to work with custom initialization logic while preserving the factory's behavior. **What's Added:** - Support for two-parameter generic factory: `AddScoped<TService, TImpl>(sp => ...)` - Support for single-parameter generic factory: `AddScoped<TService>(sp => ...)` - All lifetimes supported: Scoped, Transient, Singleton - Complex dependency resolution from IServiceProvider - Factory logic preserved while decorators wrap the result **Technical Changes:** - Added `RegistrationKind` enum to distinguish factory patterns - Extended `ClosedGenericRegistration` with optional `FactoryParameterName` - Updated `ClosedGenericRegistrationProvider` to detect factory signatures - Modified `InterceptorEmitter` to generate factory-specific interceptor code - Factory delegates registered as keyed services, then wrapped with decorators **Testing:** - Updated test case 022 (renamed to `FactoryDelegate_SingleDecorator`) - Added 6 comprehensive test cases (033-038) covering all scenarios - Updated sample project with factory delegate examples - All 38 tests passing on net8.0, net9.0, net10.0 **Documentation:** - Updated version to 1.0.2-beta - Added release notes in releasenotes.props - Updated README.md and CLAUDE.md with factory delegate examples - Added comprehensive documentation in docs/usage/class-level-decorators.md - Updated changelog and index.md Related to #3 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
@codex review |
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 implements Phase 1 of factory delegate support for DecoWeaver, enabling decorators to work with custom initialization logic via factory delegates. The implementation handles both two-parameter (AddScoped<TService, TImpl>(factory)) and single-parameter (AddScoped<TService>(factory)) generic factories across all DI lifetimes, while preserving user-defined factory logic and applying decorators around the result.
Key changes:
- Extended registration detection to identify and intercept factory delegate signatures
- Implemented three distinct interceptor generation patterns based on registration kind
- Factory delegates are registered as keyed services, then wrapped with decorator composition logic
Reviewed Changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
ClosedGenericRegistrationProvider.cs |
Extended provider to detect factory delegate signatures and classify registration kind |
InterceptorEmitter.cs |
Added factory-specific interceptor generation for two-parameter and single-parameter overloads |
TypeId.cs / TypeDefId.cs |
Refactored type identity code into dedicated files with extension methods |
| Test cases (033-038) | Added comprehensive test coverage for factory delegate scenarios |
| Sample project | Updated with factory delegate examples demonstrating complex dependency resolution |
| Documentation | Added factory delegate documentation and updated version to 1.0.2-beta |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…d add 1.0.1-beta notes - Changed condition syntax from exact match to StartsWith() to support build number suffixes - Added missing 1.0.1-beta release notes from changelog - Now supports versions like 1.0.2-beta.1, 1.0.2-beta.2, etc. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
j-d-ha
left a comment
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.
👍 Looks good! One question/suggestion.
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.
You need to start auto-generating this stuff 😄
|
|
||
| internal static class TypeDefIdExtensions | ||
| { | ||
| extension(TypeDefId typeDefId) |
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.
NICEEEEE
| internal static TypeDefId Create(ITypeSymbol t) | ||
| { | ||
| // For named types: collect containers + assembly | ||
| var assembly = t.ContainingAssembly?.Name ?? "unknown"; |
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.
This unknown bothers me a little. Will this cause issues if the assembly is undefined? I almost feel that this might be error-worthy, and then just catch the error and emit an error diagnostic.
Summary
Implements Phase 1 of #3: Factory Delegate Support
This PR adds support for factory delegate registrations, allowing decorators to work with custom initialization logic while preserving the factory's behavior.
What's Added
AddScoped<TService, TImpl>(sp => new Impl(...))AddScoped<TService>(sp => new Impl(...))AddScoped,AddTransient,AddSingletonIServiceProviderTechnical Changes
RegistrationKindenum (Parameterless, FactoryTwoTypeParams, FactorySingleTypeParam)ClosedGenericRegistrationmodel with optionalFactoryParameterNamefieldClosedGenericRegistrationProviderto detectFunc<IServiceProvider, T>signaturesInterceptorEmitterto generate three different interceptor types based on registration kindTesting
FactoryDelegate_SingleDecorator)AddTransientwith factoryAddSingletonwith factoryIServiceProviderresolutionDocumentation
releasenotes.propsREADME.mdandCLAUDE.mdwith factory delegate featuredocs/usage/class-level-decorators.mddocs/changelog.mdanddocs/index.mdExample Usage
Test Plan
Breaking Changes
None - this is a purely additive change. All existing functionality remains unchanged.
Related Issues
Related to #3 (Phase 1 complete)
🤖 Generated with Claude Code