diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2a6cd279..5c20e6c3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1088,6 +1088,18 @@ jobs: || { echo "FAIL: expected one OWN001 (listener) + one EFF001 (object dep)"; exit 1; } echo "$hard" | grep -q "scroll" \ || { echo "FAIL: the leak must be the options-dropped scroll listener"; exit 1; } + - name: Expression-bodied cleanup with an options object is silent + run: | + python frontend/ownts/ownts.py frontend/ownts/examples/EffectExprCleanup.tsx \ + -o "$RUNNER_TEMP/expr.facts.json" + # no `|| true`: this case expects ZERO findings (rc 0), so a parser/core + # crash (rc 2) must FAIL the step, not be swallowed into an empty result. + expr=$(python -m ownlang ownir "$RUNNER_TEMP/expr.facts.json") + echo "$expr" + # the `{` of `{capture: true}` belongs to the call, not the cleanup block — + # the listener is released, so no false-positive leak + [ "$(echo "$expr" | grep -cE '\[(OWN|EFF|DI)[0-9]{3}\]')" -eq 0 ] \ + || { echo "FAIL: a properly-released listener must not be reported"; exit 1; } - name: The clean fixture (cleanups + useMemo'd dep) is silent run: | python frontend/ownts/ownts.py frontend/ownts/examples/DashboardClean.tsx \ diff --git a/frontend/ownts/examples/EffectExprCleanup.tsx b/frontend/ownts/examples/EffectExprCleanup.tsx new file mode 100644 index 00000000..da7286d6 --- /dev/null +++ b/frontend/ownts/examples/EffectExprCleanup.tsx @@ -0,0 +1,23 @@ +// Regression (CodeRabbit): an EXPRESSION-bodied cleanup whose removeEventListener +// passes an options OBJECT must still parse — the `{` of `{capture: true}` is part +// of the call, not the cleanup block, so the listener is correctly released. +// python frontend/ownts/ownts.py frontend/ownts/examples/EffectExprCleanup.tsx --check +// Expect ZERO findings (no false-positive leak). +import { useEffect } from "react"; + +export function ExprCleanup() { + useEffect(() => { + window.addEventListener("scroll", onScroll, { capture: true }); + return () => window.removeEventListener("scroll", onScroll, { capture: true }); + }, []); + + // a MULTI-LINE expression-bodied cleanup must not be cut at the first newline + // after `=>` (which would leave an empty cleanup and a false leak). + useEffect(() => { + el.addEventListener("resize", onResize); + return () => + el.removeEventListener("resize", onResize); + }, []); + + return
expr cleanup
; +} diff --git a/frontend/ownts/ownts.py b/frontend/ownts/ownts.py index ef0f84c1..7ebb7313 100644 --- a/frontend/ownts/ownts.py +++ b/frontend/ownts/ownts.py @@ -390,12 +390,30 @@ def _cleanup_span(mbody: str) -> tuple[int, int] | None: prefix = mbody[:m.start()] if prefix.count("{") - prefix.count("}") != 1: # 1 == the effect body's own brace continue - brace = mbody.find("{", m.end()) - if brace == -1: - # `return () => clearInterval(id)` — single-expression cleanup, no block. - nl = mbody.find("\n", m.end()) - return (m.start(), nl if nl != -1 else len(mbody)) - return (m.start(), _match_block(mbody, brace)) + rest = mbody[m.end():] + stripped = rest.lstrip() + if stripped.startswith("{"): # block-bodied cleanup: `=> { ... }` + brace = m.end() + (len(rest) - len(stripped)) + return (m.start(), _match_block(mbody, brace)) + # single-expression cleanup. Consume the WHOLE expression across line breaks + # — stopping at a top-level `;` or the effect body's own closing brace, NOT + # the first newline (which truncates a multi-line call) and NOT a `find("{")` + # (which would mistake an options object like `{capture: true}` inside the + # call for the cleanup block). Both truncations drop the closing `)` and turn + # a released listener into a false leak. + i, depth = m.end(), 0 + while i < len(mbody): + c = mbody[i] + if c in "([{": + depth += 1 + elif c in ")]}": + if depth == 0: # the effect body's closing brace — expression ends + break + depth -= 1 + elif c == ";" and depth == 0: + break + i += 1 + return (m.start(), i) return None diff --git a/frontend/ownts/test_ownts.py b/frontend/ownts/test_ownts.py index 5c50a881..d04d806f 100644 --- a/frontend/ownts/test_ownts.py +++ b/frontend/ownts/test_ownts.py @@ -57,6 +57,12 @@ def main() -> int: hardening = codes("EffectHardening.tsx") assert hardening == ["OWN001", "EFF001"], f"EffectHardening -> {hardening}" + # an expression-bodied cleanup whose removeEventListener carries an options + # object must parse (the `{` belongs to the call, not the cleanup block) — the + # listener is released, so no false-positive leak. + expr_cleanup = codes("EffectExprCleanup.tsx") + assert expr_cleanup == [], f"EffectExprCleanup should be silent -> {expr_cleanup}" + # addEventListener release must match the receiver and capture/options, not just # the handler (dropping `true` or changing the target still leaks). sub = next(a for a in ownts.ACQUIRES if a.resource == "subscription")