From 634594fe7e6b7a8a64a2684a44e545b7b40289e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 04:05:19 +0000 Subject: [PATCH 1/3] spike(ownts): React useEffect leaks through the OwnIR seam (P-020 Own.React) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an OwnTS frontend spike — the TS/TSX sibling of the Roslyn C# extractor. It scans a React .tsx, emits OwnIR facts, and lets the existing Python core flag the leak: a useEffect acquire (setInterval/setTimeout → timer, .subscribe → subscribe, addEventListener → subscription) with no cleanup return is the core's OWN001, exactly as a C# `event +=` without `-=` is. Same seam, same one checker, different skin — proving the leak model is cross-language, not .NET-only. Honest scope, per P-020: this is the EFF003/EFF004 slice that *is* the existing acquire→release model. EFF001/002 (the unstable-dependency effect storm) is NOT built — it needs a new core stability analysis and must not masquerade as OWN001; the scanner only emits a clearly-labelled, frontend-only heuristic note for it on stderr, never a core finding. - frontend/ownts/ownts.py: heuristic extractor (brace match + cleanup-verb detection; not a TS parser) with --check to run straight through the core - frontend/ownts/examples/: leaky Dashboard.tsx (3× OWN001) + clean variant - frontend/ownts/test_ownts.py: pins leaky=3, clean=0, kinds, EFF001 note - CI: new ownts-react-effects job (.tsx → OwnIR → core, no dotnet needed) - docs/proposals/P-020: status note that the EFF003/004 spike landed Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PVeuchch67CtjSbizYm8gi --- .github/workflows/ci.yml | 36 +++ docs/proposals/P-020-ownts-react-effects.md | 6 + frontend/ownts/README.md | 72 +++++ frontend/ownts/examples/Dashboard.tsx | 39 +++ frontend/ownts/examples/DashboardClean.tsx | 33 +++ frontend/ownts/ownts.py | 294 ++++++++++++++++++++ frontend/ownts/test_ownts.py | 46 +++ 7 files changed, 526 insertions(+) create mode 100644 frontend/ownts/README.md create mode 100644 frontend/ownts/examples/Dashboard.tsx create mode 100644 frontend/ownts/examples/DashboardClean.tsx create mode 100644 frontend/ownts/ownts.py create mode 100644 frontend/ownts/test_ownts.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 66e7c0c4..78bbc8f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -998,6 +998,42 @@ jobs: || { echo "FAIL: --project flag and positional .csproj emit different OwnIR facts"; exit 1; } echo "OK: .csproj input resolves to its source set and feeds the core identically (positional == --project, fact-level parity)" + # The OwnTS frontend spike (P-020 Own.React): the SAME OwnIR seam, fed from a + # React .tsx instead of C#. A useEffect acquire (timer / subscribe / listener) + # with no cleanup return is the core's OWN001 — proving the leak model is + # cross-language, not .NET-only. No dotnet needed; pure-Python frontend. + ownts-react-effects: + name: OwnTS (React useEffect) -> OwnIR -> core + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.13" + - name: Pin the spike (leaky=3xOWN001, clean=0, EFF001 heuristic) + run: python frontend/ownts/test_ownts.py + - name: Extract OwnIR facts from a React .tsx and check through the core + run: | + python frontend/ownts/ownts.py frontend/ownts/examples/Dashboard.tsx \ + -o "$RUNNER_TEMP/dash.facts.json" + cat "$RUNNER_TEMP/dash.facts.json" + out=$(python -m ownlang ownir "$RUNNER_TEMP/dash.facts.json" || true) + echo "$out" + echo "$out" | grep -q "Dashboard.tsx" \ + || { echo "FAIL: expected findings located at the .tsx"; exit 1; } + echo "$out" | grep -q "resource: timer" \ + || { echo "FAIL: expected the setInterval [resource: timer] leak"; exit 1; } + echo "$out" | grep -qE "3 finding" \ + || { echo "FAIL: expected three OWN001 leaks from the leaky effects"; exit 1; } + - name: The clean fixture (cleanup returns) is silent + run: | + python frontend/ownts/ownts.py frontend/ownts/examples/DashboardClean.tsx \ + -o "$RUNNER_TEMP/clean.facts.json" + clean=$(python -m ownlang ownir "$RUNNER_TEMP/clean.facts.json" || true) + echo "$clean" + echo "$clean" | grep -q "0 finding" \ + || { echo "FAIL: cleaned-up effects must not leak"; exit 1; } + # The distribution surface (Уровень 1): the own-check.sh orchestrator walks a # directory of real C# and prints findings in the host-parseable formats the # GitHub Action (PR annotations) and a VS Error List (MSBuild) consume — and diff --git a/docs/proposals/P-020-ownts-react-effects.md b/docs/proposals/P-020-ownts-react-effects.md index 48f4ecb6..72c697e4 100644 --- a/docs/proposals/P-020-ownts-react-effects.md +++ b/docs/proposals/P-020-ownts-react-effects.md @@ -3,6 +3,12 @@ - **Status:** draft — **experiment / proposal, explicitly not mainline.** A marketing-shaped spike under the OwnTS frontend, on the record so the framing is honest before any code. +- **Spike landed:** the honest `EFF003`/`EFF004` slice (timer / subscribe / + listener acquire with no cleanup `return` → core `OWN001`) is implemented in + `frontend/ownts/` and CI-pinned (the `ownts-react-effects` job). `EFF001/002` + (the effect-storm stability analysis) is **not** built — the spike only emits a + clearly-labelled frontend-only heuristic note for it, never a core finding, per + Open question 1 below. - **Depends on:** [P-017](P-017-multi-stack-frontends.md) (the OwnTS frontend & confidence tiers that this profile feeds), `spec/OwnCore.md` (the acquire/release vocabulary), [P-004](P-004-wpf-lifetime-profile.md) (WPF — the same lifecycle diff --git a/frontend/ownts/README.md b/frontend/ownts/README.md new file mode 100644 index 00000000..30bed83a --- /dev/null +++ b/frontend/ownts/README.md @@ -0,0 +1,72 @@ +# OwnTS — React `useEffect` frontend spike (`Own.React`) + +The TypeScript sibling of the [Roslyn C# extractor](../roslyn/). It scans a +React `.tsx`, emits **OwnIR** facts, and lets the **existing** Python core +(`python -m ownlang ownir`) flag the leak. Same seam, same checker, different +skin — a `useEffect` acquire without a cleanup `return` is the core's `OWN001`, +exactly as a C# `event +=` without `-=` is. + +> The same OwnIR idea behind WPF subscription leaks can model React effect storms. + +This is a **spike**, per [P-020](../../docs/proposals/P-020-ownts-react-effects.md) +— deliberately *not* a TypeScript analyzer. Extraction is heuristic (brace +matching + cleanup-verb detection), not a real TS parse. Its only job is to prove +the seam is cross-language. + +## Run it + +```bash +# print the OwnIR facts extracted from a .tsx +python frontend/ownts/ownts.py frontend/ownts/examples/Dashboard.tsx + +# extract + run straight through the core (the "catch") +python frontend/ownts/ownts.py frontend/ownts/examples/Dashboard.tsx --check + +# or the real two-step CLI, same as the C# side: +python frontend/ownts/ownts.py frontend/ownts/examples/Dashboard.tsx -o facts.json +python -m ownlang ownir facts.json --format sarif + +# pin the spike +python frontend/ownts/test_ownts.py +``` + +`Dashboard.tsx` drops three `OWN001` leaks; `DashboardClean.tsx` (every acquire +has a cleanup `return`) is silent. + +``` +Dashboard.tsx:13: error: [OWN001] timer 'setInterval(() =>' ... never stopped ... (leak) [resource: timer] +Dashboard.tsx:21: error: [OWN001] the result of '.subscribe(...)' is ignored ... (leak) [resource: subscription token] +Dashboard.tsx:27: error: [OWN001] event '.addEventListener(...)' ... never unsubscribed ... (leak) [resource: subscription token] +``` + +## What it catches — the honest `Own.React` slice + +| EFF | Pattern | OwnIR `resource` | Core verdict | +|-----|---------|------------------|--------------| +| `EFF004` | `setInterval`/`setTimeout` in an effect, no `clearInterval`/`clearTimeout` cleanup | `timer` | `OWN001` | +| `EFF003` | `X.subscribe(...)` with no `unsubscribe` cleanup | `subscribe` | `OWN001` | +| `EFF003` | `addEventListener` with no `removeEventListener` cleanup | `subscription` | `OWN001` | + +These three **are** the existing acquire→release model, just emitted by an OwnTS +frontend. The core is untouched. + +## What it does NOT do (and why that's honest) + +`EFF001/002` — the unstable-dependency "effect storm" (the Cloudflare 12-Sep-2025 +shape: a `useEffect` whose dep object is re-created each render, re-firing the +effect and storming the API) — **is not an acquire/release leak.** It needs a new +core capability (dependency-identity *stability*), which the core does not have. +Per P-020, `EFF001` must **not** masquerade as `OWN001`. + +So the scanner only emits a *frontend-only, clearly-labelled* heuristic note for +`EFF001` candidates on **stderr** — never a core-verified finding: + +``` +Dashboard.tsx:34: EFF001 (frontend heuristic, NOT core-verified): dependency +'filters' is a fresh object/array identity every render; the effect does IO — +possible request storm. Stabilise with useMemo. +``` + +The one-liner the spike pays for: **"Not all lifecycle bugs leak memory. Some +leak requests."** We make **no** "Own would have prevented the Cloudflare outage" +claim — see P-020's Non-goals. diff --git a/frontend/ownts/examples/Dashboard.tsx b/frontend/ownts/examples/Dashboard.tsx new file mode 100644 index 00000000..bf1d8ca1 --- /dev/null +++ b/frontend/ownts/examples/Dashboard.tsx @@ -0,0 +1,39 @@ +// Leaky dashboard — three useEffect acquires with NO cleanup return. +// Each is the React skin of the same acquire->release contract the .NET core +// already checks (timer / subscription token). Run: +// +// python frontend/ownts/ownts.py frontend/ownts/examples/Dashboard.tsx --check +// +// Expect three OWN001 findings (EFF004 timer, EFF003 subscribe, EFF003 listener). +import { useEffect } from "react"; + +export function Dashboard({ tenantId }: { tenantId: string }) { + // EFF004 — interval started, never cleared: the timer keeps the component alive. + useEffect(() => { + const id = setInterval(() => { + fetch(`/api/tenant/${tenantId}/metrics`); + }, 1000); + // no `return () => clearInterval(id)` — leak + }, [tenantId]); + + // EFF003 — observable subscription whose teardown is never returned. + useEffect(() => { + bus.subscribe((msg) => console.log(msg)); + // no `return () => sub.unsubscribe()` — leak + }, []); + + // EFF003 — DOM listener added, never removed. + useEffect(() => { + window.addEventListener("resize", onResize); + // no `return () => window.removeEventListener("resize", onResize)` — leak + }, []); + + // EFF001 (frontend heuristic only — NOT a core OWN001): `filters` is a fresh + // object identity every render, and the effect does IO -> request storm. + const filters = { tenantId }; + useEffect(() => { + fetch(`/api/tenant/${filters.tenantId}`); + }, [filters]); + + return
dashboard
; +} diff --git a/frontend/ownts/examples/DashboardClean.tsx b/frontend/ownts/examples/DashboardClean.tsx new file mode 100644 index 00000000..ddfffdba --- /dev/null +++ b/frontend/ownts/examples/DashboardClean.tsx @@ -0,0 +1,33 @@ +// Clean dashboard — every acquire has a matching cleanup return, and the unstable +// dependency is stabilised with useMemo. The extractor sees `released: true` for +// each resource, so the core stays silent (zero findings). Run: +// +// python frontend/ownts/ownts.py frontend/ownts/examples/DashboardClean.tsx --check +import { useEffect, useMemo } from "react"; + +export function Dashboard({ tenantId }: { tenantId: string }) { + useEffect(() => { + const id = setInterval(() => { + fetch(`/api/tenant/${tenantId}/metrics`); + }, 1000); + return () => clearInterval(id); // released + }, [tenantId]); + + useEffect(() => { + const sub = bus.subscribe((msg) => console.log(msg)); + return () => sub.unsubscribe(); // released + }, []); + + useEffect(() => { + window.addEventListener("resize", onResize); + return () => window.removeEventListener("resize", onResize); // released + }, []); + + // stable identity across renders -> no effect storm + const filters = useMemo(() => ({ tenantId }), [tenantId]); + useEffect(() => { + fetch(`/api/tenant/${filters.tenantId}`); + }, [filters]); + + return
dashboard
; +} diff --git a/frontend/ownts/ownts.py b/frontend/ownts/ownts.py new file mode 100644 index 00000000..e45a9ad7 --- /dev/null +++ b/frontend/ownts/ownts.py @@ -0,0 +1,294 @@ +#!/usr/bin/env python3 +"""OwnTS frontend spike — React `useEffect` acquire/release facts -> OwnIR. + +This is the TS/TSX sibling of the Roslyn C# extractor (frontend/roslyn/): it scans +real `.tsx` and emits *facts* in the OwnIR vocabulary, which the existing Python +core (`python -m ownlang ownir`) routes through the same OWN001 acquire->release +checker. The core stays the one checker — we do not reimplement leak analysis in +JS-land (that would drift), exactly as the C# side does not. + +Scope is a deliberate **spike**, per docs/proposals/P-020 ("Not a TypeScript +analyzer"). It implements the honest, end-to-end-today slice of the `Own.React` +EFF catalog — the rules that *are* the existing acquire->release model: + + EFF003 effect subscribes (`X.subscribe(...)`) with no cleanup return -> OWN001 + EFF004 setInterval/setTimeout in an effect with no cleanup -> OWN001 + (addEventListener with no removeEventListener cleanup) -> OWN001 + +It does NOT implement EFF001/002 (the unstable-dependency "effect storm"): that is +a genuinely new core analysis (dependency-identity stability), not an +acquire->release leak, and P-020 is explicit that EFF001 must not masquerade as +OWN001. As a courtesy the scanner emits a clearly-labelled, *frontend-only* +heuristic note for EFF001 candidates on stderr — never as a core-verified finding. + +The extraction is heuristic (brace matching + verb detection), not a real TS parse. +That is fine for a spike whose job is to prove the seam, not to ship a frontend. + +Usage:: + + python frontend/ownts/ownts.py App.tsx # print OwnIR JSON + python frontend/ownts/ownts.py App.tsx -o app.facts.json + python frontend/ownts/ownts.py App.tsx --check # run through the core +""" +from __future__ import annotations + +import json +import re +import sys +from dataclasses import dataclass, field + + +# --- acquire catalog: how each React acquire maps onto a core resource kind ----- +# +# Each acquire has (a) a regex that spots the acquire call, (b) the OwnIR `resource` +# discriminator the core understands, (c) the cleanup verb that releases it, and +# (d) the EFF id + tag for provenance. The `resource` value is the *only* field the +# core acts on; `eff`/`profile` ride along as additive provenance the core ignores. +@dataclass(frozen=True) +class Acquire: + name: str # human label, e.g. "setInterval" + pattern: re.Pattern # spots the acquire call + resource: str # OwnIR resource kind: timer / subscribe / subscription + release: re.Pattern # the cleanup verb that releases it + eff: str # Own.React catalog id + +ACQUIRES: list[Acquire] = [ + Acquire("setInterval", re.compile(r"\bsetInterval\s*\("), + "timer", re.compile(r"\bclearInterval\s*\("), "EFF004"), + Acquire("setTimeout", re.compile(r"\bsetTimeout\s*\("), + "timer", re.compile(r"\bclearTimeout\s*\("), "EFF004"), + Acquire(".subscribe", re.compile(r"\.subscribe\s*\("), + "subscribe", re.compile(r"\.unsubscribe\s*\(|\.remove\s*\("), "EFF003"), + Acquire("addEventListener", re.compile(r"\.addEventListener\s*\("), + "subscription", re.compile(r"\.removeEventListener\s*\("), "EFF003"), +] + +# A React component is a function whose name is Capitalized (the JSX convention). +_COMPONENT = re.compile( + r"(?:function\s+([A-Z]\w*)\s*\(|" + r"(?:const|let|var)\s+([A-Z]\w*)\s*=\s*(?:\([^)]*\)|\w+)\s*(?::[^=]+)?=>)" +) +_USE_EFFECT = re.compile(r"\buseEffect\s*\(") + + +@dataclass +class Resource: + event: str + line: int + released: bool + resource: str + eff: str + + +@dataclass +class Component: + name: str + file: str + resources: list[Resource] = field(default_factory=list) + + +def _strip_comments(text: str) -> str: + """Blank out `//` and `/* */` comments (preserving newlines so line numbers + survive) without touching string/template contents. Stops the scanner from + mistaking a `// no return () => clearInterval(id)` note for real cleanup.""" + out = [] + i = 0 + while i < len(text): + c = text[i] + if c in "\"'`": + out.append(c) + i += 1 + while i < len(text) and text[i] != c: + out.append(text[i]) + if text[i] == "\\" and i + 1 < len(text): + out.append(text[i + 1]) + i += 2 + continue + i += 1 + if i < len(text): + out.append(text[i]) + i += 1 + elif c == "/" and i + 1 < len(text) and text[i + 1] == "/": + while i < len(text) and text[i] != "\n": + out.append(" ") + i += 1 + elif c == "/" and i + 1 < len(text) and text[i + 1] == "*": + while i < len(text): + if text[i] == "*" and i + 1 < len(text) and text[i + 1] == "/": + break + out.append("\n" if text[i] == "\n" else " ") + i += 1 + out.append(" ") + i += 2 + else: + out.append(c) + i += 1 + return "".join(out) + + +def _match_block(text: str, open_idx: int) -> int: + """Index just past the `}` that closes the `{` at/after `open_idx`. Skips over + string/template/comment content so a brace inside a string does not fool us.""" + i = text.index("{", open_idx) + depth = 0 + while i < len(text): + c = text[i] + if c in "\"'`": + quote = c + i += 1 + while i < len(text) and text[i] != quote: + if text[i] == "\\": + i += 1 + i += 1 + elif c == "/" and i + 1 < len(text) and text[i + 1] == "/": + i = text.find("\n", i) + if i == -1: + return len(text) + elif c == "/" and i + 1 < len(text) and text[i + 1] == "*": + end = text.find("*/", i) + i = len(text) if end == -1 else end + 1 + elif c == "{": + depth += 1 + elif c == "}": + depth -= 1 + if depth == 0: + return i + 1 + i += 1 + return len(text) + + +def _component_at(text: str, idx: int) -> str: + """Name of the React component enclosing position `idx` — the nearest preceding + Capitalized function declaration. Falls back to a synthetic name.""" + name = None + for m in _COMPONENT.finditer(text, 0, idx): + name = m.group(1) or m.group(2) + return name or "AnonymousComponent" + + +def _split_cleanup(body: str) -> tuple[str, str]: + """Split an effect body into (setup, cleanup). Cleanup is the block of the + `return () => { ... }` the effect hands back to React; setup is the rest.""" + m = re.search(r"return\s*(?:\(\s*\)|\w+)\s*=>", body) + if not m: + return body, "" + brace = body.find("{", m.end()) + if brace == -1: + # `return () => clearInterval(id)` — single-expression cleanup, no block. + nl = body.find("\n", m.end()) + tail = body[m.end(): nl if nl != -1 else len(body)] + return body[: m.start()], tail + end = _match_block(body, brace) + return body[: m.start()] + body[end:], body[brace:end] + + +def extract(path: str) -> list[Component]: + text = _strip_comments(open(path, encoding="utf-8").read()) + comps: dict[str, Component] = {} + for eff in _USE_EFFECT.finditer(text): + end = _match_block(text, eff.end()) + body = text[eff.end():end] + setup, cleanup = _split_cleanup(body) + cname = _component_at(text, eff.start()) + comp = comps.setdefault(cname, Component(cname, path)) + for acq in ACQUIRES: + for hit in acq.pattern.finditer(setup): + line = text.count("\n", 0, eff.end() + hit.start()) + 1 + released = bool(acq.release.search(cleanup)) + # the acquire expression, trimmed to the call head for a readable tag + snippet = setup[hit.start():].splitlines()[0].strip().rstrip("{").strip() + comp.resources.append( + Resource(snippet or acq.name, line, released, acq.resource, acq.eff)) + return [c for c in comps.values() if c.resources] + + +def to_ownir(comps: list[Component], module: str) -> dict: + return { + "ownir_version": 0, + "module": module, + "components": [ + { + "name": c.name, + "file": c.file, + # historically named "subscriptions"; it is the owned-resource list. + "subscriptions": [ + {"event": r.event, "line": r.line, "released": r.released, + "resource": r.resource, + # additive provenance the core ignores: + "profile": "react", "eff": r.eff} + for r in c.resources + ], + } + for c in comps + ], + } + + +def _eff001_notes(path: str) -> list[str]: + """Frontend-only heuristic for EFF001 (unstable dependency -> effect storm). + NOT a core finding — the core has no stability model (P-020 open question 1). + Flags `useEffect(..., [dep])` where `dep` is a local object/array literal that + does IO, i.e. a fresh identity every render.""" + text = _strip_comments(open(path, encoding="utf-8").read()) + notes = [] + for eff in _USE_EFFECT.finditer(text): + end = _match_block(text, eff.end()) + block = text[eff.end():end] + # deps array sits just past the effect body block: `}, [a, b])` + deps = re.search(r",\s*\[([^\]]*)\]\s*\)", text[end - 1:end + 120]) + if not deps: + continue + does_io = re.search(r"\bfetch\s*\(|\baxios\b|\.get\s*\(|\.post\s*\(", block) + for dep in (d.strip() for d in deps.group(1).split(",") if d.strip()): + decl = re.search( + rf"(?:const|let|var)\s+{re.escape(dep)}\s*=\s*(\{{|\[)", text) + if decl and does_io: + line = text.count("\n", 0, eff.start()) + 1 + notes.append( + f"{path}:{line}: EFF001 (frontend heuristic, NOT core-verified): " + f"dependency '{dep}' is a fresh object/array identity every render; " + f"the effect does IO — possible request storm. Stabilise with useMemo.") + return notes + + +def main(argv: list[str]) -> int: + args = [a for a in argv if not a.startswith("-")] + flags = {a for a in argv if a.startswith("-")} + out = None + if "-o" in argv: + out = argv[argv.index("-o") + 1] + if not args: + print(__doc__.splitlines()[0], file=sys.stderr) + print("usage: ownts.py FILE.tsx [--check] [-o facts.json]", file=sys.stderr) + return 2 + path = args[0] + module = re.sub(r"\.[jt]sx?$", "", path.rsplit("/", 1)[-1]) + comps = extract(path) + facts = to_ownir(comps, module) + + for note in _eff001_notes(path): + print(note, file=sys.stderr) + + if "--check" in flags: + # Run the extracted facts straight through the existing core. + import os + sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..")) + from ownlang.ownir import check_facts, render_finding + findings = check_facts(facts) + for f in findings: + print(render_finding(f, "human")) + n = len(findings) + print(f"\n{n} finding{'s' if n != 1 else ''} (via OwnTS -> OwnIR -> core).") + return 1 if findings else 0 + + payload = json.dumps(facts, indent=2) + if out: + open(out, "w", encoding="utf-8").write(payload + "\n") + print(f"wrote {out}", file=sys.stderr) + else: + print(payload) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv[1:])) diff --git a/frontend/ownts/test_ownts.py b/frontend/ownts/test_ownts.py new file mode 100644 index 00000000..e9e03a77 --- /dev/null +++ b/frontend/ownts/test_ownts.py @@ -0,0 +1,46 @@ +#!/usr/bin/env python3 +"""Pins the OwnTS spike: the leaky fixture drops exactly three OWN001 leaks through +the core, the clean fixture drops none, and the EFF001 heuristic fires only on the +unstable-dependency effect. Zero deps. Run: python frontend/ownts/test_ownts.py""" +from __future__ import annotations + +import os +import sys + +HERE = os.path.dirname(__file__) +sys.path.insert(0, os.path.join(HERE, "..", "..")) + +import ownts # noqa: E402 + +from ownlang.ownir import check_facts # noqa: E402 + + +def codes(tsx: str) -> list[str]: + comps = ownts.extract(os.path.join(HERE, "examples", tsx)) + return [f.code for f in check_facts(ownts.to_ownir(comps, tsx))] + + +def main() -> int: + leaky = codes("Dashboard.tsx") + assert leaky == ["OWN001", "OWN001", "OWN001"], f"leaky -> {leaky}" + + clean = codes("DashboardClean.tsx") + assert clean == [], f"clean should be silent -> {clean}" + + # resource-kind mapping is preserved through the bridge + comps = ownts.extract(os.path.join(HERE, "examples", "Dashboard.tsx")) + kinds = sorted(r.resource for c in comps for r in c.resources) + assert kinds == ["subscribe", "subscription", "timer"], kinds + + # EFF001 is a frontend-only heuristic (not core-verified): fires on the leaky + # unstable-dep effect, silent on the useMemo'd clean one. + assert ownts._eff001_notes(os.path.join(HERE, "examples", "Dashboard.tsx")) + assert not ownts._eff001_notes(os.path.join(HERE, "examples", "DashboardClean.tsx")) + + print("OwnTS spike OK: leaky=3xOWN001, clean=0, kinds=timer/subscribe/subscription, " + "EFF001 heuristic fires only on the unstable dep.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From 0406650e06236ecd9a277617403a6b4c2f4c3b36 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 04:28:31 +0000 Subject: [PATCH 2/3] =?UTF-8?q?feat(effects):=20EFF001=20effect-storm=20?= =?UTF-8?q?=E2=80=94=20a=20real=20core=20stability=20analysis=20(P-020)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Answer P-020's gating question (Open question 1) for real: the unstable- dependency "effect storm" is a genuinely NEW core analysis, not an OWN001 leak and not a frontend heuristic. The honest fact/verdict split holds end to end — the OwnTS frontend emits only facts; the core decides. Core (ownlang/effects.py): a dependency-identity stability lattice (STABLE < UNKNOWN < UNSTABLE) computed to a fixpoint over the render-scope binding references. An object/array literal in render scope is UNSTABLE (fresh identity each render); useMemo/useCallback/useRef/prop/primitive are STABLE; an alias/derivation takes the worst of its refs (instability propagates); an opaque call is UNKNOWN (conservative — no finding). An effect is EFF001 iff it does IO AND a dep is provably UNSTABLE. Its own core code (like DI001), never masquerading as OWN001. Bridge (ownlang/ownir.py): a new optional, additive `effects` block in OwnIR (does not bump the version, mirrors `services`); _effect_findings routes it through effects.py to an EFF001 Finding at the effect call site, with a reachability slice (effect -> the fix site) and a SARIF rule. load() shape-checks the block; malformed input degrades to no findings. Frontend (frontend/ownts/): emits the `effects` facts — per useEffect, the dep list, an IO flag, and the component's render-scope binding table with each binding's syntactic init kind + references. The old stderr-only EFF001 heuristic is removed in favour of the core verdict. - ownlang/effects.py: the analysis (lattice, propagation, cycle-safe) - ownlang/diagnostics.py: register EFF001 title - examples/EffectStorm.tsx: showcase — 2 storms fire (object dep + derived alias), memo/ref/call/primitive/no-IO stay silent (low false positives) - tests/test_effects.py (29 checks) wired into run_tests.py; ownts spike test now pins 3xOWN001+EFF001 / 0 / 2xEFF001 - CI ownts-react-effects job extended; P-020 + README updated mypy --strict clean; ruff clean; full suite green. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PVeuchch67CtjSbizYm8gi --- .github/workflows/ci.yml | 34 +++- docs/proposals/P-020-ownts-react-effects.md | 29 ++- frontend/ownts/README.md | 51 +++-- frontend/ownts/examples/EffectStorm.tsx | 54 ++++++ frontend/ownts/ownts.py | 119 ++++++++---- frontend/ownts/test_ownts.py | 32 ++-- ownlang/diagnostics.py | 2 + ownlang/effects.py | 200 ++++++++++++++++++++ ownlang/ownir.py | 85 +++++++++ tests/run_tests.py | 10 +- tests/test_effects.py | 131 +++++++++++++ 11 files changed, 665 insertions(+), 82 deletions(-) create mode 100644 frontend/ownts/examples/EffectStorm.tsx create mode 100644 ownlang/effects.py create mode 100644 tests/test_effects.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 78bbc8f6..00ed106a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -999,9 +999,12 @@ jobs: echo "OK: .csproj input resolves to its source set and feeds the core identically (positional == --project, fact-level parity)" # The OwnTS frontend spike (P-020 Own.React): the SAME OwnIR seam, fed from a - # React .tsx instead of C#. A useEffect acquire (timer / subscribe / listener) - # with no cleanup return is the core's OWN001 — proving the leak model is - # cross-language, not .NET-only. No dotnet needed; pure-Python frontend. + # React .tsx instead of C#. Two analyses over the one core: (1) a useEffect + # acquire (timer / subscribe / listener) with no cleanup return is the core's + # OWN001 — the cross-language leak model; (2) EFF001, a NEW core analysis + # (ownlang/effects.py) — an IO effect whose dependency identity is unstable + # (a render-scope object literal) re-fires every render: the effect storm. The + # frontend emits only facts; the stability verdict is the core's. No dotnet. ownts-react-effects: name: OwnTS (React useEffect) -> OwnIR -> core runs-on: ubuntu-latest @@ -1010,7 +1013,7 @@ jobs: - uses: actions/setup-python@v5 with: python-version: "3.13" - - name: Pin the spike (leaky=3xOWN001, clean=0, EFF001 heuristic) + - name: Pin the spike (leaky=3xOWN001+EFF001, clean=0, showcase=2xEFF001) run: python frontend/ownts/test_ownts.py - name: Extract OwnIR facts from a React .tsx and check through the core run: | @@ -1023,16 +1026,31 @@ jobs: || { echo "FAIL: expected findings located at the .tsx"; exit 1; } echo "$out" | grep -q "resource: timer" \ || { echo "FAIL: expected the setInterval [resource: timer] leak"; exit 1; } - echo "$out" | grep -qE "3 finding" \ - || { echo "FAIL: expected three OWN001 leaks from the leaky effects"; exit 1; } - - name: The clean fixture (cleanup returns) is silent + [ "$(echo "$out" | grep -c 'OWN001')" -eq 3 ] \ + || { echo "FAIL: expected three OWN001 leaks"; exit 1; } + echo "$out" | grep -q "\[EFF001\].*request storm" \ + || { echo "FAIL: expected the EFF001 effect-storm verdict"; exit 1; } + echo "$out" | grep -qE "4 finding" \ + || { echo "FAIL: expected 3 OWN001 + 1 EFF001 = 4 findings"; exit 1; } + - name: EFF001 stability showcase — only provable storms fire (low FP) + run: | + python frontend/ownts/ownts.py frontend/ownts/examples/EffectStorm.tsx \ + -o "$RUNNER_TEMP/storm.facts.json" + storm=$(python -m ownlang ownir "$RUNNER_TEMP/storm.facts.json" || true) + echo "$storm" + # the direct object dep and its derived alias fire; memo/ref/call/primitive/no-IO stay silent + echo "$storm" | grep -qE "2 finding" \ + || { echo "FAIL: expected exactly two EFF001 (object dep + derived alias)"; exit 1; } + echo "$storm" | grep -q "derives from" \ + || { echo "FAIL: expected the derivation (propagation) verdict"; exit 1; } + - name: The clean fixture (cleanups + useMemo'd dep) is silent run: | python frontend/ownts/ownts.py frontend/ownts/examples/DashboardClean.tsx \ -o "$RUNNER_TEMP/clean.facts.json" clean=$(python -m ownlang ownir "$RUNNER_TEMP/clean.facts.json" || true) echo "$clean" echo "$clean" | grep -q "0 finding" \ - || { echo "FAIL: cleaned-up effects must not leak"; exit 1; } + || { echo "FAIL: cleaned-up + memoised effects must not fire"; exit 1; } # The distribution surface (Уровень 1): the own-check.sh orchestrator walks a # directory of real C# and prints findings in the host-parseable formats the diff --git a/docs/proposals/P-020-ownts-react-effects.md b/docs/proposals/P-020-ownts-react-effects.md index 72c697e4..dc02d578 100644 --- a/docs/proposals/P-020-ownts-react-effects.md +++ b/docs/proposals/P-020-ownts-react-effects.md @@ -5,10 +5,15 @@ honest before any code. - **Spike landed:** the honest `EFF003`/`EFF004` slice (timer / subscribe / listener acquire with no cleanup `return` → core `OWN001`) is implemented in - `frontend/ownts/` and CI-pinned (the `ownts-react-effects` job). `EFF001/002` - (the effect-storm stability analysis) is **not** built — the spike only emits a - clearly-labelled frontend-only heuristic note for it, never a core finding, per - Open question 1 below. + `frontend/ownts/` and CI-pinned (the `ownts-react-effects` job). +- **`EFF001` is now a real core analysis** (`ownlang/effects.py`), answering Open + question 1 below: a self-contained **dependency-identity stability** lattice with + reference propagation, computed by the **core** over an OwnIR `effects` fact block + the frontend emits — *not* a frontend heuristic, and *not* an `OWN001`. It is its + own core code (`EFF001`), exactly as the DI captive check is `DI001`. The frontend + states only what each render-scope binding syntactically is; the core decides + stability. `EFF002` (network IO with no stable guard) is governed by the same + analysis and left as the next increment. - **Depends on:** [P-017](P-017-multi-stack-frontends.md) (the OwnTS frontend & confidence tiers that this profile feeds), `spec/OwnCore.md` (the acquire/release vocabulary), [P-004](P-004-wpf-lifetime-profile.md) (WPF — the same lifecycle @@ -151,12 +156,16 @@ has on the .NET side (P-013/P-015); 1/2/6/7 are this proposal's actual work. ## Open questions -1. **The new analysis for `EFF001/002`.** Detecting "dependency identity is unstable - across renders" needs render-scope object-literal/identity reasoning the core - has no model for. Is that a small, self-contained *stability* fact - (`unstable(dep, effect)`) the OwnTS frontend can emit and the core treat like an - acquire-site, or a genuinely new core analysis? This is the gating question — do - not let `EFF001` masquerade as an `OWN001` leak. +1. **The new analysis for `EFF001/002`.** ✅ **Answered (implemented).** It is a + genuinely new core analysis, not an `OWN001` acquire-site — and *not* a frontend + verdict either. The resolution: the frontend emits per-binding **facts** (each + render-scope binding's syntactic `init` kind + the names it references, the dep + list, whether the body does IO) in an OwnIR `effects` block; the **core** + (`ownlang/effects.py`) runs an identity-stability lattice (`STABLE < UNKNOWN < + UNSTABLE`) to a fixpoint over the references and decides `unstable(dep)`. An + effect is `EFF001` iff it does IO **and** a dep is *provably* `UNSTABLE` + (`UNKNOWN`/memoised/primitive clear it — low false positives). The gating + discipline held: `EFF001` is its own core code (like `DI001`), never an `OWN001`. 2. **Confidence tier.** `EFF001` clearly wants TS-mode type info (is the dep an object literal? does the body do IO?). What, if anything, survives into JS mode (P-017's heuristic tier) as a warning? diff --git a/frontend/ownts/README.md b/frontend/ownts/README.md index 30bed83a..4bfd3b46 100644 --- a/frontend/ownts/README.md +++ b/frontend/ownts/README.md @@ -22,6 +22,9 @@ python frontend/ownts/ownts.py frontend/ownts/examples/Dashboard.tsx # extract + run straight through the core (the "catch") python frontend/ownts/ownts.py frontend/ownts/examples/Dashboard.tsx --check +# the EFF001 stability showcase (propagation + the conservative silent cases) +python frontend/ownts/ownts.py frontend/ownts/examples/EffectStorm.tsx --check + # or the real two-step CLI, same as the C# side: python frontend/ownts/ownts.py frontend/ownts/examples/Dashboard.tsx -o facts.json python -m ownlang ownir facts.json --format sarif @@ -30,43 +33,57 @@ python -m ownlang ownir facts.json --format sarif python frontend/ownts/test_ownts.py ``` -`Dashboard.tsx` drops three `OWN001` leaks; `DashboardClean.tsx` (every acquire -has a cleanup `return`) is silent. +`Dashboard.tsx` drops three `OWN001` leaks **and** one `EFF001` effect storm; +`DashboardClean.tsx` (every acquire has a cleanup `return`, and the unstable dep is +`useMemo`'d) is silent. ``` Dashboard.tsx:13: error: [OWN001] timer 'setInterval(() =>' ... never stopped ... (leak) [resource: timer] Dashboard.tsx:21: error: [OWN001] the result of '.subscribe(...)' is ignored ... (leak) [resource: subscription token] Dashboard.tsx:27: error: [OWN001] event '.addEventListener(...)' ... never unsubscribed ... (leak) [resource: subscription token] +Dashboard.tsx:34: error: [EFF001] effect re-runs on every render: dependency 'filters' is an object literal ... can become a request storm ... [resource: react effect] ``` ## What it catches — the honest `Own.React` slice -| EFF | Pattern | OwnIR `resource` | Core verdict | -|-----|---------|------------------|--------------| -| `EFF004` | `setInterval`/`setTimeout` in an effect, no `clearInterval`/`clearTimeout` cleanup | `timer` | `OWN001` | -| `EFF003` | `X.subscribe(...)` with no `unsubscribe` cleanup | `subscribe` | `OWN001` | -| `EFF003` | `addEventListener` with no `removeEventListener` cleanup | `subscription` | `OWN001` | +| EFF | Pattern | OwnIR fact | Core verdict | +|-----|---------|-----------|--------------| +| `EFF004` | `setInterval`/`setTimeout` in an effect, no `clearInterval`/`clearTimeout` cleanup | `resource: timer` | `OWN001` | +| `EFF003` | `X.subscribe(...)` with no `unsubscribe` cleanup | `resource: subscribe` | `OWN001` | +| `EFF003` | `addEventListener` with no `removeEventListener` cleanup | `resource: subscription` | `OWN001` | +| `EFF001` | IO effect with a render-unstable dependency identity | `effects` block | `EFF001` (new core analysis — see below) | These three **are** the existing acquire→release model, just emitted by an OwnTS frontend. The core is untouched. -## What it does NOT do (and why that's honest) +## EFF001 — a real core analysis (not a heuristic, not OWN001) -`EFF001/002` — the unstable-dependency "effect storm" (the Cloudflare 12-Sep-2025 +`EFF001` — the unstable-dependency "effect storm" (the Cloudflare 12-Sep-2025 shape: a `useEffect` whose dep object is re-created each render, re-firing the -effect and storming the API) — **is not an acquire/release leak.** It needs a new -core capability (dependency-identity *stability*), which the core does not have. -Per P-020, `EFF001` must **not** masquerade as `OWN001`. +effect and storming the API) — **is not an acquire/release leak.** It is a new core +analysis: **dependency-identity stability** (`ownlang/effects.py`), its own core +code like `DI001`, *never* an `OWN001`. -So the scanner only emits a *frontend-only, clearly-labelled* heuristic note for -`EFF001` candidates on **stderr** — never a core-verified finding: +The honest split is preserved end-to-end. The frontend emits only **facts** — for +each `useEffect`, its dep list, whether the body does IO, and a render-scope +**binding table** (what each binding syntactically is: `object`/`array`/`new`, +`memo`/`callback`/`ref`, `ident` derivation + the names it references, `call`, …). +It does **not** pre-judge stability. The **core** runs an identity-stability lattice +(`STABLE < UNKNOWN < UNSTABLE`) to a fixpoint over the references and decides: ``` -Dashboard.tsx:34: EFF001 (frontend heuristic, NOT core-verified): dependency -'filters' is a fresh object/array identity every render; the effect does IO — -possible request storm. Stabilise with useMemo. +EFF001 fires ⟺ the effect does IO ∧ some dep is *provably* UNSTABLE ``` +- object/array literal in render scope → UNSTABLE (fresh identity every render) +- `useMemo`/`useCallback`/`useRef`, prop, primitive → STABLE +- an alias/derivation → the worst of what it references (instability *propagates*) +- an opaque `call(...)` → UNKNOWN → **no finding** (conservative; low false positives) + +See `examples/EffectStorm.tsx`: two storms fire (a direct object dep and the alias +that derives from it — `derives from 'filters' ... (via alias -> filters)`), while +the memoised, ref, opaque-call, primitive, and no-IO effects all stay silent. + The one-liner the spike pays for: **"Not all lifecycle bugs leak memory. Some leak requests."** We make **no** "Own would have prevented the Cloudflare outage" claim — see P-020's Non-goals. diff --git a/frontend/ownts/examples/EffectStorm.tsx b/frontend/ownts/examples/EffectStorm.tsx new file mode 100644 index 00000000..97e834ff --- /dev/null +++ b/frontend/ownts/examples/EffectStorm.tsx @@ -0,0 +1,54 @@ +// EFF001 showcase — the core's dependency-identity *stability* analysis, not a +// leak. Only the IO effects whose dep is PROVABLY unstable fire; memoised, +// primitive, opaque-call and no-IO cases stay silent (low false positives). +// +// python frontend/ownts/ownts.py frontend/ownts/examples/EffectStorm.tsx --check +// +// Expect exactly two EFF001 findings: the direct object dep, and the alias that +// derives from it. +import { useEffect, useMemo, useRef } from "react"; + +export function StormBoard({ tenantId }: { tenantId: string }) { + // (1) FIRES — fresh object identity every render + IO. + const filters = { tenantId }; + useEffect(() => { + fetch(`/api/tenant/${filters.tenantId}`); + }, [filters]); + + // (2) FIRES — `alias` derives from the unstable `filters`; instability propagates. + const alias = filters; + useEffect(() => { + fetch(`/api/alias/${alias.tenantId}`); + }, [alias]); + + // (3) SILENT — useMemo gives a stable identity across renders. + const stable = useMemo(() => ({ tenantId }), [tenantId]); + useEffect(() => { + fetch(`/api/stable/${stable.tenantId}`); + }, [stable]); + + // (4) SILENT — useRef identity is stable. + const box = useRef({ tenantId }); + useEffect(() => { + fetch(`/api/ref/${box.current.tenantId}`); + }, [box]); + + // (5) SILENT — opaque call: the core stays conservative (UNKNOWN, no finding). + const computed = makeFilters(tenantId); + useEffect(() => { + fetch(`/api/computed/${computed.tenantId}`); + }, [computed]); + + // (6) SILENT — primitive dependency has a stable value identity. + useEffect(() => { + fetch(`/api/primitive/${tenantId}`); + }, [tenantId]); + + // (7) SILENT — unstable dep but NO IO: re-running is cheap, not a storm. + const opts = { verbose: true }; + useEffect(() => { + console.log(opts.verbose); + }, [opts]); + + return
storm board
; +} diff --git a/frontend/ownts/ownts.py b/frontend/ownts/ownts.py index e45a9ad7..a0801f8b 100644 --- a/frontend/ownts/ownts.py +++ b/frontend/ownts/ownts.py @@ -15,11 +15,14 @@ EFF004 setInterval/setTimeout in an effect with no cleanup -> OWN001 (addEventListener with no removeEventListener cleanup) -> OWN001 -It does NOT implement EFF001/002 (the unstable-dependency "effect storm"): that is -a genuinely new core analysis (dependency-identity stability), not an -acquire->release leak, and P-020 is explicit that EFF001 must not masquerade as -OWN001. As a courtesy the scanner emits a clearly-labelled, *frontend-only* -heuristic note for EFF001 candidates on stderr — never as a core-verified finding. +It ALSO feeds the new **EFF001** core analysis (the unstable-dependency "effect +storm"): a genuinely new dimension — dependency-identity *stability*, not an +acquire->release leak. The honest split is preserved end-to-end: this frontend +emits only *facts* (each render-scope binding's syntactic shape, the dep list, and +whether the effect body does IO) into the OwnIR `effects` block; the stability +VERDICT is the core's (ownlang/effects.py), exactly as the DI captive check decides +over the `services` graph. EFF001 does NOT masquerade as OWN001 — it is its own +core code, like DI001. The extraction is heuristic (brace matching + verb detection), not a real TS parse. That is fine for a spike whose job is to prove the seam, not to ship a frontend. @@ -202,8 +205,9 @@ def extract(path: str) -> list[Component]: return [c for c in comps.values() if c.resources] -def to_ownir(comps: list[Component], module: str) -> dict: - return { +def to_ownir(comps: list[Component], module: str, + effects: list[dict] | None = None) -> dict: + facts = { "ownir_version": 0, "module": module, "components": [ @@ -222,33 +226,82 @@ def to_ownir(comps: list[Component], module: str) -> dict: for c in comps ], } - - -def _eff001_notes(path: str) -> list[str]: - """Frontend-only heuristic for EFF001 (unstable dependency -> effect storm). - NOT a core finding — the core has no stability model (P-020 open question 1). - Flags `useEffect(..., [dep])` where `dep` is a local object/array literal that - does IO, i.e. a fresh identity every render.""" + if effects: + # The EFF001 stability facts the core's effects analysis decides on. The + # frontend states only what each binding syntactically IS — it does NOT + # pre-judge stability (that gate lives in ownlang/effects.py). + facts["effects"] = effects + return facts + + +# Network-IO calls in an effect body — the "leaks requests, not memory" trigger. +_IO = re.compile(r"\bfetch\s*\(|\baxios\b|\.(?:get|post|put|patch|delete)\s*\(|XMLHttpRequest") +# `const/let/var NAME = RHS` (simple binding; destructures fall through to stable). +_BINDING = re.compile(r"\b(?:const|let|var)\s+([A-Za-z_$][\w$]*)\s*=\s*([^\n;]+)") + + +def _classify_rhs(rhs: str) -> tuple[str, list[str]]: + """Map a binding's right-hand side to an OwnIR identity `init` kind (+ the names + it references, for derivations). Purely syntactic — the stability VERDICT is the + core's; this only reports the shape the core reasons over.""" + r = rhs.strip() + if r.startswith("{"): + return "object", [] + if r.startswith("["): + return "array", [] + if r.startswith("new "): + return "new", [] + if r.startswith("useMemo"): + return "memo", [] + if r.startswith("useCallback"): + return "callback", [] + if r.startswith("useRef"): + return "ref", [] + if r.startswith("function") or re.match(r"^(?:async\s+)?\(?[\w$,\s]*\)?\s*=>", r): + return "fn", [] + if re.match(r"^(?:['\"`]|-?\d|true\b|false\b|null\b|undefined\b)", r): + return "primitive", [] + # a bare identifier or member chain (`a`, `a.b.c`) — an alias/derivation of one root + m = re.match(r"^([A-Za-z_$][\w$]*)(?:\.[A-Za-z_$][\w$]*)*$", r) + if m: + return "ident", [m.group(1)] + # a function call returns an opaque (possibly fresh) identity -> let the core stay + # conservative (UNKNOWN, no finding) rather than guess. + if re.match(r"^[A-Za-z_$][\w$.]*\s*\(", r): + return "call", [] + return "unknown", [] + + +def extract_effects(path: str) -> list[dict]: + """Extract the EFF001 stability facts: for each `useEffect`, its dependency list, + whether its body does network IO, and the render-scope binding table of the + component it lives in. The core's effects analysis turns these into a verdict.""" text = _strip_comments(open(path, encoding="utf-8").read()) - notes = [] + # render-scope bindings, attributed to their enclosing component. + binds_by_comp: dict[str, list[dict]] = {} + for m in _BINDING.finditer(text): + kind, refs = _classify_rhs(m.group(2)) + binds_by_comp.setdefault(_component_at(text, m.start()), []).append({ + "name": m.group(1), "init": kind, "refs": refs, + "line": text.count("\n", 0, m.start()) + 1, + }) + effects: list[dict] = [] for eff in _USE_EFFECT.finditer(text): end = _match_block(text, eff.end()) - block = text[eff.end():end] - # deps array sits just past the effect body block: `}, [a, b])` - deps = re.search(r",\s*\[([^\]]*)\]\s*\)", text[end - 1:end + 120]) - if not deps: + body = text[eff.end():end] + deps_m = re.search(r",\s*\[([^\]]*)\]\s*\)", text[end - 1:end + 200]) + if not deps_m: # no dep array -> not an EFF001 candidate (by-design re-run cadence) continue - does_io = re.search(r"\bfetch\s*\(|\baxios\b|\.get\s*\(|\.post\s*\(", block) - for dep in (d.strip() for d in deps.group(1).split(",") if d.strip()): - decl = re.search( - rf"(?:const|let|var)\s+{re.escape(dep)}\s*=\s*(\{{|\[)", text) - if decl and does_io: - line = text.count("\n", 0, eff.start()) + 1 - notes.append( - f"{path}:{line}: EFF001 (frontend heuristic, NOT core-verified): " - f"dependency '{dep}' is a fresh object/array identity every render; " - f"the effect does IO — possible request storm. Stabilise with useMemo.") - return notes + deps = [d.strip() for d in deps_m.group(1).split(",") if d.strip()] + cname = _component_at(text, eff.start()) + effects.append({ + "component": cname, "file": path, + "line": text.count("\n", 0, eff.start()) + 1, + "io": bool(_IO.search(body)), + "deps": deps, + "bindings": binds_by_comp.get(cname, []), + }) + return effects def main(argv: list[str]) -> int: @@ -264,10 +317,8 @@ def main(argv: list[str]) -> int: path = args[0] module = re.sub(r"\.[jt]sx?$", "", path.rsplit("/", 1)[-1]) comps = extract(path) - facts = to_ownir(comps, module) - - for note in _eff001_notes(path): - print(note, file=sys.stderr) + effects = extract_effects(path) + facts = to_ownir(comps, module, effects) if "--check" in flags: # Run the extracted facts straight through the existing core. diff --git a/frontend/ownts/test_ownts.py b/frontend/ownts/test_ownts.py index e9e03a77..47d26288 100644 --- a/frontend/ownts/test_ownts.py +++ b/frontend/ownts/test_ownts.py @@ -1,7 +1,12 @@ #!/usr/bin/env python3 -"""Pins the OwnTS spike: the leaky fixture drops exactly three OWN001 leaks through -the core, the clean fixture drops none, and the EFF001 heuristic fires only on the -unstable-dependency effect. Zero deps. Run: python frontend/ownts/test_ownts.py""" +"""Pins the OwnTS spike end-to-end through the real core: + - Dashboard.tsx -> three OWN001 leaks (timer/subscribe/listener) + one EFF001 + effect storm (unstable object dep + IO); + - DashboardClean.tsx -> silent (cleanups + useMemo'd dep); + - EffectStorm.tsx -> exactly two EFF001 (direct object dep + its derived alias), + every memo/ref/call/primitive/no-IO case staying silent. +EFF001 here is a real core verdict (ownlang/effects.py), not a frontend heuristic. +Zero deps. Run: python frontend/ownts/test_ownts.py""" from __future__ import annotations import os @@ -16,13 +21,14 @@ def codes(tsx: str) -> list[str]: - comps = ownts.extract(os.path.join(HERE, "examples", tsx)) - return [f.code for f in check_facts(ownts.to_ownir(comps, tsx))] + path = os.path.join(HERE, "examples", tsx) + facts = ownts.to_ownir(ownts.extract(path), tsx, ownts.extract_effects(path)) + return [f.code for f in check_facts(facts)] def main() -> int: leaky = codes("Dashboard.tsx") - assert leaky == ["OWN001", "OWN001", "OWN001"], f"leaky -> {leaky}" + assert leaky == ["OWN001", "OWN001", "OWN001", "EFF001"], f"leaky -> {leaky}" clean = codes("DashboardClean.tsx") assert clean == [], f"clean should be silent -> {clean}" @@ -32,13 +38,15 @@ def main() -> int: kinds = sorted(r.resource for c in comps for r in c.resources) assert kinds == ["subscribe", "subscription", "timer"], kinds - # EFF001 is a frontend-only heuristic (not core-verified): fires on the leaky - # unstable-dep effect, silent on the useMemo'd clean one. - assert ownts._eff001_notes(os.path.join(HERE, "examples", "Dashboard.tsx")) - assert not ownts._eff001_notes(os.path.join(HERE, "examples", "DashboardClean.tsx")) + # EFF001 is a real core verdict: the showcase fires on exactly the direct + # object dep and its derived alias; memo/ref/opaque-call/primitive/no-IO are + # all silent (the core's conservative, low-false-positive stability analysis). + storm = codes("EffectStorm.tsx") + assert storm == ["EFF001", "EFF001"], f"EffectStorm -> {storm}" - print("OwnTS spike OK: leaky=3xOWN001, clean=0, kinds=timer/subscribe/subscription, " - "EFF001 heuristic fires only on the unstable dep.") + print("OwnTS spike OK: leaky=3xOWN001+EFF001, clean=0, kinds=timer/subscribe/" + "subscription, EffectStorm=2xEFF001 (core stability analysis, propagation " + "+ conservative).") return 0 diff --git a/ownlang/diagnostics.py b/ownlang/diagnostics.py index 48417161..a0561f7a 100644 --- a/ownlang/diagnostics.py +++ b/ownlang/diagnostics.py @@ -80,6 +80,8 @@ class Severity(Enum): "OWN041": "call argument mismatch", # ---- C# front-end resolution coverage (P-014; advisory) ---- "OWN050": "declaring type unresolved -- leakage analysis skipped", + # ---- reactive-effect stability (P-020; a separate analysis, like DI001) ---- + "EFF001": "reactive effect re-runs on an unstable dependency identity (render-time IO storm)", } diff --git a/ownlang/effects.py b/ownlang/effects.py new file mode 100644 index 00000000..ffa23501 --- /dev/null +++ b/ownlang/effects.py @@ -0,0 +1,200 @@ +"""Reactive-effect stability analysis — EFF001, the effect storm (P-020). + +Not every lifecycle bug leaks memory; some leak *requests*. A React `useEffect` +re-runs whenever one of its declared dependencies changes **identity**. A +dependency that is an object/array literal created in render scope gets a brand +new identity on every render, so the effect re-fires every render — and if the +effect does IO (a `fetch`), that is a render-rate request storm (the Cloudflare +12-Sep-2025 shape). + +This is NOT the acquire/release leak model the rest of the core checks +(EFF003/004/005 -> OWN001 are that). It is a deterministic property of the +**render-scope binding graph**: which names are bound to fresh-identity +expressions, how those names derive from one another, and which effect depends on +which name. So — exactly like `di.py` over the DI registration graph — it lives in +its own small analyzer the OwnIR bridge feeds facts to. One checker, several +analyses; the frontend still only *produces facts* (what each binding's initialiser +syntactically is, the dep list, whether the body does IO) and the **core decides** +stability. The frontend must NOT pre-judge "unstable" — that gating call is here. + +The stability lattice (join = worst case), computed to a fixpoint over the binding +references so a chain `a = {..}; b = a; c = b` carries instability to `c`: + + STABLE < UNKNOWN < UNSTABLE + + - object / array / new literal in render scope -> UNSTABLE (fresh identity) + - useMemo / useCallback / useRef result -> STABLE (memoised) + - prop / state / primitive / import / fn / param -> STABLE (referential) + - identifier / spread / ternary (a derivation) -> join of what it references + - call (an opaque return value) -> UNKNOWN (conservative) + +An effect is an **EFF001 storm** iff it performs IO AND at least one of its +dependencies is provably UNSTABLE. UNKNOWN never fires (low false positives is the +whole point — P-020); a `useMemo`/primitive dep clears it. +""" +from __future__ import annotations + +from dataclasses import dataclass + +# stability lattice +STABLE = "stable" +UNKNOWN = "unknown" +UNSTABLE = "unstable" +_RANK = {STABLE: 0, UNKNOWN: 1, UNSTABLE: 2} + +# initialiser kinds the frontend can observe syntactically +_FRESH = frozenset({"object", "array", "new"}) # fresh identity every render +_MEMOISED = frozenset({"memo", "callback", "ref"}) # useMemo/useCallback/useRef +_REFERENTIAL = frozenset({"prop", "state", "primitive", "import", "fn", "param"}) +_DERIVED = frozenset({"ident", "spread", "ternary", "derive"}) # join over refs +# "call" and any unknown kind fall through to UNKNOWN (opaque return identity). + + +def _join(a: str, b: str) -> str: + return a if _RANK[a] >= _RANK[b] else b + + +@dataclass(frozen=True) +class Binding: + """One render-scope binding: a `name` bound to an initialiser of kind `init`, + which may reference other binding names (`refs`) for derivations. `line` is the + declaration site (the finding's evidence hop).""" + + name: str + init: str + refs: tuple[str, ...] = () + line: int = 0 + + +@dataclass(frozen=True) +class Effect: + """One `useEffect`: the `component` it lives in, the `deps` it declares, whether + its body does `io`, the render-scope `bindings` visible to it, and the call + `line` (the finding's anchor — where the effect re-fires).""" + + component: str + deps: tuple[str, ...] + io: bool + bindings: tuple[Binding, ...] + file: str = "?" + line: int = 0 + + +@dataclass(frozen=True) +class EffectStorm: + """An EFF001 finding: `dep` (the unstable dependency) makes the effect re-run; + `origin`/`origin_kind` name the binding whose fresh identity is the root cause + (the same as `dep` for a direct literal, or an upstream one for a derivation).""" + + component: str + dep: str + origin: str + origin_kind: str + file: str + line: int + decl_line: int + path: tuple[str, ...] = () + + @property + def _kind_phrase(self) -> str: + return { + "object": "an object literal", + "array": "an array literal", + "new": "a freshly constructed object", + }.get(self.origin_kind, "a value with an unstable identity") + + @property + def message(self) -> str: + via = "" + if len(self.path) > 1: + via = f" (via {' -> '.join(self.path)})" + root = (f"dependency '{self.dep}' is {self._kind_phrase} created in render " + f"scope, so its identity changes on every render" + if self.origin == self.dep else + f"dependency '{self.dep}' derives from '{self.origin}', " + f"{self._kind_phrase} created in render scope{via}, so its identity " + f"changes on every render") + return (f"effect re-runs on every render: {root}; the effect performs IO, " + f"which can become a request storm — stabilise '{self.origin}' with " + f"useMemo/useCallback (or move it out of render)") + + +class _Lattice: + """Stability of each binding name, computed to a fixpoint over references with a + cycle guard. Also records, for an UNSTABLE name, the upstream `origin` binding + and the reference `path` that carried the instability (for the evidence slice).""" + + def __init__(self, bindings: list[Binding]) -> None: + self._by_name = {b.name: b for b in bindings} + self._stab: dict[str, str] = {} + self._origin: dict[str, str] = {} + self._path: dict[str, tuple[str, ...]] = {} + + def stability(self, name: str) -> str: + return self._resolve(name, frozenset())[0] + + def origin(self, name: str) -> str: + return self._origin.get(name, name) + + def path(self, name: str) -> tuple[str, ...]: + return self._path.get(name, (name,)) + + def _resolve(self, name: str, on_stack: frozenset[str]) -> tuple[str, str, tuple[str, ...]]: + if name in self._stab: + return self._stab[name], self._origin.get(name, name), self._path.get(name, (name,)) + b = self._by_name.get(name) + if b is None: + # a name with no render-scope binding is a prop/state/global — stable. + return STABLE, name, (name,) + if name in on_stack: + # an identity cycle (a = b; b = a): cannot prove unstable — stay safe. + return UNKNOWN, name, (name,) + stab, origin, path = self._classify(b, on_stack | {name}) + self._stab[name] = stab + self._origin[name] = origin + self._path[name] = path + return stab, origin, path + + def _classify(self, b: Binding, on_stack: frozenset[str]) -> tuple[str, str, tuple[str, ...]]: + if b.init in _FRESH: + return UNSTABLE, b.name, (b.name,) + if b.init in _MEMOISED or b.init in _REFERENTIAL: + return STABLE, b.name, (b.name,) + if b.init in _DERIVED: + if not b.refs: + return UNKNOWN, b.name, (b.name,) + worst: str = STABLE + worst_origin: str = b.name + worst_path: tuple[str, ...] = (b.name,) + for r in b.refs: + s, o, p = self._resolve(r, on_stack) + if _RANK[s] > _RANK[worst]: + worst, worst_origin, worst_path = s, o, (b.name, *p) + return worst, worst_origin, worst_path + # "call" or any unrecognised kind: opaque identity -> conservative. + return UNKNOWN, b.name, (b.name,) + + +def find_effect_storms(effects: list[Effect]) -> list[EffectStorm]: + """Return every EFF001 effect storm: an IO effect with a provably UNSTABLE + dependency. Deterministic; sorted by location. One storm per effect (the first + unstable dep) — re-running once is the bug, the count of culprits is noise.""" + out: list[EffectStorm] = [] + for e in effects: + if not e.io: + continue + lat = _Lattice(list(e.bindings)) + decl = {b.name: b.line for b in e.bindings} + for dep in e.deps: + if lat.stability(dep) != UNSTABLE: + continue + origin = lat.origin(dep) + b = next((x for x in e.bindings if x.name == origin), None) + out.append(EffectStorm( + component=e.component, dep=dep, origin=origin, + origin_kind=b.init if b else "object", + file=e.file, line=e.line, + decl_line=decl.get(origin, e.line), path=lat.path(dep))) + break # one finding per effect + out.sort(key=lambda f: (f.file, f.line, f.dep)) + return out diff --git a/ownlang/ownir.py b/ownlang/ownir.py index 13040d6a..7d8b698f 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -139,6 +139,9 @@ find_weak_captive_dependencies, ) from .diagnostics import TITLES, Severity +from .effects import Binding as EffectBinding +from .effects import Effect as ReactEffect +from .effects import find_effect_storms from .evidence import code_flow, di_path_steps from .ownership import ( MethodSkeleton, @@ -542,6 +545,36 @@ def load(path: str) -> dict[str, Any]: raise OwnIRError( "service 'scope_cache_sites' must be an array of " "{type:str, file:str, line:int} objects") + # Optional reactive-effect graph (EFF001 — effect storm, P-020). Additive and + # optional: an older core simply ignores it. Each effect carries its render-scope + # binding table; the core (ownlang/effects.py) decides identity stability. + effs = result.get("effects", []) + if not isinstance(effs, list) or not all(isinstance(eff, dict) for eff in effs): + raise OwnIRError("OwnIR 'effects' must be a JSON array of objects") + for eff in effs: + deps = eff.get("deps", []) + if not isinstance(deps, list) or not all(isinstance(d, str) for d in deps): + raise OwnIRError("effect 'deps' must be an array of strings") + io = eff.get("io", False) + if not isinstance(io, bool): + raise OwnIRError(f"effect 'io' must be a boolean, got {io!r}") + eln = eff.get("line", 0) + if not isinstance(eln, int) or isinstance(eln, bool): + raise OwnIRError("effect 'line' must be an integer") + binds = eff.get("bindings", []) + if not isinstance(binds, list) or not all(isinstance(b, dict) for b in binds): + raise OwnIRError("effect 'bindings' must be a JSON array of objects") + for b in binds: + if not isinstance(b.get("name", ""), str): + raise OwnIRError("binding 'name' must be a string") + if not isinstance(b.get("init", "unknown"), str): + raise OwnIRError("binding 'init' must be a string") + refs = b.get("refs", []) + if not isinstance(refs, list) or not all(isinstance(r, str) for r in refs): + raise OwnIRError("binding 'refs' must be an array of strings") + bln = b.get("line", 0) + if not isinstance(bln, int) or isinstance(bln, bool): + raise OwnIRError("binding '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", []) @@ -1996,6 +2029,12 @@ def check_facts(facts: dict[str, Any]) -> list[Finding]: # it (see ownlang/di.py). Findings carry the registration site as file/line. findings.extend(_di_findings(facts)) + # EFF001 (effect storm): a separate core analysis over the render-scope binding + # graph (ownlang/effects.py), NOT the acquire/release model. The bridge routes + # the optional `effects` facts to it; the frontend only states what each binding + # syntactically is — the stability verdict is the core's. + findings.extend(_effect_findings(facts)) + # OWN050 (P-014 Tier A): a `+=` whose declaring type could not be resolved — # an advisory "leakage analysis skipped" note, never a leak. Routed through # this side path so it bypasses the ERROR-only diagnostic mapping above. @@ -2202,6 +2241,52 @@ def _di_findings(facts: dict[str, Any]) -> list[Finding]: return out +def _effect_findings(facts: dict[str, Any]) -> list[Finding]: + """Run the reactive-effect stability check over the facts' `effects` graph and + map each EFF001 storm to a Finding at the effect's call site (ownlang/effects.py). + Additive/optional, like `services`: absent or malformed `effects` -> no findings.""" + raw = facts.get("effects", []) + if not isinstance(raw, list): + return [] + effects: list[ReactEffect] = [] + for e in raw: + if not isinstance(e, dict): + continue + bindings = tuple( + EffectBinding( + name=str(b.get("name", "?")), + init=str(b.get("init", "unknown")), + refs=tuple(str(r) for r in b.get("refs", [])), + line=_as_int(b.get("line", 0)), + ) + for b in e.get("bindings", []) if isinstance(b, dict) + ) + effects.append(ReactEffect( + component=str(e.get("component", "?")), + deps=tuple(str(d) for d in e.get("deps", [])), + io=e.get("io") is True, + bindings=bindings, + file=str(e.get("file", "?")), + line=_as_int(e.get("line", 0)), + )) + out: list[Finding] = [] + for s in find_effect_storms(effects): + # reachability slice: where the effect re-fires -> where the unstable + # identity is minted (the fix site). + flow: tuple[tuple[str, int, str], ...] = () + if s.line >= 1 and s.decl_line >= 1: + flow = ( + (s.file, s.line, f"effect re-runs here on '{s.dep}'"), + (s.file, s.decl_line, + f"'{s.origin}' gets a fresh identity here — stabilise with useMemo"), + ) + out.append(Finding( + file=s.file, line=s.line, code="EFF001", + component=s.component, event=s.dep, handler="", + message=s.message, kind="react effect", flow=flow)) + return out + + def _unresolved_findings(facts: dict[str, Any]) -> list[Finding]: """Surface every "unresolved-subscription" marker as an advisory OWN050 finding (P-014 Tier A): the extractor saw a `+=` that looks like an event diff --git a/tests/run_tests.py b/tests/run_tests.py index 39fa90ec..965b66b4 100644 --- a/tests/run_tests.py +++ b/tests/run_tests.py @@ -1117,11 +1117,19 @@ def run() -> int: import test_diagnostics diag_rc = test_diagnostics.run() + # Reactive-effect stability (P-020): the EFF001 effect-storm analysis — the + # identity lattice, reference propagation, cycle safety, and the OwnIR bridge + # mapping the optional `effects` block to an EFF001 finding (a new core + # analysis dimension, like DI001, not the acquire/release leak model). + import test_effects + effects_rc = test_effects.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 or rid_rc or diag_rc) else 0 + or spec_rc or ownir_rc or own5_rc or rid_rc or diag_rc + or effects_rc) else 0 if __name__ == "__main__": diff --git a/tests/test_effects.py b/tests/test_effects.py new file mode 100644 index 00000000..bbbd7041 --- /dev/null +++ b/tests/test_effects.py @@ -0,0 +1,131 @@ +#!/usr/bin/env python3 +"""Reactive-effect stability tests — EFF001, the effect storm (P-020). + +Two layers, both with no dotnet/JS dependency: + 1. the pure core analysis (ownlang/effects.py): the identity-stability lattice, + reference propagation, cycle safety, and the IO + unstable -> storm rule; + 2. the OwnIR bridge (ownlang/ownir.py): the optional `effects` block routes + through check_facts to an EFF001 Finding at the effect's call site, and the + code reaches the SARIF rules catalogue. + +Run: python tests/test_effects.py + python tests/run_tests.py (runs it as part of the suite) +""" +from __future__ import annotations + +import os +import sys + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +from ownlang.effects import ( + STABLE, + UNKNOWN, + UNSTABLE, + Binding, + Effect, + _Lattice, + find_effect_storms, +) +from ownlang.ownir import build_sarif, check_facts + + +def _stab(binds: list[Binding]) -> dict[str, str]: + lat = _Lattice(binds) + return {b.name: lat.stability(b.name) for b in binds} + + +def run() -> int: + fails: list[str] = [] + checks = 0 + + def check(cond: bool, msg: str) -> None: + nonlocal checks + checks += 1 + if not cond: + fails.append(msg) + + # ---- the stability lattice ---- + check(_stab([Binding("f", "object", (), 1)]) == {"f": UNSTABLE}, "object literal is UNSTABLE") + check(_stab([Binding("f", "array", (), 1)]) == {"f": UNSTABLE}, "array literal is UNSTABLE") + check(_stab([Binding("f", "new", (), 1)]) == {"f": UNSTABLE}, "new expr must be UNSTABLE") + for stable_kind in ("memo", "callback", "ref", "prop", "primitive", "import", "fn"): + check(_stab([Binding("f", stable_kind, (), 1)]) == {"f": STABLE}, + f"{stable_kind} must be STABLE") + check(_stab([Binding("f", "call", (), 1)]) == {"f": UNKNOWN}, "opaque call must be UNKNOWN") + + # propagation: alias of an unstable literal is unstable; alias of a memo is stable. + check(_stab([Binding("a", "object", (), 1), Binding("c", "ident", ("a",), 2)]) + == {"a": UNSTABLE, "c": UNSTABLE}, "instability must propagate through an alias") + check(_stab([Binding("m", "memo", (), 1), Binding("d", "ident", ("m",), 2)]) + == {"m": STABLE, "d": STABLE}, "alias of a memo stays STABLE") + # join is worst-case: derive from a stable AND an unstable ref -> unstable. + check(_stab([Binding("s", "memo", (), 1), Binding("u", "object", (), 2), + Binding("d", "derive", ("s", "u"), 3)])["d"] == UNSTABLE, + "a derivation is as unstable as its worst input") + # identity cycle must not hang and stays conservative. + check(_stab([Binding("a", "ident", ("b",), 1), Binding("b", "ident", ("a",), 2)]) + == {"a": UNKNOWN, "b": UNKNOWN}, "an identity cycle must resolve to UNKNOWN") + + # ---- the storm rule ---- + fire = find_effect_storms([ + Effect("D", ("filters",), True, (Binding("filters", "object", (), 32),), "D.tsx", 33)]) + check(len(fire) == 1 and fire[0].dep == "filters" and fire[0].decl_line == 32, + "IO + unstable dep must fire one EFF001 anchored at the effect line") + check("request storm" in fire[0].message and "object literal" in fire[0].message, + "the message must name the storm and the unstable kind") + + silent_cases = [ + ("no IO", Effect("D", ("f",), False, (Binding("f", "object", (), 1),), "D.tsx", 2)), + ("memo dep", Effect("D", ("f",), True, (Binding("f", "memo", (), 1),), "D.tsx", 2)), + ("opaque call", Effect("D", ("f",), True, (Binding("f", "call", (), 1),), "D.tsx", 2)), + ("primitive dep", Effect("D", ("n",), True, (Binding("n", "primitive", (), 1),), "f", 2)), + ("dep with no binding (prop)", Effect("D", ("tenantId",), True, (), "D.tsx", 2)), + ] + for label, e in silent_cases: + check(find_effect_storms([e]) == [], f"{label} must stay silent (no false positive)") + + # derivation finding names the upstream origin and the path. + derived = find_effect_storms([ + Effect("D", ("c",), True, + (Binding("a", "object", (), 1), Binding("c", "ident", ("a",), 2)), "D.tsx", 3)])[0] + check(derived.origin == "a" and derived.path == ("c", "a"), + "a derived storm must point at the upstream unstable origin") + check("derives from 'a'" in derived.message, "the message must explain the derivation") + + # ---- the OwnIR bridge ---- + facts = { + "ownir_version": 0, "module": "D", "components": [], + "effects": [ + {"component": "Dashboard", "file": "Dashboard.tsx", "line": 33, "io": True, + "deps": ["filters"], + "bindings": [{"name": "filters", "init": "object", "refs": [], "line": 32}]}, + {"component": "Dashboard", "file": "Dashboard.tsx", "line": 40, "io": True, + "deps": ["stable"], + "bindings": [{"name": "stable", "init": "memo", "refs": ["x"], "line": 39}]}, + ], + } + findings = check_facts(facts) + eff = [f for f in findings if f.code == "EFF001"] + check(len(eff) == 1, f"bridge must yield one EFF001 (memo silent), got {len(eff)}") + check(eff[0].file == "Dashboard.tsx" and eff[0].line == 33, + "EFF001 must anchor at the effect call site") + check(bool(eff[0].flow), "EFF001 must carry a reachability slice (effect -> fix site)") + + # the code reaches the SARIF rules catalogue with its title. + rules = {r["id"]: r["shortDescription"]["text"] for r in + build_sarif(eff)["runs"][0]["tool"]["driver"]["rules"]} + check("EFF001" in rules and "storm" in rules["EFF001"], "EFF001 must appear in the SARIF rules") + + # malformed effects degrade gracefully (additive/optional, never a crash). + check(check_facts({"ownir_version": 0, "components": [], "effects": "nope"}) == [], + "a malformed effects block must not crash check_facts") + + for f in fails: + print(f"EFFECTS FAIL: {f}") + print(f"effects: {checks - len(fails)}/{checks} EFF001 stability checks passed") + return 1 if fails else 0 + + +if __name__ == "__main__": + raise SystemExit(run()) From f61a1ca82033179d1c27672b4402b503b94464ec Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 04:48:21 +0000 Subject: [PATCH 3/3] =?UTF-8?q?fix(ownts):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20real=20bugs=20+=20robustness=20(Codex=20+=20CodeRabbit)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Frontend correctness (Codex P2 + CodeRabbit Major): - Per-resource cleanup matching: `released` was kind-level, so two setIntervals with one clearInterval marked both released (a swallowed leak). Now matched to each acquire's own token/handler (_is_released) — the uncleared one still leaks. - Render-scope-only bindings: extract_effects walked every const/let/var in the file and attributed by name, so a like-named local inside a useEffect callback shadowed the real memoized dep and minted a false EFF001. Now only component-body depth-1 bindings count (_render_bindings + _body_brace, which also skips a destructured param list). - Top-level cleanup only: _split_cleanup accepted the first `return ... =>` anywhere; a nested callback's disposer wrongly suppressed OWN001. Now only the effect body's own (brace-depth 1) return is the cleanup. - Expression-bodied effects: `useEffect(() => fetch(url), [dep])` blindly hit _match_block and jumped to an unrelated `{` or ran off the end. _effect_callback now detects block vs expression bodies. Core robustness (CodeRabbit Major): - effects.py: a dep with no render-scope binding is STABLE only if identifier-like (`tenantId`, `props.id`); a forwarded non-identifier (`{}`, `new URL(x)`) is UNKNOWN, not silently STABLE. - ownir.py _effect_findings: validate each effect/binding shape and SKIP malformed entries (mirrors load()) instead of coercing `deps:"a"` into `("a",)` on the direct check_facts() path. Tests/CI/docs: - New fixture EffectEdges.tsx (partial timer cleanup + nested shadow) → 1 OWN001, pinned in test_ownts.py + a CI step; test_effects.py guards list derefs and adds a malformed-skip case (now 31 checks). - CI finding-count asserts are exact (grep -c on the code tag), not substrings. - Docs: P-020 marks only EFF001 shipped (EFF002 pending); Dashboard.tsx comments updated (3xOWN001 + 1 EFF001, EFF001 is a core analysis not a heuristic); README fences get a language tag (MD040). Full suite green (effects 31/31, ownir 194/194, explain); ruff + mypy --strict clean. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PVeuchch67CtjSbizYm8gi --- .github/workflows/ci.yml | 18 +- docs/proposals/P-020-ownts-react-effects.md | 4 +- frontend/ownts/README.md | 4 +- frontend/ownts/examples/Dashboard.tsx | 5 +- frontend/ownts/examples/EffectEdges.tsx | 24 +++ frontend/ownts/ownts.py | 190 +++++++++++++++----- frontend/ownts/test_ownts.py | 11 +- ownlang/effects.py | 14 +- ownlang/ownir.py | 39 ++-- tests/test_effects.py | 29 ++- 10 files changed, 263 insertions(+), 75 deletions(-) create mode 100644 frontend/ownts/examples/EffectEdges.tsx diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 710efc27..79d74be7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1051,7 +1051,8 @@ jobs: || { echo "FAIL: expected three OWN001 leaks"; exit 1; } echo "$out" | grep -q "\[EFF001\].*request storm" \ || { echo "FAIL: expected the EFF001 effect-storm verdict"; exit 1; } - echo "$out" | grep -qE "4 finding" \ + # exact finding count (a code-tagged line each), not a substring of "4 finding" + [ "$(echo "$out" | grep -cE '\[(OWN|EFF|DI)[0-9]{3}\]')" -eq 4 ] \ || { echo "FAIL: expected 3 OWN001 + 1 EFF001 = 4 findings"; exit 1; } - name: EFF001 stability showcase — only provable storms fire (low FP) run: | @@ -1060,17 +1061,28 @@ jobs: storm=$(python -m ownlang ownir "$RUNNER_TEMP/storm.facts.json" || true) echo "$storm" # the direct object dep and its derived alias fire; memo/ref/call/primitive/no-IO stay silent - echo "$storm" | grep -qE "2 finding" \ + [ "$(echo "$storm" | grep -c '\[EFF001\]')" -eq 2 ] \ || { echo "FAIL: expected exactly two EFF001 (object dep + derived alias)"; exit 1; } echo "$storm" | grep -q "derives from" \ || { echo "FAIL: expected the derivation (propagation) verdict"; exit 1; } + - name: Edge cases — partial timer cleanup + nested-scope shadow + run: | + python frontend/ownts/ownts.py frontend/ownts/examples/EffectEdges.tsx \ + -o "$RUNNER_TEMP/edges.facts.json" + edges=$(python -m ownlang ownir "$RUNNER_TEMP/edges.facts.json" || true) + echo "$edges" + # only the SECOND, uncleared interval leaks; the memoized dep is not shadowed + [ "$(echo "$edges" | grep -cE '\[(OWN|EFF|DI)[0-9]{3}\]')" -eq 1 ] \ + || { echo "FAIL: expected exactly one OWN001 (the uncleared timer)"; exit 1; } + echo "$edges" | grep -q "pollB" \ + || { echo "FAIL: the leak must be the second (uncleared) interval"; exit 1; } - name: The clean fixture (cleanups + useMemo'd dep) is silent run: | python frontend/ownts/ownts.py frontend/ownts/examples/DashboardClean.tsx \ -o "$RUNNER_TEMP/clean.facts.json" clean=$(python -m ownlang ownir "$RUNNER_TEMP/clean.facts.json" || true) echo "$clean" - echo "$clean" | grep -q "0 finding" \ + [ "$(echo "$clean" | grep -cE '\[(OWN|EFF|DI)[0-9]{3}\]')" -eq 0 ] \ || { echo "FAIL: cleaned-up + memoised effects must not fire"; exit 1; } # The distribution surface (Уровень 1): the own-check.sh orchestrator walks a diff --git a/docs/proposals/P-020-ownts-react-effects.md b/docs/proposals/P-020-ownts-react-effects.md index dc02d578..bde4ba28 100644 --- a/docs/proposals/P-020-ownts-react-effects.md +++ b/docs/proposals/P-020-ownts-react-effects.md @@ -156,7 +156,7 @@ has on the .NET side (P-013/P-015); 1/2/6/7 are this proposal's actual work. ## Open questions -1. **The new analysis for `EFF001/002`.** ✅ **Answered (implemented).** It is a +1. **The new analysis for `EFF001`.** ✅ **Answered (implemented).** It is a genuinely new core analysis, not an `OWN001` acquire-site — and *not* a frontend verdict either. The resolution: the frontend emits per-binding **facts** (each render-scope binding's syntactic `init` kind + the names it references, the dep @@ -166,6 +166,8 @@ has on the .NET side (P-013/P-015); 1/2/6/7 are this proposal's actual work. effect is `EFF001` iff it does IO **and** a dep is *provably* `UNSTABLE` (`UNKNOWN`/memoised/primitive clear it — low false positives). The gating discipline held: `EFF001` is its own core code (like `DI001`), never an `OWN001`. + `EFF002` (network IO with no stable guard) is intended to reuse this same + lattice but is **not yet implemented** — it remains the next increment. 2. **Confidence tier.** `EFF001` clearly wants TS-mode type info (is the dep an object literal? does the body do IO?). What, if anything, survives into JS mode (P-017's heuristic tier) as a warning? diff --git a/frontend/ownts/README.md b/frontend/ownts/README.md index 4bfd3b46..7f097366 100644 --- a/frontend/ownts/README.md +++ b/frontend/ownts/README.md @@ -37,7 +37,7 @@ python frontend/ownts/test_ownts.py `DashboardClean.tsx` (every acquire has a cleanup `return`, and the unstable dep is `useMemo`'d) is silent. -``` +```text Dashboard.tsx:13: error: [OWN001] timer 'setInterval(() =>' ... never stopped ... (leak) [resource: timer] Dashboard.tsx:21: error: [OWN001] the result of '.subscribe(...)' is ignored ... (leak) [resource: subscription token] Dashboard.tsx:27: error: [OWN001] event '.addEventListener(...)' ... never unsubscribed ... (leak) [resource: subscription token] @@ -71,7 +71,7 @@ each `useEffect`, its dep list, whether the body does IO, and a render-scope It does **not** pre-judge stability. The **core** runs an identity-stability lattice (`STABLE < UNKNOWN < UNSTABLE`) to a fixpoint over the references and decides: -``` +```text EFF001 fires ⟺ the effect does IO ∧ some dep is *provably* UNSTABLE ``` diff --git a/frontend/ownts/examples/Dashboard.tsx b/frontend/ownts/examples/Dashboard.tsx index bf1d8ca1..97e378f0 100644 --- a/frontend/ownts/examples/Dashboard.tsx +++ b/frontend/ownts/examples/Dashboard.tsx @@ -4,7 +4,8 @@ // // python frontend/ownts/ownts.py frontend/ownts/examples/Dashboard.tsx --check // -// Expect three OWN001 findings (EFF004 timer, EFF003 subscribe, EFF003 listener). +// Expect three OWN001 findings (EFF004 timer, EFF003 subscribe, EFF003 listener) +// plus one EFF001 effect-storm finding (the unstable `filters` dependency below). import { useEffect } from "react"; export function Dashboard({ tenantId }: { tenantId: string }) { @@ -28,7 +29,7 @@ export function Dashboard({ tenantId }: { tenantId: string }) { // no `return () => window.removeEventListener("resize", onResize)` — leak }, []); - // EFF001 (frontend heuristic only — NOT a core OWN001): `filters` is a fresh + // EFF001 (a separate core analysis — NOT an OWN001 leak): `filters` is a fresh // object identity every render, and the effect does IO -> request storm. const filters = { tenantId }; useEffect(() => { diff --git a/frontend/ownts/examples/EffectEdges.tsx b/frontend/ownts/examples/EffectEdges.tsx new file mode 100644 index 00000000..e5eae34e --- /dev/null +++ b/frontend/ownts/examples/EffectEdges.tsx @@ -0,0 +1,24 @@ +// Edge cases the per-resource + render-scope fixes must get right (Codex/CodeRabbit): +// python frontend/ownts/ownts.py frontend/ownts/examples/EffectEdges.tsx --check +// Expect exactly ONE finding: OWN001 on the second, uncleared interval. No EFF001. +import { useEffect, useMemo } from "react"; + +export function EdgeBoard({ id }: { id: string }) { + // Two timers; only the first is cleared -> the SECOND still leaks. A kind-level + // "is there any clearInterval?" check would wrongly mark both released. + useEffect(() => { + const a = setInterval(pollA, 1000); + const b = setInterval(pollB, 2000); + return () => clearInterval(a); // only `a` cleared; `b` leaks (one OWN001) + }, []); + + // The render-scope dep is memoized (stable). A like-named local INSIDE the effect + // callback must not shadow it into a false EFF001 storm. + const filters = useMemo(() => ({ id }), [id]); + useEffect(() => { + const filters = { id }; // nested-scope local — NOT the component's render scope + fetch(`/api/${filters.id}`); + }, [filters]); // refers to the stable outer (memoized) `filters` + + return
edges
; +} diff --git a/frontend/ownts/ownts.py b/frontend/ownts/ownts.py index a0801f8b..dccdb9f9 100644 --- a/frontend/ownts/ownts.py +++ b/frontend/ownts/ownts.py @@ -44,33 +44,79 @@ # --- acquire catalog: how each React acquire maps onto a core resource kind ----- # # Each acquire has (a) a regex that spots the acquire call, (b) the OwnIR `resource` -# discriminator the core understands, (c) the cleanup verb that releases it, and -# (d) the EFF id + tag for provenance. The `resource` value is the *only* field the -# core acts on; `eff`/`profile` ride along as additive provenance the core ignores. +# discriminator the core understands, and (c) the EFF id + tag for provenance. The +# `resource` value is the *only* field the core acts on; `eff`/`profile` ride along +# as additive provenance the core ignores. Whether a *specific* acquire is released +# is decided per-resource by `_is_released` (matched to its own token/handler — not +# a kind-level "is there any cleanup verb", which would mark every same-kind acquire +# released as soon as one is cleaned up). @dataclass(frozen=True) class Acquire: name: str # human label, e.g. "setInterval" pattern: re.Pattern # spots the acquire call resource: str # OwnIR resource kind: timer / subscribe / subscription - release: re.Pattern # the cleanup verb that releases it eff: str # Own.React catalog id ACQUIRES: list[Acquire] = [ - Acquire("setInterval", re.compile(r"\bsetInterval\s*\("), - "timer", re.compile(r"\bclearInterval\s*\("), "EFF004"), - Acquire("setTimeout", re.compile(r"\bsetTimeout\s*\("), - "timer", re.compile(r"\bclearTimeout\s*\("), "EFF004"), - Acquire(".subscribe", re.compile(r"\.subscribe\s*\("), - "subscribe", re.compile(r"\.unsubscribe\s*\(|\.remove\s*\("), "EFF003"), + Acquire("setInterval", re.compile(r"\bsetInterval\s*\("), "timer", "EFF004"), + Acquire("setTimeout", re.compile(r"\bsetTimeout\s*\("), "timer", "EFF004"), + Acquire(".subscribe", re.compile(r"\.subscribe\s*\("), "subscribe", "EFF003"), Acquire("addEventListener", re.compile(r"\.addEventListener\s*\("), - "subscription", re.compile(r"\.removeEventListener\s*\("), "EFF003"), + "subscription", "EFF003"), ] + +def _lhs_token(setup: str, pos: int) -> str | None: + """The variable an acquire's result is bound to, e.g. `id` in + `const id = setInterval(...)` or `sub` in `const sub = obs.subscribe(...)`. + Scoped to the current statement so an earlier `const` does not bleed in.""" + head = re.split(r"[;\n{}]", setup[:pos])[-1] + m = re.search(r"(?:const|let|var)\s+([A-Za-z_$][\w$]*)\s*=", head) + return m.group(1) if m else None + + +def _is_released(acq: Acquire, setup: str, pos: int, cleanup: str) -> bool: + """Whether THIS acquire (at `pos` in `setup`) is released by the effect's cleanup + — matched to its own handle, so two `setInterval`s with one `clearInterval` leave + the other a leak. A resource with no capturable handle (a bare `setInterval(...)` + or an ignored `.subscribe(...)` result) can never be released → False.""" + if acq.resource == "timer": + tok = _lhs_token(setup, pos) + return bool(tok and re.search( + rf"\bclear(?:Interval|Timeout)\s*\(\s*{re.escape(tok)}\b", cleanup)) + if acq.resource == "subscribe": + tok = _lhs_token(setup, pos) + return bool(tok and re.search( + rf"\b{re.escape(tok)}\s*\.\s*(?:unsubscribe|remove)\s*\(", cleanup)) + if acq.resource == "subscription": # addEventListener(event, handler) + hm = re.search(r"addEventListener\s*\(\s*[^,]+,\s*([A-Za-z_$][\w$.]*)", + setup[pos:]) + handler = hm.group(1) if hm else None + return bool(handler and re.search( + rf"removeEventListener\s*\(\s*[^,]+,\s*{re.escape(handler)}\b", cleanup)) + return False + # A React component is a function whose name is Capitalized (the JSX convention). _COMPONENT = re.compile( - r"(?:function\s+([A-Z]\w*)\s*\(|" + r"(?:function\s+([A-Z]\w*)\b|" r"(?:const|let|var)\s+([A-Z]\w*)\s*=\s*(?:\([^)]*\)|\w+)\s*(?::[^=]+)?=>)" ) + + +def _body_brace(text: str, start: int) -> int: + """Index of a component's body `{`, scanning from `start` (just past the name or + `=>`). Skips the parameter list by paren depth, so a destructured param + `({ x }: { x: T })` is not mistaken for the body block.""" + depth = 0 + for i in range(start, len(text)): + c = text[i] + if c == "(": + depth += 1 + elif c == ")": + depth -= 1 + elif c == "{" and depth == 0: + return i + return -1 _USE_EFFECT = re.compile(r"\buseEffect\s*\(") @@ -169,35 +215,77 @@ def _component_at(text: str, idx: int) -> str: return name or "AnonymousComponent" +def _expr_end(text: str, i: int) -> int: + """End index of an expression body — the first top-level `,` or `)` from `i` + (so the `, [deps])` tail and the `useEffect(` close are not swallowed).""" + depth = 0 + while i < len(text): + c = text[i] + if c in "([{": + depth += 1 + elif c in ")]}": + if depth == 0: + return i + depth -= 1 + elif c == "," and depth == 0: + return i + i += 1 + return i + + +def _effect_callback(text: str, after_open: int) -> tuple[str, int, list[str] | None, int]: + """Parse `useEffect(, [deps])` from just past the `(`. Returns + (body, body_start, deps, end). Handles BOTH a block `() => { ... }` and an + expression `() => fetch(url)` callback — calling `_match_block` blindly on the + latter would jump to an unrelated `{` or run off the end. `deps` is None when no + dependency array is present; `body` includes the braces for a block callback.""" + arrow = text.find("=>", after_open) + i = (arrow + 2) if arrow != -1 else after_open + while i < len(text) and text[i] in " \t\r\n": + i += 1 + if i < len(text) and text[i] == "{": + end = _match_block(text, i) + else: + end = _expr_end(text, i) + body = text[i:end] + deps_m = re.search(r"\s*,\s*\[([^\]]*)\]", text[end:end + 200]) + deps = ([d.strip() for d in deps_m.group(1).split(",") if d.strip()] + if deps_m else None) + return body, i, deps, end + + def _split_cleanup(body: str) -> tuple[str, str]: - """Split an effect body into (setup, cleanup). Cleanup is the block of the - `return () => { ... }` the effect hands back to React; setup is the rest.""" - m = re.search(r"return\s*(?:\(\s*\)|\w+)\s*=>", body) - if not m: - return body, "" - brace = body.find("{", m.end()) - if brace == -1: - # `return () => clearInterval(id)` — single-expression cleanup, no block. - nl = body.find("\n", m.end()) - tail = body[m.end(): nl if nl != -1 else len(body)] - return body[: m.start()], tail - end = _match_block(body, brace) - return body[: m.start()] + body[end:], body[brace:end] + """Split an effect block body into (setup, cleanup). Cleanup is the block of the + effect's OWN top-level `return () => { ... }` — a `return ... =>` nested inside a + callback (brace-depth > 1) is NOT the effect cleanup and must not suppress a + leak. For an expression-bodied effect there is no cleanup.""" + for m in re.finditer(r"return\s*(?:\(\s*\)|\w+)\s*=>", body): + prefix = body[:m.start()] + if prefix.count("{") - prefix.count("}") != 1: # 1 == the effect body's own brace + continue + brace = body.find("{", m.end()) + if brace == -1: + # `return () => clearInterval(id)` — single-expression cleanup, no block. + nl = body.find("\n", m.end()) + tail = body[m.end(): nl if nl != -1 else len(body)] + return body[: m.start()], tail + end = _match_block(body, brace) + return body[: m.start()] + body[end:], body[brace:end] + return body, "" def extract(path: str) -> list[Component]: text = _strip_comments(open(path, encoding="utf-8").read()) comps: dict[str, Component] = {} for eff in _USE_EFFECT.finditer(text): - end = _match_block(text, eff.end()) - body = text[eff.end():end] + body, body_start, _deps, _end = _effect_callback(text, eff.end()) setup, cleanup = _split_cleanup(body) cname = _component_at(text, eff.start()) comp = comps.setdefault(cname, Component(cname, path)) for acq in ACQUIRES: for hit in acq.pattern.finditer(setup): - line = text.count("\n", 0, eff.end() + hit.start()) + 1 - released = bool(acq.release.search(cleanup)) + line = text.count("\n", 0, body_start + hit.start()) + 1 + released = _is_released(acq, setup, hit.start(), cleanup) # the acquire expression, trimmed to the call head for a readable tag snippet = setup[hit.start():].splitlines()[0].strip().rstrip("{").strip() comp.resources.append( @@ -272,27 +360,45 @@ def _classify_rhs(rhs: str) -> tuple[str, list[str]]: return "unknown", [] +def _render_bindings(text: str) -> dict[str, list[dict]]: + """The render-scope binding table per component: only bindings declared DIRECTLY + in the component body (brace-depth 1). A `const filters = {...}` inside a + `useEffect` callback, an event handler, or any nested block is NOT render scope — + it must not shadow the real outer dependency of the same name and mint a false + EFF001. Excluding a render-level `if`/`try` binding only costs a missed finding + (the dep reads as stable), never a false one — the safe direction.""" + out: dict[str, list[dict]] = {} + for cm in _COMPONENT.finditer(text): + name = cm.group(1) or cm.group(2) + brace = _body_brace(text, cm.end()) # skips a destructured param list + if brace == -1: + continue + body_end = _match_block(text, brace) + body = text[brace + 1:body_end] + base = text.count("\n", 0, brace + 1) # 0-based line index of the body start + binds: list[dict] = [] + for m in _BINDING.finditer(body): + prefix = body[:m.start()] + if prefix.count("{") != prefix.count("}"): + continue # inside a nested block -> not the component's render scope + kind, refs = _classify_rhs(m.group(2)) + binds.append({"name": m.group(1), "init": kind, "refs": refs, + "line": base + prefix.count("\n") + 1}) + out[name] = binds + return out + + def extract_effects(path: str) -> list[dict]: """Extract the EFF001 stability facts: for each `useEffect`, its dependency list, whether its body does network IO, and the render-scope binding table of the component it lives in. The core's effects analysis turns these into a verdict.""" text = _strip_comments(open(path, encoding="utf-8").read()) - # render-scope bindings, attributed to their enclosing component. - binds_by_comp: dict[str, list[dict]] = {} - for m in _BINDING.finditer(text): - kind, refs = _classify_rhs(m.group(2)) - binds_by_comp.setdefault(_component_at(text, m.start()), []).append({ - "name": m.group(1), "init": kind, "refs": refs, - "line": text.count("\n", 0, m.start()) + 1, - }) + binds_by_comp = _render_bindings(text) effects: list[dict] = [] for eff in _USE_EFFECT.finditer(text): - end = _match_block(text, eff.end()) - body = text[eff.end():end] - deps_m = re.search(r",\s*\[([^\]]*)\]\s*\)", text[end - 1:end + 200]) - if not deps_m: # no dep array -> not an EFF001 candidate (by-design re-run cadence) + body, _start, deps, _end = _effect_callback(text, eff.end()) + if deps is None: # no dep array -> not an EFF001 candidate (by-design re-run cadence) continue - deps = [d.strip() for d in deps_m.group(1).split(",") if d.strip()] cname = _component_at(text, eff.start()) effects.append({ "component": cname, "file": path, diff --git a/frontend/ownts/test_ownts.py b/frontend/ownts/test_ownts.py index 47d26288..255d5241 100644 --- a/frontend/ownts/test_ownts.py +++ b/frontend/ownts/test_ownts.py @@ -44,9 +44,16 @@ def main() -> int: storm = codes("EffectStorm.tsx") assert storm == ["EFF001", "EFF001"], f"EffectStorm -> {storm}" + # Edge cases: two same-kind timers with only one cleared -> exactly ONE OWN001 + # (per-resource cleanup matching, not kind-level); a like-named local inside an + # effect callback must NOT shadow the memoized render-scope dep into a false + # EFF001 (render-scope-only bindings). + edges = codes("EffectEdges.tsx") + assert edges == ["OWN001"], f"EffectEdges -> {edges}" + print("OwnTS spike OK: leaky=3xOWN001+EFF001, clean=0, kinds=timer/subscribe/" - "subscription, EffectStorm=2xEFF001 (core stability analysis, propagation " - "+ conservative).") + "subscription, EffectStorm=2xEFF001, EffectEdges=1xOWN001 (per-resource " + "cleanup + render-scope-only bindings).") return 0 diff --git a/ownlang/effects.py b/ownlang/effects.py index ffa23501..f05b8ca3 100644 --- a/ownlang/effects.py +++ b/ownlang/effects.py @@ -34,6 +34,7 @@ """ from __future__ import annotations +import re from dataclasses import dataclass # stability lattice @@ -42,6 +43,10 @@ UNSTABLE = "unstable" _RANK = {STABLE: 0, UNKNOWN: 1, UNSTABLE: 2} +# a plain identifier or member chain (`tenantId`, `props.id`) — referentially stable +# when it has no render-scope binding; anything else (a literal/ctor/call) is not. +_IDENT = re.compile(r"^[A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)*$") + # initialiser kinds the frontend can observe syntactically _FRESH = frozenset({"object", "array", "new"}) # fresh identity every render _MEMOISED = frozenset({"memo", "callback", "ref"}) # useMemo/useCallback/useRef @@ -144,8 +149,13 @@ def _resolve(self, name: str, on_stack: frozenset[str]) -> tuple[str, str, tuple return self._stab[name], self._origin.get(name, name), self._path.get(name, (name,)) b = self._by_name.get(name) if b is None: - # a name with no render-scope binding is a prop/state/global — stable. - return STABLE, name, (name,) + # A dep with no render-scope binding: a plain identifier or member chain + # (`tenantId`, `props.id`) is a prop/state/global — referentially stable. + # A non-identifier dep the frontend forwarded verbatim (`{}`, `new URL(x)`, + # `f()`) is NOT provably stable — stay conservative (UNKNOWN, no finding) + # rather than assert STABLE and silence a real fresh-identity storm. + stab = STABLE if _IDENT.match(name) else UNKNOWN + return stab, name, (name,) if name in on_stack: # an identity cycle (a = b; b = a): cannot prove unstable — stay safe. return UNKNOWN, name, (name,) diff --git a/ownlang/ownir.py b/ownlang/ownir.py index 7d8b698f..d9bf1781 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -2250,22 +2250,37 @@ def _effect_findings(facts: dict[str, Any]) -> list[Finding]: return [] effects: list[ReactEffect] = [] for e in raw: + # Validate each entry's shape and SKIP a malformed one (do not coerce). This + # mirrors load(), but also guards the direct check_facts() path (tests / + # embedders) that never went through load() — a `deps: "a"` must not become + # `("a",)` and emit a spurious EFF001. if not isinstance(e, dict): continue - bindings = tuple( - EffectBinding( - name=str(b.get("name", "?")), - init=str(b.get("init", "unknown")), - refs=tuple(str(r) for r in b.get("refs", [])), - line=_as_int(b.get("line", 0)), - ) - for b in e.get("bindings", []) if isinstance(b, dict) - ) + deps_raw = e.get("deps", []) + io = e.get("io", False) + binds_raw = e.get("bindings", []) + if (not isinstance(deps_raw, list) or not all(isinstance(d, str) for d in deps_raw) + or not isinstance(io, bool) or not isinstance(binds_raw, list)): + continue + bindings: list[EffectBinding] = [] + malformed = False + for b in binds_raw: + refs = b.get("refs", []) if isinstance(b, dict) else None + if (not isinstance(b, dict) or not isinstance(b.get("name", ""), str) + or not isinstance(b.get("init", "unknown"), str) + or not isinstance(refs, list) or not all(isinstance(r, str) for r in refs)): + malformed = True + break + bindings.append(EffectBinding( + name=str(b.get("name", "?")), init=str(b.get("init", "unknown")), + refs=tuple(refs), line=_as_int(b.get("line", 0)))) + if malformed: + continue effects.append(ReactEffect( component=str(e.get("component", "?")), - deps=tuple(str(d) for d in e.get("deps", [])), - io=e.get("io") is True, - bindings=bindings, + deps=tuple(deps_raw), + io=io, + bindings=tuple(bindings), file=str(e.get("file", "?")), line=_as_int(e.get("line", 0)), )) diff --git a/tests/test_effects.py b/tests/test_effects.py index bbbd7041..ed13aff7 100644 --- a/tests/test_effects.py +++ b/tests/test_effects.py @@ -72,8 +72,9 @@ def check(cond: bool, msg: str) -> None: Effect("D", ("filters",), True, (Binding("filters", "object", (), 32),), "D.tsx", 33)]) check(len(fire) == 1 and fire[0].dep == "filters" and fire[0].decl_line == 32, "IO + unstable dep must fire one EFF001 anchored at the effect line") - check("request storm" in fire[0].message and "object literal" in fire[0].message, - "the message must name the storm and the unstable kind") + if fire: # guard the deref so a regression reports all failures, not an IndexError + check("request storm" in fire[0].message and "object literal" in fire[0].message, + "the message must name the storm and the unstable kind") silent_cases = [ ("no IO", Effect("D", ("f",), False, (Binding("f", "object", (), 1),), "D.tsx", 2)), @@ -88,10 +89,13 @@ def check(cond: bool, msg: str) -> None: # derivation finding names the upstream origin and the path. derived = find_effect_storms([ Effect("D", ("c",), True, - (Binding("a", "object", (), 1), Binding("c", "ident", ("a",), 2)), "D.tsx", 3)])[0] - check(derived.origin == "a" and derived.path == ("c", "a"), - "a derived storm must point at the upstream unstable origin") - check("derives from 'a'" in derived.message, "the message must explain the derivation") + (Binding("a", "object", (), 1), Binding("c", "ident", ("a",), 2)), "D.tsx", 3)]) + check(len(derived) == 1, "the derived-alias case must produce one EFF001") + if derived: + d = derived[0] + check(d.origin == "a" and d.path == ("c", "a"), + "a derived storm must point at the upstream unstable origin") + check("derives from 'a'" in d.message, "the message must explain the derivation") # ---- the OwnIR bridge ---- facts = { @@ -108,9 +112,10 @@ def check(cond: bool, msg: str) -> None: findings = check_facts(facts) eff = [f for f in findings if f.code == "EFF001"] check(len(eff) == 1, f"bridge must yield one EFF001 (memo silent), got {len(eff)}") - check(eff[0].file == "Dashboard.tsx" and eff[0].line == 33, - "EFF001 must anchor at the effect call site") - check(bool(eff[0].flow), "EFF001 must carry a reachability slice (effect -> fix site)") + if eff: + check(eff[0].file == "Dashboard.tsx" and eff[0].line == 33, + "EFF001 must anchor at the effect call site") + check(bool(eff[0].flow), "EFF001 must carry a reachability slice (effect -> fix site)") # the code reaches the SARIF rules catalogue with its title. rules = {r["id"]: r["shortDescription"]["text"] for r in @@ -120,6 +125,12 @@ def check(cond: bool, msg: str) -> None: # malformed effects degrade gracefully (additive/optional, never a crash). check(check_facts({"ownir_version": 0, "components": [], "effects": "nope"}) == [], "a malformed effects block must not crash check_facts") + # a per-entry malformed effect is SKIPPED, not coerced: `deps: "a"` must NOT + # become `("a",)` and emit a spurious EFF001 on the direct check_facts() path. + coerce = check_facts({"ownir_version": 0, "components": [], "effects": [ + {"component": "X", "file": "X.tsx", "line": 1, "io": True, "deps": "a", + "bindings": [{"name": "a", "init": "object", "refs": [], "line": 1}]}]}) + check(coerce == [], f"a malformed deps='a' entry must be skipped, got {coerce}") for f in fails: print(f"EFFECTS FAIL: {f}")