feat: Add instance registration support for singleton decorators (Phase 3 of #3) #13

ncipollina · 2025-11-13T19:33:57Z

Summary

This PR completes issue #3 by implementing Phase 3: Instance Registration Support. DecoWeaver now supports decorating singleton instances registered with AddSingleton<TService>(instance).

Closes #3

Features

Instance registration support: Decorators work with AddSingleton<TService>(instance)
Automatic type extraction: Generator extracts actual implementation type from instance argument
Factory lambda wrapping: Instances wrapped in factory since keyed services don't have instance overloads
Singleton only: Only AddSingleton supported (Scoped/Transient don't have instance parameters in .NET DI)

Implementation

Core Changes

Added InstanceSingleTypeParam to RegistrationKind enum
Added InstanceParameterName field to ClosedGenericRegistration model
Updated ClosedGenericRegistrationProvider to detect instance registrations (non-delegate parameters)
Updated InterceptorEmitter to generate instance interceptors with factory lambda wrapping
Instance type extracted via SemanticModel.GetTypeInfo(instanceArg).Type

Key Technical Details

Instance Detection: Parameter type must match type parameter AND not be a Func<> delegate
Type Extraction: Uses Roslyn semantic model to get actual type from expression (e.g., new SqlRepository<Customer>())
Factory Wrapping: services.AddKeyedSingleton<T>(key, (sp, _) => capturedInstance)
ArgumentList Handling: Extension methods don't include this parameter, so instance is at args[0]

Testing

✅ 3 new test cases with snapshot verification (047-049)
- 047: Single type parameter with decorator
- 048: Multiple ordered decorators
- 049: No decorators (pass-through)
✅ Updated sample project with instance registration example
✅ All 48 existing tests still passing
✅ Build successful with no errors

Documentation

Created docs/advanced/instance-registrations.md with comprehensive guide
Updated CLAUDE.md with supported patterns section
Updated README.md feature list
Updated docs/changelog.md for v1.0.4-beta
Updated releasenotes.props with release notes
Version bumped to 1.0.4-beta

Examples

Basic Usage

[DecoratedBy<LoggingRepository<>>]
public class SqlRepository<T> : IRepository<T> { /* ... */ }

// Register pre-created instance
var instance = new SqlRepository<Customer>();
services.AddSingleton<IRepository<Customer>>(instance);

// Decorators are applied around the instance
var repo = serviceProvider.GetRequiredService<IRepository<Customer>>();
// Returns: LoggingRepository wrapping SqlRepository instance

Generated Code

// User code:
services.AddSingleton<IRepository<Customer>>(new SqlRepository<Customer>());

// DecoWeaver generates:
var key = DecoratorKeys.For(typeof(IRepository<Customer>), typeof(SqlRepository<Customer>));
var capturedInstance = (IRepository<Customer>)(object)implementationInstance;
services.AddKeyedSingleton<IRepository<Customer>>(key, (sp, _) => capturedInstance);

services.AddSingleton<IRepository<Customer>>(sp =>
{
    var current = sp.GetRequiredKeyedService<IRepository<Customer>>(key);
    current = (IRepository<Customer>)DecoratorFactory.Create(sp, typeof(IRepository<Customer>), typeof(LoggingRepository<>), current);
    return current;
});

Test Plan

All unit tests pass (48 tests)
Build succeeds with no errors
Sample project runs successfully with instance registration example
Generated code compiles without errors
Snapshot tests verified and accepted

Breaking Changes

None - this is purely additive functionality.

Related Issues

Completes Support factory delegates, keyed services, and instance registrations #3 (Phase 3: Instance Registration Support)
Builds on Phase 1 (feat: Add factory delegate support for decorator registration #9 - Factory Delegates) and Phase 2 (feat: Add keyed service support for decorator registration (Phase 2 of #3) #12 - Keyed Services)

🤖 Generated with Claude Code

…se 3 of #3) This completes issue #3 by adding support for singleton instance registrations. **New Features:** - Support for `AddSingleton<TService>(instance)` registrations - Decorators are applied around pre-created instances - Instance type extraction from argument expressions - Factory lambda wrapping for keyed service compatibility **Implementation Details:** - Added `InstanceSingleTypeParam` to `RegistrationKind` enum - Added `InstanceParameterName` field to model - Updated provider to detect non-delegate instance parameters - Extended emitter to generate instance interceptors - Instance wrapped in factory: `(sp, _) => capturedInstance` - Type extraction via `SemanticModel.GetTypeInfo(instanceArg).Type` **Limitations:** - Only `AddSingleton` supported (Scoped/Transient don't have instance overloads) - Instance must be created before registration **Testing:** - 3 new test cases (047-049) with snapshot verification - Updated sample project with instance registration example - All tests passing **Documentation:** - Created docs/advanced/instance-registrations.md - Updated CLAUDE.md with supported patterns - Updated README.md with feature list - Updated changelog and release notes - Version bumped to 1.0.4-beta 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

chatgpt-codex-connector

💡 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".

src/LayeredCraft.DecoWeaver.Generators/Emit/InterceptorEmitter.cs

ChatGPT identified that wrapping instances in factory lambdas broke .NET DI disposal semantics. The container would dispose user-provided instances when it shouldn't. **Root Cause:** Line 421 wrapped instances in factory lambda: `(sp, _) => capturedInstance` This changed disposal ownership - factory-created services are disposed by the container, but instance registrations should NOT be. **Fix:** Use direct instance overload: `AddKeyedSingleton<T>(key, instance)` This preserves expected .NET DI disposal behavior. **Changes:** - InterceptorEmitter.cs: Removed factory lambda wrapper (line 421) - docs/advanced/instance-registrations.md: Updated to reflect direct registration - CLAUDE.md: Updated generated code example - Test snapshots regenerated with correct code **Verification:** - Build succeeds - Sample project runs correctly - Disposal semantics now match .NET DI expectations Fixes disposal bug reported by ChatGPT in PR #13 feedback. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

Added Example 8 demonstrating keyed instance registration pattern. Note: This currently does NOT apply decorators because we don't yet intercept the AddKeyedSingleton<T>(key, instance) signature. This is a known limitation and potential future enhancement. The example shows correct usage and expected resolution pattern using GetRequiredKeyedService<T>(key). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

Extends instance registration support to handle keyed singleton instances. Users can now register pre-created instances with keys and have decorators applied automatically. Key changes: - Add KeyedInstanceSingleTypeParam to RegistrationKind enum - Update ClosedGenericRegistrationProvider to detect keyed instance pattern - Validates AddKeyedSingleton<T>(key, instance) signature - Extracts implementation type from instance argument (2nd parameter) - Add EmitKeyedInstanceSingleTypeParamInterceptor to InterceptorEmitter - Uses nested key strategy to avoid circular resolution - Registers instance with nested key, decorated factory with user key - Add test case 051_KeyedInstanceRegistration_SingleDecorator - Update instance registrations documentation with keyed examples Pattern supported: services.AddKeyedSingleton<IRepository<Customer>>("primary", instance); Generated code preserves disposal semantics by using direct instance registration instead of factory lambda wrapper. Fixes sample project Example 8 - now applies decorators to keyed instances. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

ncipollina · 2025-11-14T00:36:17Z

@codex review

Copilot

Pull Request Overview

This PR implements instance registration support for singleton decorators, completing Phase 3 of issue #3. DecoWeaver can now intercept and decorate pre-created instances registered with AddSingleton<TService>(instance) and AddKeyedSingleton<TService>(key, instance).

Key Changes:

Added instance registration detection by distinguishing non-delegate parameters from factory delegates
Implemented type extraction from instance arguments using Roslyn's semantic model
Generated interceptors that wrap instances in factory lambdas for keyed service registration

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`ClosedGenericRegistrationProvider.cs`	Added instance parameter detection, type extraction logic, and new registration kinds
`InterceptorEmitter.cs`	Added methods to emit instance interceptors with factory lambda wrapping
`DecoWeaverGeneratorTests.cs`	Added four new test cases covering instance registration scenarios
Test case files (047-051)	New test input files for instance registration scenarios
Generated snapshot files	Verified output for instance registration interceptors
Documentation files	Added comprehensive guide and updated changelog/release notes
Sample program	Added instance registration examples
Version files	Bumped version to 1.0.4-beta

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-14T00:37:43Z

...raft.DecoWeaver.Generator.Tests/Cases/047_InstanceRegistration_SingleTypeParam/Repository.cs

+    void Save(T entity);
+}
+
+[DecoratedBy(typeof(LoggingRepository<>))]


[nitpick] The DecoratedBy attribute uses typeof() syntax instead of the generic type parameter syntax shown in other examples. For consistency with the codebase style and C# 11+ syntax, consider using [DecoratedBy<LoggingRepository<>>].

Suggested change

[DecoratedBy(typeof(LoggingRepository<>))]

[DecoratedBy<LoggingRepository<>>]

Copilot · 2025-11-14T00:37:43Z

docs/changelog.md

 ### Added
 - No changes yet

+## [1.0.4-beta] - 2025-11-13


The date shows November 13, 2025, but the current date is November 2025. Verify this is the intended release date or update to the actual date when this version was released/will be released.

Suggested change

## [1.0.4-beta] - 2025-11-13

## [1.0.4-beta] - [Unreleased]

Copilot · 2025-11-14T00:37:43Z

releasenotes.props

+  - `AddSingleton<TService>(instance)` - Single type parameter with instance
+  - Decorators are applied around the provided instance
+  - Only `AddSingleton` is supported (instance registrations don't exist for Scoped/Transient in .NET DI)
+


The release notes mention 'Single type parameter with instance' but don't mention keyed instance registration support (AddKeyedSingleton), which is also implemented in this PR and shown in the test cases. Consider adding a bullet point for keyed instance registration.

Suggested change

* **Keyed instance registration support** - Decorators now work with keyed singleton instance registrations:

- `AddKeyedSingleton<TService>(key, instance)` - Register a singleton instance with a key

- Decorators are applied around the provided keyed instance

Copilot · 2025-11-14T00:37:44Z

src/LayeredCraft.DecoWeaver.Generators/Providers/ClosedGenericRegistrationProvider.cs

+            var svcTypeParam = symbolToCheck.TypeParameters.Length >= 1 ? symbolToCheck.TypeParameters[0] : null;
+            if (svcTypeParam == null || !SymbolEqualityComparer.Default.Equals(param2.Type, svcTypeParam))
+                return null;


[nitpick] The check symbolToCheck.TypeParameters.Length >= 1 could be simplified to symbolToCheck.TypeParameters.Length > 0 for clarity, or better yet, use symbolToCheck.TypeParameters.FirstOrDefault() to avoid the ternary operator.

Copilot · 2025-11-14T00:37:44Z

src/LayeredCraft.DecoWeaver.Generators/Providers/ClosedGenericRegistrationProvider.cs

+            var svcTypeParam = symbolToCheck.TypeParameters.Length >= 1 ? symbolToCheck.TypeParameters[0] : null;
+            if (svcTypeParam == null || !SymbolEqualityComparer.Default.Equals(param3.Type, svcTypeParam))
+                return null;


This code is duplicated from lines 154-156. Consider extracting this validation into a helper method like ValidateInstanceParameter(IParameterSymbol param, IMethodSymbol symbol) to reduce duplication.

chatgpt-codex-connector · 2025-11-14T00:42:31Z

Codex Review: Didn't find any major issues. Can't wait for the next one!

ℹ️ 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".

chatgpt-codex-connector bot reviewed Nov 13, 2025

View reviewed changes

src/LayeredCraft.DecoWeaver.Generators/Emit/InterceptorEmitter.cs Show resolved Hide resolved

ncipollina and others added 3 commits November 13, 2025 14:53

ncipollina requested review from Copilot and j-d-ha November 14, 2025 00:36

Copilot AI reviewed Nov 14, 2025

View reviewed changes

ncipollina merged commit 3273c36 into main Nov 14, 2025
3 checks passed

ncipollina deleted the issue-3-phase-3-instance-registrations branch November 14, 2025 00:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat: Add instance registration support for singleton decorators (Phase 3 of #3) #13

feat: Add instance registration support for singleton decorators (Phase 3 of #3) #13

Uh oh!

ncipollina commented Nov 13, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

Uh oh!

ncipollina commented Nov 14, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Uh oh!

chatgpt-codex-connector bot commented Nov 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	[DecoratedBy(typeof(LoggingRepository<>))]
	[DecoratedBy<LoggingRepository<>>]

+* **Keyed instance registration support** - Decorators now work with keyed singleton instance registrations:
+  - `AddKeyedSingleton<TService>(key, instance)` - Register a singleton instance with a key
+  - Decorators are applied around the provided keyed instance

Uh oh!

feat: Add instance registration support for singleton decorators (Phase 3 of #3) #13

feat: Add instance registration support for singleton decorators (Phase 3 of #3) #13

Uh oh!

Conversation

ncipollina commented Nov 13, 2025

Summary

Features

Implementation

Core Changes

Key Technical Details

Testing

Documentation

Examples

Basic Usage

Generated Code

Test Plan

Breaking Changes

Related Issues

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

ncipollina commented Nov 14, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chatgpt-codex-connector bot commented Nov 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants