feat(extractor): DI registration graph → DI001 captive dependency on real C# (P-006)#49
Conversation
…(P-006)
The DI001 captive-dependency check has lived in the core (ownlang/di.py),
validated only on hand-written `services` facts. Give it a C# front end: a
syntactic pass over the parsed trees builds the registration + constructor graph
and emits the `services` facts the core already consumes, so DI001 fires
end-to-end on real C# with ZERO core changes.
Extractor (frontend/roslyn/OwnSharp.Extractor/Program.cs):
- ExtractServices(): a constructor-graph pass (each class's widest ctor's
parameter types) + a registration pass over conventional IServiceCollection
calls — Add{Singleton,Scoped,Transient} in the generic `<TService[, TImpl]>`
form and the `typeof(...)` form. Each registration emits one service fact
{name=service type, lifetime, deps=impl ctor params, file, line@registration}.
- Factory / reflection / open-generic shapes we cannot read become unknown-dep
nodes (deps: []) — silent, never guessed (P-006 non-goals).
- `services` is added to the emitted facts (empty when a scan has no
registrations).
Sample + CI (frontend/roslyn/samples/DiCaptiveSample.cs, wpf-extractor job):
asserts a direct capture (singleton EmailSender -> scoped AppDbContext), a
transitive one through a transient (ReportService -> UnitOfWork -> AppDbContext),
an interface-registration one (AddScoped<IRepo, Repo>; CacheService -> IRepo),
exactly three findings, and silence on singleton->singleton (Metrics -> Clock)
and the clean leaves.
Validated bridge-side locally (the emitted facts -> core -> the 3 expected DI001s,
clean ones silent); the C#->facts step is validated in CI (no local .NET SDK).
docs: P-006 status advanced (extractor built, end-to-end), ROADMAP milestone 3 +
indices, and docs/notes/di-captive-extractor.md.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
![github-advanced-security[bot]](https://cdn.github.com/in/57789?s=60&v=4){kind=small}
| services.AddScoped<IRepo, Repo>(); // interface -> impl, scoped | ||
|
|
||
| // FLAGGED — singleton captures a scoped service directly. | ||
| services.AddSingleton<EmailSender>(); |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR implements DI001 captive-dependency detection end-to-end: a new C# sample file defines flagged and silent DI registrations; the Roslyn extractor gains an ChangesDI001 Captive Dependency End-to-End
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://cdn.github.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.
Reviewed commit: e4495cb693
ℹ️ 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".
| foreach (var m in cls.Members) | ||
| if (m is ConstructorDeclarationSyntax ctor |
There was a problem hiding this comment.
When a registered service uses C# 12 primary-constructor injection, such as public sealed class EmailSender(AppDbContext db), there is no ConstructorDeclarationSyntax member; the parameters live on the class declaration itself. This loop leaves widest null and emits an empty dependency list, so DI001 misses singleton-to-scoped captures in current .NET 8/ASP.NET-style code.
Useful? React with 👍 / 👎.
| if (m is ConstructorDeclarationSyntax ctor | ||
| && (widest is null | ||
| || ctor.ParameterList.Parameters.Count | ||
| > widest.ParameterList.Parameters.Count)) | ||
| widest = ctor; |
There was a problem hiding this comment.
When a service has a wider private/protected constructor that takes a scoped dependency plus a public constructor that DI can actually use, this code selects the non-public constructor and emits those parameters as real dependencies. The default IServiceProvider only uses public constructors, so DI001 can report a captive dependency at the registration site even though that scoped service is never consumed by DI.
Useful? React with 👍 / 👎.
…x review) Two real ctor-selection gaps in ExtractServices that the classic-ctor sample did not exercise: - Primary constructors (C# 12). `class EmailSender(AppDbContext db)` has no ConstructorDeclarationSyntax member — its parameters live on the class declaration. The loop left `widest` null and emitted empty deps, so DI001 missed captures in modern .NET 8 / ASP.NET code. Seed `widest` from `cls.ParameterList` (the primary ctor). - Non-public constructors. The default IServiceProvider resolves through PUBLIC ctors only; a wider private/protected ctor taking a scoped dependency must not count, or DI001 reports a captive DI never creates. Gate explicit ctors on IsPublicCtor. Sample + CI pin both: a primary-ctor singleton -> scoped capture is flagged (PrimaryCtorService -> AppDbContext); a service whose only public ctor is parameterless but which also has a wider private ctor with a scoped dep stays silent (PublicCtorOnly). Count is now exactly four captives. Re-validated bridge-side; the C#->facts step is validated in CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
| services.AddSingleton<ReportService>(); | ||
|
|
||
| // FLAGGED — through the interface registration: singleton -> IRepo (scoped). | ||
| services.AddSingleton<CacheService>(); |
| services.AddSingleton<CacheService>(); | ||
|
|
||
| // FLAGGED — primary-constructor injection: singleton -> scoped AppDbContext. | ||
| services.AddSingleton<PrimaryCtorService>(); |
3 participants
What
Gives the built-but-dormant DI001 captive-dependency check (
ownlang/di.py) a C# front end. The Roslyn extractor now builds the registration + constructor graph from real code, so DI001 fires end-to-end on C# with zero core changes — a whole new diagnostic class purely by producing theservicesfacts the lifetime core already consumes (P-006, ROADMAP milestone 3).The bug: a
singletondepending — directly, transitively through atransient, or through an interface registration — on ascopedservice captures that scoped instance for the app lifetime (the classic ASP.NET Core "Cannot consume scoped service from singleton"; aDbContextheld by a singleton is the canonical case).How (extractor,
Program.cs)A syntactic pass (no SemanticModel — narrow-frontend spirit):
Add{Singleton,Scoped,Transient}in the generic<TService[, TImpl]>form and thetypeof(...)form;servicesfact per registration:{name = service type, lifetime, deps = impl ctor params, file, line@registration}.deps: []): silent, never guessed (P-006 non-goals).Validation
wpf-extractorjob overDiCaptiveSample.cs) — the C# the extractor cannot be compiled locally, so this is the real test, and it's green:The assertions pin: direct + transitive-through-transient + interface-registration captures, exactly 3 findings, and silence on
Metrics -> Clock(singleton→singleton) and the clean leaves. Theservicesgraph also flows through theown-check-surface+own-check-codescanjobs (DI001 now rides into the dog-food SARIF too).Why it matters
The captive dependency is an ASP.NET-specific lifetime contract general-purpose analyzers don't model — the same complementary story as the subscription-leak class the cross-tool oracle documented. And it's a clean reuse win: a new diagnostic on real C# with no core changes.
Files
frontend/roslyn/OwnSharp.Extractor/Program.cs—ExtractServices()+ theservicesfact.frontend/roslyn/samples/DiCaptiveSample.cs— the CI sample (3 flagged, 2 silent)..github/workflows/ci.yml—wpf-extractorwiring + DI001 assertions.docs/— P-006 status, ROADMAP milestone 3 + indices,docs/notes/di-captive-extractor.md.Next (separate slices): DI002 (weak-ref), DI003 (transient-
IDisposable-from-root).🤖 Generated with Claude Code
https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation
CI/Validation