Skip to content

fix(core): scope-aware shadow tracking for CORRECT004/CORRECT005 - #141

Merged
oekazuma merged 1 commit into
mainfrom
fix/140-scope-aware-shadow-tracking
Jul 7, 2026
Merged

oekazuma merged 1 commit into
mainfrom
fix/140-scope-aware-shadow-tracking

Conversation

@oekazuma

@oekazuma oekazuma commented Jul 7, 2026 •

Copy link
Copy Markdown
Owner

Summary

Fixes #140 — the follow-up filed from CodeRabbit's review on #139. Extends that PR's partial shadow-tracking fix (function parameters + {#each} context only, applied only to CORRECT005) into a shared utility applied consistently to both affected rules, with wider coverage.

Both collectStateWrites (CORRECT004) and collectPropMutations (CORRECT005) matched writes by base identifier name alone, with no scope resolution — a nested local that reused a tracked $state/prop's name was misattributed as writing the outer binding:

  • For CORRECT005 this was a false positive (mutating an unrelated local flagged as a prop-mutation violation) — the worse failure mode for a precision-focused tool.
  • For CORRECT004 it was a false negative (a shadowed write made a genuinely-constable $state look mutated, silently suppressing the finding).

Fix

A new shared walkScoped/scopeIntroducedNames utility threads a "shadowed names" set down through scope-introducing constructs while walking: function/arrow-function parameters, a catch clause's parameter, a block's own let/const declarations, a for/for-of/for-in loop's declared variable, and a Svelte {#each ... as x} block's context. Both collectors now check the current shadow set before treating an identifier match as real, replacing the plain walkEstree they used before.

{#snippet}/{:then}/{:catch} bindings remain untracked — genuinely rarer as a real-world collision, and determining their exact AST shape would expand this into full lexical scope resolution (the "heavy lift" CodeRabbit itself flagged). Documented as a known, deliberately partial mitigation in code comments and the CORRECT005 rule docs (en/ja).

Test plan

  • New parse-level tests: CORRECT004 still flags a $state correctly when only a shadowing function param / {#each} var / for-loop var writes the local, not the state; CORRECT005 doesn't flag mutation of a block-scoped let/const, for-loop variable, or catch-clause parameter that shadows the prop, while still flagging the real prop mutation elsewhere in the same file.
  • pnpm build && pnpm typecheck && pnpm test && pnpm lint: all pass (core 372, cli 345, vite 84, mcp 13 — no regressions across the rest of the suite, including all pre-existing CORRECT001–004 tests).
  • pnpm --filter docs check: 0 errors.
  • Patch changeset for @svelte-vitals/core + svelte-vitals (precision fix to shipped rules).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved detection of state and prop mutations in nested scopes, reducing false positives when names are shadowed by local variables.
    • Kept constability checks accurate even when only shadowing locals are written.
  • Documentation
    • Clarified when CORRECT005 warnings are triggered or suppressed, including additional shadowing cases.
    • Noted current limitations for some untracked binding types.

#140)

Both collectStateWrites (CORRECT004) and collectPropMutations (CORRECT005)
matched writes by base identifier name alone, with no scope resolution —
a nested local that reused a tracked $state/prop's name was misattributed
as writing the outer binding. For CORRECT005 this was a false positive
(mutating an unrelated local flagged as a prop violation); for CORRECT004
it was a false negative (a shadowed write made a genuinely-constable
$state look mutated, suppressing the finding).

Adds a shared walkScoped/scopeIntroducedNames utility that threads a
"shadowed names" set down through scope-introducing constructs while
walking: function/arrow-function parameters, a catch clause's parameter,
a block's own let/const declarations, a for/for-of/for-in loop's declared
variable, and a Svelte {#each ... as x} block's context. Both collectors
now check the current shadow set before treating an identifier match as
real. This extends PR #139's partial fix (function params + each-context
only) to also cover block-scoped let/const and for-loops, and applies the
same guard to CORRECT004 for consistency.

{#snippet}/{:then}/{:catch} bindings remain untracked — documented as a
known, deliberately partial mitigation in code comments and the CORRECT005
rule docs (en/ja), not full lexical scope resolution.

Fixes #140

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 7, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a65f1730-b680-4192-ab43-e7519937d081

📥 Commits

Reviewing files that changed from the base of the PR and between a8d7578 and 4513f97.

📒 Files selected for processing (5)
  • .changeset/scope-aware-shadow-tracking.md
  • docs/src/content/docs/ja/rules/correct005.md
  • docs/src/content/docs/rules/correct005.md
  • packages/core/src/component-parse.ts
  • packages/core/test/component-parse.test.ts

📝 Walkthrough

Walkthrough

This PR adds scope-aware AST traversal (scopeIntroducedNames, walkScoped) to component-parse.ts so that local bindings shadowing a tracked $state/prop name no longer cause false positives in prop mutation detection (CORRECT005) or false negatives in constability detection (CORRECT004). Documentation and a changeset are updated accordingly, with new tests added.

Changes

Scope-aware shadow tracking

Layer / File(s) Summary
Scope tracking utilities
packages/core/src/component-parse.ts
Adds scopeIntroducedNames to compute names introduced at scope boundaries (params, catch clauses, block declarations, loop variables, each-block context) and walkScoped to traverse nodes while threading a growing shadowed-name set to visitors.
State write detection guard
packages/core/src/component-parse.ts, packages/core/test/component-parse.test.ts
collectStateWrites now uses walkScoped so assignments/updates/deletes/mutating calls are only recorded as writes when the root name isn't shadowed; a new test confirms $state is still flagged when only shadowing locals are written.
Prop mutation detection guard
packages/core/src/component-parse.ts, packages/core/test/component-parse.test.ts
collectPropMutations replaces its previous custom shadowing logic with walkScoped, suppressing findings when a local shadows the prop name; new tests verify shadowed block/loop/catch bindings are ignored while real prop mutations are still flagged.
Docs and release notes
docs/src/content/docs/rules/correct005.md, docs/src/content/docs/ja/rules/correct005.md, .changeset/scope-aware-shadow-tracking.md
Expands documented shadowing exceptions for CORRECT005 to cover block-scoped redeclarations, loop variables, and catch parameters, and adds a changeset bumping patch versions for both packages.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related issues

Possibly related PRs

  • oekazuma/svelte-vitals#78: Both PRs modify the same $state/prop mutation tracking logic underlying CORRECT004, with this PR refining the write/escape analysis introduced there.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: scope-aware shadow tracking for CORRECT004 and CORRECT005.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@oekazuma

oekazuma commented Jul 7, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 7, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tech-debt: identifier-only matching in component-parse.ts can misattribute mutations across shadowed bindings

1 participant