From d977dc8cf62e9dbc0ad4be7678a92f4b753cb7c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 21 Jun 2026 09:45:44 +0000 Subject: [PATCH 1/4] feat(di): anchor captive findings at the consuming constructor (P-006 Q#1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DI001/DI002/DI003 keep their primary anchor at the registration site but now also name the CONSUMING CONSTRUCTOR — where the captive dependency is injected — so the developer is pointed at the code, not just the wiring. P-006 open question #1 ("anchor at the registration line, the consuming constructor, or both?") answered: both, with the capture path shown, like OWN014. - Extractor records each implementation's ctor location (the widest public ctor's declaration, or the class declaration for a C# 12 primary / implicit ctor) into ctor_file / ctor_line on the service fact. - Core appends ` [consumed by the '' constructor at :]` to the captive message (every surface) and carries it as a structured Finding.related triple, emitted as a SARIF relatedLocation — clickable + cross-file (registration in Startup.cs, ctor in EmailSender.cs). Degrades cleanly (no tail, no related) when the location is unknown, so hand-authored facts / an older extractor still produce the prior form. - ownir load() validates ctor_file/ctor_line like file/line. Pinned end-to-end by DiCaptiveSample.cs (explicit-ctor EmailSender:25, primary-ctor PrimaryCtorService:33, ConnectionWarmer:50, WeakCache:57) in the wpf-extractor CI job, the cross-file case by the di.facts.json fixture, and the SARIF related location by tests/test_ownir.py. DI004's consumer is a resolution call site (not a ctor) — its anchor is the sibling follow-up. ruff + mypy --strict clean, ownir 109/109; DI counts unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .github/workflows/ci.yml | 13 ++++++ docs/notes/di-captive-extractor.md | 31 ++++++++++++- docs/proposals/P-006-di-lifetimes.md | 12 ++++- frontend/roslyn/OwnSharp.Extractor/Program.cs | 19 +++++++- ownlang/di.py | 41 ++++++++++++++--- ownlang/ownir.py | 46 +++++++++++++++++-- tests/fixtures/ownir/di.facts.json | 4 +- tests/test_ownir.py | 40 ++++++++++++++++ 8 files changed, 189 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6902d261..51633a9c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -308,6 +308,13 @@ jobs: nd=$(echo "$out" | grep -cE "DiCaptiveSample\.cs:[0-9]+:.*\[DI001\]") [ "$nd" = "4" ] \ || { echo "FAIL: expected exactly 4 DI001 captive findings, got $nd"; exit 1; } + # P-006 Q#1: each captive finding anchors at the registration site but ALSO names its + # CONSUMING CONSTRUCTOR (where the capture is injected) — the explicit ctor line for + # EmailSender, and the class-declaration line for the C# 12 primary ctor. + echo "$out" | grep -q "consumed by the 'EmailSender' constructor at DiCaptiveSample.cs:25" \ + || { echo "FAIL: expected the consuming-constructor anchor (EmailSender ctor, line 25)"; exit 1; } + echo "$out" | grep -q "consumed by the 'PrimaryCtorService' constructor at DiCaptiveSample.cs:33" \ + || { echo "FAIL: expected the primary-constructor consuming anchor (PrimaryCtorService, line 33)"; exit 1; } # P-006 DI003 (transient IDisposable captured by a singleton, WARNING): the # singleton ConnectionWarmer holds the transient IDisposable PooledConnection # for the app lifetime (disposed only at root disposal). A warning, distinct @@ -317,6 +324,9 @@ jobs: nw=$(echo "$out" | grep -cE "DiCaptiveSample\.cs:[0-9]+:.*\[DI003\]") [ "$nw" = "1" ] \ || { echo "FAIL: expected exactly 1 DI003 finding, got $nw"; exit 1; } + # DI003 carries the consuming-constructor anchor too (ConnectionWarmer's ctor, line 50). + echo "$out" | grep -q "consumed by the 'ConnectionWarmer' constructor at DiCaptiveSample.cs:50" \ + || { echo "FAIL: expected the DI003 consuming-constructor anchor (ConnectionWarmer, line 50)"; exit 1; } # P-006 DI002 (scoped service held by a singleton via WeakReference, WARNING): # the weak ref is the usual "fix" for a DI001 captive, but scoped AppDbContext is # still root-resolved and app-lived — the lifetime contract is still violated. The @@ -341,6 +351,9 @@ jobs: nwk=$(echo "$out" | grep -cE "DiCaptiveSample\.cs:[0-9]+:.*\[DI002\]") [ "$nwk" = "3" ] \ || { echo "FAIL: expected exactly 3 DI002 findings, got $nwk"; exit 1; } + # DI002 carries the consuming-constructor anchor too (WeakCache's ctor, line 57). + echo "$out" | grep -q "consumed by the 'WeakCache' constructor at DiCaptiveSample.cs:57" \ + || { echo "FAIL: expected the DI002 consuming-constructor anchor (WeakCache, line 57)"; exit 1; } if echo "$out" | grep -q "WeakClockHolder"; then echo "FAIL: a weak ref to a singleton (WeakClockHolder) was wrongly flagged"; exit 1 fi diff --git a/docs/notes/di-captive-extractor.md b/docs/notes/di-captive-extractor.md index 6aa4e60e..a424bc92 100644 --- a/docs/notes/di-captive-extractor.md +++ b/docs/notes/di-captive-extractor.md @@ -175,9 +175,36 @@ end-to-end by `DiCaptiveSample.cs` — `ConnectionResolver` (block ctor), `ExprB `wpf-extractor` CI job, and at the graph level by `tests/test_ownir.py`. No general-purpose analyzer models this DI-container resolution contract. +## The consuming-constructor anchor (DI001/2/3, shipped) + +A captive finding's primary anchor is the **registration site** — where you wire the +lifetime, and one place to fix it. But the capture is *introduced* somewhere else: the +**consuming constructor** that injects the captive dependency (P-006 open question #1, now +answered "both"). So each DI001/DI002/DI003 finding now also names that constructor — + +- in the **message tail**: `… [consumed by the 'EmailSender' constructor at EmailSender.cs:25]` + (visible on every surface — human, GitHub annotation, MSBuild, SARIF), and +- as a structured **SARIF `relatedLocation`** (a `Finding.related` triple), which GitHub code + scanning renders as a second, clickable, labelled location — **cross-file** (the registration + may live in `Startup.cs`, the ctor in `EmailSender.cs`). + +The extractor records each implementation's ctor location — the widest **public** constructor's +declaration, or the class declaration for a C# 12 primary / implicit constructor — into +`ctor_file` / `ctor_line` on the service fact (`classCtorLoc`). The core appends the anchor when +the location is known and **degrades cleanly** (no tail, no related location) when it is not, so +hand-authored facts and an older extractor still produce the registration-anchored form. The +finding is on the **singleton**, so the consuming constructor is the singleton's — the entry of +the (possibly transitive) capture chain the path already spells out. Pinned end-to-end by +`DiCaptiveSample.cs` (the explicit-ctor `EmailSender:25`, the primary-ctor `PrimaryCtorService:33`, +`ConnectionWarmer:50`, `WeakCache:57`) in the `wpf-extractor` CI job, the cross-file case by the +`tests/fixtures/ownir/di.facts.json` fixture, and the SARIF related location by `tests/test_ownir.py`. + ## Next (separate slices) -- Anchoring the finding at the **consuming constructor** (DI001/2/3) or the **resolution call - site** (DI004) as well as the registration site (P-006 open question #1), with the path shown. +- The same **consumer anchor for DI004**, whose consumer is a **resolution call site** + (`GetRequiredService()`), not a constructor — the call-site location needs threading + through `root_resolves`; until then DI004 keeps its registration-site anchor. +- 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). diff --git a/docs/proposals/P-006-di-lifetimes.md b/docs/proposals/P-006-di-lifetimes.md index 71f7896a..a4525125 100644 --- a/docs/proposals/P-006-di-lifetimes.md +++ b/docs/proposals/P-006-di-lifetimes.md @@ -111,8 +111,16 @@ rather than guessed. ## Open questions 1. Where to anchor the diagnostic — the registration line, the consuming - constructor, or both? (Both, with the capture path shown, like OWN014's - "expected: Window — actual: App — path: …".) + constructor, or both? **(Resolved — both, with the capture path shown, like OWN014's + "expected: Window — actual: App — path: …".)** For the captive family (DI001/DI002/DI003) + the finding keeps its **primary** anchor at the registration site and names the + **consuming constructor** — where the captive is injected — both in the message tail + (`[consumed by the '' constructor at :]`) and as a structured + **SARIF `relatedLocation`** (clickable, cross-file). The extractor records each + implementation's ctor location (the widest public ctor, or the class declaration for a + primary/implicit ctor); the core appends it when known and degrades cleanly when not. + *Still open:* DI004's consumer is a **resolution call site**, not a ctor — anchoring it + there is the sibling follow-up (the call-site location is not yet threaded through). 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* diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index dd82f1ca..0febf064 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -953,7 +953,12 @@ static List ExtractServices(List<(string file, SyntaxTree tree)> parsed) // through a scope (`scope.ServiceProvider.GetRequiredService()`) has a different // receiver and is deliberately NOT recorded — that pattern is correct. var classRootResolves = new Dictionary>(); - foreach (var (_, tree) in parsed) + // 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 + // the developer is pointed at the code, not just the wiring (P-006 open question #1). + var classCtorLoc = new Dictionary(); + foreach (var (ctorFile, tree) in parsed) foreach (var node in tree.GetRoot().DescendantNodes()) { if (node is not ClassDeclarationSyntax cls) @@ -988,6 +993,12 @@ static List ExtractServices(List<(string file, SyntaxTree tree)> parsed) } ctorDeps[cls.Identifier.Text] = deps; // last decl wins (core dedups by name) ctorWeakDeps[cls.Identifier.Text] = weakDeps; + // the consuming-constructor anchor: the widest public ctor's declaration (its + // Parent), or the class declaration for a primary/implicit ctor. Points at the + // code that injects the captive, surfaced as a finding's related location. + var ctorNode = widest?.Parent ?? cls; + classCtorLoc[cls.Identifier.Text] = (ctorFile, + ctorNode.GetLocation().GetLineSpan().StartLinePosition.Line + 1); // OR across partial declarations: any part that names IDisposable makes the // type disposable, so a later `partial class C { }` (no base list, e.g. a // generated/designer file) cannot clear an earlier `partial class C : IDisposable`. @@ -1061,6 +1072,8 @@ static List ExtractServices(List<(string file, SyntaxTree tree)> parsed) ? wd : new List(); var rootResolves = impl is not null && classRootResolves.TryGetValue(impl, out var rr) ? rr : new List(); + var (ctorFile, ctorLine) = impl is not null && classCtorLoc.TryGetValue(impl, out var cl) + ? cl : ("?", 0); services.Add(new { name = service, @@ -1076,6 +1089,10 @@ static List ExtractServices(List<(string file, SyntaxTree tree)> parsed) root_resolves = rootResolves, file, line = node.GetLocation().GetLineSpan().StartLinePosition.Line + 1, + // the IMPLEMENTATION's consuming-constructor location (P-006 Q#1): where the + // captive is injected, a finding's second anchor beside the registration site. + ctor_file = ctorFile, + ctor_line = ctorLine, }); } return services; diff --git a/ownlang/di.py b/ownlang/di.py index a7a5eecf..dcc586d4 100644 --- a/ownlang/di.py +++ b/ownlang/di.py @@ -33,6 +33,17 @@ LIFETIMES = frozenset({SINGLETON, SCOPED, TRANSIENT}) +def _consumed_suffix(singleton: str, file: str, line: int) -> str: + """The ` [consumed by the '' constructor at :]` tail a captive + finding appends so it names **both** the registration site (its primary anchor) and the + *consuming constructor* where the capture is introduced (P-006 open question #1). Empty + when the ctor location is unknown (an older extractor / hand-authored facts), so the + message degrades cleanly to just the registration-anchored form.""" + if line < 1: + return "" + return f" [consumed by the '{singleton}' constructor at {file}:{line}]" + + @dataclass(frozen=True) class Service: """One DI registration: a service `name`, its `lifetime`, and the service @@ -59,6 +70,12 @@ class Service: # resolved this way is tracked to app shutdown: DI004. Off the registration graph (it is # a call site, not a ctor edge); declared LAST, after `weak_deps`, for the same reason. root_resolves: tuple[str, ...] = () + # the **consuming constructor** of this service's implementation — the ctor that injects + # the captive dependency (P-006 open question #1). `file`/`line` above point at the + # *registration* site; these point at the *code* where the capture is introduced, so a + # captive finding can name both. Declared LAST (positional-contract safe), default unknown. + ctor_file: str = "?" + ctor_line: int = 0 @dataclass(frozen=True) @@ -71,12 +88,15 @@ class CaptiveDependency: path: tuple[str, ...] file: str line: int + consumed_file: str = "?" + consumed_line: int = 0 @property def message(self) -> str: chain = " -> ".join(self.path) return (f"singleton '{self.singleton}' captures scoped service " - f"'{self.captured}' (captive dependency: {chain})") + f"'{self.captured}' (captive dependency: {chain})" + + _consumed_suffix(self.singleton, self.consumed_file, self.consumed_line)) def find_captive_dependencies(services: list[Service]) -> list[CaptiveDependency]: @@ -109,7 +129,8 @@ def find_captive_dependencies(services: list[Service]) -> list[CaptiveDependency reported.add(dep) findings.append(CaptiveDependency( singleton=s.name, captured=dep, path=npath, - file=s.file, line=s.line)) + file=s.file, line=s.line, + consumed_file=s.ctor_file, consumed_line=s.ctor_line)) continue # the violating edge is found; don't recurse past it if dnode.lifetime == TRANSIENT and dep not in visited: visited.add(dep) @@ -133,6 +154,8 @@ class WeakCaptiveDependency: path: tuple[str, ...] file: str line: int + consumed_file: str = "?" + consumed_line: int = 0 @property def message(self) -> str: @@ -141,7 +164,8 @@ def message(self) -> str: f"'{self.captured}' (WeakReference): '{self.captured}' is still resolved " f"from the root provider and promoted to application lifetime — the weak " f"reference avoids pinning it for the GC but does not fix the " - f"captive-dependency lifetime violation ({chain})") + f"captive-dependency lifetime violation ({chain})" + + _consumed_suffix(self.singleton, self.consumed_file, self.consumed_line)) def find_weak_captive_dependencies( @@ -174,7 +198,8 @@ def find_weak_captive_dependencies( reported.add(cur) findings.append(WeakCaptiveDependency( singleton=s.name, captured=cur, path=path, - file=s.file, line=s.line)) + file=s.file, line=s.line, + consumed_file=s.ctor_file, consumed_line=s.ctor_line)) continue # the violating scoped edge is found; don't recurse past it if cnode.lifetime == TRANSIENT and cur not in visited: visited.add(cur) @@ -197,13 +222,16 @@ class CapturedTransientDisposable: path: tuple[str, ...] file: str line: int + consumed_file: str = "?" + consumed_line: int = 0 @property def message(self) -> str: chain = " -> ".join(self.path) return (f"singleton '{self.singleton}' captures transient IDisposable " f"'{self.captured}': it is promoted to application lifetime and " - f"disposed only when the root provider is disposed ({chain})") + f"disposed only when the root provider is disposed ({chain})" + + _consumed_suffix(self.singleton, self.consumed_file, self.consumed_line)) def find_captured_transient_disposables( @@ -236,7 +264,8 @@ def find_captured_transient_disposables( reported.add(dep) findings.append(CapturedTransientDisposable( singleton=s.name, captured=dep, path=npath, - file=s.file, line=s.line)) + file=s.file, line=s.line, + consumed_file=s.ctor_file, consumed_line=s.ctor_line)) if dep not in visited: visited.add(dep) stack.append((dep, npath)) diff --git a/ownlang/ownir.py b/ownlang/ownir.py index b3b5c0a2..47594d6b 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -273,6 +273,11 @@ class Finding: # --severity, default error). Still a leak verdict (counts in the exit code); # only the displayed level differs. severity: str | None = None + # secondary, structured locations that explain the finding — each a + # (file, line, label) triple. The primary `file`/`line` stays the anchor; these ride + # along as SARIF `relatedLocations` (e.g. a DI captive's *consuming constructor*, distinct + # from its registration-site anchor). Empty for findings with a single location. + related: tuple[tuple[str, int, str], ...] = () def render(self, severity: str = "error") -> str: return (f"{self.file}:{self.line}: {severity}: [{self.code}] " @@ -346,13 +351,24 @@ def _sarif_result(f: Finding, severity: str) -> dict[str, Any]: ("handler", f.handler)): if val: props[key] = val - return { + result: dict[str, Any] = { "ruleId": f.code, "level": _sarif_level(f, severity), "message": {"text": f"{f.message} [resource: {f.kind}]"}, "locations": [{"physicalLocation": phys}], "properties": props, } + # secondary locations (e.g. a DI captive's consuming constructor) — a SARIF consumer + # (GitHub code scanning) renders these as clickable, labelled related locations. + related = [ + {"physicalLocation": {"artifactLocation": {"uri": rf.replace("\\", "/")}, + "region": {"startLine": rl}}, + "message": {"text": rmsg}} + for (rf, rl, rmsg) in f.related if rl >= 1 + ] + if related: + result["relatedLocations"] = related + return result def build_sarif(findings: list[Finding], severity: str = "error") -> dict[str, Any]: @@ -466,6 +482,12 @@ def load(path: str) -> dict[str, Any]: ln = s.get("line", 0) if not isinstance(ln, int) or isinstance(ln, bool): raise OwnIRError("service 'line' must be an integer") + # the consuming-constructor location (optional, P-006 Q#1) is validated like file/line. + if not isinstance(s.get("ctor_file", "?"), str): + raise OwnIRError("service 'ctor_file' must be a string") + cln = s.get("ctor_line", 0) + if not isinstance(cln, int) or isinstance(cln, bool): + raise OwnIRError("service 'ctor_line' must be an integer") # 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", []) @@ -1234,6 +1256,16 @@ def _as_int(v: Any) -> int: return v if isinstance(v, int) and not isinstance(v, bool) else 0 +def _consumer_related(c: Any) -> tuple[tuple[str, int, str], ...]: + """The captive finding's **consuming constructor** as a structured related location + (file, line, label), or empty when the extractor did not record a ctor location. The + primary anchor stays the registration site; this is the second, code-side anchor.""" + if getattr(c, "consumed_line", 0) >= 1: + return ((c.consumed_file, c.consumed_line, + f"consuming constructor of singleton '{c.singleton}'"),) + return () + + def _di_findings(facts: dict[str, Any]) -> list[Finding]: """Run the DI captive-dependency check over the facts' `services` graph and map each result to a DI001 Finding at its registration site.""" @@ -1256,6 +1288,10 @@ def _di_findings(facts: dict[str, Any]) -> list[Finding]: disposable=s.get("disposable") is True, file=str(s.get("file", "?")), line=_as_int(s.get("line", 0)), + # the consuming constructor's location (where the capture is injected) — a + # secondary anchor distinct from the registration site above (P-006 Q#1). + ctor_file=str(s.get("ctor_file", "?")), + ctor_line=_as_int(s.get("ctor_line", 0)), ) for s in raw if isinstance(s, dict) ] @@ -1263,7 +1299,7 @@ def _di_findings(facts: dict[str, Any]) -> list[Finding]: Finding( file=c.file, line=c.line, code="DI001", component=c.singleton, event=c.captured, handler="", - message=c.message, kind="DI lifetime") + message=c.message, kind="DI lifetime", related=_consumer_related(c)) for c in find_captive_dependencies(services) ] # DI003: a transient IDisposable captured by a singleton is promoted to application @@ -1274,7 +1310,8 @@ def _di_findings(facts: dict[str, Any]) -> list[Finding]: Finding( file=c.file, line=c.line, code="DI003", component=c.singleton, event=c.captured, handler="", - message=c.message, kind="DI lifetime", severity="warning") + message=c.message, kind="DI lifetime", severity="warning", + related=_consumer_related(c)) for c in find_captured_transient_disposables(services) ] # DI002: a singleton holding a scoped service via WeakReference (P-006). The weak @@ -1285,7 +1322,8 @@ def _di_findings(facts: dict[str, Any]) -> list[Finding]: Finding( file=c.file, line=c.line, code="DI002", component=c.singleton, event=c.captured, handler="", - message=c.message, kind="DI lifetime", severity="warning") + message=c.message, kind="DI lifetime", severity="warning", + related=_consumer_related(c)) for c in find_weak_captive_dependencies(services) ] # DI004: a singleton that resolves a transient IDisposable BY HAND from its injected diff --git a/tests/fixtures/ownir/di.facts.json b/tests/fixtures/ownir/di.facts.json index a3d0e1c2..15d44423 100644 --- a/tests/fixtures/ownir/di.facts.json +++ b/tests/fixtures/ownir/di.facts.json @@ -3,10 +3,10 @@ "module": "DiDemo", "components": [], "services": [ - {"name": "EmailSender", "lifetime": "singleton", "file": "Startup.cs", "line": 12, "deps": ["AppDbContext"]}, + {"name": "EmailSender", "lifetime": "singleton", "file": "Startup.cs", "line": 12, "deps": ["AppDbContext"], "ctor_file": "EmailSender.cs", "ctor_line": 5}, {"name": "AppDbContext", "lifetime": "scoped", "file": "Startup.cs", "line": 13, "deps": []}, {"name": "Clock", "lifetime": "singleton", "file": "Startup.cs", "line": 14, "deps": []}, - {"name": "ReportService", "lifetime": "singleton", "file": "Startup.cs", "line": 15, "deps": ["UnitOfWork"]}, + {"name": "ReportService", "lifetime": "singleton", "file": "Startup.cs", "line": 15, "deps": ["UnitOfWork"], "ctor_file": "ReportService.cs", "ctor_line": 7}, {"name": "UnitOfWork", "lifetime": "transient", "file": "Startup.cs", "line": 16, "deps": ["AppDbContext"]}, {"name": "RequestLog", "lifetime": "scoped", "file": "Startup.cs", "line": 17, "deps": ["AppDbContext"]} ] diff --git a/tests/test_ownir.py b/tests/test_ownir.py index f0e92f59..9aca1482 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -645,6 +645,40 @@ def _sub(source: str | None) -> list[Finding]: fails.append(f"DI001 message missing captive text: " f"{em.message if em else None!r}") checks += 1 + # P-006 Q#1: the finding anchors at the REGISTRATION site (line 12) but names the + # CONSUMING CONSTRUCTOR too — here a DIFFERENT file (EmailSender.cs:5), both in the + # message tail and as a structured related location (-> SARIF relatedLocations). + consumed = "[consumed by the 'EmailSender' constructor at EmailSender.cs:5]" + if em is None or consumed not in em.message: + fails.append(f"DI001 message missing consuming-constructor anchor: " + f"{em.message if em else None!r}") + checks += 1 + if em is None or em.related != (("EmailSender.cs", 5, + "consuming constructor of singleton 'EmailSender'"),): + fails.append(f"DI001 related (consuming ctor) location wrong: " + f"{em.related if em else None!r}") + checks += 1 + # the related location rides into SARIF as a relatedLocations entry (clickable in + # GitHub code scanning), distinct from the primary registration-site location. + em_sarif = next((r for r in build_sarif(difindings)["runs"][0]["results"] + if r["properties"].get("component") == "EmailSender"), None) + rel = (em_sarif or {}).get("relatedLocations") + if (not rel or rel[0]["physicalLocation"]["artifactLocation"]["uri"] != "EmailSender.cs" + or rel[0]["physicalLocation"]["region"]["startLine"] != 5): + fails.append(f"DI001 SARIF relatedLocations wrong: {rel!r}") + checks += 1 + # a DI001 whose ctor location is UNKNOWN degrades cleanly — no suffix, no related. + nolocf = check_facts({"ownir_version": 0, "module": "X", "components": [], "functions": [], + "services": [ + {"name": "Cap", "lifetime": "singleton", "deps": ["Sc"], + "file": "S.cs", "line": 3}, + {"name": "Sc", "lifetime": "scoped", "deps": [], + "file": "S.cs", "line": 4}]}) + cap = next((x for x in nolocf if x.code == "DI001"), None) + if cap is None or "consumed by the" in cap.message or cap.related != (): + fails.append(f"DI001 without ctor loc should omit the consuming-ctor anchor: " + f"{(cap.message, cap.related) if cap else None!r}") + checks += 1 # an unknown lifetime must fail loudly at load (external input). if not _load_raises({"ownir_version": OWNIR_VERSION, "components": [], "services": [{"name": "X", "lifetime": "perpetual"}]}): @@ -671,6 +705,12 @@ def _sub(source: str | None) -> list[Finding]: "services": [{"name": "X", "lifetime": "singleton", "root_resolves": "abc"}]}): fails.append("a non-array service root_resolves did not raise OwnIRError") + checks += 1 + # ctor_line (the consuming-constructor anchor, P-006 Q#1) is validated like line. + if not _load_raises({"ownir_version": OWNIR_VERSION, "components": [], + "services": [{"name": "X", "lifetime": "singleton", + "ctor_line": "NaN"}]}): + fails.append("a non-integer service ctor_line 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 f152254e9520f6a5de70b56583ceeb299c2c64a1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 21 Jun 2026 09:48:03 +0000 Subject: [PATCH 2/4] test(ci): match the full sample path in the consuming-constructor assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The extractor emits the ctor location with the full relative path (frontend/roslyn/samples/DiCaptiveSample.cs:25), but the new DI consuming-constructor assertions grepped for the bare filename (at DiCaptiveSample.cs:25), which is not a substring — so the wpf-extractor job failed even though the output was correct. Switch the four assertions to a regex that allows the path prefix (`at .*DiCaptiveSample\.cs:NN]`), matching how the existing DI greps already tolerate it. Feature unchanged; CI-only. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 51633a9c..ae1718c4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -311,9 +311,9 @@ jobs: # P-006 Q#1: each captive finding anchors at the registration site but ALSO names its # CONSUMING CONSTRUCTOR (where the capture is injected) — the explicit ctor line for # EmailSender, and the class-declaration line for the C# 12 primary ctor. - echo "$out" | grep -q "consumed by the 'EmailSender' constructor at DiCaptiveSample.cs:25" \ + echo "$out" | grep -qE "consumed by the 'EmailSender' constructor at .*DiCaptiveSample\.cs:25\]" \ || { echo "FAIL: expected the consuming-constructor anchor (EmailSender ctor, line 25)"; exit 1; } - echo "$out" | grep -q "consumed by the 'PrimaryCtorService' constructor at DiCaptiveSample.cs:33" \ + echo "$out" | grep -qE "consumed by the 'PrimaryCtorService' constructor at .*DiCaptiveSample\.cs:33\]" \ || { echo "FAIL: expected the primary-constructor consuming anchor (PrimaryCtorService, line 33)"; exit 1; } # P-006 DI003 (transient IDisposable captured by a singleton, WARNING): the # singleton ConnectionWarmer holds the transient IDisposable PooledConnection @@ -325,7 +325,7 @@ jobs: [ "$nw" = "1" ] \ || { echo "FAIL: expected exactly 1 DI003 finding, got $nw"; exit 1; } # DI003 carries the consuming-constructor anchor too (ConnectionWarmer's ctor, line 50). - echo "$out" | grep -q "consumed by the 'ConnectionWarmer' constructor at DiCaptiveSample.cs:50" \ + echo "$out" | grep -qE "consumed by the 'ConnectionWarmer' constructor at .*DiCaptiveSample\.cs:50\]" \ || { echo "FAIL: expected the DI003 consuming-constructor anchor (ConnectionWarmer, line 50)"; exit 1; } # P-006 DI002 (scoped service held by a singleton via WeakReference, WARNING): # the weak ref is the usual "fix" for a DI001 captive, but scoped AppDbContext is @@ -352,7 +352,7 @@ jobs: [ "$nwk" = "3" ] \ || { echo "FAIL: expected exactly 3 DI002 findings, got $nwk"; exit 1; } # DI002 carries the consuming-constructor anchor too (WeakCache's ctor, line 57). - echo "$out" | grep -q "consumed by the 'WeakCache' constructor at DiCaptiveSample.cs:57" \ + echo "$out" | grep -qE "consumed by the 'WeakCache' constructor at .*DiCaptiveSample\.cs:57\]" \ || { echo "FAIL: expected the DI002 consuming-constructor anchor (WeakCache, line 57)"; exit 1; } if echo "$out" | grep -q "WeakClockHolder"; then echo "FAIL: a weak ref to a singleton (WeakClockHolder) was wrongly flagged"; exit 1 From 3b3ecea69175a0318f824f1b6c15813f1422c886 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 21 Jun 2026 09:54:59 +0000 Subject: [PATCH 3/4] fix(di): name the implementation type as the consuming-constructor owner (Codex) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For an interface registration (AddSingleton), the singleton's service name is the ctor-less interface IFoo, but the consuming constructor is Foo's — so naming the service ('IFoo') as the ctor owner pointed users at a non-existent IFoo constructor. Carry the implementation type through the fact (ctor_type = impl) and name IT in the message tail and the SARIF related-location label. For concrete registrations ctor_type == name, so the message is unchanged; degrades to a plain "the constructor" when the impl is unknown. Pinned by an interface-registration case in tests/test_ownir.py (IBilling -> Billing names Billing, never IBilling) and ctor_type load-validation. Also split the dense P-006 Q#1 sentence for readability (CodeRabbit nitpick). ruff + mypy --strict clean, ownir 112/112. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- docs/notes/di-captive-extractor.md | 14 ++++--- docs/proposals/P-006-di-lifetimes.md | 19 ++++++---- frontend/roslyn/OwnSharp.Extractor/Program.cs | 3 ++ ownlang/di.py | 38 +++++++++++++------ ownlang/ownir.py | 15 ++++++-- tests/fixtures/ownir/di.facts.json | 4 +- tests/test_ownir.py | 28 +++++++++++++- 7 files changed, 90 insertions(+), 31 deletions(-) diff --git a/docs/notes/di-captive-extractor.md b/docs/notes/di-captive-extractor.md index a424bc92..3305ff59 100644 --- a/docs/notes/di-captive-extractor.md +++ b/docs/notes/di-captive-extractor.md @@ -190,11 +190,15 @@ answered "both"). So each DI001/DI002/DI003 finding now also names that construc The extractor records each implementation's ctor location — the widest **public** constructor's declaration, or the class declaration for a C# 12 primary / implicit constructor — into -`ctor_file` / `ctor_line` on the service fact (`classCtorLoc`). The core appends the anchor when -the location is known and **degrades cleanly** (no tail, no related location) when it is not, so -hand-authored facts and an older extractor still produce the registration-anchored form. The -finding is on the **singleton**, so the consuming constructor is the singleton's — the entry of -the (possibly transitive) capture chain the path already spells out. Pinned end-to-end by +`ctor_file` / `ctor_line` on the service fact (`classCtorLoc`), plus the **implementation type** +that owns it in `ctor_type`. The core appends the anchor when the location is known and +**degrades cleanly** (no tail, no related location) when it is not, so hand-authored facts and an +older extractor still produce the registration-anchored form. The owner named is the *impl*, not +the service name: for an interface registration (`AddSingleton`) the captor's service +name is the ctor-less interface `IFoo`, but the consuming ctor is `Foo`'s — so the finding names +`Foo` (a Codex review catch). The finding is on the **singleton**, so the consuming constructor is +the singleton implementation's — the entry of the (possibly transitive) capture chain the path +already spells out. Pinned end-to-end by `DiCaptiveSample.cs` (the explicit-ctor `EmailSender:25`, the primary-ctor `PrimaryCtorService:33`, `ConnectionWarmer:50`, `WeakCache:57`) in the `wpf-extractor` CI job, the cross-file case by the `tests/fixtures/ownir/di.facts.json` fixture, and the SARIF related location by `tests/test_ownir.py`. diff --git a/docs/proposals/P-006-di-lifetimes.md b/docs/proposals/P-006-di-lifetimes.md index a4525125..c20530c7 100644 --- a/docs/proposals/P-006-di-lifetimes.md +++ b/docs/proposals/P-006-di-lifetimes.md @@ -111,14 +111,17 @@ rather than guessed. ## Open questions 1. Where to anchor the diagnostic — the registration line, the consuming - constructor, or both? **(Resolved — both, with the capture path shown, like OWN014's - "expected: Window — actual: App — path: …".)** For the captive family (DI001/DI002/DI003) - the finding keeps its **primary** anchor at the registration site and names the - **consuming constructor** — where the captive is injected — both in the message tail - (`[consumed by the '' constructor at :]`) and as a structured - **SARIF `relatedLocation`** (clickable, cross-file). The extractor records each - implementation's ctor location (the widest public ctor, or the class declaration for a - primary/implicit ctor); the core appends it when known and degrades cleanly when not. + constructor, or both? **Resolved: both**, with the capture path shown, like OWN014's + "expected: Window — actual: App — path: …". + For the captive family (DI001/DI002/DI003) the finding keeps its **primary** anchor at + the registration site and names the **consuming constructor** — where the captive is + injected — both in the message tail + (`[consumed by the '' constructor at :]`) and as a structured + **SARIF `relatedLocation`** (clickable, cross-file). The owner named is the + **implementation** type that owns the ctor (for an interface registration that is the + impl, not the ctor-less service interface). The extractor records each implementation's + ctor location (the widest public ctor, or the class declaration for a primary/implicit + ctor); the core appends it when known and degrades cleanly when not. *Still open:* DI004's consumer is a **resolution call site**, not a ctor — anchoring it there is the sibling follow-up (the call-site location is not yet threaded through). 2. How far to chase transitive captures through the constructor graph before the diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 0febf064..37dbfb07 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -1093,6 +1093,9 @@ static List ExtractServices(List<(string file, SyntaxTree tree)> parsed) // captive is injected, a finding's second anchor beside the registration site. ctor_file = ctorFile, ctor_line = ctorLine, + // the IMPLEMENTATION type that owns that ctor — named in the finding instead of + // the (possibly interface) service `name`, which has no constructor (Codex). + ctor_type = impl ?? service, }); } return services; diff --git a/ownlang/di.py b/ownlang/di.py index dcc586d4..e0157c3c 100644 --- a/ownlang/di.py +++ b/ownlang/di.py @@ -33,15 +33,19 @@ LIFETIMES = frozenset({SINGLETON, SCOPED, TRANSIENT}) -def _consumed_suffix(singleton: str, file: str, line: int) -> str: - """The ` [consumed by the '' constructor at :]` tail a captive - finding appends so it names **both** the registration site (its primary anchor) and the - *consuming constructor* where the capture is introduced (P-006 open question #1). Empty - when the ctor location is unknown (an older extractor / hand-authored facts), so the - message degrades cleanly to just the registration-anchored form.""" +def _consumed_suffix(ctor_type: str, file: str, line: int) -> str: + """The ` [consumed by the '' constructor at :]` tail a captive finding + appends so it names **both** the registration site (its primary anchor) and the *consuming + constructor* where the capture is introduced (P-006 open question #1). The owner named is + the **implementation** type whose ctor it is — for an interface registration that is the + impl, never the (ctor-less) service interface (Codex). Empty when the ctor location is + unknown (older extractor / hand-authored facts); the type name is dropped (not guessed) + when only the location is known, so the message always degrades cleanly.""" if line < 1: return "" - return f" [consumed by the '{singleton}' constructor at {file}:{line}]" + owner = (f"the '{ctor_type}' constructor" + if ctor_type and ctor_type != "?" else "the constructor") + return f" [consumed by {owner} at {file}:{line}]" @dataclass(frozen=True) @@ -76,6 +80,11 @@ class Service: # captive finding can name both. Declared LAST (positional-contract safe), default unknown. ctor_file: str = "?" ctor_line: int = 0 + # the IMPLEMENTATION type that owns that constructor. For an interface registration + # (`AddSingleton`) `name` is the service `IFoo` (no ctor), but the consuming + # ctor is `Foo`'s — so the finding must name `Foo`, not the interface (Codex). Empty when + # unknown, in which case the suffix names "the constructor" without a (wrong) type. + ctor_type: str = "" @dataclass(frozen=True) @@ -90,13 +99,14 @@ class CaptiveDependency: line: int consumed_file: str = "?" consumed_line: int = 0 + consumed_type: str = "" @property def message(self) -> str: chain = " -> ".join(self.path) return (f"singleton '{self.singleton}' captures scoped service " f"'{self.captured}' (captive dependency: {chain})" - + _consumed_suffix(self.singleton, self.consumed_file, self.consumed_line)) + + _consumed_suffix(self.consumed_type, self.consumed_file, self.consumed_line)) def find_captive_dependencies(services: list[Service]) -> list[CaptiveDependency]: @@ -130,7 +140,8 @@ def find_captive_dependencies(services: list[Service]) -> list[CaptiveDependency findings.append(CaptiveDependency( singleton=s.name, captured=dep, path=npath, file=s.file, line=s.line, - consumed_file=s.ctor_file, consumed_line=s.ctor_line)) + consumed_file=s.ctor_file, consumed_line=s.ctor_line, + consumed_type=s.ctor_type)) continue # the violating edge is found; don't recurse past it if dnode.lifetime == TRANSIENT and dep not in visited: visited.add(dep) @@ -156,6 +167,7 @@ class WeakCaptiveDependency: line: int consumed_file: str = "?" consumed_line: int = 0 + consumed_type: str = "" @property def message(self) -> str: @@ -165,7 +177,7 @@ def message(self) -> str: f"from the root provider and promoted to application lifetime — the weak " f"reference avoids pinning it for the GC but does not fix the " f"captive-dependency lifetime violation ({chain})" - + _consumed_suffix(self.singleton, self.consumed_file, self.consumed_line)) + + _consumed_suffix(self.consumed_type, self.consumed_file, self.consumed_line)) def find_weak_captive_dependencies( @@ -199,7 +211,8 @@ def find_weak_captive_dependencies( findings.append(WeakCaptiveDependency( singleton=s.name, captured=cur, path=path, file=s.file, line=s.line, - consumed_file=s.ctor_file, consumed_line=s.ctor_line)) + consumed_file=s.ctor_file, consumed_line=s.ctor_line, + consumed_type=s.ctor_type)) continue # the violating scoped edge is found; don't recurse past it if cnode.lifetime == TRANSIENT and cur not in visited: visited.add(cur) @@ -224,6 +237,7 @@ class CapturedTransientDisposable: line: int consumed_file: str = "?" consumed_line: int = 0 + consumed_type: str = "" @property def message(self) -> str: @@ -231,7 +245,7 @@ def message(self) -> str: return (f"singleton '{self.singleton}' captures transient IDisposable " f"'{self.captured}': it is promoted to application lifetime and " f"disposed only when the root provider is disposed ({chain})" - + _consumed_suffix(self.singleton, self.consumed_file, self.consumed_line)) + + _consumed_suffix(self.consumed_type, self.consumed_file, self.consumed_line)) def find_captured_transient_disposables( diff --git a/ownlang/ownir.py b/ownlang/ownir.py index 47594d6b..37904d37 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -488,6 +488,8 @@ def load(path: str) -> dict[str, Any]: cln = s.get("ctor_line", 0) if not isinstance(cln, int) or isinstance(cln, bool): raise OwnIRError("service 'ctor_line' must be an integer") + if not isinstance(s.get("ctor_type", ""), str): + raise OwnIRError("service 'ctor_type' must be a string") # 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", []) @@ -1259,10 +1261,14 @@ def _as_int(v: Any) -> int: def _consumer_related(c: Any) -> tuple[tuple[str, int, str], ...]: """The captive finding's **consuming constructor** as a structured related location (file, line, label), or empty when the extractor did not record a ctor location. The - primary anchor stays the registration site; this is the second, code-side anchor.""" + primary anchor stays the registration site; this is the second, code-side anchor. The + label names the **implementation** type that owns the ctor (not the possibly-interface + service name — Codex), falling back to a plain label when the impl type is unknown.""" if getattr(c, "consumed_line", 0) >= 1: - return ((c.consumed_file, c.consumed_line, - f"consuming constructor of singleton '{c.singleton}'"),) + owner = getattr(c, "consumed_type", "") or "" + label = (f"consuming constructor of '{owner}'" if owner and owner != "?" + else "consuming constructor") + return ((c.consumed_file, c.consumed_line, label),) return () @@ -1292,6 +1298,9 @@ def _di_findings(facts: dict[str, Any]) -> list[Finding]: # secondary anchor distinct from the registration site above (P-006 Q#1). ctor_file=str(s.get("ctor_file", "?")), ctor_line=_as_int(s.get("ctor_line", 0)), + # the IMPLEMENTATION type owning that ctor — named in the finding instead of the + # (possibly interface) service name, which would point at a ctor-less type (Codex). + ctor_type=str(s.get("ctor_type", "")), ) for s in raw if isinstance(s, dict) ] diff --git a/tests/fixtures/ownir/di.facts.json b/tests/fixtures/ownir/di.facts.json index 15d44423..3b91a586 100644 --- a/tests/fixtures/ownir/di.facts.json +++ b/tests/fixtures/ownir/di.facts.json @@ -3,10 +3,10 @@ "module": "DiDemo", "components": [], "services": [ - {"name": "EmailSender", "lifetime": "singleton", "file": "Startup.cs", "line": 12, "deps": ["AppDbContext"], "ctor_file": "EmailSender.cs", "ctor_line": 5}, + {"name": "EmailSender", "lifetime": "singleton", "file": "Startup.cs", "line": 12, "deps": ["AppDbContext"], "ctor_file": "EmailSender.cs", "ctor_line": 5, "ctor_type": "EmailSender"}, {"name": "AppDbContext", "lifetime": "scoped", "file": "Startup.cs", "line": 13, "deps": []}, {"name": "Clock", "lifetime": "singleton", "file": "Startup.cs", "line": 14, "deps": []}, - {"name": "ReportService", "lifetime": "singleton", "file": "Startup.cs", "line": 15, "deps": ["UnitOfWork"], "ctor_file": "ReportService.cs", "ctor_line": 7}, + {"name": "ReportService", "lifetime": "singleton", "file": "Startup.cs", "line": 15, "deps": ["UnitOfWork"], "ctor_file": "ReportService.cs", "ctor_line": 7, "ctor_type": "ReportService"}, {"name": "UnitOfWork", "lifetime": "transient", "file": "Startup.cs", "line": 16, "deps": ["AppDbContext"]}, {"name": "RequestLog", "lifetime": "scoped", "file": "Startup.cs", "line": 17, "deps": ["AppDbContext"]} ] diff --git a/tests/test_ownir.py b/tests/test_ownir.py index 9aca1482..89fe2ff5 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -654,7 +654,7 @@ def _sub(source: str | None) -> list[Finding]: f"{em.message if em else None!r}") checks += 1 if em is None or em.related != (("EmailSender.cs", 5, - "consuming constructor of singleton 'EmailSender'"),): + "consuming constructor of 'EmailSender'"),): fails.append(f"DI001 related (consuming ctor) location wrong: " f"{em.related if em else None!r}") checks += 1 @@ -679,6 +679,26 @@ def _sub(source: str | None) -> list[Finding]: fails.append(f"DI001 without ctor loc should omit the consuming-ctor anchor: " f"{(cap.message, cap.related) if cap else None!r}") checks += 1 + # an INTERFACE registration (AddSingleton): the singleton is 'IBilling' + # (no ctor) but the consuming ctor is 'Billing's, so the finding must name the IMPL Billing, + # never the interface (Codex). ctor_type carries the impl through the fact. + ifacef = check_facts({"ownir_version": 0, "module": "X", "components": [], "functions": [], + "services": [ + {"name": "IBilling", "lifetime": "singleton", "deps": ["Db"], + "file": "Startup.cs", "line": 8, "ctor_file": "Billing.cs", + "ctor_line": 11, "ctor_type": "Billing"}, + {"name": "Db", "lifetime": "scoped", "deps": [], + "file": "Startup.cs", "line": 9}]}) + ib = next((x for x in ifacef if x.code == "DI001"), None) + if ib is None or "[consumed by the 'Billing' constructor at Billing.cs:11]" not in ib.message: + fails.append(f"DI001 interface-registration must name the IMPL ctor (Billing), not " + f"the interface: {ib.message if ib else None!r}") + checks += 1 + if ib is None or "'IBilling' constructor" in ib.message \ + or ib.related != (("Billing.cs", 11, "consuming constructor of 'Billing'"),): + fails.append(f"DI001 interface-registration named the interface ctor or wrong related: " + f"{(ib.message, ib.related) if ib else None!r}") + checks += 1 # an unknown lifetime must fail loudly at load (external input). if not _load_raises({"ownir_version": OWNIR_VERSION, "components": [], "services": [{"name": "X", "lifetime": "perpetual"}]}): @@ -711,6 +731,12 @@ def _sub(source: str | None) -> list[Finding]: "services": [{"name": "X", "lifetime": "singleton", "ctor_line": "NaN"}]}): fails.append("a non-integer service ctor_line did not raise OwnIRError") + checks += 1 + # ctor_type (the impl owning the ctor) is validated as a string. + if not _load_raises({"ownir_version": OWNIR_VERSION, "components": [], + "services": [{"name": "X", "lifetime": "singleton", + "ctor_type": 5}]}): + fails.append("a non-string service ctor_type 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 cc2d8624420bc1cc7e2369a6f496fc1a82d2a131 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 21 Jun 2026 09:58:55 +0000 Subject: [PATCH 4/4] fix(di): pass consumed_type on the DI003 collector too (interface ctor name) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Codex impl-naming fix added consumed_type to the DI001 and DI002 collectors but missed find_captured_transient_disposables (DI003), so a DI003 finding rendered the empty-type fallback "[consumed by the constructor at …]" instead of naming the impl ("[consumed by the 'ConnectionWarmer' constructor at …]"), failing the wpf-extractor CI assertion. Pass consumed_type=s.ctor_type here too, and strengthen the DI003 unit test to set ctor_type and assert the named anchor (the gap that let this through — the DI003 fixtures carried no ctor location). ruff + mypy --strict clean, ownir 113/113. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED --- ownlang/di.py | 3 ++- tests/test_ownir.py | 11 ++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/ownlang/di.py b/ownlang/di.py index e0157c3c..e98908fb 100644 --- a/ownlang/di.py +++ b/ownlang/di.py @@ -279,7 +279,8 @@ def find_captured_transient_disposables( findings.append(CapturedTransientDisposable( singleton=s.name, captured=dep, path=npath, file=s.file, line=s.line, - consumed_file=s.ctor_file, consumed_line=s.ctor_line)) + consumed_file=s.ctor_file, consumed_line=s.ctor_line, + consumed_type=s.ctor_type)) if dep not in visited: visited.add(dep) stack.append((dep, npath)) diff --git a/tests/test_ownir.py b/tests/test_ownir.py index 89fe2ff5..6b4847ba 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -493,7 +493,8 @@ def _sub(source: str | None) -> list[Finding]: # AND disposable; a non-disposable transient and a scoped capture stay silent. from ownlang.di import find_captured_transient_disposables dsvcs = [ - Service("Cache", "singleton", ("Conn",)), # -> transient disposable: DI003 + Service("Cache", "singleton", ("Conn",), # -> transient disposable: DI003 + ctor_file="Cache.cs", ctor_line=4, ctor_type="Cache"), Service("Conn", "transient", (), disposable=True), Service("Warm", "singleton", ("Mid",)), # -> transient -> disposable Service("Mid", "transient", ("Pool",), disposable=False), @@ -511,6 +512,14 @@ def _sub(source: str | None) -> list[Finding]: checks += 1 if any("IDisposable" not in c.message for c in di3): fails.append("DI003 message missing 'IDisposable'") + checks += 1 + # DI003 carries the consuming-constructor anchor too, naming the IMPL that owns the ctor + # (regression guard: the DI003 collector must pass consumed_type, like DI001/DI002). + cache3 = next((c for c in di3 if c.singleton == "Cache"), None) + c3want = "[consumed by the 'Cache' constructor at Cache.cs:4]" + if cache3 is None or c3want not in cache3.message: + fails.append(f"DI003 message missing consuming-constructor anchor: " + f"{cache3.message if cache3 else None!r}") # bridge: DI003 surfaces as a WARNING-severity finding; `disposable` is parsed. di3facts = {"ownir_version": 0, "module": "X", "components": [], "functions": [], "services": [