Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 26 additions & 5 deletions ownlang/ownir.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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}] "
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -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
Expand All @@ -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<T> (P-006). The weak
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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


Expand Down
7 changes: 6 additions & 1 deletion tests/run_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__":
Expand Down
60 changes: 60 additions & 0 deletions tests/test_ownir.py
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> is a
# weak captive (warning). The weak edge lives in `weak_deps`, OFF the DI001 strong
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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": [
Expand Down
Loading