diff --git a/ownlang/ownir.py b/ownlang/ownir.py index 2848e177..3e54ce8a 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -138,6 +138,7 @@ find_weak_captive_dependencies, ) from .diagnostics import TITLES, Severity +from .evidence import code_flow, di_path_steps from .ownership import ( MethodSkeleton, ParamSkeleton, @@ -292,6 +293,11 @@ class Finding: # 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], ...] = () + # an ORDERED reachability slice — each an (file, line, label) triple — that explains the + # finding by the path it walks: a DI captive's `singleton -> transient -> scoped` retention + # chain, each hop anchored at a real registration site. Rides along as a SARIF `codeFlows` + # (the step-through trace `relatedLocations` cannot express). Empty for a single-point finding. + flow: tuple[tuple[str, int, str], ...] = () def render(self, severity: str = "error") -> str: return (f"{self.file}:{self.line}: {severity}: [{self.code}] " @@ -382,6 +388,12 @@ def _sarif_result(f: Finding, severity: str) -> dict[str, Any]: ] if related: result["relatedLocations"] = related + # the ordered reachability slice (e.g. a DI captive's singleton -> transient -> scoped + # retention path): a SARIF consumer renders codeFlows as a click-through step trace, which + # is exactly the "why is this held, and through what?" question relatedLocations can't answer. + flows = code_flow(f.flow) + if flows: + result["codeFlows"] = flows return result @@ -1746,11 +1758,16 @@ def _di_findings(facts: dict[str, Any]) -> list[Finding]: ) for s in raw if isinstance(s, dict) ] + # registration site of each service, to anchor the hops of a finding's reachability slice: + # DI001..005 each carry a `path` of service names (singleton -> ... -> captured), and + # di_path_steps turns those names into ordered (file, line, label) steps via this map. + loc_by_name = {s.name: (s.file, s.line) for s in services if s.line >= 1} out = [ Finding( file=c.file, line=c.line, code="DI001", component=c.singleton, event=c.captured, handler="", - message=c.message, kind="DI lifetime", related=_consumer_related(c)) + message=c.message, kind="DI lifetime", related=_consumer_related(c), + flow=di_path_steps(c.path, loc_by_name, "captures scoped service")) for c in find_captive_dependencies(services) ] # DI003: a transient IDisposable captured by a singleton is promoted to application @@ -1762,7 +1779,8 @@ def _di_findings(facts: dict[str, Any]) -> list[Finding]: file=c.file, line=c.line, code="DI003", component=c.singleton, event=c.captured, handler="", message=c.message, kind="DI lifetime", severity="warning", - related=_consumer_related(c)) + related=_consumer_related(c), + flow=di_path_steps(c.path, loc_by_name, "captures transient IDisposable")) for c in find_captured_transient_disposables(services) ] # DI002: a singleton holding a scoped service via WeakReference (P-006). The weak @@ -1774,7 +1792,8 @@ def _di_findings(facts: dict[str, Any]) -> list[Finding]: file=c.file, line=c.line, code="DI002", component=c.singleton, event=c.captured, handler="", message=c.message, kind="DI lifetime", severity="warning", - related=_consumer_related(c)) + related=_consumer_related(c), + flow=di_path_steps(c.path, loc_by_name, "weakly captures scoped service")) for c in find_weak_captive_dependencies(services) ] # DI004: a singleton that resolves a transient IDisposable BY HAND from its injected @@ -1790,7 +1809,8 @@ def _di_findings(facts: dict[str, Any]) -> list[Finding]: file=pf, line=pl, code="DI004", component=c.singleton, event=c.resolved, handler="", message=c.message, kind="DI lifetime", severity="warning", - related=_di004_related(c))) + related=_di004_related(c), + flow=di_path_steps(c.path, loc_by_name, "leaks transient IDisposable"))) # 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) @@ -1803,7 +1823,8 @@ def _di_findings(facts: dict[str, Any]) -> list[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))) + related=_di005_related(sc), + flow=di_path_steps(sc.path, loc_by_name, "caches scoped service"))) return out diff --git a/tests/run_tests.py b/tests/run_tests.py index 191c8054..19659f3e 100644 --- a/tests/run_tests.py +++ b/tests/run_tests.py @@ -1106,11 +1106,16 @@ def run() -> int: import test_ownership own5_rc = test_ownership.run() + # Diagnostic rendering (P-015): the empty-evidence invariant (render()/render_pretty() + # byte-for-byte unchanged) plus the ordered `note:` slice and the SARIF evidence builders. + import test_diagnostics + diag_rc = test_diagnostics.run() + return 1 if (failed or cg_fail or golden_fails or buffer_fails or escape_fails or branchy_fails or nest_fails or order_fails or helper_fails or cc_rc or pf_rc or gl_rc or co_rc or wpf_rc or lt_rc or loops_rc - or spec_rc or ownir_rc or own5_rc) else 0 + or spec_rc or ownir_rc or own5_rc or diag_rc) else 0 if __name__ == "__main__": diff --git a/tests/test_ownir.py b/tests/test_ownir.py index 00431a26..7377ce6d 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -570,6 +570,13 @@ def _sub(source: str | None) -> list[Finding]: if len(di3b) != 1 or di3b[0].severity != "warning" or di3b[0].component != "Cache": fails.append(f"DI003 bridge finding wrong: " f"{[(x.component, x.severity) for x in di3b]}") + checks += 1 + # P-015 flow: captor singleton -> captured transient IDisposable, each at its registration + # site, ending with the DI003 family label. + if not di3b or di3b[0].flow != ( + ("S.cs", 7, "singleton 'Cache' (captor)"), + ("S.cs", 8, "captures transient IDisposable 'Conn'")): + fails.append(f"DI003 flow wrong: {di3b[0].flow if di3b else None!r}") # --- DI002 (P-006): a scoped service held by a singleton via WeakReference is a # weak captive (warning). The weak edge lives in `weak_deps`, OFF the DI001 strong @@ -618,6 +625,12 @@ def _sub(source: str | None) -> list[Finding]: checks += 1 if any(x.code == "DI001" for x in di2b): fails.append("DI002 bridge wrongly also produced a DI001") + checks += 1 + # P-015 flow: ends with the DI002 family label "weakly captures scoped service". + if not di2only or di2only[0].flow != ( + ("S.cs", 9, "singleton 'WeakCache' (captor)"), + ("S.cs", 10, "weakly captures scoped service 'Db'")): + fails.append(f"DI002 flow wrong: {di2only[0].flow if di2only else None!r}") # --- DI004 (P-006): a transient IDisposable resolved BY HAND from a singleton's injected # root IServiceProvider (the service-locator anti-pattern, warning). Only singletons are @@ -690,6 +703,17 @@ def _sub(source: str | None) -> list[Finding]: # also produce a DI001/DI002/DI003 (the singleton has no scoped/transient ctor dependency). if any(x.code in ("DI001", "DI002", "DI003") for x in di4b): fails.append("DI004 wrongly also produced a graph DI00x finding") + checks += 1 + # P-015 flow: DI004 anchors at the CALL site (R.cs:42), but the reachability slice begins at + # the REGISTRATION site (S.cs:5) and ends with "leaks transient IDisposable" — the flow's + # first hop deliberately differs from the finding's own (call-site) location. + if not di4only or di4only[0].flow != ( + ("S.cs", 5, "singleton 'Resolver' (captor)"), + ("S.cs", 6, "leaks transient IDisposable 'Conn'")): + fails.append(f"DI004 flow wrong: {di4only[0].flow if di4only else None!r}") + checks += 1 + if di4only and di4only[0].flow[0][:2] == (di4only[0].file, di4only[0].line): + fails.append("DI004 flow must start at the registration site, not the call-site anchor") # --- 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 @@ -763,6 +787,16 @@ def _sub(source: str | None) -> list[Finding]: # 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") + checks += 1 + # P-015 flow: DI005 anchors at the field-STORE site (C.cs:21), but the slice begins at the + # REGISTRATION site (S.cs:7) and ends with "caches scoped service". + if not di5only or di5only[0].flow != ( + ("S.cs", 7, "singleton 'Cacher' (captor)"), + ("S.cs", 8, "caches scoped service 'Db'")): + fails.append(f"DI005 flow wrong: {di5only[0].flow if di5only else None!r}") + checks += 1 + if di5only and di5only[0].flow[0][:2] == (di5only[0].file, di5only[0].line): + fails.append("DI005 flow must start at the registration site, not the store-site anchor") # bridge: the fixture surfaces exactly the two captive singletons as DI001 # at their registration lines; the clock/scoped-to-scoped stay silent. @@ -805,6 +839,32 @@ def _sub(source: str | None) -> list[Finding]: or rel[0]["physicalLocation"]["region"]["startLine"] != 5): fails.append(f"DI001 SARIF relatedLocations wrong: {rel!r}") checks += 1 + # P-015: the captive's retention path also rides along as an ORDERED reachability slice + # (SARIF codeFlows) — the "why is this held, and through what?" trace relatedLocations + # cannot express. The direct captive is a 2-hop path: captor singleton -> captured scoped, + # each anchored at its registration site. + if em is None or em.flow != ( + ("Startup.cs", 12, "singleton 'EmailSender' (captor)"), + ("Startup.cs", 13, "captures scoped service 'AppDbContext'")): + fails.append(f"DI001 EmailSender reachability flow wrong: {em.flow if em else None!r}") + checks += 1 + em_flows = (em_sarif or {}).get("codeFlows") + em_locs = em_flows[0]["threadFlows"][0]["locations"] if em_flows else [] + if (len(em_locs) != 2 + or em_locs[0]["location"]["physicalLocation"]["region"]["startLine"] != 12 + or em_locs[-1]["location"]["physicalLocation"]["region"]["startLine"] != 13): + fails.append(f"DI001 SARIF codeFlows wrong: {em_flows!r}") + checks += 1 + # the TRANSITIVE captive (ReportService -> UnitOfWork(transient) -> AppDbContext(scoped)) + # renders a 3-hop slice, with the transient a labelled pass-through middle step. + rs = next((x for x in difindings + if x.component == "ReportService" and x.code == "DI001"), None) + if rs is None or rs.flow != ( + ("Startup.cs", 15, "singleton 'ReportService' (captor)"), + ("Startup.cs", 16, "via 'UnitOfWork'"), + ("Startup.cs", 13, "captures scoped service 'AppDbContext'")): + fails.append(f"DI001 ReportService transitive flow wrong: {rs.flow if rs else None!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": [