feat(di): anchor captive findings at the consuming constructor (P-006 Q#1)

.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 -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 -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
           # 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 -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<T>, 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 -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
           fi

docs/notes/di-captive-extractor.md

@@ -175,9 +175,40 @@ 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`), 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<IFoo, Foo>`) 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`.
+
 ## 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<T>()`), 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<T>()` 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<T>()` and the `CreateScope()` → scope-provider form).

docs/proposals/P-006-di-lifetimes.md

@@ -111,8 +111,19 @@ 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 '<impl>' constructor at <file>:<line>]`) 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
    dynamic cases make it unreliable? (Bounded depth; stop at unknown edges.)
 3. Is `IServiceScopeFactory` usage inside a singleton recognised as the *fix*

frontend/roslyn/OwnSharp.Extractor/Program.cs

@@ -953,7 +953,12 @@
     // through a scope (`scope.ServiceProvider.GetRequiredService<T>()`) has a different
     // receiver and is deliberately NOT recorded — that pattern is correct.
     var classRootResolves = new Dictionary<string, List<string>>();
-    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<string, (string file, int line)>();
+    foreach (var (ctorFile, tree) in parsed)
         foreach (var node in tree.GetRoot().DescendantNodes())
         {
             if (node is not ClassDeclarationSyntax cls)
@@ -988,6 +993,12 @@
                 }
             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 @@
                 ? wd : new List<string>();
             var rootResolves = impl is not null && classRootResolves.TryGetValue(impl, out var rr)
                 ? rr : new List<string>();
+            var (ctorFile, ctorLine) = impl is not null && classCtorLoc.TryGetValue(impl, out var cl)
+                ? cl : ("?", 0);
             services.Add(new
             {
                 name = service,
@@ -1076,6 +1089,13 @@
                 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,
+                // 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;
@@ -1256,7 +1276,7 @@
     .Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries)
     .Where(p => p.EndsWith(".dll", StringComparison.OrdinalIgnoreCase))
     .ToList();
 var refNames = new HashSet<string>(tpa.Select(Path.GetFileName), StringComparer.OrdinalIgnoreCase);
 var references = tpa.Select(p => (MetadataReference)MetadataReference.CreateFromFile(p)).ToList();
 // P-004 WPF profile: widen the reference set with assemblies named by the
 // OWN_EXTRA_REF_DIRS env var (colon-separated dirs) — e.g. the WindowsDesktop ref

ownlang/di.py

@@ -33,6 +33,21 @@
 LIFETIMES = frozenset({SINGLETON, SCOPED, TRANSIENT})
 
 
+def _consumed_suffix(ctor_type: str, file: str, line: int) -> str:
+    """The ` [consumed by the '<impl>' constructor at <file>:<line>]` 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 ""
+    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)
 class Service:
     """One DI registration: a service `name`, its `lifetime`, and the service
@@ -59,6 +74,17 @@ 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
+    # the IMPLEMENTATION type that owns that constructor. For an interface registration
+    # (`AddSingleton<IFoo, Foo>`) `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)
@@ -71,12 +97,16 @@ class CaptiveDependency:
     path: tuple[str, ...]
     file: str
     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})")
+                f"'{self.captured}' (captive dependency: {chain})"
+                + _consumed_suffix(self.consumed_type, self.consumed_file, self.consumed_line))
 
 
 def find_captive_dependencies(services: list[Service]) -> list[CaptiveDependency]:
@@ -109,7 +139,9 @@ 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,
+                            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)
@@ -133,6 +165,9 @@ class WeakCaptiveDependency:
     path: tuple[str, ...]
     file: str
     line: int
+    consumed_file: str = "?"
+    consumed_line: int = 0
+    consumed_type: str = ""
 
     @property
     def message(self) -> str:
@@ -141,7 +176,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.consumed_type, self.consumed_file, self.consumed_line))
 
 
 def find_weak_captive_dependencies(
@@ -174,7 +210,9 @@ 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,
+                        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)
@@ -197,13 +235,17 @@ class CapturedTransientDisposable:
     path: tuple[str, ...]
     file: str
     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 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.consumed_type, self.consumed_file, self.consumed_line))
 
 
 def find_captured_transient_disposables(
@@ -236,7 +278,9 @@ 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,
+                        consumed_type=s.ctor_type))
                 if dep not in visited:
                     visited.add(dep)
                     stack.append((dep, npath))