diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 770063b4..8b6599fd 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -218,9 +218,12 @@ architectural strictness, and the borrow-checker showcase): the scope is disposed and is promoted to app lifetime) is built end to end too, a store-site property anchored at the field assignment. The family now also has its first **real-world corpus case** — a singleton injecting a scoped EF `DbContext` → DI001 (`corpus/di/`, a - benchmark-only corpus since DI has no `.own` form). Remaining (deliberate-deferral / future): - directly-injected `IServiceScopeFactory`-as-a-positive-signal recognition (P-006 OQ#3), and - the dynamic registrations that are explicit non-goals. + benchmark-only corpus since DI has no `.own` form). Directly-injected + `IServiceScopeFactory`-as-a-positive-signal recognition (P-006 OQ#3) is **done** — shipped in + PR #126, reconciled in #200: the correct scope-per-operation pattern is silent *by construction* + (a scope-resolved value used in the scope is a local, not a `scope_cached` field store), and + caching it into a field is DI005. Remaining (deliberate non-goals): the dynamic registrations a + static graph cannot see. 4. **Pool/Span** — `Rent`/`Return`, borrowed views, return-invalidates-views, known-bug replay corpus (P-007). The borrow checker on stage at full height. ◑ *In progress* — POOL001 (leak), POOL002 (view-after-return → OWN002), diff --git a/docs/notes/di-captive-extractor.md b/docs/notes/di-captive-extractor.md index f39da915..ed0c8d20 100644 --- a/docs/notes/di-captive-extractor.md +++ b/docs/notes/di-captive-extractor.md @@ -216,9 +216,41 @@ hand-resolved, not at the container-built `PooledConnection`. Falls back to the when the call is unknown. Pinned by `DiCaptiveSample.cs` (`ConnectionResolver:79`, `ExprBodiedResolver:123`, transitive `WrapperResolver:137`). +## DI005 — scope-resolved scoped service cached into a field (shipped), and the OQ#3 fix recognised + +The remedy DI001/DI002 point at is **scope-per-operation**: inject `IServiceScopeFactory`, and per +operation `using var scope = factory.CreateScope();` then resolve the scoped dependency *inside* the +scope. DI005 catches that remedy done wrong — the scope-resolved **scoped** service **cached into a +field**. The field outlives the `using` scope, so the cached instance dangles after the scope (and +the service) is disposed *and* is promoted to the singleton's application lifetime: the captive is +back, hidden behind the API meant to fix it (a **warning**, anchored at the field-store site). + +The extractor (still purely syntactic) records the **scope-creator** names with the same this-field +discipline as DI004 — a **directly-injected `IServiceScopeFactory`** *and* an injected +`IServiceProvider` (both expose `CreateScope()`) — then the scope locals their `CreateScope()` +produces, and every `scope.ServiceProvider.Get(Required)Service()` whose result is **assigned to +a field** into a `scope_cached` list with its store site. `find_scope_cached_captives` +(`ownlang/di.py`) walks each cached entry's strong transient graph like DI001; the field-store site +is the finding's primary anchor, the registration the secondary. + +**This is the answer to P-006 open question #3 — recognising the directly-injected +`IServiceScopeFactory` fix.** It needs no separate "approval" fact: the correct pattern (resolve +inside the scope, use, **discard** — a local, not a field store) simply **produces no `scope_cached` +entry**, so it is silent **by construction**. The "positive signal" is the *absence* of a captive +fact. Recognising the factory injection as licence to suppress *other* captive findings would be +wrong — a singleton that also injects a scoped service directly is still DI001. Pinned end-to-end by +`DiCaptiveSample.cs` (`ScopeCachingService` DI005 direct, `UnitOfWorkCachingService` DI005 +transitive; `ScopeUsingService` — the correct scope-per-operation use — and `ClockCachingService` +— a cached *singleton*, shareable — both silent) in the `wpf-extractor` CI job, and at the graph +level by `tests/test_ownir.py`. + ## Next (separate slices) - Per-**parameter** precision for the captive anchor (the specific injecting parameter, not just the constructor). -- The plural `GetServices()` and non-generic `GetService(typeof(T))` resolution forms, and a - directly-injected `IServiceScopeFactory` as the recognised fix (DI004 currently reads the - generic singular `Get(Required)Service()` and the `CreateScope()` → scope-provider form). +- The plural `GetServices()` and non-generic `GetService(typeof(T))` resolution forms (DI004 + currently reads the generic singular `Get(Required)Service()`). +- **A scope-resolved scoped service that *escapes* its scope by being returned (or passed out as a + `ref`/`out`/method argument)** rather than cached into a field — the same lifetime promotion as + DI005, but through a data-flow edge the store-site pass does not model. Silent today + (precision-safe: the extractor records only field stores, so an escaping local is no + `scope_cached` fact). A candidate for a future flow-aware slice, not the store-site model. diff --git a/docs/proposals/P-006-di-lifetimes.md b/docs/proposals/P-006-di-lifetimes.md index d14e7a44..0e792168 100644 --- a/docs/proposals/P-006-di-lifetimes.md +++ b/docs/proposals/P-006-di-lifetimes.md @@ -162,11 +162,21 @@ rather than guessed. 2. How far to chase transitive captures through the constructor graph before the dynamic cases make it unreliable? (Bounded depth; stop at unknown edges.) 3. Is `IServiceScopeFactory` usage inside a singleton recognised as the *fix* - (so we stay silent), as it should be? (For the explicit form (DI004): **yes**, by - construction — DI004 records only `GetService()` / `GetRequiredService()` on the - injected `IServiceProvider` names and **excludes** a scope's `.ServiceProvider` receiver, so - resolving from a scope created with `CreateScope()` is silent. Modelling a directly-injected - `IServiceScopeFactory` is not yet implemented — a natural future extension.) + (so we stay silent), as it should be? **Resolved — shipped in PR #126, reconciled in #200.** + For the explicit form (DI004): **yes**, by construction — DI004 records only `GetService()` / + `GetRequiredService()` on the injected `IServiceProvider` names and **excludes** a scope's + `.ServiceProvider` receiver, so resolving from a scope created with `CreateScope()` is silent. + A **directly-injected `IServiceScopeFactory`** is modelled the same way: the extractor + recognises it as a scope-creator name (`Program.cs`, alongside the injected provider) and emits + a `scope_cached` fact **only** for a value stored into a *field*. So the correct scope-per- + operation pattern — resolve inside the scope, use, discard (a local) — produces **no fact** and + stays silent *by construction*, while caching the scope-resolved scoped service into a field is + DI005. That silence **is** the finished state: the "positive signal" is the **absence** of a + captive fact, not a separate approval marker. Recognising the factory injection as licence to + suppress *other* captive findings would be wrong — a singleton that also injects a scoped service + directly is still DI001, regardless of any scope it opens elsewhere. Pinned by + `DiCaptiveSample.cs` (`ScopeUsingService` silent / `ScopeCachingService` DI005), the + `wpf-extractor` CI DI-contrast, and `tests/test_ownir.py`. 4. Treat transient-`IDisposable`-from-root (DI003/DI004) as warning or error? (Warning — it is a slow leak, not always a bug. DI004's call-site form is repeated at runtime, arguably worse, but kept a warning for consistency with DI003.)