refactor: Improve code generation quality and fix registration bug#4
refactor: Improve code generation quality and fix registration bug#4ncipollina merged 13 commits intomainfrom
Conversation
- Implemented compile-time decorator registration using C# 11+ interceptors - Added incremental source generator with attribute-based decoration discovery - Support for both generic [DecoratedBy<T>] and non-generic [DecoratedBy(typeof(...))] attributes - Open generic type support with proper arity handling (e.g., Dictionary<,>) - Keyed services integration to prevent circular dependencies - Proper indentation and global:: namespace prefixes in generated code - Comprehensive README with usage examples and comparison to Scrutor - Unit test infrastructure with snapshot verification - CI/CD workflows for build and PR validation Generated code features: - Only emits interceptor methods for actually used lifetimes (AddScoped/Transient/Singleton) - Block-scoped namespace declarations for proper C# formatting - Type-safe interceptor attributes with file-scoped visibility - Decorator factory with runtime type closing for open generics 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add multi-targeting for net8.0, net9.0, and net10.0 - Update Microsoft.NET.Test.Sdk to 18.0.0 - Update Verify.XunitV3 to 31.3.0 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Update Description and PackageTags in Generators.csproj for Sculptor - Add Directory.Build.props with common package metadata - Add icon.png for NuGet package - Configure package icon and README for NuGet publishing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit implements compile-time decorator registration for closed generic types using C# interceptors. Key changes: - Added ClosedGenericRegistrationProvider to discover closed generic registrations (e.g., AddScoped<IRepository<Customer>, DynamoDbRepository<Customer>>()) - Fixed TypeDefId equality by replacing array fields with EquatableArray<string> for proper structural equality - Updated InterceptorEmitter to generate interceptors with generic signatures but concrete type bodies - Open generic decorators (e.g., CachingRepository<>) are closed at runtime using MakeGenericType - Updated README to reflect closed generic registration requirement and current implementation - Added CollectionBuilder support to EquatableArray for collection expression syntax Technical details: - Interceptor methods have generic signatures to match original DI extension methods - Method bodies use concrete closed types from the registration call site - Each registration gets its own specialized interceptor method - Decorators are applied using keyed services to prevent circular dependencies 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Fixed multiple decorator discovery on same class by using CreateSyntaxProvider with SelectMany - Added TransformMultiple method to process all [DecoratedBy] attributes per class - Fixed pattern matching for constructor order argument syntax - Changed attribute properties to support named arguments (get/set instead of get-only) - Skip interceptor generation when no interceptable decorators exist - Created 9 comprehensive test cases for open generic decorators: * 001: Single decorator * 002: Multiple ordered decorators * 003: Constructor order syntax * 004: Multiple default order * 005: Mixed order syntax * 006: Non-generic decorator * 007: IsInterceptable=false (no code generation) * 008: No decorators (baseline) * 009: Three decorators (deep nesting) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit addresses all critical, medium, and low priority issues identified during code review to improve code quality, safety, and performance. ## Critical Fixes - Add null-coalescing for AssemblyQualifiedName in generated code Prevents NullReferenceException for generic type parameters and unloaded assemblies by falling back to FullName then Name ## Medium Priority Improvements - Remove unused EmitAddOverloadWithAttributes method (61 lines) - Remove unused GetServiceDescriptorMethod and GetServiceLifetime helpers - Replace fragile StartsWith type check with MetadataName + namespace Uses proper Roslyn API for type-safe attribute detection - Remove unnecessary ToList() allocation in BuildDecorationMap - Improve dictionary lookup clarity with explicit TryGetValue pattern ## Low Priority Optimizations - Replace ContainsKey + access with single TryGetValue call Eliminates redundant dictionary lookup - Remove unused ToFqnClosed method (18 lines) ## Test Updates - Update all snapshot tests to reflect improved null-safe generated code - Add comprehensive test coverage for new decorator scenarios (Generic, Transient, Singleton with single/multiple decorators) Total impact: Removed ~100 lines of dead code, improved safety and performance of both generator and generated code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Removes DecorateServiceAttribute and DoNotDecorateAttribute which were placeholder implementations not yet functional. This cleans up the codebase for the initial release. Future implementation tracked in issue #2. Changes: - Remove DecorateServiceAttribute class definition from DecoratedByAttribute.cs - Remove DecorateServiceAttribute and DoNotDecorateAttribute constants from Attributes.cs - Verified no other references to removed code exist 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Added complete MkDocs documentation site with Material theme - Created documentation for getting started, core concepts, usage patterns, and examples - Added GitHub Actions workflow for deploying docs to GitHub Pages - Configured NuGet packaging for source generator with proper analyzer paths - Added Sculptor.props and Sculptor.targets for MSBuild integration - Moved common package metadata to Directory.Build.props - Enabled XML documentation generation for Attributes project 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Complete rebranding of the project from Sculptor to DecoWeaver, including: - Updated all namespaces, class names, and identifiers - Renamed solution file: LayeredCraft.Sculptor.slnx → LayeredCraft.DecoWeaver.slnx - Renamed all project files and directories - Updated documentation: - CLAUDE.md: Project overview, structure, and conventions - README.md: Trimmed down and streamlined with links to full docs - All docs/ content: Installation, usage guides, examples, and API reference - Updated configuration files: - Directory.Build.props: Repository URLs - mkdocs.yml: Site name and URLs - GitHub workflows: Solution file reference - Updated diagnostics prefix: SCULPT### → DECOW### - Updated conditional compilation: SCULPTOR_EMIT_ATTRIBUTE_METADATA → DECOWEAVER_EMIT_ATTRIBUTE_METADATA - Updated all code comments and sample applications - Updated test infrastructure and snapshots Breaking Changes: - Package name changed from Sculptor to DecoWeaver - Namespaces changed from LayeredCraft.Sculptor to LayeredCraft.DecoWeaver - Generated file names changed from Sculptor.Interceptors.* to DecoWeaver.Interceptors.* 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…d support for closed generic registrations and clarify registration requirements
…e generator - Use SymbolDisplay.FormatLiteral for proper string escaping in InterceptorEmitter - Handles all edge cases (backslashes, quotes, newlines, unicode) - More robust than manual escaping - Replace string comparison with pattern matching for namespace verification - DecoratedByGenericProvider: Use pattern matching for attribute validation - DecoratedByNonGenericProvider: Use pattern matching for attribute validation - Add metadata name constants to AttributeNames class - More efficient (no string allocation from ToDisplayString) - Type-safe compile-time verification - Update documentation with actual GitHub issue link (#3) for factory delegate support 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Improves maintainability by deriving full attribute names from metadata names using string interpolation, reducing duplication and potential for inconsistency. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
@codex review |
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".
|
|
||
| private static string Escape(string s) => | ||
| s.Replace("\\", "\\\\").Replace("\"", "\\\""); | ||
| SymbolDisplay.FormatLiteral(s, quote: true); |
There was a problem hiding this comment.
Add missing using for SymbolDisplay
The new Escape helper calls SymbolDisplay.FormatLiteral, but this type lives in the Microsoft.CodeAnalysis namespace, not Microsoft.CodeAnalysis.CSharp. Because the file only imports Microsoft.CodeAnalysis.CSharp, the SymbolDisplay identifier is unresolved and the generator no longer compiles. Import Microsoft.CodeAnalysis (or fully qualify the call) so the project builds.
Useful? React with 👍 / 👎.
Summary
This PR includes code quality improvements to the source generator and a critical bug fix for registration interception.
Bug Fix: Registration Interception
Problem: The generator was intercepting ALL generic
AddScoped/Transient/Singletoncalls regardless of parameters, but only generating parameterless interceptor signatures. This caused silent data loss for:AddScoped<T1, T2>(sp => new T2())AddKeyedScoped<T1, T2>("key")AddSingleton<T1>(instance)Solution: Added parameter count check in
ClosedGenericRegistrationProvider.cs:57to only match the parameterless overload.Related:
022_FactoryDelegate_ShouldNotInterceptDECOW020for unsupported patternsCode Quality Improvements
String Escaping:
SymbolDisplay.FormatLiteralinstead of manual escapingNamespace Verification:
ToDisplayString()AttributeNamesclassPattern matching example:
Documentation Updates
docs/usage/open-generics.mdwith actual issue link (Support factory delegates, keyed services, and instance registrations #3)Files Changed
src/LayeredCraft.DecoWeaver.Generators/Providers/ClosedGenericRegistrationProvider.cs- Bug fixsrc/LayeredCraft.DecoWeaver.Generators/Emit/InterceptorEmitter.cs- Better string escapingsrc/LayeredCraft.DecoWeaver.Generators/Providers/DecoratedByGenericProvider.cs- Pattern matchingsrc/LayeredCraft.DecoWeaver.Generators/Providers/DecoratedByNonGenericProvider.cs- Pattern matchingsrc/LayeredCraft.DecoWeaver.Generators/AttributeNames.cs- Added metadata name constantssrc/LayeredCraft.DecoWeaver.Generators/Descriptors.cs- Added DECOW020 diagnostictest/Cases/022_FactoryDelegate_ShouldNotIntercept/- New test casedocs/usage/open-generics.md- Documentation updatesTesting
🤖 Generated with Claude Code