feat: Add Decorator pattern source generator #91
+2,092
−0
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
Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add source generator for decorator pattern implementations
feat: Add Decorator pattern source generator
Jan 22, 2026
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
Test Results158 tests 158 ✅ 56s ⏱️ Results for commit 7047b99. |
Contributor
Code Coverage |
Contributor
🔍 PR Validation ResultsVersion: `` ✅ Validation Steps
📊 ArtifactsDry-run artifacts have been uploaded and will be available for 7 days. This comment was automatically generated by the PR validation workflow. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #91 +/- ##
==========================================
+ Coverage 85.83% 89.15% +3.31%
==========================================
Files 143 146 +3
Lines 12288 12739 +451
Branches 1641 1697 +56
==========================================
+ Hits 10548 11358 +810
- Misses 1329 1381 +52
+ Partials 411 0 -411
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a new Decorator-pattern source generator to produce base decorator implementations (plus optional composition helpers) for interfaces and abstract classes, backed by new attributes, tests, and an example.
Changes:
- Introduces
[GenerateDecorator],[DecoratorIgnore], andDecoratorCompositionModein the abstractions package. - Adds
DecoratorGeneratorto emit forwarding base decorator types and (optionally) aComposehelper. - Adds a new test suite and an examples/demo file showcasing generated decorators.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
src/PatternKit.Generators/DecoratorGenerator.cs |
Implements the incremental source generator and code emission for decorators/helpers. |
src/PatternKit.Generators.Abstractions/Decorator/GenerateDecoratorAttribute.cs |
Adds public generator surface area (attribute + enum) to configure decorator generation. |
src/PatternKit.Generators.Abstractions/Decorator/DecoratorIgnoreAttribute.cs |
Adds attribute for per-member generation behavior control. |
src/PatternKit.Generators/AnalyzerReleases.Unshipped.md |
Registers new diagnostic IDs for the decorator generator. |
test/PatternKit.Generators.Tests/DecoratorGeneratorTests.cs |
Adds compilation-based tests for generated output. |
src/PatternKit.Examples/Decorators/StorageDecoratorExample.cs |
Adds a real-world example demonstrating decorator chaining and usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/PatternKit.Generators.Abstractions/Decorator/DecoratorIgnoreAttribute.cs
Outdated
Show resolved
Hide resolved
src/PatternKit.Generators.Abstractions/Decorator/GenerateDecoratorAttribute.cs
Outdated
Show resolved
Hide resolved
Owner
|
@copilot apply changes based on the comments in this thread |
…rwarding, and add comprehensive tests Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Owner
|
@copilot apply changes based on the comments in this thread |
… setters, signature deduplication Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
…fix protected member access Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
…m lookup, fix indentation Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Skip special methods (constructors, operators, property accessors, etc.) | ||
| if (method.MethodKind != MethodKind.Ordinary) | ||
| continue; | ||
|
|
There was a problem hiding this comment.
Static members aren’t filtered out. If a contract interface contains static members (including C# 11 static abstract interface members) or static properties, the generator will try to emit instance forwarding members, which won’t compile. Consider detecting IsStatic for methods/properties and reporting PKDEC002 instead of generating invalid code.
Suggested change
| // Skip static methods; decorators only forward instance members | |
| if (method.IsStatic) | |
| continue; |
Contributor
Author
There was a problem hiding this comment.
Fixed in 7783db2. Static methods now filtered out and ignored silently. Added test to verify static members are not included in generated code.
Comment on lines
+269
to
+273
| var isAsync = IsAsyncMethod(method); | ||
| var returnType = method.ReturnType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); | ||
|
|
||
| members.Add(new MemberInfo | ||
| { |
There was a problem hiding this comment.
Generic methods aren’t handled: the generator records only method.Name and parameters, but never emits method type parameters (<T>) or constraints. Any contract with a generic method (even if the contract type itself is non-generic) will result in uncompilable generated code. Either implement generic method emission or emit PKDEC002 and skip these members.
Contributor
Author
There was a problem hiding this comment.
Fixed in 7783db2. Generic methods now emit PKDEC002 diagnostic. Added test for generic method detection.
Comment on lines
+309
to
+313
| // Skip inaccessible properties | ||
| if (!IsAccessibleForDecorator(property.DeclaredAccessibility)) | ||
| { | ||
| context.ReportDiagnostic(Diagnostic.Create( | ||
| InaccessibleMemberDescriptor, |
There was a problem hiding this comment.
Property forwarding only checks property.DeclaredAccessibility, but accessor accessibility can differ. A common pattern like public string Name { get; protected set; } will pass this check, but the generated setter will call Inner.Name = value; which won’t compile because the setter is protected and Inner is typed as the base contract. The accessibility logic should consider getter/setter accessibility (or omit forwarding for inaccessible accessors) and report PKDEC004/PKDEC002 rather than emitting uncompilable code.
Contributor
Author
There was a problem hiding this comment.
Fixed in 7783db2. Now checks accessor-level accessibility separately. Properties with inaccessible accessors (e.g., protected set) emit PKDEC004. Added test for protected setter scenario.
Comment on lines
359
to
367
| else if (member is IFieldSymbol or INamedTypeSymbol) | ||
| { | ||
| // Fields and nested types are not supported | ||
| context.ReportDiagnostic(Diagnostic.Create( | ||
| UnsupportedMemberDescriptor, | ||
| member.Locations.FirstOrDefault(), | ||
| member.Name, | ||
| member.Kind.ToString())); | ||
| } |
There was a problem hiding this comment.
This branch reports PKDEC002 for all fields and nested types returned by type.GetMembers(). For abstract class contracts this will raise errors even for private implementation details (fields/nested types), which makes many real-world abstract classes unusable as contracts. Consider only reporting PKDEC002 for forwardable API-surface members, and silently ignore private/internal implementation details like fields/nested types.
Suggested change
| else if (member is IFieldSymbol or INamedTypeSymbol) | |
| { | |
| // Fields and nested types are not supported | |
| context.ReportDiagnostic(Diagnostic.Create( | |
| UnsupportedMemberDescriptor, | |
| member.Locations.FirstOrDefault(), | |
| member.Name, | |
| member.Kind.ToString())); | |
| } | |
| else if (member is IFieldSymbol fieldSymbol) | |
| { | |
| // Fields are not supported; only report for forwardable API-surface members | |
| if (IsAccessibleForDecorator(fieldSymbol.DeclaredAccessibility)) | |
| { | |
| context.ReportDiagnostic(Diagnostic.Create( | |
| UnsupportedMemberDescriptor, | |
| member.Locations.FirstOrDefault(), | |
| member.Name, | |
| member.Kind.ToString())); | |
| } | |
| } | |
| else if (member is INamedTypeSymbol typeSymbol) | |
| { | |
| // Nested types are not supported; only report for forwardable API-surface members | |
| if (IsAccessibleForDecorator(typeSymbol.DeclaredAccessibility)) | |
| { | |
| context.ReportDiagnostic(Diagnostic.Create( | |
| UnsupportedMemberDescriptor, | |
| member.Locations.FirstOrDefault(), | |
| member.Name, | |
| member.Kind.ToString())); | |
| } | |
| } |
Contributor
Author
There was a problem hiding this comment.
Fixed in 7783db2. PKDEC002 now only reported for accessible fields/nested types. Private implementation details silently ignored.
| private static readonly DiagnosticDescriptor InaccessibleMemberDescriptor = new( | ||
| id: DiagIdInaccessibleMember, | ||
| title: "Member is not accessible for decorator generation", | ||
| messageFormat: "Member '{0}' cannot be accessed for decorator forwarding. Ensure the member is public or internal with InternalsVisibleTo.", |
There was a problem hiding this comment.
PKDEC004’s message text is misleading: the generator allows protected internal members, and InternalsVisibleTo doesn’t address the real unsupported case (pure protected/private protected forwarding through Inner). Consider updating the diagnostic message to reflect the actual supported accessibilities and why protected members can’t be forwarded.
Suggested change
| messageFormat: "Member '{0}' cannot be accessed for decorator forwarding. Ensure the member is public or internal with InternalsVisibleTo.", | |
| messageFormat: "Member '{0}' cannot be forwarded by the generated decorator. Only members accessible from the decorator type (public, internal, or protected internal, with InternalsVisibleTo as needed) can be forwarded; purely protected or private protected members on the inner type are not accessible.", |
Contributor
Author
There was a problem hiding this comment.
Fixed in 7783db2. Updated PKDEC004 message to accurately describe supported accessibilities and explain why pure protected members can't be forwarded.
Comment on lines
5
to
6
| /// The generator will still emit a forwarding member in the decorator, but it will | ||
| /// strip <c>virtual</c>/<c>override</c> so the member is not decorated or overridden. |
There was a problem hiding this comment.
The XML doc says the generator “strips virtual/override” for ignored members, but for abstract-class contracts the generator emits sealed override (it must still override to satisfy the abstract/virtual contract). Please update the doc to match the actual behavior.
Suggested change
| /// The generator will still emit a forwarding member in the decorator, but it will | |
| /// strip <c>virtual</c>/<c>override</c> so the member is not decorated or overridden. | |
| /// The generator will still emit a forwarding member in the decorator. For concrete | |
| /// members it strips <c>virtual</c>/<c>override</c> so the member is not decorated, | |
| /// but when required to satisfy an abstract or virtual contract it emits a | |
| /// <c>sealed override</c> instead. |
Contributor
Author
There was a problem hiding this comment.
Fixed in 7783db2. Updated DecoratorIgnoreAttribute documentation to clarify sealed override behavior for abstract/virtual contracts.
Comment on lines
628
to
640
| string modifierKeyword; | ||
| if (contractInfo.IsAbstractClass) | ||
| { | ||
| // For abstract class contracts, always override the contract member. | ||
| // If the member is ignored, seal the override to prevent further overriding | ||
| // while still satisfying the abstract/virtual contract. | ||
| modifierKeyword = member.IsIgnored ? "sealed override " : "override "; | ||
| } | ||
| else | ||
| { | ||
| // For non-abstract contracts (e.g., interfaces), only non-ignored members are virtual. | ||
| modifierKeyword = member.IsIgnored ? "" : "virtual "; | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
Suggested change
| string modifierKeyword; | |
| if (contractInfo.IsAbstractClass) | |
| { | |
| // For abstract class contracts, always override the contract member. | |
| // If the member is ignored, seal the override to prevent further overriding | |
| // while still satisfying the abstract/virtual contract. | |
| modifierKeyword = member.IsIgnored ? "sealed override " : "override "; | |
| } | |
| else | |
| { | |
| // For non-abstract contracts (e.g., interfaces), only non-ignored members are virtual. | |
| modifierKeyword = member.IsIgnored ? "" : "virtual "; | |
| } | |
| string modifierKeyword = contractInfo.IsAbstractClass | |
| // For abstract class contracts, always override the contract member. | |
| // If the member is ignored, seal the override to prevent further overriding | |
| // while still satisfying the abstract/virtual contract. | |
| ? (member.IsIgnored ? "sealed override " : "override ") | |
| // For non-abstract contracts (e.g., interfaces), only non-ignored members are virtual. | |
| : (member.IsIgnored ? "" : "virtual "); |
Contributor
Author
There was a problem hiding this comment.
Fixed in 7783db2. Simplified to ternary operator for better code clarity.
Comment on lines
707
to
718
| string modifierKeyword; | ||
| if (contractInfo.IsAbstractClass) | ||
| { | ||
| // For abstract class contracts, always override. | ||
| // If ignored, seal the override to prevent further overriding. | ||
| modifierKeyword = member.IsIgnored ? "sealed override " : "override "; | ||
| } | ||
| else | ||
| { | ||
| // For interfaces, only non-ignored members are virtual. | ||
| modifierKeyword = member.IsIgnored ? "" : "virtual "; | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
Suggested change
| string modifierKeyword; | |
| if (contractInfo.IsAbstractClass) | |
| { | |
| // For abstract class contracts, always override. | |
| // If ignored, seal the override to prevent further overriding. | |
| modifierKeyword = member.IsIgnored ? "sealed override " : "override "; | |
| } | |
| else | |
| { | |
| // For interfaces, only non-ignored members are virtual. | |
| modifierKeyword = member.IsIgnored ? "" : "virtual "; | |
| } | |
| string modifierKeyword = contractInfo.IsAbstractClass | |
| ? (member.IsIgnored ? "sealed override " : "override ") | |
| : (member.IsIgnored ? "" : "virtual "); |
Contributor
Author
There was a problem hiding this comment.
Fixed in 7783db2. Simplified to ternary operator for better code clarity.
Owner
|
@copilot apply changes based on the comments in this thread |
…accessibility, improved diagnostics Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
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.
Decorator Pattern Source Generator - PR Review Round 3 ✅
✅ All PR Review Comments Addressed (8/8)
1. Static Members Handling
2. Generic Methods
3. Property Accessor Accessibility
public get; protected set;scenarios4. Fields & Nested Types
5. PKDEC004 Message
6. DecoratorIgnoreAttribute Documentation
7. Code Style
📊 Test Results
🔧 Key Improvements
protected setand similar scenariosProduction ready with enhanced edge case handling!
Original prompt
This section details on the original issue you should resolve
<issue_title>Generator: Create Decorator Pattern</issue_title>
<issue_description>## Summary
Add a source generator that produces boilerplate-free, GoF-consistent Decorator pattern implementations for interfaces and abstract classes.
The generator lives in
PatternKit.Generatorsand emits self-contained, readable C# with no runtime PatternKit dependency.Primary goals:
ValueTaskfor generated hooks).Motivation / Problem
Decorator chains are frequently hand-written and drift:
Source generation can emit correct forwarding + consistent hooks once.
Supported Targets (must-have)
Decorator generation must support:
interfaceabstract class(virtual/abstract members only)Decorator generation is opt-in via attribute on the contract.
Proposed User Experience
A) Generate base decorator + optional pipeline builder
Generated (representative shape):
B) Typed decorators written by users
The generator ensures the base class always stays in sync with the contract.
Attributes / Surface Area
Namespace:
PatternKit.Generators.Decorator[GenerateDecorator]on contract typestring? BaseTypeName(default:<ContractName>DecoratorBase)string? HelpersTypeName(default:<ContractName>Decorators)DecoratorCompositionMode Composition(default:HelpersOnly)bool GenerateAsync(default: inferred)bool ForceAsync(default: false)Optional:
[DecoratorIgnore]for members to exclude.Enums:
DecoratorCompositionMode:None,HelpersOnly,PipelineNextStyle(v2)Semantics (must-have)
Forwarding correctness
Inner.Composition helper ordering
If helper
Composeis generated:decorators applied in array order.
ordering must be documented:
decorators[0]is outermost by default.Async
Diagnostics (must-have)
PKDEC001Type marked[GenerateDecorator]must bepartial.PKDEC002Unsupported member kind (e.g., event) for v1.PKDEC003Name conflict for generated base/helper types.PKDEC004Member not accessible for generation.Generated Code Layout
ContractName.Decorator.g.csDeterminism:
Testing Expectations
Acceptance Criteria
[GenerateDecorator]works for interfaces and abstract classes.</issue_description>
<agent_instructions>Ensure complete BDD code coverage, comprehensive realworld usage examples and demos, user guides, and congruence with our existing implementations.</agent_instructions>
Comments on the Issue (you are @copilot in this section)
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.