From e967309da821774c912609658b8e0bea68a8cf49 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 25 Jun 2026 16:51:57 +0000 Subject: [PATCH 1/6] =?UTF-8?q?docs:=20correct=20stale=20DI=20roadmap=20st?= =?UTF-8?q?atus=20(DI002=E2=80=93DI004=20shipped,=20not=20remaining)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ROADMAP listed DI002 (weak-ref), the root-`GetService` form, and the consuming-constructor anchor as remaining, but all are built and tested: `find_weak_captive_dependencies` (DI002), `find_explicit_root_resolutions` (DI004), and the `consuming_ctor` anchor (P-006 OQ#1, "both"). Update the Milestone-3 status and the P-006 index row to DI001–DI004 end-to-end; the real remaining items are a real-world corpus case, IServiceScopeFactory recognition, and the explicit dynamic-registration non-goals. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Fmwk5ruExKy5fQ77CJ2NtF --- docs/ROADMAP.md | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index edc2abec..496ba2db 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -147,9 +147,16 @@ architectural strictness, and the borrow-checker showcase): registration + constructor graph from `Add{Singleton,Scoped,Transient}` (generic and `typeof(...)` forms) and the core flags the captive — direct, transitive through a transient, or through an interface registration — CI-validated on - `DiCaptiveSample.cs`. **DI003** (a transient `IDisposable` captured by a singleton, - warning) now also fires on the same sample. Remaining: DI002 (weak-ref), the explicit - root-`GetService` form of DI003, and the consuming-constructor anchor. + `DiCaptiveSample.cs`. The whole captive family is now built end to end on the same + sample: **DI002** (a scoped service held weakly via `WeakReference` — still + root-resolved/app-lived), **DI003** (a transient `IDisposable` captured by a singleton), + and **DI004** (the service-locator form — a transient `IDisposable` resolved by hand from + a singleton's injected **root** `IServiceProvider` via `GetService`/`GetRequiredService`), + all warnings; plus the **consuming-constructor anchor** (a captive names both its + registration site and the ctor that injects it, as message tail + SARIF relatedLocation). + Remaining (all deliberate-deferral / future): a real-world DI corpus case (the family is + pinned only on the synthetic sample), directly-injected `IServiceScopeFactory` recognition + (P-006 OQ#3), and the dynamic registrations that are explicit non-goals. 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), @@ -225,7 +232,7 @@ own scan. Label them as estimates wherever they appear. | [P-003](proposals/P-003-lifetime-visualization.md) | Lifetime visualization (RustOwl-style) | horizon | draft | | [P-004](proposals/P-004-wpf-lifetime-profile.md) | WPF / UI lifetime leak profile | P0 | in progress (WPF001–005 built) | | [P-005](proposals/P-005-idisposable-ownership.md) | `IDisposable` ownership profile | P0 | draft | -| [P-006](proposals/P-006-di-lifetimes.md) | DI lifetime / captive dependency | P0 | in progress (DI001 end-to-end: core + extractor) | +| [P-006](proposals/P-006-di-lifetimes.md) | DI lifetime / captive dependency | P0 | in progress (DI001–DI004 end-to-end: core + extractor) | | [P-007](proposals/P-007-arraypool-span.md) | ArrayPool / Span borrow-view | P1 | in progress (POOL001–003 built; 004/005 first slices) | | [P-008](proposals/P-008-effects-and-resources.md) | Effects & resources (`Own.Effects`) | P1/P2 | draft | | [P-009](proposals/P-009-nogc-regions.md) | No-GC / allocation-free regions | horizon | draft | From 8c5ad57a9f9d22ae1a81904fa55dd4572dfaf927 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 25 Jun 2026 17:16:52 +0000 Subject: [PATCH 2/6] feat(di): first real-world captive-dependency corpus case (DI001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DI captive family (DI001–DI004) was pinned only on the synthetic DiCaptiveSample.cs. Add a real-world-shaped case: a singleton NotificationService injecting a scoped EF AppDbContext -> DI001, with the standard IServiceScopeFactory fix in after.cs (silent). DI has no `.own` reduction (registrations live in the `services` fact graph, not the resource/flow DSL), so this can't be a corpus/real-world case.own — it lives in a new benchmark-only `corpus/di/` directory, added to scripts/benchmark.py's default corpus dirs and scored by the dotnet corpus-benchmark job (not the Python test_corpus `.own` runner). Bump the recall floor 24 -> 25 to pin the newly-caught case. P-006 / ROADMAP updated. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Fmwk5ruExKy5fQ77CJ2NtF --- .github/workflows/ci.yml | 5 ++- .../after.cs | 36 +++++++++++++++++ .../before.cs | 34 ++++++++++++++++ .../expected-diagnostics.txt | 1 + .../notes.md | 40 +++++++++++++++++++ docs/ROADMAP.md | 8 ++-- docs/proposals/P-006-di-lifetimes.md | 6 ++- scripts/benchmark.py | 5 ++- 8 files changed, 128 insertions(+), 7 deletions(-) create mode 100644 corpus/di/singleton-captures-scoped-dbcontext/after.cs create mode 100644 corpus/di/singleton-captures-scoped-dbcontext/before.cs create mode 100644 corpus/di/singleton-captures-scoped-dbcontext/expected-diagnostics.txt create mode 100644 corpus/di/singleton-captures-scoped-dbcontext/notes.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3610f9fd..8637ca09 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1108,7 +1108,10 @@ jobs: # _owner.Memory`) read in such a handler is the view-in-a-field dangle -> OWN002 (`pooled-view-after- # dispose`). The POOL005 field pass now also catches a full-length view of an ArrayPool `byte[]` # buffer FIELD read past its logical length -> OWN025 (`arraypool-field-fullspan-overread`). + # use, and an injected-source region-escape. The DI captive family also has its first real-world + # case now — a singleton injecting a scoped EF `DbContext` -> DI001 (`corpus/di/`, a benchmark-only + # corpus: DI has no `.own` form, so it is not scanned by the Python `test_corpus` runner). # Remaining backlog: a full-length view STORED into another field, a TWO-plus-hop indirect field # use, and an injected-source region-escape. A drop below the floor is a regression. - run: python scripts/benchmark.py --min-recall 24 + run: python scripts/benchmark.py --min-recall 25 diff --git a/corpus/di/singleton-captures-scoped-dbcontext/after.cs b/corpus/di/singleton-captures-scoped-dbcontext/after.cs new file mode 100644 index 00000000..38d18009 --- /dev/null +++ b/corpus/di/singleton-captures-scoped-dbcontext/after.cs @@ -0,0 +1,36 @@ +// AFTER (fixed). The standard remedy for a singleton that needs a scoped service: +// inject `IServiceScopeFactory` (a singleton itself) instead of the scoped +// `AppDbContext`, and open a fresh scope per operation — `using var scope = +// _scopes.CreateScope();` — resolving the DbContext inside it so it lives and is +// disposed within that operation. The singleton's constructor no longer depends on +// a scoped service, so there is no captive edge in the registration graph (DI001 +// silent), and the resolve is off the scope's provider (not an injected root +// `IServiceProvider`), so the service-locator rule (DI004) stays silent too. +using System; + +namespace Corpus +{ + public sealed class AppDbContext { } + + public sealed class NotificationService + { + private readonly IServiceScopeFactory _scopes; + public NotificationService(IServiceScopeFactory scopes) { _scopes = scopes; } // no scoped captured + + public void Notify() + { + using var scope = _scopes.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); // per-operation scope + // ... use db within the scope ... + } + } + + public static class Startup + { + public static void ConfigureServices(IServiceCollection services) + { + services.AddScoped(); + services.AddSingleton(); // SILENT — injects IServiceScopeFactory, not the scoped service + } + } +} diff --git a/corpus/di/singleton-captures-scoped-dbcontext/before.cs b/corpus/di/singleton-captures-scoped-dbcontext/before.cs new file mode 100644 index 00000000..4d6f60e0 --- /dev/null +++ b/corpus/di/singleton-captures-scoped-dbcontext/before.cs @@ -0,0 +1,34 @@ +// BEFORE (buggy). The canonical ASP.NET Core captive dependency (P-006 DI001): +// a SINGLETON service takes a SCOPED EF Core `DbContext` in its constructor, so +// the container builds one `AppDbContext` with the singleton and holds it for the +// whole application lifetime — an open DB connection pinned for the process, and +// request state shared across requests. Microsoft calls this "Cannot consume +// scoped service 'AppDbContext' from singleton 'NotificationService'." The +// extractor reads the conventional `IServiceCollection` registration graph +// (`Add{Singleton,Scoped}`) plus each implementation's constructor parameters, and +// ownlang/di.py flags the capture at the registration site, naming the consuming +// constructor. Representative of the pattern (a singleton background/notification +// service injecting a scoped DbContext), not verbatim from one project. The fix is +// a scope boundary — inject `IServiceScopeFactory` and resolve per operation (see +// after.cs). +using System; + +namespace Corpus +{ + public sealed class AppDbContext { } // scoped (an EF Core DbContext is scoped) + + // registered as a SINGLETON below, but it captures the scoped DbContext: + public sealed class NotificationService + { + public NotificationService(AppDbContext db) { } // <-- captures scoped (DI001) + } + + public static class Startup + { + public static void ConfigureServices(IServiceCollection services) + { + services.AddScoped(); // scoped + services.AddSingleton(); // FLAGGED: singleton -> scoped (DI001) + } + } +} diff --git a/corpus/di/singleton-captures-scoped-dbcontext/expected-diagnostics.txt b/corpus/di/singleton-captures-scoped-dbcontext/expected-diagnostics.txt new file mode 100644 index 00000000..d0014773 --- /dev/null +++ b/corpus/di/singleton-captures-scoped-dbcontext/expected-diagnostics.txt @@ -0,0 +1 @@ +DI001 diff --git a/corpus/di/singleton-captures-scoped-dbcontext/notes.md b/corpus/di/singleton-captures-scoped-dbcontext/notes.md new file mode 100644 index 00000000..a59a6252 --- /dev/null +++ b/corpus/di/singleton-captures-scoped-dbcontext/notes.md @@ -0,0 +1,40 @@ +# Singleton captures a scoped DbContext (DI001) + +**Pattern:** the canonical ASP.NET Core *captive dependency*. A service registered +`AddSingleton` takes a `AddScoped` service (here an EF Core `AppDbContext`) in its +constructor. The container builds **one** scoped instance with the singleton and +holds it for the whole application lifetime — a DB connection pinned for the +process, request-specific state shared across requests, and a `DbContext` used +concurrently from multiple threads (it is not thread-safe). Microsoft surfaces it +at startup as *"Cannot consume scoped service 'AppDbContext' from singleton +'NotificationService'."* + +**Why it is exactly OwnLang's lifetime model.** A captive dependency *is* the +OWN014 region-escape rule in DI clothing: `Scoped < Singleton` (request < app), and +storing a shorter-lived value into a longer-lived owner is the violation. The core +runs the same lifetime ordering it uses for OWN014; `ownlang/di.py` +(`find_captive_dependencies`) walks the registration + constructor graph and flags +the capture **at the registration site**, naming the consuming constructor. + +**The fix (after.cs).** Inject `IServiceScopeFactory` (a singleton) instead of the +scoped service, and open a fresh scope per operation +(`using var scope = _scopes.CreateScope();`), resolving the `DbContext` inside it. +The singleton's constructor no longer depends on a scoped service, so the captive +edge is gone (DI001 silent), and the resolve is off the scope's provider rather than +an injected root `IServiceProvider`, so the service-locator rule (DI004) is silent +too. + +**Honesty / scope.** This is the DI family's first **real-world** corpus case; the +captive classifier was previously pinned only on the synthetic +`frontend/roslyn/samples/DiCaptiveSample.cs`. There is **no `case.own`**: the `.own` +DSL has no service-registration surface (DI lives in the `services` fact graph, not +the resource/flow language), so the captive cannot be hand-reduced to `.own` the way +an ownership bug can — `corpus/di/` is therefore scanned by the **dotnet +`corpus-benchmark` job only** (extractor → `services` graph → DI001), not the +Python `test_corpus` `.own` runner. `before.cs` / `after.cs` are representative of +the pattern, not a verbatim diff. The transitive, interface-registration, weak +(`DI002`), transient-`IDisposable` (`DI003`), and service-locator (`DI004`) variants +remain pinned on the synthetic sample. + +Reference: [P-006](../../../docs/proposals/P-006-di-lifetimes.md); Microsoft "DI +guidelines — scoped service as singleton" (the captive-dependency anti-pattern). diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 496ba2db..8fd8344a 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -154,9 +154,11 @@ architectural strictness, and the borrow-checker showcase): a singleton's injected **root** `IServiceProvider` via `GetService`/`GetRequiredService`), all warnings; plus the **consuming-constructor anchor** (a captive names both its registration site and the ctor that injects it, as message tail + SARIF relatedLocation). - Remaining (all deliberate-deferral / future): a real-world DI corpus case (the family is - pinned only on the synthetic sample), directly-injected `IServiceScopeFactory` recognition - (P-006 OQ#3), and the dynamic registrations that are explicit non-goals. + 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` + recognition (P-006 OQ#3), a DI005 "scope-factory misuse" detection, and the dynamic + registrations that are explicit non-goals. 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/proposals/P-006-di-lifetimes.md b/docs/proposals/P-006-di-lifetimes.md index 1a2e48a4..f467267c 100644 --- a/docs/proposals/P-006-di-lifetimes.md +++ b/docs/proposals/P-006-di-lifetimes.md @@ -17,7 +17,11 @@ `IDisposable` resolved by hand from a singleton's injected **root** `IServiceProvider` — `GetService()` / `GetRequiredService()`, the service-locator anti-pattern) extends the family to a **call site** the registration graph cannot see, also a CI-validated - warning on the same sample. + warning on the same sample. A first **real-world corpus case** now grounds DI001 outside + the synthetic sample — a singleton injecting a scoped EF `DbContext` + (`corpus/di/singleton-captures-scoped-dbcontext`), scored by the dotnet `corpus-benchmark` + job (a benchmark-only corpus: DI has no `.own` reduction, so it is not run by the Python + `test_corpus` `.own` suite). - **Depends on:** `spec/Lifetimes.md` (the region-ordering model behind OWN014), [P-001](P-001-csharp-extractor.md) (the C# seam). See [`docs/ROADMAP.md`](../ROADMAP.md) (Milestone 3). diff --git a/scripts/benchmark.py b/scripts/benchmark.py index 94263c67..9a9d3fc1 100644 --- a/scripts/benchmark.py +++ b/scripts/benchmark.py @@ -356,7 +356,7 @@ def main(argv: list[str]) -> int: help="validate the harness logic with no .NET SDK") ap.add_argument("--root", default=None, help="repo root (default: this script's repo)") ap.add_argument("--corpus", action="append", default=None, metavar="DIR", - help="corpus base dir(s) (default: corpus/real-world + corpus/wpf)") + help="corpus base dir(s) (default: corpus/real-world + corpus/wpf + corpus/di)") ap.add_argument("--min-recall", type=_non_negative_int, default=0, metavar="N", help="fail if fewer than N before.cs cases are caught (the pinned " "recall floor; specificity + zero-FP are always required)") @@ -365,7 +365,8 @@ def main(argv: list[str]) -> int: return _selftest() root = args.root or os.path.dirname(os.path.dirname(os.path.abspath(__file__))) corpus_dirs = args.corpus or [os.path.join(root, "corpus", "real-world"), - os.path.join(root, "corpus", "wpf")] + os.path.join(root, "corpus", "wpf"), + os.path.join(root, "corpus", "di")] return run(root, corpus_dirs, args.min_recall) From 04a8a92fef80ccb37c5a0996209b213bc21574de Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 25 Jun 2026 17:31:02 +0000 Subject: [PATCH 3/6] =?UTF-8?q?feat(di):=20DI005=20=E2=80=94=20scoped=20se?= =?UTF-8?q?rvice=20cached=20from=20a=20created=20scope=20(the=20fix=20done?= =?UTF-8?q?=20wrong)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A new captive-family member. A singleton that injects IServiceScopeFactory and opens a scope (CreateScope()) — the *correct* remedy for a DI001 captive — but then CACHES the scope-resolved scoped service into a field. The `using` scope is disposed when the operation ends, so the cached instance dangles (use-after-dispose) and is promoted to application lifetime: the captive returns, hidden behind the very API meant to fix it, invisible to a static surface that "sees the fix". - Core (ownlang/di.py): `scope_cached` / `scope_cache_sites` Service fields, a `ScopeCachedCaptive` finding, and `find_scope_cached_captives` — a singleton whose scope_cached names a SCOPED service (a cached singleton is shareable; a cached transient is the DI003/DI004 family). A store-site property, anchored at the field assignment like DI004 anchors at its call site. - Bridge (ownlang/ownir.py): DI005 Finding at the store site (warning) with the registration as the related secondary; strict load() validation of the new fields. Pinned by 5 new ownir checks (locally verified, 118 -> 123). - Extractor (Program.cs): collect scope-creator names (injected IServiceScopeFactory / provider, same this-field discipline as DI004), the scope locals their CreateScope() produces, and each `scope.ServiceProvider.Get(Required)Service()` ASSIGNED TO A FIELD -> scope_cached + its store site. - Sample + CI: DiCaptiveSample.cs gains ScopeCachingService (flagged) and the ScopeUsingService / ClockCachingService controls (used-in-scope local; cached singleton) that must stay silent; CI asserts the DI005 finding, count, store-site anchor (line 154), and the controls. P-006 / ROADMAP updated. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Fmwk5ruExKy5fQ77CJ2NtF --- .github/workflows/ci.yml | 24 +++- docs/ROADMAP.md | 13 ++- docs/proposals/P-006-di-lifetimes.md | 26 ++++- frontend/roslyn/OwnSharp.Extractor/Program.cs | 107 ++++++++++++++++++ frontend/roslyn/samples/DiCaptiveSample.cs | 61 ++++++++++ ownlang/di.py | 75 ++++++++++++ ownlang/ownir.py | 52 +++++++++ tests/test_ownir.py | 55 +++++++++ 8 files changed, 405 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8637ca09..aed7b417 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -586,7 +586,7 @@ jobs: # the registration site rides along as the SECONDARY anchor (named in EACH DI004 # message tail) — exactly 3, one per finding, so a partial-suffix regression (the tail # on some findings but not all) fails CI too (CodeRabbit review; mirrors the counts above). - nreg=$(echo "$out" | grep -c "singleton registered at ") + nreg=$(echo "$out" | grep -cE "\[DI004\].*singleton registered at ") [ "$nreg" = "3" ] \ || { echo "FAIL: expected exactly 3 DI004 registration-site suffixes, got $nreg"; exit 1; } # the three controls each pin one precision guard and must stay SILENT: ScopedResolver @@ -597,7 +597,27 @@ jobs: if echo "$out" | grep -qE "(ScopedResolver|PlainResolver|RequestResolver)"; then echo "FAIL: a correct/non-leaking resolver (scope-resolved, non-disposable, or scoped) was wrongly flagged DI004"; exit 1 fi - echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) + DI001 (captive dependency) + DI002 (scoped captured weakly) + DI003 (transient IDisposable captured by a singleton) + DI004 (transient IDisposable service-located from the root provider) at the C# location" + # P-006 DI005 (scope-cached captive, WARNING): a singleton that resolves a SCOPED service + # from a scope it CREATES (the correct IServiceScopeFactory pattern) but CACHES it into a + # field — the scope is disposed when the operation ends, so the cached instance dangles + # and is promoted to application lifetime (the captive returns, hidden behind the fix). + echo "$out" | grep -qE "\[DI005\].*'ScopeCachingService' caches scoped service 'AppDbContext'" \ + || { echo "FAIL: expected DI005 (ScopeCachingService caches scope-resolved scoped AppDbContext)"; exit 1; } + n5=$(echo "$out" | grep -cE "DiCaptiveSample\.cs:[0-9]+:.*\[DI005\]") + [ "$n5" = "1" ] \ + || { echo "FAIL: expected exactly 1 DI005 finding, got $n5"; exit 1; } + # DI005's consumer is the field-STORE site (not a ctor), the PRIMARY anchor — line 154, + # the `_db = scope.ServiceProvider.GetRequiredService()` store — with the + # registration as the secondary suffix. + echo "$out" | grep -qE "DiCaptiveSample\.cs:154: warning: \[DI005\].*'ScopeCachingService'" \ + || { echo "FAIL: expected DI005 anchored at the field-store site (line 154)"; exit 1; } + # two controls stay SILENT: ScopeUsingService USES the scope-resolved service within the + # scope (a local, not a field store); ClockCachingService caches a SINGLETON (shareable, + # not a scoped service). Neither is a captive. + if echo "$out" | grep -qE "(ScopeUsingService|ClockCachingService)"; then + echo "FAIL: a correct scope use (used-in-scope, or a cached singleton) was wrongly flagged DI005"; exit 1 + fi + echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) + DI001 (captive dependency) + DI002 (scoped captured weakly) + DI003 (transient IDisposable captured by a singleton) + DI004 (transient IDisposable service-located from the root provider) + DI005 (scoped service cached from a created scope) at the C# location" - name: Flow-sensitive local IDisposables (--flow-locals, P-016 B0b/B2) run: | # Path-sensitive flow analysis of local IDisposables — bugs the flat D1 diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 8fd8344a..29b2652e 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -154,11 +154,14 @@ architectural strictness, and the borrow-checker showcase): a singleton's injected **root** `IServiceProvider` via `GetService`/`GetRequiredService`), all warnings; plus the **consuming-constructor anchor** (a captive names both its registration site and the ctor that injects it, as message tail + SARIF relatedLocation). - 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` - recognition (P-006 OQ#3), a DI005 "scope-factory misuse" detection, and the dynamic - registrations that are explicit non-goals. + **DI005** (the fix done wrong — a singleton that injects `IServiceScopeFactory`, opens a + scope, but **caches** the scope-resolved **scoped** service into a field, so it dangles after + 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. 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/proposals/P-006-di-lifetimes.md b/docs/proposals/P-006-di-lifetimes.md index f467267c..b5827504 100644 --- a/docs/proposals/P-006-di-lifetimes.md +++ b/docs/proposals/P-006-di-lifetimes.md @@ -17,7 +17,14 @@ `IDisposable` resolved by hand from a singleton's injected **root** `IServiceProvider` — `GetService()` / `GetRequiredService()`, the service-locator anti-pattern) extends the family to a **call site** the registration graph cannot see, also a CI-validated - warning on the same sample. A first **real-world corpus case** now grounds DI001 outside + warning on the same sample. **DI005** (a singleton that resolves a **scoped** service from a + scope it *creates* — the correct `IServiceScopeFactory` pattern — but **caches it into a + field**, so the cached instance dangles after the scope is disposed and is promoted to + application lifetime: the captive returns, hidden behind the API meant to fix it) is the + newest member — a **store-site** property (the field assignment), anchored there like DI004 + anchors at its call site; CI-validated on the same sample (`ScopeCachingService` flagged; + the used-in-scope `ScopeUsingService` and the cached-singleton `ClockCachingService` silent). + A first **real-world corpus case** now grounds DI001 outside the synthetic sample — a singleton injecting a scoped EF `DbContext` (`corpus/di/singleton-captures-scoped-dbcontext`), scored by the dotnet `corpus-benchmark` job (a benchmark-only corpus: DI has no `.own` reduction, so it is not run by the Python @@ -76,6 +83,23 @@ to a longer-lived region) already models it. restricted to real fields (no local-alias false match) — each pinned by a control on `DiCaptiveSample.cs` (`ConnectionResolver` / `ExprBodiedResolver` / transitive `WrapperResolver` flagged; `ScopedResolver` / `PlainResolver` / `RequestResolver` silent). +- **DI005 (warning) — shipped:** the *fix done wrong*. A singleton that **does** inject + `IServiceScopeFactory` (or its provider) and opens a scope (`CreateScope()`) — the remedy + suggested below — but then **caches the scope-resolved scoped service into a field**. The + `using` scope is disposed when the operation ends, so the cached instance is used after the + scope (and the service) is disposed (use-after-dispose) *and* lives for the application + lifetime: the captive the scope was meant to avoid, now invisible to the static surface that + "sees the fix". The extractor records the scope-creator names (injected `IServiceScopeFactory` + / provider, with the same this-field discipline as DI004), the scope locals their + `CreateScope()` produces, and each `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` flags a singleton whose `scope_cached` names a **scoped** service. + A **store-site** property (anchored at the field assignment, like DI004's call site), filed as a + distinct code (different detection, different fix: resolve inside the scope per use, do not + cache). Precision guards — singleton-only, scoped-cached-type-only (a cached singleton is + shareable, a cached transient is the DI003/DI004 family), real-field store only (a value used + in the scope and discarded is a local, not a field) — pinned on `DiCaptiveSample.cs` + (`ScopeCachingService` flagged; `ScopeUsingService` / `ClockCachingService` silent). Suggested fix attached to DI001/DI002: inject `IServiceScopeFactory`, and per operation `using var scope = factory.CreateScope();` then resolve the scoped diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 7189d5c5..2b29a5fa 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -1909,6 +1909,13 @@ static List ExtractServices(List<(string file, SyntaxTree tree)> parsed) // DI004's consumer is the GetRequiredService call site (not a ctor), so a finding anchors // at it; emitted as `root_resolve_sites` alongside `root_resolves`. var classRootResolveSites = new Dictionary>(); + // class name -> service types it resolves from a scope it CREATES (`factory.CreateScope()` / + // an injected provider's `.CreateScope()`) and then CACHES into a FIELD (DI005 — the + // scope-per-operation fix done wrong), with the field-STORE call site of each. A scope-resolved + // value USED in the scope and discarded (the correct shape) is not assigned to a field, so it + // never enters here. + var classScopeCached = new Dictionary>(); + var classScopeCacheSites = new Dictionary>(); // class name -> its CONSUMING CONSTRUCTOR location (file, 1-based line): the widest // public ctor (or the class/primary-ctor declaration), where a captive dependency is // injected. A captive finding anchors at the registration site but names this too, so @@ -2011,6 +2018,45 @@ static List ExtractServices(List<(string file, SyntaxTree tree)> parsed) } classRootResolves[cls.Identifier.Text] = rootResolves; classRootResolveSites[cls.Identifier.Text] = rootResolveSites; + + // DI005 — names whose `.CreateScope()` creates a child scope: an injected + // `IServiceScopeFactory`, plus the injected provider itself (an injected provider's + // `.CreateScope()` is equally a scope). A scoped service resolved off such a scope and + // CACHED into a field (rather than used within the scope) is the scope-per-operation fix + // done wrong. Collected with the SAME this-field discipline as providerNames so a local + // alias never enters (no false store-match). + var scopeCreatorNames = new HashSet(providerNames); + if (widest is not null) + foreach (var p in widest.Parameters) + if (p.Type is not null && DiTypeName(p.Type) == "IServiceScopeFactory") + scopeCreatorNames.Add(p.Identifier.Text); + foreach (var mem in cls.Members) + if (mem is ConstructorDeclarationSyntax scfCtor) + foreach (var asg in scfCtor.DescendantNodes().OfType()) + if (asg.Right is IdentifierNameSyntax scfRhs + && scopeCreatorNames.Contains(scfRhs.Identifier.Text) + && AssignedFieldName(asg.Left) is { } scfFld + && classFieldNames.Contains(scfFld)) + scopeCreatorNames.Add(scfFld); + foreach (var fdecl in cls.Members.OfType()) + foreach (var v in fdecl.Declaration.Variables) + if (v.Initializer?.Value is IdentifierNameSyntax scfInit + && scopeCreatorNames.Contains(scfInit.Identifier.Text)) + scopeCreatorNames.Add(v.Identifier.Text); + var scopeCached = new List(); + var scopeCacheSites = new List(); + if (scopeCreatorNames.Count > 0) + { + var seenCached = new HashSet(); + foreach (var (cachedType, line) in ScopeCachedTypes(cls, scopeCreatorNames, classFieldNames)) + if (seenCached.Add(cachedType)) // first store per type wins + { + scopeCached.Add(cachedType); + scopeCacheSites.Add(new { type = cachedType, file = ctorFile, line }); + } + } + classScopeCached[cls.Identifier.Text] = scopeCached; + classScopeCacheSites[cls.Identifier.Text] = scopeCacheSites; } // 2. registrations -> service facts at the registration site. @@ -2035,6 +2081,10 @@ static List ExtractServices(List<(string file, SyntaxTree tree)> parsed) ? rr : new List(); var rootResolveSites = impl is not null && classRootResolveSites.TryGetValue(impl, out var rrs) ? rrs : new List(); + var scopeCached = impl is not null && classScopeCached.TryGetValue(impl, out var sca) + ? sca : new List(); + var scopeCacheSites = impl is not null + && classScopeCacheSites.TryGetValue(impl, out var scas) ? scas : new List(); var (ctorFile, ctorLine) = impl is not null && classCtorLoc.TryGetValue(impl, out var cl) ? cl : ("?", 0); services.Add(new @@ -2047,6 +2097,11 @@ static List ExtractServices(List<(string file, SyntaxTree tree)> parsed) // disposes the impl, so a transient-disposable impl captured by a // singleton is held to app exit (DI003). disposable = impl is not null && ctorDisposable.TryGetValue(impl, out var disp) && disp, + // the IMPLEMENTATION's scope-cached scoped services (DI005): a scoped service + // resolved off a scope it creates and cached into a field — the captive the scope + // was meant to avoid, plus the field-store site each finding anchors at. + scope_cached = scopeCached, + scope_cache_sites = scopeCacheSites, // the IMPLEMENTATION's by-hand resolutions off its injected provider — for a // singleton, the root; a transient IDisposable resolved this way is DI004. root_resolves = rootResolves, @@ -2182,6 +2237,7 @@ static void ResolveRegistration(SimpleNameSyntax name, ArgumentListSyntax args, // (identifier) or `this._provider` (this-qualified field)? A `scope.ServiceProvider` // receiver is a member access NOT qualified by `this`, so it returns false: only the // injected ROOT provider, never a scope's provider, is treated as a root resolution. +// Reused by DI005 to test a `.CreateScope()` receiver against the scope-creator names. static bool ReceiverIsProvider(ExpressionSyntax recv, HashSet providerNames) => recv switch { IdentifierNameSyntax id => providerNames.Contains(id.Identifier.Text), @@ -2190,6 +2246,57 @@ static void ResolveRegistration(SimpleNameSyntax name, ArgumentListSyntax args, _ => false, }; +// Is `inv` a `.CreateScope()` call whose receiver is one of the scope-creator names (an +// injected IServiceScopeFactory or provider)? The `IServiceScopeFactory.CreateScope()` / +// `IServiceProvider.CreateScope()` that opens a child scope (DI005). +static bool IsCreateScopeOff(InvocationExpressionSyntax inv, HashSet creators) => + inv.Expression is MemberAccessExpressionSyntax { Name.Identifier.Text: "CreateScope" } m + && ReceiverIsProvider(m.Expression, creators); + +// The type `T` of a `.ServiceProvider.GetService()` / `GetRequiredService()` +// expression, where `` is a scope-local declared from `CreateScope()` OR an inline +// `.CreateScope()` chain — else null (DI005). Single type argument only (the generic +// resolve form). The `.ServiceProvider` receiver is what distinguishes a scope resolution from a +// root one (DI004, off the injected provider directly). +static string? ScopeResolvedType( + ExpressionSyntax expr, HashSet scopeLocals, HashSet creators) +{ + if (expr is not InvocationExpressionSyntax inv + || inv.Expression is not MemberAccessExpressionSyntax ma + || ma.Name is not GenericNameSyntax gen + || gen.TypeArgumentList.Arguments.Count != 1 + || gen.Identifier.Text is not ("GetService" or "GetRequiredService") + || ma.Expression is not MemberAccessExpressionSyntax { Name.Identifier.Text: "ServiceProvider" } sp) + return null; + var fromScope = sp.Expression switch + { + IdentifierNameSyntax id => scopeLocals.Contains(id.Identifier.Text), + InvocationExpressionSyntax cs => IsCreateScopeOff(cs, creators), // inline CreateScope().ServiceProvider + _ => false, + }; + return fromScope ? DiTypeName(gen.TypeArgumentList.Arguments[0]) : null; +} + +// The service types a class resolves off a scope it CREATES and CACHES into a FIELD, with the +// 1-based line of the field store (DI005). Recognises a field assignment +// `_f = scope.ServiceProvider.Get(Required)Service()` where `scope` is a local declared from +// `.CreateScope()`, and the inline `_f = .CreateScope().ServiceProvider. +// Get(Required)Service()`. The LHS must be a real field (AssignedFieldName + classFieldNames) — +// a scope-resolved value used within the scope and discarded (the CORRECT pattern) is a local, not +// a field store, so it is never recorded. +static IEnumerable<(string type, int line)> ScopeCachedTypes( + ClassDeclarationSyntax cls, HashSet creators, HashSet classFieldNames) +{ + var scopeLocals = new HashSet(StringComparer.Ordinal); + foreach (var v in cls.DescendantNodes().OfType()) + if (v.Initializer?.Value is InvocationExpressionSyntax cs && IsCreateScopeOff(cs, creators)) + scopeLocals.Add(v.Identifier.Text); + foreach (var asg in cls.DescendantNodes().OfType()) + if (AssignedFieldName(asg.Left) is { } fld && classFieldNames.Contains(fld) + && ScopeResolvedType(asg.Right, scopeLocals, creators) is { } t) + yield return (t, asg.GetLocation().GetLineSpan().StartLinePosition.Line + 1); +} + // DI's default IServiceProvider resolves through PUBLIC constructors only — an // explicit ctor with no access modifier defaults to private and DI never uses it. static bool IsPublicCtor(SyntaxTokenList modifiers) diff --git a/frontend/roslyn/samples/DiCaptiveSample.cs b/frontend/roslyn/samples/DiCaptiveSample.cs index 147045b8..b49ae9a8 100644 --- a/frontend/roslyn/samples/DiCaptiveSample.cs +++ b/frontend/roslyn/samples/DiCaptiveSample.cs @@ -137,6 +137,54 @@ public sealed class WrapperResolver(IServiceProvider sp) public void Warm() { var m = _sp.GetRequiredService(); } // DI004 (transitive) } + // DI005 — a singleton that resolves a SCOPED service from a scope it CREATES (the correct + // IServiceScopeFactory pattern) but CACHES it into a FIELD. The `using` scope is disposed when + // the operation ends, so `_db` dangles (use-after-dispose) and is promoted to the singleton's + // application lifetime — the captive returns, hidden behind the very API meant to fix it. The + // static surface "sees the fix" (CreateScope) and would otherwise stay silent, which is exactly + // why this needs a dedicated check. A warning, anchored at the field store. + public sealed class ScopeCachingService + { + private readonly IServiceScopeFactory _scopes; + private AppDbContext _db; // cached scoped service (app-lived) + public ScopeCachingService(IServiceScopeFactory scopes) { _scopes = scopes; } + public void Warm() + { + using var scope = _scopes.CreateScope(); + _db = scope.ServiceProvider.GetRequiredService(); // DI005 — cached into a field + } + } + + // control: creates a scope and USES the scoped service WITHIN the scope, not caching it — the + // CORRECT scope-per-operation pattern. The resolved value is a LOCAL, never a field store, so + // DI005 stays SILENT. + public sealed class ScopeUsingService + { + private readonly IServiceScopeFactory _scopes; + public ScopeUsingService(IServiceScopeFactory scopes) { _scopes = scopes; } + public void Run() + { + using var scope = _scopes.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); // SILENT — used locally + _ = db; + } + } + + // control: caches a SINGLETON service (Clock) resolved from a scope into a field — a singleton + // is shareable for the whole app, so caching it is no lifetime violation. SILENT (the cached + // type is not scoped; the core gates DI005 on a scoped cached type). + public sealed class ClockCachingService + { + private readonly IServiceScopeFactory _scopes; + private Clock _clock; + public ClockCachingService(IServiceScopeFactory scopes) { _scopes = scopes; } + public void Warm() + { + using var scope = _scopes.CreateScope(); + _clock = scope.ServiceProvider.GetRequiredService(); // SILENT — Clock is singleton + } + } + public static class Startup { public static void ConfigureServices(IServiceCollection services) @@ -203,6 +251,14 @@ public static void ConfigureServices(IServiceCollection services) // off the root, which drags in the transient IDisposable PooledConnection (tracked to app exit). services.AddTransient(); services.AddSingleton(); + + // FLAGGED (DI005, warning) — ScopeCachingService resolves scoped AppDbContext from a + // scope it creates and CACHES it into a field (the scope-per-operation fix done wrong). + services.AddSingleton(); + // SILENT — ScopeUsingService uses the scope-resolved service within the scope (not cached). + services.AddSingleton(); + // SILENT — ClockCachingService caches a SINGLETON (shareable), not a scoped service. + services.AddSingleton(); } } @@ -225,6 +281,11 @@ public static class ServiceCollectionExtensions // Microsoft.Extensions.DependencyInjection extensions — provided here so the sample binds. public interface IServiceScope : IDisposable { IServiceProvider ServiceProvider { get; } } + // IServiceScopeFactory — the singleton-safe way to open a child scope (DI005). Injecting it + // and resolving scoped services per operation is the FIX for a captive; caching what it + // resolves into a field is the misuse this check catches. + public interface IServiceScopeFactory { IServiceScope CreateScope(); } + public static class ServiceProviderExtensions { public static T GetRequiredService(this IServiceProvider sp) => default!; diff --git a/ownlang/di.py b/ownlang/di.py index bdc21818..b8320ef8 100644 --- a/ownlang/di.py +++ b/ownlang/di.py @@ -90,6 +90,17 @@ class Service: # site. The consumer of a DI004 leak is this call site (not a ctor), so the finding anchors # at it; optional presentation metadata, declared LAST (positional-contract safe). root_resolve_sites: tuple[tuple[str, str, int], ...] = () + # service types this class resolves from a scope it CREATES (`IServiceScopeFactory. + # CreateScope()` / an injected provider's `.CreateScope()`) and then CACHES into a FIELD — + # the "scope-per-operation fix" done wrong (DI005). The scope is disposed at the end of the + # operation, so a cached scoped service both dangles (use-after-dispose) and is promoted to + # the singleton's application lifetime. Off the registration graph (a call site + a field + # store), gated on SINGLETON + cached type SCOPED in the core. Declared LAST (positional + # contract safe), with its cache-site metadata after it. + scope_cached: tuple[str, ...] = () + # for DI005: where each `scope_cached` type was cached — `(type, file, line)` of the field + # assignment, the finding's anchor (the leak is that store). Optional, declared LAST. + scope_cache_sites: tuple[tuple[str, str, int], ...] = () @dataclass(frozen=True) @@ -381,3 +392,67 @@ def find_explicit_root_resolutions( stack.append((dep, (*path, dep))) findings.sort(key=lambda f: (f.file, f.line, f.singleton, f.resolved)) return findings + + +@dataclass(frozen=True) +class ScopeCachedCaptive: + """A singleton that resolves a **scoped** service from a scope it CREATES + (`IServiceScopeFactory.CreateScope()`) and then **caches it into a field** (DI005). + The scope-per-operation pattern is the *correct* fix for a DI001 captive — but only + when the resolved service is used within the scope and discarded. Caching it into a + field defeats that twice over: the field outlives the `using` scope, so the cached + instance is used after the scope (and the service) is disposed (use-after-dispose), + and it lives for the singleton's application lifetime — the captive is back, hidden + behind the API that was supposed to fix it. The static surface "sees the fix" + (`CreateScope`) and would otherwise stay silent, which is exactly what makes this + worth a dedicated check.""" + + singleton: str + captured: str + file: str + line: int + # the field-assignment call site where the scope-resolved service was cached — DI005's + # consumer (the leak is that store), so the bridge anchors the finding here; the + # registration `file`/`line` become the secondary. Unknown -> 0. + cached_file: str = "?" + cached_line: int = 0 + + @property + def message(self) -> str: + reg = (f" [singleton registered at {self.file}:{self.line}]" + if self.cached_line >= 1 and self.line >= 1 else "") + return (f"singleton '{self.singleton}' caches scoped service '{self.captured}', " + f"resolved from a scope it creates, into a field: the scope is disposed when " + f"the operation ends, so the cached instance dangles (use-after-dispose) and " + f"is promoted to application lifetime — the captive the scope was meant to " + f"avoid. Resolve it inside the scope per use and do not cache it{reg}") + + +def find_scope_cached_captives( + services: list[Service]) -> list[ScopeCachedCaptive]: + """Return every scoped service a singleton resolves from a scope it creates and caches + into a field (DI005). Only **singletons** are considered (a scoped/transient consumer's + cached value lives no longer than its own short scope — no promotion). A cached type that + is `scoped` in the registration graph is the captive; a cached `singleton`/`transient` + type is not this violation (a singleton is shareable; a transient cached in a field is the + DI003/DI004 promotion family, surfaced there). The extractor records only values cached + into a FIELD off a self-created scope — a value used within the scope and discarded (the + correct pattern) produces no `scope_cached` entry, so it stays silent.""" + by_name = {s.name: s for s in services} + findings: list[ScopeCachedCaptive] = [] + for s in services: + if s.lifetime != SINGLETON: + continue + sites = {t: (f, ln) for (t, f, ln) in s.scope_cache_sites} + reported: set[str] = set() + for dep in s.scope_cached: + node = by_name.get(dep) + if node is None or node.lifetime != SCOPED or dep in reported: + continue + reported.add(dep) + cf, cl = sites.get(dep, ("?", 0)) + findings.append(ScopeCachedCaptive( + singleton=s.name, captured=dep, file=s.file, line=s.line, + cached_file=cf, cached_line=cl)) + findings.sort(key=lambda f: (f.file, f.line, f.singleton, f.captured)) + return findings diff --git a/ownlang/ownir.py b/ownlang/ownir.py index 1ed9791f..e7e6f8ff 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -130,6 +130,7 @@ find_captive_dependencies, find_captured_transient_disposables, find_explicit_root_resolutions, + find_scope_cached_captives, find_weak_captive_dependencies, ) from .diagnostics import TITLES, Severity @@ -501,6 +502,21 @@ def load(path: str) -> dict[str, Any]: raise OwnIRError( "service 'root_resolve_sites' must be an array of " "{type:str, file:str, line:int} objects") + # DI005 (scope-cached captive): types resolved from a self-created scope and cached + # into a field, plus their field-store sites — validated like root_resolves / its sites. + scope_cached = s.get("scope_cached", []) + if not isinstance(scope_cached, list) or not all( + isinstance(d, str) for d in scope_cached): + raise OwnIRError("service 'scope_cached' must be an array of strings") + csites = s.get("scope_cache_sites", []) + if not isinstance(csites, list) or not all( + isinstance(x, dict) and isinstance(x.get("type", ""), str) + and isinstance(x.get("file", "?"), str) + and isinstance(x.get("line", 0), int) and not isinstance(x.get("line", 0), bool) + for x in csites): + raise OwnIRError( + "service 'scope_cache_sites' must be an array of " + "{type:str, file:str, line:int} objects") # Optional per-method flow bodies (P-016 B0b/B2 — local IDisposable # acquire/use/release over a CFG). Additive/optional; an older core ignores it. fns = result.get("functions", []) @@ -1344,6 +1360,25 @@ def _di004_related(c: Any) -> tuple[tuple[str, int, str], ...]: return () +def _di005_primary(c: Any) -> tuple[str, int]: + """DI005's primary anchor — the field-assignment **cache site** (where the scope-resolved + service is stored and where the leak is), falling back to the registration site when the + extractor did not record it. Like DI004, DI005 is a call-/store-site property, not a + registration-graph one, so the store site is the primary.""" + if getattr(c, "cached_line", 0) >= 1: + return (c.cached_file, c.cached_line) + return (c.file, c.line) + + +def _di005_related(c: Any) -> tuple[tuple[str, int, str], ...]: + """The DI005 **registration** site as a structured related location — the secondary anchor + beside the cache-site primary. Empty when the cache site is unknown (then the registration + is already the primary) or the registration line is unknown.""" + if getattr(c, "cached_line", 0) >= 1 and getattr(c, "line", 0) >= 1: + return ((c.file, c.line, f"registration of singleton '{c.singleton}'"),) + return () + + def _resolve_sites(raw: Any) -> tuple[tuple[str, str, int], ...]: """Parse a service's optional `root_resolve_sites` (DI004 call-site metadata) into `(type, file, line)` triples. Tolerant for direct `check_facts` callers; `load()` does the @@ -1388,6 +1423,10 @@ def _di_findings(facts: dict[str, Any]) -> list[Finding]: # DI004 call-site metadata: where each root_resolves type was hand-resolved, so the # finding can anchor at the GetRequiredService call site (its real consumer). root_resolve_sites=_resolve_sites(s.get("root_resolve_sites", [])), + # DI005: types resolved from a self-created scope and cached into a field, plus + # where each was cached (the field-store site the finding anchors at). + scope_cached=tuple(s.get("scope_cached", [])), + scope_cache_sites=_resolve_sites(s.get("scope_cache_sites", [])), ) for s in raw if isinstance(s, dict) ] @@ -1436,6 +1475,19 @@ def _di_findings(facts: dict[str, Any]) -> list[Finding]: component=c.singleton, event=c.resolved, handler="", message=c.message, kind="DI lifetime", severity="warning", related=_di004_related(c))) + # DI005: a singleton that resolves a scoped service from a scope it CREATES + # (IServiceScopeFactory.CreateScope()) and CACHES it into a field — the scope-per-operation + # fix done wrong. The cached instance dangles after the scope is disposed (use-after-dispose) + # and is promoted to application lifetime (the captive returns, hidden behind the API that + # was meant to fix it). A warning, anchored at the field-store site (its real consumer), with + # the registration as the secondary — the store-site twin of DI004's call-site anchoring. + for sc in find_scope_cached_captives(services): + pf, pl = _di005_primary(sc) + out.append(Finding( + file=pf, line=pl, code="DI005", + component=sc.singleton, event=sc.captured, handler="", + message=sc.message, kind="DI lifetime", severity="warning", + related=_di005_related(sc))) return out diff --git a/tests/test_ownir.py b/tests/test_ownir.py index a7dcc02a..83ded9f3 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -691,6 +691,61 @@ def _sub(source: str | None) -> list[Finding]: if any(x.code in ("DI001", "DI002", "DI003") for x in di4b): fails.append("DI004 wrongly also produced a graph DI00x finding") + # --- DI005 (P-006): a singleton that resolves a SCOPED service from a scope it CREATES + # (IServiceScopeFactory.CreateScope()) and CACHES it into a field — the scope-per-op + # fix done wrong (warning). Only singletons; only a SCOPED cached type (a transient or + # singleton cached value is not this violation); a scope-resolved value USED in the scope + # and not cached produces no `scope_cached` entry, so it stays silent. + from ownlang.di import find_scope_cached_captives + csvcs = [ + # caches scoped -> DI005, store site 21: + Service("Cacher", "singleton", deps=(), scope_cached=("Db",), + scope_cache_sites=(("Db", "C.cs", 21),)), + Service("Db", "scoped", ()), + Service("CacheTmp", "singleton", deps=(), scope_cached=("Tmp",)), # silent: not scoped + Service("Tmp", "transient", (), disposable=True), + Service("CacheClk", "singleton", deps=(), scope_cached=("Clk",)), # silent: singleton + Service("Clk", "singleton", ()), + Service("ReqCacher", "scoped", deps=(), scope_cached=("Db",)), # silent: not singleton + Service("GoodScope", "singleton", deps=(), scope_cached=()), # silent: not cached + ] + di5 = find_scope_cached_captives(csvcs) + checks += 1 + got5 = sorted((c.singleton, c.captured) for c in di5) + if got5 != [("Cacher", "Db")]: + fails.append(f"DI005 set wrong: {got5}") + checks += 1 + if not di5 or "use-after-dispose" not in di5[0].message: + fails.append("DI005 message missing 'use-after-dispose'") + checks += 1 + # DI005 records the field-STORE site (C.cs:21) — its real consumer — for anchoring. + c5 = di5[0] if di5 else None + if c5 is None or (c5.cached_file, c5.cached_line) != ("C.cs", 21): + fails.append(f"DI005 cache-site wrong: {(c5.cached_file, c5.cached_line) if c5 else None}") + # bridge: DI005 surfaces as a WARNING anchored at the STORE site (C.cs:21), with the + # REGISTRATION (S.cs:7) as the related secondary and named in the message tail. + di5facts = {"ownir_version": 0, "module": "X", "components": [], "functions": [], + "services": [ + {"name": "Cacher", "lifetime": "singleton", "deps": [], + "scope_cached": ["Db"], "file": "S.cs", "line": 7, + "scope_cache_sites": [{"type": "Db", "file": "C.cs", "line": 21}]}, + {"name": "Db", "lifetime": "scoped", "deps": [], "file": "S.cs", "line": 8}, + ]} + di5b = check_facts(di5facts) + checks += 1 + di5only = [x for x in di5b if x.code == "DI005"] + if (len(di5only) != 1 or di5only[0].severity != "warning" + or (di5only[0].file, di5only[0].line) != ("C.cs", 21) + or di5only[0].related != (("S.cs", 7, "registration of singleton 'Cacher'"),) + or "[singleton registered at S.cs:7]" not in di5only[0].message): + fails.append("DI005 bridge finding wrong: " + f"{[(x.file, x.line, x.related) for x in di5only]}") + checks += 1 + # DI005 is a store-site property, not a registration-graph edge: the singleton has no scoped + # ctor dependency, so it must not also produce a DI001/DI002/DI003/DI004. + if any(x.code in ("DI001", "DI002", "DI003", "DI004") for x in di5b): + fails.append("DI005 wrongly also produced another DI00x finding") + # bridge: the fixture surfaces exactly the two captive singletons as DI001 # at their registration lines; the clock/scoped-to-scoped stay silent. with open(_DI_FIXTURE, encoding="utf-8") as f: From 02d919291ac343824e99c827b26a9f6a3f61a458 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 25 Jun 2026 17:43:02 +0000 Subject: [PATCH 4/6] fix(di): DI005 follows the transient graph + review hardening (#107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex: a singleton that caches a TRANSIENT (resolved from its scope) which ctor-injects a scoped service is the same captive — the scope disposes the scoped service, but the singleton keeps the transient holding it (use-after-dispose), a shape DI001/DI003/DI004 cannot see. find_scope_cached_captives now walks each cached entry's strong transient graph like DI001 (cached scoped = direct; cached transient -> scoped = transitive), anchored at the cached entry's store site. New sample case UnitOfWorkCachingService + CI assertion (line 201, path text); core pinned by 2 new ownir checks. CodeRabbit: - ROADMAP index row DI001–DI004 -> DI001–DI005. - CI: assert the DI005 registration-site suffix on each finding (nreg5 == 2), mirroring DI004's nreg. - tests: negative load() coverage for scope_cached / scope_cache_sites (a non-array and a malformed site must raise OwnIRError), like weak_deps / root_resolve_sites. Not changed: the scope-cache-site `file` uses the per-class tree file (`ctorFile`), which IS the file containing the class and its method bodies where the store lives — identical to DI004's proven rootResolveSites; not the ctor's file in a misleading sense. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Fmwk5ruExKy5fQ77CJ2NtF --- .github/workflows/ci.yml | 23 ++++++-- docs/ROADMAP.md | 2 +- docs/proposals/P-006-di-lifetimes.md | 5 +- frontend/roslyn/samples/DiCaptiveSample.cs | 20 +++++++ ownlang/di.py | 69 +++++++++++++++------- tests/test_ownir.py | 42 ++++++++++--- 6 files changed, 127 insertions(+), 34 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aed7b417..67acefac 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -603,14 +603,29 @@ jobs: # and is promoted to application lifetime (the captive returns, hidden behind the fix). echo "$out" | grep -qE "\[DI005\].*'ScopeCachingService' caches scoped service 'AppDbContext'" \ || { echo "FAIL: expected DI005 (ScopeCachingService caches scope-resolved scoped AppDbContext)"; exit 1; } + # transitive DI005: a singleton caches the TRANSIENT UnitOfWork (which ctor-injects scoped + # AppDbContext) from a created scope — the DFS follows the cached transient's strong edges + # like DI001, so the dragged-in scoped service is found. A captive DI001/3/4 cannot see. + echo "$out" | grep -qE "\[DI005\].*'UnitOfWorkCachingService' caches scoped service 'AppDbContext'" \ + || { echo "FAIL: expected transitive DI005 (UnitOfWorkCachingService -> UnitOfWork -> AppDbContext)"; exit 1; } + echo "$out" | grep -q "UnitOfWorkCachingService -> UnitOfWork -> AppDbContext" \ + || { echo "FAIL: expected the transitive DI005 path text"; exit 1; } n5=$(echo "$out" | grep -cE "DiCaptiveSample\.cs:[0-9]+:.*\[DI005\]") - [ "$n5" = "1" ] \ - || { echo "FAIL: expected exactly 1 DI005 finding, got $n5"; exit 1; } - # DI005's consumer is the field-STORE site (not a ctor), the PRIMARY anchor — line 154, - # the `_db = scope.ServiceProvider.GetRequiredService()` store — with the + [ "$n5" = "2" ] \ + || { echo "FAIL: expected exactly 2 DI005 findings (direct + transitive), got $n5"; exit 1; } + # DI005's consumer is the field-STORE site (not a ctor), the PRIMARY anchor — the direct + # case at line 154 (`_db = ...AppDbContext`), the transitive case at the cached ENTRY's + # store (line 201, `_uow = ...UnitOfWork`, NOT the dragged-in AppDbContext) — with the # registration as the secondary suffix. echo "$out" | grep -qE "DiCaptiveSample\.cs:154: warning: \[DI005\].*'ScopeCachingService'" \ || { echo "FAIL: expected DI005 anchored at the field-store site (line 154)"; exit 1; } + echo "$out" | grep -qE "DiCaptiveSample\.cs:201: warning: \[DI005\].*'UnitOfWorkCachingService'" \ + || { echo "FAIL: expected transitive DI005 anchored at the cached-entry store site (line 201)"; exit 1; } + # the registration site rides along as the SECONDARY anchor in EACH DI005 message tail — + # exactly 2 (one per finding), so a partial-suffix regression fails CI (like DI004's nreg). + nreg5=$(echo "$out" | grep -cE "\[DI005\].*singleton registered at ") + [ "$nreg5" = "2" ] \ + || { echo "FAIL: expected exactly 2 DI005 registration-site suffixes, got $nreg5"; exit 1; } # two controls stay SILENT: ScopeUsingService USES the scope-resolved service within the # scope (a local, not a field store); ClockCachingService caches a SINGLETON (shareable, # not a scoped service). Neither is a captive. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 29b2652e..7887745c 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -237,7 +237,7 @@ own scan. Label them as estimates wherever they appear. | [P-003](proposals/P-003-lifetime-visualization.md) | Lifetime visualization (RustOwl-style) | horizon | draft | | [P-004](proposals/P-004-wpf-lifetime-profile.md) | WPF / UI lifetime leak profile | P0 | in progress (WPF001–005 built) | | [P-005](proposals/P-005-idisposable-ownership.md) | `IDisposable` ownership profile | P0 | draft | -| [P-006](proposals/P-006-di-lifetimes.md) | DI lifetime / captive dependency | P0 | in progress (DI001–DI004 end-to-end: core + extractor) | +| [P-006](proposals/P-006-di-lifetimes.md) | DI lifetime / captive dependency | P0 | in progress (DI001–DI005 end-to-end: core + extractor) | | [P-007](proposals/P-007-arraypool-span.md) | ArrayPool / Span borrow-view | P1 | in progress (POOL001–003 built; 004/005 first slices) | | [P-008](proposals/P-008-effects-and-resources.md) | Effects & resources (`Own.Effects`) | P1/P2 | draft | | [P-009](proposals/P-009-nogc-regions.md) | No-GC / allocation-free regions | horizon | draft | diff --git a/docs/proposals/P-006-di-lifetimes.md b/docs/proposals/P-006-di-lifetimes.md index b5827504..d14e7a44 100644 --- a/docs/proposals/P-006-di-lifetimes.md +++ b/docs/proposals/P-006-di-lifetimes.md @@ -93,7 +93,10 @@ to a longer-lived region) already models it. / provider, with the same this-field discipline as DI004), the scope locals their `CreateScope()` produces, and each `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` flags a singleton whose `scope_cached` names a **scoped** service. + `find_scope_cached_captives` walks each cached entry's **strong transient graph** like DI001 — + a cached **scoped** service is the captive directly, and a cached **transient** that ctor-injects + a scoped service (directly or transitively) drags it into the singleton's lifetime too (the scope + disposed it; the singleton keeps the transient holding it). A cached singleton is shareable. A **store-site** property (anchored at the field assignment, like DI004's call site), filed as a distinct code (different detection, different fix: resolve inside the scope per use, do not cache). Precision guards — singleton-only, scoped-cached-type-only (a cached singleton is diff --git a/frontend/roslyn/samples/DiCaptiveSample.cs b/frontend/roslyn/samples/DiCaptiveSample.cs index b49ae9a8..c10f0f0e 100644 --- a/frontend/roslyn/samples/DiCaptiveSample.cs +++ b/frontend/roslyn/samples/DiCaptiveSample.cs @@ -185,6 +185,23 @@ public void Warm() } } + // DI005 (transitive) — caches a TRANSIENT (UnitOfWork) resolved from a created scope into a + // field. UnitOfWork ctor-injects scoped AppDbContext; the scope disposes that DbContext when + // the operation ends, but the singleton keeps the UnitOfWork holding it (use-after-dispose). + // The DFS follows the cached transient's strong edges like DI001, so the dragged-in scoped + // service is found — a captive DI001/DI003/DI004 cannot see (no ctor edge, no root resolution). + public sealed class UnitOfWorkCachingService + { + private readonly IServiceScopeFactory _scopes; + private UnitOfWork _uow; + public UnitOfWorkCachingService(IServiceScopeFactory scopes) { _scopes = scopes; } + public void Warm() + { + using var scope = _scopes.CreateScope(); + _uow = scope.ServiceProvider.GetRequiredService(); // DI005 (transitive -> AppDbContext) + } + } + public static class Startup { public static void ConfigureServices(IServiceCollection services) @@ -259,6 +276,9 @@ public static void ConfigureServices(IServiceCollection services) services.AddSingleton(); // SILENT — ClockCachingService caches a SINGLETON (shareable), not a scoped service. services.AddSingleton(); + // FLAGGED (DI005, transitive) — caches the transient UnitOfWork (which ctor-injects + // scoped AppDbContext) from a created scope into a field. + services.AddSingleton(); } } diff --git a/ownlang/di.py b/ownlang/di.py index b8320ef8..774cd249 100644 --- a/ownlang/di.py +++ b/ownlang/di.py @@ -409,8 +409,13 @@ class ScopeCachedCaptive: singleton: str captured: str - file: str - line: int + # the cache path: singleton -> cached entry -> ... -> the scoped service. A direct cache is + # `(singleton, scoped)`; a TRANSIENT cached entry that drags in a scoped service transitively + # is `(singleton, transient, ..., scoped)` — the same captive, since the singleton keeps the + # transient and the transient holds the scoped service the scope already disposed. + path: tuple[str, ...] = () + file: str = "?" + line: int = 0 # the field-assignment call site where the scope-resolved service was cached — DI005's # consumer (the leak is that store), so the bridge anchors the finding here; the # registration `file`/`line` become the secondary. Unknown -> 0. @@ -421,38 +426,60 @@ class ScopeCachedCaptive: def message(self) -> str: reg = (f" [singleton registered at {self.file}:{self.line}]" if self.cached_line >= 1 and self.line >= 1 else "") + chain = " -> ".join(self.path) return (f"singleton '{self.singleton}' caches scoped service '{self.captured}', " f"resolved from a scope it creates, into a field: the scope is disposed when " f"the operation ends, so the cached instance dangles (use-after-dispose) and " - f"is promoted to application lifetime — the captive the scope was meant to " - f"avoid. Resolve it inside the scope per use and do not cache it{reg}") + f"'{self.captured}' is promoted to application lifetime — the captive the scope " + f"was meant to avoid. Resolve it inside the scope per use and do not cache it " + f"({chain}){reg}") def find_scope_cached_captives( services: list[Service]) -> list[ScopeCachedCaptive]: - """Return every scoped service a singleton resolves from a scope it creates and caches - into a field (DI005). Only **singletons** are considered (a scoped/transient consumer's - cached value lives no longer than its own short scope — no promotion). A cached type that - is `scoped` in the registration graph is the captive; a cached `singleton`/`transient` - type is not this violation (a singleton is shareable; a transient cached in a field is the - DI003/DI004 promotion family, surfaced there). The extractor records only values cached - into a FIELD off a self-created scope — a value used within the scope and discarded (the - correct pattern) produces no `scope_cached` entry, so it stays silent.""" + """Return every scoped service a singleton reaches by caching, into a field, a value it + resolved from a scope it creates (DI005). Only **singletons** are considered (a + scoped/transient consumer's cached value lives no longer than its own short scope — no + promotion). From each cached entry, walk the STRONG transient graph exactly as DI001 does: + a cached **scoped** service is the captive directly; a cached **transient** that ctor-injects + a scoped service (directly or through further transients) drags that scoped service into the + singleton's lifetime too — the scope disposed it when the operation ended, but the singleton + keeps the transient holding it (use-after-dispose), a captive DI001/DI003/DI004 cannot see (it + is neither a registration edge nor a root-provider resolution). A cached **singleton** is + shareable — not followed. The finding anchors at the cached ENTRY's field-store site even when + the scoped is reached transitively (like DI004 anchors at its entry call site). The extractor + records only values cached into a FIELD off a self-created scope — a value used within the + scope and discarded (the correct pattern) produces no `scope_cached` entry, so it stays silent. + Cycles are guarded.""" by_name = {s.name: s for s in services} findings: list[ScopeCachedCaptive] = [] for s in services: if s.lifetime != SINGLETON: continue sites = {t: (f, ln) for (t, f, ln) in s.scope_cache_sites} - reported: set[str] = set() - for dep in s.scope_cached: - node = by_name.get(dep) - if node is None or node.lifetime != SCOPED or dep in reported: + reported: set[str] = set() # cached entries already reported (one finding per entry) + for entry in s.scope_cached: + if entry in reported: continue - reported.add(dep) - cf, cl = sites.get(dep, ("?", 0)) - findings.append(ScopeCachedCaptive( - singleton=s.name, captured=dep, file=s.file, line=s.line, - cached_file=cf, cached_line=cl)) + cf, cl = sites.get(entry, ("?", 0)) + visited: set[str] = set() + # DFS from the cached entry through TRANSIENTS; a SCOPED reached is the captive. + stack: list[tuple[str, tuple[str, ...]]] = [(entry, (s.name, entry))] + while stack: + cur, path = stack.pop() + node = by_name.get(cur) + if node is None: + continue + if node.lifetime == SCOPED: + reported.add(entry) + findings.append(ScopeCachedCaptive( + singleton=s.name, captured=cur, path=path, + file=s.file, line=s.line, cached_file=cf, cached_line=cl)) + break # first scoped reached from this entry — one finding per cached entry + if node.lifetime == TRANSIENT and cur not in visited: + visited.add(cur) + for dep in node.deps: # the singleton keeps the transient, so its deps too + stack.append((dep, (*path, dep))) + # a singleton dependency is shareable — not followed findings.sort(key=lambda f: (f.file, f.line, f.singleton, f.captured)) return findings diff --git a/tests/test_ownir.py b/tests/test_ownir.py index 83ded9f3..7351084b 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -702,7 +702,11 @@ def _sub(source: str | None) -> list[Finding]: Service("Cacher", "singleton", deps=(), scope_cached=("Db",), scope_cache_sites=(("Db", "C.cs", 21),)), Service("Db", "scoped", ()), - Service("CacheTmp", "singleton", deps=(), scope_cached=("Tmp",)), # silent: not scoped + # caches a TRANSIENT that ctor-injects scoped Db -> transitive DI005, store site 30: + Service("TransCacher", "singleton", deps=(), scope_cached=("Uow",), + scope_cache_sites=(("Uow", "C.cs", 30),)), + Service("Uow", "transient", deps=("Db",)), + Service("CacheTmp", "singleton", deps=(), scope_cached=("Tmp",)), # silent: no scoped dep Service("Tmp", "transient", (), disposable=True), Service("CacheClk", "singleton", deps=(), scope_cached=("Clk",)), # silent: singleton Service("Clk", "singleton", ()), @@ -712,16 +716,28 @@ def _sub(source: str | None) -> list[Finding]: di5 = find_scope_cached_captives(csvcs) checks += 1 got5 = sorted((c.singleton, c.captured) for c in di5) - if got5 != [("Cacher", "Db")]: + if got5 != [("Cacher", "Db"), ("TransCacher", "Db")]: fails.append(f"DI005 set wrong: {got5}") checks += 1 - if not di5 or "use-after-dispose" not in di5[0].message: + # the transitive cache carries the full path through the cached transient. + tpath = next((c.path for c in di5 if c.singleton == "TransCacher"), None) + if tpath != ("TransCacher", "Uow", "Db"): + fails.append(f"DI005 transitive path wrong: {tpath}") + checks += 1 + direct5 = next((c for c in di5 if c.singleton == "Cacher"), None) + if direct5 is None or "use-after-dispose" not in direct5.message: fails.append("DI005 message missing 'use-after-dispose'") checks += 1 - # DI005 records the field-STORE site (C.cs:21) — its real consumer — for anchoring. - c5 = di5[0] if di5 else None - if c5 is None or (c5.cached_file, c5.cached_line) != ("C.cs", 21): - fails.append(f"DI005 cache-site wrong: {(c5.cached_file, c5.cached_line) if c5 else None}") + # DI005 records the field-STORE site of the cached ENTRY (Cacher@C.cs:21) for anchoring — + # and the transitive case anchors at the ENTRY (Uow) store, not the dragged-in Db. + if direct5 is None or (direct5.cached_file, direct5.cached_line) != ("C.cs", 21): + fails.append(f"DI005 cache-site wrong: " + f"{(direct5.cached_file, direct5.cached_line) if direct5 else None}") + checks += 1 + trans5 = next((c for c in di5 if c.singleton == "TransCacher"), None) + if trans5 is None or (trans5.cached_file, trans5.cached_line) != ("C.cs", 30): + fails.append(f"DI005 transitive cache-site wrong: " + f"{(trans5.cached_file, trans5.cached_line) if trans5 else None}") # bridge: DI005 surfaces as a WARNING anchored at the STORE site (C.cs:21), with the # REGISTRATION (S.cs:7) as the related secondary and named in the message tail. di5facts = {"ownir_version": 0, "module": "X", "components": [], "functions": [], @@ -863,6 +879,18 @@ def _sub(source: str | None) -> list[Finding]: "services": [{"name": "X", "lifetime": "singleton", "root_resolve_sites": [{"type": "T", "line": "NaN"}]}]}): fails.append("a malformed service root_resolve_sites did not raise OwnIRError") + checks += 1 + # scope_cached (DI005) is validated like root_resolves — a non-array must raise at load. + if not _load_raises({"ownir_version": OWNIR_VERSION, "components": [], + "services": [{"name": "X", "lifetime": "singleton", + "scope_cached": "abc"}]}): + fails.append("a non-array service scope_cached did not raise OwnIRError") + checks += 1 + # scope_cache_sites (DI005 store-site metadata) must be an array of {type,file,line} objects. + if not _load_raises({"ownir_version": OWNIR_VERSION, "components": [], + "services": [{"name": "X", "lifetime": "singleton", + "scope_cache_sites": [{"type": "Db", "line": "NaN"}]}]}): + fails.append("a malformed service scope_cache_sites did not raise OwnIRError") # --- P-014 Tier A: an "unresolved-subscription" marker (the extractor could # not bind the `+=` LHS to an event) is NOT a leak — the lowering skips it From 7c399620cdad613a7de3e98401c842a3a3f2a27f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 25 Jun 2026 17:44:53 +0000 Subject: [PATCH 5/6] fix(di): DI005 scope-cache site uses the assignment's own file (CodeRabbit) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `scope_cache_sites` serialized `file = ctorFile` (the per-class tree file). That is the store's file in the single-file case, but to be unambiguous — and correct even if the cache write lives in another partial-class file — read the file from the assignment's own location span (`asg.GetLocation().GetLineSpan().Path`) instead. ScopeCachedTypes now yields (type, file, line); the emission uses that file. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Fmwk5ruExKy5fQ77CJ2NtF --- frontend/roslyn/OwnSharp.Extractor/Program.cs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 2b29a5fa..45482b35 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -2048,11 +2048,11 @@ static List ExtractServices(List<(string file, SyntaxTree tree)> parsed) if (scopeCreatorNames.Count > 0) { var seenCached = new HashSet(); - foreach (var (cachedType, line) in ScopeCachedTypes(cls, scopeCreatorNames, classFieldNames)) + foreach (var (cachedType, cacheFile, line) in ScopeCachedTypes(cls, scopeCreatorNames, classFieldNames)) if (seenCached.Add(cachedType)) // first store per type wins { scopeCached.Add(cachedType); - scopeCacheSites.Add(new { type = cachedType, file = ctorFile, line }); + scopeCacheSites.Add(new { type = cachedType, file = cacheFile, line }); } } classScopeCached[cls.Identifier.Text] = scopeCached; @@ -2284,7 +2284,7 @@ static bool IsCreateScopeOff(InvocationExpressionSyntax inv, HashSet cre // Get(Required)Service()`. The LHS must be a real field (AssignedFieldName + classFieldNames) — // a scope-resolved value used within the scope and discarded (the CORRECT pattern) is a local, not // a field store, so it is never recorded. -static IEnumerable<(string type, int line)> ScopeCachedTypes( +static IEnumerable<(string type, string file, int line)> ScopeCachedTypes( ClassDeclarationSyntax cls, HashSet creators, HashSet classFieldNames) { var scopeLocals = new HashSet(StringComparer.Ordinal); @@ -2294,7 +2294,13 @@ static bool IsCreateScopeOff(InvocationExpressionSyntax inv, HashSet cre foreach (var asg in cls.DescendantNodes().OfType()) if (AssignedFieldName(asg.Left) is { } fld && classFieldNames.Contains(fld) && ScopeResolvedType(asg.Right, scopeLocals, creators) is { } t) - yield return (t, asg.GetLocation().GetLineSpan().StartLinePosition.Line + 1); + { + // the store site's file comes from the assignment's OWN location, not the per-class + // `ctorFile` — correct even if the cache write lives in another partial-class file + // (CodeRabbit). DI005 anchors at this store, so the file must be the store's file. + var span = asg.GetLocation().GetLineSpan(); + yield return (t, span.Path, span.StartLinePosition.Line + 1); + } } // DI's default IServiceProvider resolves through PUBLIC constructors only — an From d73c33aef4fd8767b9b6b77fb24cd7b867a1f861 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 25 Jun 2026 17:49:40 +0000 Subject: [PATCH 6/6] docs(test): narrow DI005 test header for the transitive case (CodeRabbit) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header still said a cached transient is not the violation, but the transient DFS now flags a cached transient that drags in a scoped service. Reword to "the cached value must REACH a scoped service — a cached scoped type, or a transient that ctor-injects one; a cached singleton or a transient with no scoped path is not the violation." Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Fmwk5ruExKy5fQ77CJ2NtF --- tests/test_ownir.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test_ownir.py b/tests/test_ownir.py index 7351084b..fe8f3b83 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -693,9 +693,11 @@ def _sub(source: str | None) -> list[Finding]: # --- DI005 (P-006): a singleton that resolves a SCOPED service from a scope it CREATES # (IServiceScopeFactory.CreateScope()) and CACHES it into a field — the scope-per-op - # fix done wrong (warning). Only singletons; only a SCOPED cached type (a transient or - # singleton cached value is not this violation); a scope-resolved value USED in the scope - # and not cached produces no `scope_cached` entry, so it stays silent. + # fix done wrong (warning). Only singletons; the cached value must REACH a scoped service + # — a cached scoped type directly, or a cached transient that ctor-injects one (the DFS + # follows transients like DI001). A cached singleton, or a transient with NO scoped strong + # path, is not this violation; a scope-resolved value USED in the scope and not cached + # produces no `scope_cached` entry, so it stays silent. from ownlang.di import find_scope_cached_captives csvcs = [ # caches scoped -> DI005, store site 21: