Fix INJ0008/INJ0012 to fire for inferred open-generic registrations - #64
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe analyzer validates ChangesOpen-generic registration validation
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@copilot based on your analysis via this fix, is it possible for either case to exist without a matching |
No — both INJ0008 and INJ0012 always co-occur with INJ0002. Every path that fires either diagnostic also sets |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@tests/Immediate.Injections.Tests/AnalyzerTests/RegisterTypeAnalyzerTests.INJ0008.cs`:
- Around line 20-31: Extend RegisterTypeAnalyzerTests with coverage for inferred
Self and SelfAndImplementedInterfaces strategies, mirroring the existing
open-generic proxy-factory test. Add cases using explicit strategies and
assembly-default configuration variants, and assert each still reports INJ0008
alongside the existing registration diagnostic.
- Around line 20-46: Complete the inferred registration-strategy test matrix by
adding Self and SelfAndImplementedInterfaces cases alongside the existing
ImplementedInterfaces cases in
tests/Immediate.Injections.Tests/AnalyzerTests/RegisterTypeAnalyzerTests.INJ0008.cs
lines 20-46, covering both per-attribute and assembly-default strategies; add
the corresponding inferred factory-validation cases in
tests/Immediate.Injections.Tests/AnalyzerTests/RegisterTypeAnalyzerTests.INJ0012.cs
lines 25-61, preserving the existing analyzer test structure and expected
diagnostics.
In
`@tests/Immediate.Injections.Tests/AnalyzerTests/RegisterTypeAnalyzerTests.INJ0012.cs`:
- Around line 25-41: Expand the RegisterTypeAnalyzer tests around
FactoryMethodWithInferredOpenGenericStrategyTriggers to cover inferred
RegistrationStrategy.Self and RegistrationStrategy.SelfAndImplementedInterfaces,
including their assembly-default variants. Keep the existing generic factory
setup and assertions, ensuring each case still reports INJ0012 rather than
losing the diagnostic during strategy resolution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c48da6f9-5eb0-411a-a7a1-1f45f0ad8a02
📒 Files selected for processing (3)
src/Immediate.Injections.Analyzers/RegisterTypeAnalyzer.cstests/Immediate.Injections.Tests/AnalyzerTests/RegisterTypeAnalyzerTests.INJ0008.cstests/Immediate.Injections.Tests/AnalyzerTests/RegisterTypeAnalyzerTests.INJ0012.cs
| [Fact] | ||
| public async Task UseProxyFactoryWithInferredOpenGenericStrategyTriggers() => | ||
| await AnalyzerTestHelpers.CreateAnalyzerTest<RegisterTypeAnalyzer>( | ||
| """ | ||
| using Immediate.Injections.Shared; | ||
|
|
||
| public interface IService<T>; | ||
|
|
||
| [{|INJ0008:{|INJ0002:RegisterSingleton(RegistrationStrategy = RegistrationStrategy.ImplementedInterfaces, UseProxyFactory = true)|}|}] | ||
| public class Class<T> : IService<T>; | ||
| """ | ||
| ).RunAsync(TestContext.Current.CancellationToken); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover the remaining inferred registration strategies.
Both new tests use RegistrationStrategy.ImplementedInterfaces; add inferred Self and SelfAndImplementedInterfaces cases, including assembly-default variants, so regressions in strategy resolution cannot silently lose INJ0008.
Also applies to: 33-46
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@tests/Immediate.Injections.Tests/AnalyzerTests/RegisterTypeAnalyzerTests.INJ0008.cs`
around lines 20 - 31, Extend RegisterTypeAnalyzerTests with coverage for
inferred Self and SelfAndImplementedInterfaces strategies, mirroring the
existing open-generic proxy-factory test. Add cases using explicit strategies
and assembly-default configuration variants, and assert each still reports
INJ0008 alongside the existing registration diagnostic.
|
@copilot per review comments, please add test cases for |
Done in the latest commit. Added tests for all three strategies (per-attribute and assembly-default) for both INJ0008 and INJ0012. Note that |
Coverage Report for CI Build 30372396405Coverage increased (+0.06%) to 96.217%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
INJ0008 and INJ0012 only fired when
ServiceType = typeof(IService<>)was written explicitly. A generic class registered viaRegistrationStrategy = ImplementedInterfaces(or an assembly-wide default strategy) produces the same open-generic descriptor without spelling out the service type — those cases silently dropped the registration with no diagnostic.Changes
RegisterTypeAnalyzer.cs— InAnalyzeRegisterType0Attribute, adds anelse if (containerSymbol.IsGenericType)branch for theserviceType is nullcase: fires INJ0008 whenUseProxyFactory = true, INJ0012 whenFactory = ...is set. Covers all strategies (Self, ImplementedInterfaces, SelfAndImplementedInterfaces) since any registration strategy on a generic class produces open-generic service types.RegisterTypeAnalyzerTests.INJ0008.cs/INJ0012.cs— New tests for the per-attribute and assembly-default inferred cases.Example — previously no diagnostic
Summary by CodeRabbit
UseProxyFactoryor factory methods are specified but the registration is open-generic.UseProxyFactory = true) and open-generic factory method scenarios.