fix: Multiple bug fixes and enhancements to source generator #287
fix: Multiple bug fixes and enhancements to source generator #287glennawatson merged 10 commits intomainfrom
Conversation
…yzer This commit addresses four major bugs and adds comprehensive test coverage for the Splat.DI.SourceGenerator. All changes maintain compatibility with the incremental generator pipeline and follow AOT-first design principles. Fixes #282 - Analyzer false positives on non-Splat Register methods ============================================================ **Problem**: ConstructorAnalyzer triggered SPLATDI001 warnings on ALL classes with multiple constructors, including non-Splat code like DialogManager.Register<TView, TContext>(). **Root Cause**: Used RegisterSymbolAction(SymbolKind.NamedType) which analyzed every class/struct in the compilation, regardless of whether it was used with Splat DI. **Solution**: Changed to RegisterOperationAction(OperationKind.Invocation) to only analyze types actually registered via SplatRegistrations.Register/RegisterLazySingleton. Added IsSplatRegistrationsMethod() helper to filter invocations. **Impact**: Eliminates false positives while maintaining detection of real Splat DI issues. **Tests**: 5 new tests added, 114 analyzer tests passing. Fixes #120 - Missing dependencies silently return null ===================================================== **Problem**: Generated code used resolver.GetService(typeof(T)) which returns null when dependencies are missing, causing runtime NullReferenceException instead of clear error messages. **Solution**: - Switched to generic GetService<T>() API (no cast needed, better for AOT) - Added null-coalescing operator with descriptive InvalidOperationException - Error messages include dependency type name and contract (if applicable) **Generated Code Change**: Before: (IFoo)resolver.GetService(typeof(IFoo)) After: resolver.GetService<IFoo>() ?? throw new InvalidOperationException("Dependency 'IFoo' not registered...") **Impact**: Early failure detection at registration time with clear error messages. All 342 snapshot tests updated. **Tests**: All existing tests updated and verified. Fixes #72 - IEnumerable<T> dependency injection support ======================================================= **Problem**: Constructor parameters of type IEnumerable<T> were treated like regular dependencies, calling GetService(typeof(IEnumerable<T>)) which returns null. Should call GetServices<T>() to retrieve all registered implementations. **Solution**: - Extended ConstructorParameter model with IsCollection and CollectionItemType fields - Added IEnumerable<T> detection in ExtractConstructorParameters (similar to Lazy<T>) - Generate GetServices<T>() calls for collection parameters (returns IEnumerable<T>, never null) **Generated Code**: resolver.GetServices<IService>() for IEnumerable<IService> parameters resolver.GetServices<IService>("contract") when using contracts **Impact**: Enables injection of multiple implementations, enables common DI patterns. **Tests**: 9 new tests added covering contracts and scenarios. Validates #137 - Generic type registration support ================================================== **Status**: Already working correctly via ToDisplayString(FullyQualifiedFormat). Added comprehensive test coverage to prevent regressions. **Tests Added**: - Single type parameter: Register<IAppUpdateService, AppUpdateService<string>>() - Multiple type parameters: Register<ICache, Cache<string, int>>() - Nested generics: Register<IRepository, Repository<List<string>>>() **Impact**: 27 new tests (3 scenarios × 3 contracts × 3 frameworks) confirm generic types work correctly with fully-qualified names. Additional Improvements ====================== - All generated code now uses generic APIs (GetService<T>, Register<T>, GetServices<T>) instead of Type-based APIs for better AOT compatibility and performance - Test infrastructure updated to include SplatRegistrations class in test compilations - All tests follow TUnit framework patterns with proper async/await usage Breaking Changes =============== 1. Generated GetService calls now throw exceptions if dependencies are missing 2. ConstructorParameter record has 2 new fields (IsCollection, CollectionItemType) - internal API 3. Generated code uses generic GetService<T>() and GetServices<T>() instead of typeof() variants Test Results =========== - Total: 633 tests (36 new tests added) - Failed: 0 - Succeeded: 633 - New coverage: 5 analyzer + 4 IEnumerable + 27 generic type tests All changes follow ReactiveUI contribution guidelines with proper formatting, XML documentation, and no LINQ in hot paths.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #287 +/- ##
==========================================
+ Coverage 87.85% 94.03% +6.18%
==========================================
Files 14 17 +3
Lines 815 838 +23
Branches 145 145
==========================================
+ Hits 716 788 +72
+ Misses 58 19 -39
+ Partials 41 31 -10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Created Microsoft.CodeAnalysis.EmbeddedAttribute.g.cs for test cases. - Added Splat.DI.Reg.g.cs and Splat.DI.g.cs files for dependency registration in multiple test scenarios. - Implemented service registration logic in SplatRegistrations for various test contracts. - Included support for lazy initialization and contract-based service resolution. - Ensured proper handling of dependencies with contract parameters in the generated code.
There was a problem hiding this comment.
Pull request overview
This PR implements multiple critical bug fixes and feature enhancements for the Splat.DI.SourceGenerator, addressing analyzer false positives, missing dependency handling, and IEnumerable injection support.
Changes:
- Switches from reflection-based
typeof()API to genericGetService<T>()for better AOT compatibility - Adds null-coalescing with descriptive
InvalidOperationExceptionfor missing dependencies - Implements
IEnumerable<T>parameter detection usingGetServices<T>() - Adds comprehensive test coverage (36 new tests across 4 bug fixes)
- Updates ConstructorParameter model with IsCollection and CollectionItemType fields
Reviewed changes
Copilot reviewed 263 out of 263 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Models/ConstructorParameter.cs | Extended record with IsCollection and CollectionItemType fields for IEnumerable support |
| Generator.cs | Refactored dependency resolution to use generic APIs, added null checks with exceptions, implemented IEnumerable detection |
| Constants.cs | Fixed indentation of attribute class definitions (formatting only) |
| RegisterTests.cs | Added 4 new test methods for IEnumerable, generic types, and property injection scenarios |
| RegisterLazySingletonTests.cs | Added 2 new test methods for lazy singleton with IEnumerable and contracts |
| Models/*Tests.cs | Updated unit tests to match new ConstructorParameter constructor signature |
| *.verified.cs (342 files) | Updated snapshot tests reflecting new generic API usage and error handling |
…rate helper classes
…or accessibility and multiple marked constructors
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This PR addresses four major bugs and adds comprehensive test coverage for the Splat.DI.SourceGenerator. All changes maintain compatibility with the incremental generator pipeline and follow AOT-first design principles.
Fixes #282 - Analyzer false positives on non-Splat Register methods
Problem: ConstructorAnalyzer triggered SPLATDI001 warnings on ALL classes with multiple constructors, including non-Splat code like DialogManager.Register<TView, TContext>().
Root Cause: Used RegisterSymbolAction(SymbolKind.NamedType) which analyzed every class/struct in the compilation, regardless of whether it was used with Splat DI.
Solution: Changed to RegisterOperationAction(OperationKind.Invocation) to only analyze types actually registered via SplatRegistrations.Register/RegisterLazySingleton. Added IsSplatRegistrationsMethod() helper to filter invocations.
Impact: Eliminates false positives while maintaining detection of real Splat DI issues.
Tests: 5 new tests added, 114 analyzer tests passing.
Fixes #120 - Missing dependencies silently return null
Problem: Generated code used resolver.GetService(typeof(T)) which returns null when dependencies are missing, causing runtime NullReferenceException instead of clear error messages.
Solution:
Generated Code Change:
Before: (IFoo)resolver.GetService(typeof(IFoo))
After: resolver.GetService() ?? throw new InvalidOperationException("Dependency 'IFoo' not registered...")
Impact: Early failure detection at registration time with clear error messages. All 342 snapshot tests updated.
Tests: All existing tests updated and verified.
Fixes #72 - IEnumerable dependency injection support
Problem: Constructor parameters of type IEnumerable were treated like regular dependencies, calling GetService(typeof(IEnumerable)) which returns null. Should call GetServices() to retrieve all registered implementations.
Solution:
Generated Code:
resolver.GetServices() for IEnumerable parameters resolver.GetServices("contract") when using contracts
Impact: Enables injection of multiple implementations, enables common DI patterns.
Tests: 9 new tests added covering contracts and scenarios.
Validates and closes #137 - Generic type registration support
Status: Already working correctly via ToDisplayString(FullyQualifiedFormat). Added comprehensive test coverage to prevent regressions.
Tests Added:
Impact: 27 new tests (3 scenarios × 3 contracts × 3 frameworks) confirm generic types work correctly with fully-qualified names.
Additional Improvements
Breaking Changes
Test Results
All changes follow ReactiveUI contribution guidelines with proper formatting, XML documentation, and no LINQ in hot paths.