feat(ownts): мультиланг-PoC — React useEffect через OwnIR + EFF001 (effect storm) как настоящий core-анализ#144
Conversation
….React) 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PVeuchch67CtjSbizYm8gi
…P-020) 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PVeuchch67CtjSbizYm8gi
📝 WalkthroughWalkthroughAdds a TSX OwnTS extractor that emits OwnIR acquire and effect facts, extends core checking with EFF001 effect-storm analysis, and adds example fixtures, tests, CI, and documentation. ChangesOwnTS React effect analysis
Sequence Diagram(s)sequenceDiagram
participant ownts.py
participant ownir.py
participant effects.py
ownts.py->>ownir.py: check_facts(facts with effects)
ownir.py->>effects.py: find_effect_storms(effects)
effects.py-->>ownir.py: EFF001 findings
ownir.py-->>ownts.py: verdict codes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
PhysShell
left a comment
PhysShell
left a comment
There was a problem hiding this comment.
Гид по диффу — где смотреть и почему так
PoC = один чекер, два фронтенда (Roslyn C# + новый OwnTS), и новое измерение анализа поверх ядра. Ключевая дисциплина, которую держим везде: фронтенд эмитит факты, ядро принимает вердикт.
ownlang/effects.py— новый core-анализ: решётка стабильности идентичностиSTABLE < UNKNOWN < UNSTABLEс распространением по ссылкам до неподвижной точки. Это не утечка и не regex.ownlang/ownir.py— мост: новый additive/optional блокeffects(не бампитownir_version, зеркалитservices);_effect_findings→EFF001(свой код, какDI001).frontend/ownts/— OwnTS отдаёт факты (deps, IO-флаг, таблица render-биндингов); эвристика EFF001-на-stderr убрана в пользу реального вердикта ядра.examples/EffectStorm.tsx— витрина: 2 шторма (прямой объектный dep + производный алиас, распространение нестабильности), а memo/ref/call()/примитив/без-IO — молчат.
Инлайн-комменты повесил на 4 точки, где выбор неочевиден (гейт EFF001 ≠ OWN001, консервативный UNKNOWN, алиасы импортов из-за коллизии Effect, граница факт/вердикт во фронтенде).
CI: все 14 проверок зелёные — lint (ruff + mypy --strict), tests (py3.11–3.13), новая OwnTS (React useEffect) -> OwnIR -> core, corpus benchmark, golden C#. Полный suite локально: ownir 194/194, effects 29/29.
Открыт к спору по двум местам: (1) стоит ли call() оставлять UNKNOWN или давать опциональный warning-тир; (2) [resource: react effect] в рендере — оставить как у DI001 ([resource: DI lifetime]) ради единообразия, или ввести [profile: react].
Generated by Claude Code
| 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: |
There was a problem hiding this comment.
Сердце анализа — и осознанный выбор в пользу низкого FP. Шторм срабатывает только когда зависимость доказуемо UNSTABLE. UNKNOWN (непрозрачный call()), memo/ref и примитивы — НЕ срабатывают. Это намеренно: «непрозрачное» нельзя выдавать за баг, иначе мы заваливаем дашборды ложняком (требование P-020 — low FP). Цена — пропуск штормов через функцию, возвращающую свежий объект (makeFilters()); ловить их = звать стабильность функций, отдельный шаг.
Generated by Claude Code
| ) | ||
| from .diagnostics import TITLES, Severity | ||
| from .effects import Binding as EffectBinding | ||
| from .effects import Effect as ReactEffect |
There was a problem hiding this comment.
Алиасы намеренные — не косметика. ast_nodes.Effect уже импортирован в этом модуле (enum эффектов владения, Effect.CONSUME). Мой React-Effect затенил бы его и ронял импорт всего модуля (поймал на первом прогоне CLI). Поэтому Effect as ReactEffect / Binding as EffectBinding. Если решим переименовать — менять в effects.py, тут только алиас.
Generated by Claude Code
| f"'{s.origin}' gets a fresh identity here — stabilise with useMemo"), | ||
| ) | ||
| out.append(Finding( | ||
| file=s.file, line=s.line, code="EFF001", |
There was a problem hiding this comment.
code="EFF001", а не OWN001 — это и есть гейт P-020. Шторм маршрутизируется через _effect_findings (как _di_findings для DI001), а не через acquire/release lowering. Свой core-код = честное «новое измерение», а не подгон утечки. Привязка к s.line (вызов useEffect) — место фикса; slice ведёт к точке создания нестабильной идентичности.
Generated by Claude Code
| _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]]: |
There was a problem hiding this comment.
Граница факт/вердикт проходит ровно здесь. _classify_rhs сообщает только синтаксис RHS (object/memo/ident+refs/call/…) — он НЕ решает «стабильно/нестабильно». Вердикт целиком в ownlang/effects.py. Так фронтенд остаётся тонким (как Roslyn-экстрактор отдаёт факты, а не судит об утечках), и одну и ту же решётку стабильности позже сможет переиспользовать любой другой фронтенд (Vue watchEffect, Svelte $:).
Generated by Claude Code
![chatgpt-codex-connector[bot]](https://cdn.github.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0406650e06
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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)) |
There was a problem hiding this comment.
When an effect acquires more than one resource of the same kind, this single released boolean is applied to every hit as soon as the cleanup contains any matching verb. For example, an effect with two setInterval calls and return () => clearInterval(first) emits both timer facts as released: true, so the core reports zero OWN001 findings even though the second interval still leaks. The same false negative applies to multiple subscriptions/listeners where only one is removed.
Useful? React with 👍 / 👎.
| 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, | ||
| }) |
There was a problem hiding this comment.
This walks every const/let/var in the file, including declarations inside useEffect callbacks and nested functions, then hands them to the core as render-scope bindings. If an effect depends on a stable outer value like const filters = useMemo(...) but its callback has a local const filters = { ... }, the inner object literal overwrites the real dependency binding by name and produces a false EFF001 storm even though [filters] refers to the stable outer value.
Useful? React with 👍 / 👎.
…useeffect-27n6xg # Conflicts: # ownlang/diagnostics.py # tests/run_tests.py
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 1033-1034: The finding-count assertions in the CI workflow are
using substring matches, so extra diagnostics can still pass; update the grep
checks in the affected shell steps to match the exact count output from the
diagnostic summary rather than any string containing “4 finding”, “2 finding”,
or “0 finding”. Use the existing finding-count verification blocks in the
workflow job to ensure the checks only succeed when the total findings exactly
match the expected OWN001/EFF001 counts.
In `@docs/proposals/P-020-ownts-react-effects.md`:
- Around line 159-168: The paragraph in the proposal currently implies EFF002 is
implemented, but it should distinguish the shared analysis foundation from the
still-pending EFF002 verdict. Reword the section around the `EFF001/002`
discussion so `ownlang/effects.py` and the identity-stability lattice are
described as the implemented core analysis, while `EFF002` is explicitly called
out as the next increment or still pending. Keep the existing distinction
between frontend-emitted facts and core verdicts, and update the wording so only
EFF001 is presented as shipped.
In `@frontend/ownts/examples/Dashboard.tsx`:
- Around line 1-7: Update the fixture comments in Dashboard.tsx to reflect the
new ownership flow: the example should no longer describe three OWN001 findings
or call EFF001 a frontend heuristic. Adjust the inline commentary around the
useEffect cleanup/acquire-release example so it matches the current test
expectation in test_ownts.py, which now looks for three OWN001 findings plus one
EFF001, and keep the wording aligned with the current OWN/EFF taxonomy used by
the ownts examples.
In `@frontend/ownts/ownts.py`:
- Around line 280-287: The bindings collection in ownts.py is currently scanning
the entire file and classifying every _BINDING match with _component_at(), which
pulls in non-render-scope declarations from effects, nested callbacks, and
helper blocks. Update the bindings extraction logic to only collect top-level
render-scope declarations within each component body before populating
binds_by_comp, so the emitted binding facts align with the component’s actual
render scope and the effects analysis.
- Around line 172-185: _update _split_cleanup so it only recognizes a cleanup
return at the effect body’s top-level brace depth, not the first return ... =>
anywhere in the text._ Use the existing _match_block helper and the body
scanning logic in _split_cleanup to track nesting depth before accepting a
return as the effect cleanup, so nested callbacks inside the effect do not
suppress OWN001. Keep the same setup/cleanup split behavior, but ensure only the
outer effect callback’s return is treated as cleanup.
- Around line 191-193: Update the `useEffect` extraction logic so it detects
whether the callback body is a block or an expression before calling
`_match_block()`. In the `extract` path around the `_USE_EFFECT.finditer` loop
and the similar extractor noted in the comment, handle `useEffect(() => expr,
deps)` by parsing the single expression body separately instead of assuming
`{...}`. Keep `_match_block()` only for block-bodied callbacks, and preserve the
existing handling for arrow/function callbacks with braces.
In `@frontend/ownts/README.md`:
- Around line 40-45: The Markdown in README contains unlabeled fenced code
blocks, which triggers MD040; update both fences to include a language tag so
markdownlint stays green. Use the existing fenced examples in the README and
label the block that contains the Dashboard.tsx diagnostics and the block with
the EFF001 note consistently, preserving the content while adding the
appropriate fence language.
In `@ownlang/effects.py`:
- Around line 145-148: The dependency classification in Effects._by_name is
treating any missing render-scope binding as STABLE, which incorrectly marks raw
expressions forwarded from Ownts.deps as safe. Update the logic around the name
lookup in effects.py so non-identifier dependency strings are not assumed to be
props/state/globals; either normalize or synthesize bindings earlier in
frontend/ownts/ownts.py, or have this fallback return UNKNOWN unless the
dependency is identifier-like. Keep the fix aligned with the _by_name and deps
flow so fresh-identity expressions like object literals or constructor calls can
still trigger EFF001.
In `@ownlang/ownir.py`:
- Around line 2248-2268: The effects parser in the helper that builds
ReactEffect entries is still coercing malformed nested data instead of skipping
it. Update the logic around the effects loop in the ownir.py helper (the same
path used by check_facts() and mirrored by load()) to validate each effect and
its bindings before constructing ReactEffect/EffectBinding, and skip any entry
whose shape is invalid rather than converting values like deps="a" into tuples.
Keep malformed nested effects from producing EFF001 findings by applying the
same rejection behavior as load().
In `@tests/test_effects.py`:
- Around line 75-76: The fail-accumulating assertions in the effect tests still
dereference fire[0], derived[0], and eff[0] even after check() may have recorded
a failure, which can raise IndexError and stop the suite early. Update the
relevant assertions in test_effects.py to guard these follow-up dereferences in
the same test flow, using the existing check() pattern around the
fire/derived/eff result lists so the suite can continue reporting all failures
when no findings are returned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c37160ff-b4bf-4a03-bd8a-54d819dd7703
📒 Files selected for processing (13)
.github/workflows/ci.ymldocs/proposals/P-020-ownts-react-effects.mdfrontend/ownts/README.mdfrontend/ownts/examples/Dashboard.tsxfrontend/ownts/examples/DashboardClean.tsxfrontend/ownts/examples/EffectStorm.tsxfrontend/ownts/ownts.pyfrontend/ownts/test_ownts.pyownlang/diagnostics.pyownlang/effects.pyownlang/ownir.pytests/run_tests.pytests/test_effects.py
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PVeuchch67CtjSbizYm8gi
|
Спасибо за ревью — разобрал все пункты (Codex + CodeRabbit) в Реальные баги фронтенда (Codex P2 / CodeRabbit Major):
Робастность ядра (CodeRabbit Major):
Тесты/CI/доки (Minor): новая фикстура Локально: весь suite зелёный ( Generated by Claude Code |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/ownts/ownts.py`:
- Around line 218-273: The useEffect parser is still scanning raw punctuation,
so expressions, deps, and cleanup detection can break on strings/templates or
nested brackets. Update `_expr_end`, `_effect_callback`, and `_split_cleanup` to
use token-aware scanning like `_match_block()` so literals such as `"/a,b"` or
`"{` inside strings don’t truncate bodies or misclassify cleanup/deps. Keep the
fix localized to these helpers and preserve the existing `OWN001`/`EFF001` flow.
- Around line 91-96: The addEventListener release check in ownts.py only
compares the handler, so it can miss leaks when the target or capture/options
differ. Update the subscription branch to capture the receiver and optional
third argument from the acquire-site match in the release logic near the
addEventListener/removeEventListener handling, then require those values to
match in cleanup before returning released.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9bb88b40-7a30-4ee4-9354-2106d7ab61f4
📒 Files selected for processing (12)
.github/workflows/ci.ymldocs/proposals/P-020-ownts-react-effects.mdfrontend/ownts/README.mdfrontend/ownts/examples/Dashboard.tsxfrontend/ownts/examples/EffectEdges.tsxfrontend/ownts/ownts.pyfrontend/ownts/test_ownts.pyownlang/diagnostics.pyownlang/effects.pyownlang/ownir.pytests/run_tests.pytests/test_effects.py
🚧 Files skipped from review as they are similar to previous changes (8)
- frontend/ownts/examples/Dashboard.tsx
- tests/test_effects.py
- tests/run_tests.py
- .github/workflows/ci.yml
- docs/proposals/P-020-ownts-react-effects.md
- ownlang/ownir.py
- frontend/ownts/README.md
- ownlang/effects.py
| 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)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require the same addEventListener target/options in cleanup.
This branch only matches the handler, so foo.addEventListener("x", onX, true) is currently treated as released by bar.removeEventListener("x", onX) or by dropping the capture/options argument. Both cases still leak in the DOM, so this can suppress a real OWN001. Capture the receiver and the third argument from the acquire site and require them to match before marking the listener released.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 94-95: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.search(
rf"removeEventListener\s*(\s*[^,]+,\s*{re.escape(handler)}\b", cleanup)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/ownts/ownts.py` around lines 91 - 96, The addEventListener release
check in ownts.py only compares the handler, so it can miss leaks when the
target or capture/options differ. Update the subscription branch to capture the
receiver and optional third argument from the acquire-site match in the release
logic near the addEventListener/removeEventListener handling, then require those
values to match in cleanup before returning released.
| 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(<cb>, [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 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] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the useEffect parser token-aware instead of counting raw punctuation.
The new expression/deps/cleanup scanners still treat source as plain text. Valid code like useEffect(() => fetch("/a,b"), [items[i]]) or an effect body containing const s = "{"; return () => clearInterval(id) will be truncated or misclassified because _expr_end(), the deps regex, and the top-level cleanup check do not skip literals/templates or nested bracket pairs. That feeds malformed facts into both OWN001 and EFF001. Reuse the same quote/template-aware scanning you already have in _match_block() for these paths too.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/ownts/ownts.py` around lines 218 - 273, The useEffect parser is
still scanning raw punctuation, so expressions, deps, and cleanup detection can
break on strings/templates or nested brackets. Update `_expr_end`,
`_effect_callback`, and `_split_cleanup` to use token-aware scanning like
`_match_block()` so literals such as `"/a,b"` or `"{` inside strings don’t
truncate bodies or misclassify cleanup/deps. Keep the fix localized to these
helpers and preserve the existing `OWN001`/`EFF001` flow.
2 participants
Что и зачем
Доказываем, что модель ресурсов/жизненного цикла ядра — кросс-языковая, а не только .NET. Добавлен фронтенд OwnTS (TS/TSX-брат Roslyn-экстрактора): он сканирует React
.tsx, эмитит факты в OwnIR, и то же самое ядро ловит баг. Две аналитики над одним чекером:useEffect→OWN001(EFF003/004): acquire (setInterval/setTimeout→ timer,.subscribe(→ subscribe,addEventListener→ subscription) без cleanup-return— это ровноOWN001, как C#event += / -=.EFF001(effect storm) — новый core-анализ (ownlang/effects.py): IO-эффект, чья зависимость имеет нестабильную идентичность (объектный литерал в render-scope), перезапускается каждый рендер → шторм запросов (форма инцидента Cloudflare 12.09.2025). Это отдельное измерение (стабильность идентичности зависимостей), а не утечка — свой core-код, какDI001, и никогда не выдаёт себя заOWN001. Фронтенд эмитит только факты; вердикт о стабильности принимает ядро (честный split факт/вердикт, как у DI-графаservices).Отвечает на гейтовый вопрос P-020 (Open question 1) реализацией, а не словами.
Тип изменения
Как проверено
python tests/run_tests.py— зелёный (вкл. новыйtests/test_effects.py, 29 проверок;ownir194/194)ruff check .иmypy(--strict поownlang) — чистоpython frontend/ownts/test_ownts.py(3×OWN001+EFF001 / 0 / 2×EFF001),python -m ownlang ownirend-to-end + SARIFpython frontend/ownts/ownts.py frontend/ownts/examples/EffectStorm.tsx --checkСвязанные issue
Refs P-020 (docs/proposals/P-020-ownts-react-effects.md), P-017. Closes — нет (proposal-driven, не mainline).
Чеклист
tests/test_effects.py,frontend/ownts/test_ownts.py, CI-джобаownts-react-effects)frontend/ownts/README.md, P-020: Open question 1 помечен ✅)feat:,spike:)Что НЕ делается (честные границы)
EFF001консервативен: срабатывает только на доказуемо нестабильной зависимости (memo/ref/непрозрачный вызов/примитив/без-IO → молчит) — низкий FP.🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit
useEffectre-runs caused by unstable dependency identity, including “render-time IO storm” messaging.