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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .github/workflows/ci.yml
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 ] \
Comment thread
coderabbitai[bot] marked this conversation as resolved.
|| { 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 \
Expand Down
23 changes: 23 additions & 0 deletions frontend/ownts/examples/EffectExprCleanup.tsx
Original file line numberDiff line numberDiff line change
@@ -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 <div>expr cleanup</div>;
}
30 changes: 24 additions & 6 deletions frontend/ownts/ownts.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -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


Expand Down
6 changes: 6 additions & 0 deletions frontend/ownts/test_ownts.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -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")
Expand Down
Loading