feat: Assembly-level decorator support (Phase 2a of #2) #7
Conversation
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
…artial #2) This commit implements the foundational infrastructure for assembly-level decorator declarations, enabling decorators to be applied to all implementations of a service type within an assembly. New Attributes: - DecorateServiceAttribute: Assembly-level decorator declaration - SkipAssemblyDecorationAttribute: Opt out of all assembly-level decorations - DoNotDecorateAttribute: Surgically remove specific decorators Generator Changes: - ServiceDecoratedByProvider: Discovers [assembly: DecorateService] attributes - SkipAssemblyDecoratorProvider: Discovers [SkipAssemblyDecorators] markers - Updated pipeline to merge class-level and assembly-level decorations - Added filtering logic to respect opt-out mechanisms Test Coverage: - Test case 023: Single assembly-level decorator - Test case 024: Multiple ordered assembly-level decorators - Test case 025: SkipAssemblyDecorators opt-out behavior Documentation: - Added XML documentation to all new attributes - Added inline comments explaining merge logic and filtering - Enhanced IsInterceptable property documentation Note: This is Phase 2a implementation. Still TODO: - DoNotDecorate provider and merge logic - Service type matching (currently uses implementation type) - Full precedence/deduplication rules from issue #2 All tests passing. Build succeeds with no errors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…evel decorators This test explicitly verifies that [SkipAssemblyDecorators] correctly: - Skips assembly-level decorators (CachingRepository<>) - Still applies class-level decorators (ValidationRepository<>) Test Setup: - Assembly-level: [assembly: DecorateService(typeof(IRepository<>), typeof(CachingRepository<>))] - Class-level: [SkipAssemblyDecorators] + [DecoratedBy(typeof(ValidationRepository<>))] Expected Behavior: The generated interceptor applies ONLY ValidationRepository<User>, proving that class-level decorators are unaffected by the assembly-level opt-out. This test documents and validates the behavior described in the SkipAssemblyDecorationAttribute XML documentation. All 26 tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
j-d-ha
reviewed
Nov 7, 2025
Collaborator
j-d-ha
left a comment
j-d-ha
left a comment
There was a problem hiding this comment.
Looks good! 1 suggestion.
src/LayeredCraft.DecoWeaver.Generators/Providers/DecoratedByNonGenericProvider.cs
Show resolved
Hide resolved
src/LayeredCraft.DecoWeaver.Generators/Providers/SkipAssemblyDecoratorProvider.cs
Show resolved
Hide resolved
...ptor_sut=DecoWeaver.DecoWeaverGenerator#DecoWeaver.Interceptors.ClosedGenerics.g.verified.cs
Outdated
Show resolved
Hide resolved
This commit fixes a critical bug in the ServiceDecoratedByProvider where assembly-level decorators were being matched by implementation type instead of service type, contradicting the XML documentation and intended design. Changes: - ServiceDecoratedByProvider now returns ServiceDecoration (not DecoratorToIntercept) - ServiceDecoration properly stores ServiceDef (the service type to match against) - Updated generator pipeline to match ServiceDecorations against registrations by service type - Implemented proper conversion: ServiceDecoration → DecoratorToIntercept during merge phase - Added Attr_Service_Collected tracking name constant Architecture Improvements: - Created clean named tuple structure in pipeline to avoid deeply nested Left.Left.Left patterns - ServiceDecorations now correctly matched where reg.ServiceDef equals sd.ServiceDef - Assembly name validation ensures decorators only apply within their declaring assembly Test Fixes: - Updated test cases 023-025 to use service type (IRepository<>) instead of implementation type - This aligns test cases with the correct semantic meaning of DecorateServiceAttribute - All 26 tests pass Breaking Change Note: This fixes the semantic meaning of DecorateServiceAttribute's first parameter. It should always be the SERVICE type (interface/base class), not the implementation type. Example: Before (incorrect): [assembly: DecorateService(typeof(DynamoDbRepository<>), ...)] After (correct): [assembly: DecorateService(typeof(IRepository<>), ...)] All tests passing. Build succeeds. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…ecorators Implements Priority 2 of issue #2: Full merge and precedence logic for combining class-level and assembly-level decorators with proper deduplication and ordering. Changes: - Refactored BuildDecorationMap to accept separate class/assembly decorator lists - Implemented deduplication where class-level decorators win over assembly-level for the same decorator type - Implemented three-tier sorting: 1. Order (ascending) - primary sort 2. Source (ascending) - Class=0 before Assembly=1 at same order 3. FQN (fully qualified name) - stable tiebreaker - Added GetSortableTypeName helper for stable FQN generation - Uses MergedDecoration model internally for tracking Source metadata - Returns Dictionary<TypeDefId, EquatableArray<TypeDefId>> to avoid changes to InterceptorEmitter signature Test Coverage: - Case 027: Verifies deduplication (class-level CachingRepository@20 wins over assembly-level CachingRepository@10) - Case 028: Verifies sort order (class-level Validation@10 comes before assembly-level Logging@10) - All 28 tests passing Technical Details: - Merge logic kept in generator, not in emitter - Placeholder added for DoNotDecorateDirective filtering (Priority 3) - IsInterceptable remains hardcoded to true for assembly-level decorators - No emission when no decorators present (existing behavior preserved) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Implements Priority 3 of issue #2: DoNotDecorate attribute support. This commit adds the [DoNotDecorate(typeof(TDecorator))] attribute that allows classes to opt out of specific decorators (typically assembly-level decorators) without affecting other implementations. Changes: - Add DoNotDecorateProvider to discover [DoNotDecorate] attributes - Add DoNotDecorateDirective model with ImplementationDef and DecoratorDef - Add DoNotDecorate stream to generator pipeline with tracking - Update BuildDecorationMap to filter decorators based on directives - Support open generic decorator types in DoNotDecorate (matches all closed variants) - Add 4 comprehensive test cases (029-032) covering various scenarios Test coverage: - Test 029: Remove assembly-level decorator from specific implementation - Test 030: Multiple DoNotDecorate attributes on same class - Test 031: Open generic matching (typeof(Decorator<>) excludes all closed generics) - Test 032: Isolation check (DoNotDecorate on one impl doesn't affect others) All 32 tests passing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
Pull Request Overview
This PR implements Phase 2a of assembly-level decorator support (#2), introducing the foundational infrastructure for declaring decorators at the assembly level that apply to service implementations. The implementation includes new attributes, provider infrastructure for attribute discovery, and merge logic with opt-out mechanisms. Phase 2b (full merge precedence, service type matching, and complete diagnostic support) remains as future work.
Reviewed Changes
Copilot reviewed 53 out of 53 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| DecoratedByAttribute.cs | Adds three new attributes: DecorateServiceAttribute for assembly-level decoration, SkipAssemblyDecorationAttribute for opt-out, and DoNotDecorateAttribute for selective exclusion |
| DecoWeaverGenerator.cs | Updates generator pipeline to discover assembly-level decorations, merge them with class-level decorations, and apply filtering logic based on opt-out mechanisms |
| ServiceDecoratedByProvider.cs | New provider that discovers [assembly: DecorateService] attributes |
| SkipAssemblyDecoratorProvider.cs | New provider that discovers [SkipAssemblyDecorators] markers on implementation classes |
| DoNotDecorateProvider.cs | New provider that discovers [DoNotDecorate] attributes for surgical decorator removal |
| DecoratorToIntercept.cs | Adds new model types: ServiceDecoration, SkipAssemblyDecoratorsMarker, DoNotDecorateDirective, DecorationSource, and MergedDecoration |
| Test cases (023-032) | Nine new test cases covering single/multiple assembly decorators, skip behavior, merge precedence, deduplication, and isolation |
| TrackingNames.cs | Adds tracking names for new pipeline stages |
| AttributeNames.cs | Adds metadata names for the three new attributes |
| RoslynAdapters.cs | Removes unused ToLocationId method |
| DecoratedByGenericProvider.cs & DecoratedByNonGenericProvider.cs | Updates to use list pattern matching and removes Location field |
| Repository.cs (sample) | Adds sample types demonstrating assembly-level decoration |
| Globals.cs (sample) | Adds assembly-level decoration declaration for sample |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 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".
test/LayeredCraft.DecoWeaver.Generator.Tests/DecoWeaverGeneratorTests.cs
Show resolved
Hide resolved
Updates documentation to reflect new 1.0.1-beta features including assembly-level decorators and opt-out mechanisms. Changes: - Add new assembly-level-decorators.md usage guide (comprehensive) - Update opt-out.md from placeholder to full DoNotDecorate documentation - Add DecorateServiceAttribute and DoNotDecorateAttribute to API reference - Update changelog.md with 1.0.1-beta release notes - Add cross-references between class-level and assembly-level docs - Update index.md and README.md with new features - Add assembly-level section to quick-start.md - Update mkdocs.yml navigation with new page - Add new doc file to LayeredCraft.DecoWeaver.slnx for IDE visibility Documentation covers: - When to use assembly-level vs class-level decorators - Syntax and usage examples for both attributes - Precedence and merging rules - Opt-out scenarios and best practices - Open generic matching behavior - Troubleshooting and anti-patterns 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Adds complete documentation for the SkipAssemblyDecorationAttribute which was missing from the initial documentation update. Changes: - Add SkipAssemblyDecoration section to opt-out.md with examples and comparison - Add opt-out section to assembly-level-decorators.md covering both attributes - Add SkipAssemblyDecorationAttribute API reference with complete details - Update changelog to mention SkipAssemblyDecoration in features list - Update technical details to include SkipAssemblyDecoratorProvider - Include version bump to 1.0.1-beta in Directory.Build.props Documentation covers: - Difference between SkipAssemblyDecoration (all) vs DoNotDecorate (specific) - When to use each opt-out mechanism - Decision tree for choosing the right approach - Examples of combined usage with class-level decorators - Best practices and use cases 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Collaborator
|
LGTM! |
- Change assembly-level decorator from DynamoDbRepository<> to IRepository<> for clearer demonstration - Fix typo: ConcreateClass -> ConcreteClass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…bal:: prefix Changed ClosedGenericRegistrationProvider to use SymbolDisplayFormat.FullyQualifiedFormat directly instead of manually prepending global:: and using WithGlobalNamespaceStyle(Omitted). This ensures ALL types including nested generic type arguments get the global:: prefix, preventing potential type resolution ambiguity. Before: global::IRepository<Customer> After: global::IRepository<global::Customer> Updated all test snapshots to reflect the corrected output format. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 76 out of 76 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
j-d-ha
reviewed
Nov 10, 2025
src/LayeredCraft.DecoWeaver.Generators/Providers/ClosedGenericRegistrationProvider.cs
Show resolved
Hide resolved
...ptor_sut=DecoWeaver.DecoWeaverGenerator#DecoWeaver.Interceptors.ClosedGenerics.g.verified.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 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/Providers/ServiceDecoratedByProvider.cs
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
This PR implements the foundational infrastructure for assembly-level decorator declarations as described in issue #2. This is Phase 2a - the first part of the full assembly-level decorator feature.
What's Implemented
New Attributes
DecorateServiceAttribute: Declares decorators at assembly level that apply to all implementations of a service typeSkipAssemblyDecorationAttribute: Opts out an implementation from all assembly-level decorationsDoNotDecorateAttribute: Surgically removes specific decorators from the merged set (attribute defined, provider TODO)Generator Infrastructure
ServiceDecoratedByProvider: Discovers[assembly: DecorateService]attributesSkipAssemblyDecoratorProvider: Discovers[SkipAssemblyDecorators]markersTest Coverage
[SkipAssemblyDecorators]opt-out behaviorWhat's Still TODO (Phase 2b)
Per issue #2, these items remain to complete the full assembly-level feature:
DoNotDecorateAttributeprovider and filtering logicServiceDecorationandMergedDecorationmodel typesDoNotDecoratetargets non-existent decoratorWhy Split into 2a/2b?
This PR represents functional progress that can be reviewed and merged independently. The core infrastructure is in place and tested. The remaining work (Phase 2b) involves more complex merge logic and will be easier to review separately.
Test Plan
Breaking Changes
None - this is purely additive functionality.
🤖 Generated with Claude Code