Skip to content

fix(core): tracking of bindings and value references#9886

Merged
dyc3 merged 2 commits intomainfrom
fix/tracking-bindings-refs
Apr 9, 2026
Merged

fix(core): tracking of bindings and value references#9886
dyc3 merged 2 commits intomainfrom
fix/tracking-bindings-refs

Conversation

@ematipico
Copy link
Copy Markdown
Member

Summary

This PR fixes two bugs:

  • Correctly tracks object and array assignments inside Svelte embedded functions
  • Fixes how bindings and references were tracked. Using FxHashMap, two bindings in two different snippets that had the same range were swallowed into one. I changed the storage into Vec, which keeps the duplicate

The use of AI was limited in finding the issue and creating the code for object/array assignments (rewritten a couple of times).

Test Plan

Added new tests.

The new svelte test is where I encountered the issue.

Docs

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 9, 2026

🦋 Changeset detected

Latest commit: 97cdc63

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@biomejs/biome Patch
@biomejs/cli-win32-x64 Patch
@biomejs/cli-win32-arm64 Patch
@biomejs/cli-darwin-x64 Patch
@biomejs/cli-darwin-arm64 Patch
@biomejs/cli-linux-x64 Patch
@biomejs/cli-linux-arm64 Patch
@biomejs/cli-linux-x64-musl Patch
@biomejs/cli-linux-arm64-musl Patch
@biomejs/wasm-web Patch
@biomejs/wasm-bundler Patch
@biomejs/wasm-nodejs Patch
@biomejs/backend-jsonrpc Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ematipico ematipico requested review from a team April 9, 2026 15:08
@github-actions github-actions Bot added A-Project Area: project A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Apr 9, 2026
Copy link
Copy Markdown
Contributor

@dyc3 dyc3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunate losing the hashmap

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4e1cbfa9-cc47-44c3-9aa5-7bfef78d776b

📥 Commits

Reviewing files that changed from the base of the PR and between c1d0844 and 97cdc63.

⛔ Files ignored due to path filters (2)
  • crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/invalid_svelte_snippet_defaults.svelte.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/valid_svelte_snippet_defaults.svelte.snap is excluded by !**/*.snap and included by **
📒 Files selected for processing (8)
  • .changeset/clever-wombats-move.md
  • crates/biome_js_analyze/src/lib.rs
  • crates/biome_js_analyze/src/services/embedded_bindings.rs
  • crates/biome_js_analyze/src/services/embedded_value_references.rs
  • crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/invalid_svelte_snippet_defaults.svelte
  • crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/valid_svelte_snippet_defaults.svelte
  • crates/biome_service/src/workspace/document/services/embedded_bindings.rs
  • crates/biome_service/src/workspace/document/services/embedded_value_references.rs
✅ Files skipped from review due to trivial changes (2)
  • crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/valid_svelte_snippet_defaults.svelte
  • crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/invalid_svelte_snippet_defaults.svelte
🚧 Files skipped from review as they are similar to previous changes (4)
  • crates/biome_js_analyze/src/services/embedded_bindings.rs
  • crates/biome_js_analyze/src/services/embedded_value_references.rs
  • crates/biome_service/src/workspace/document/services/embedded_bindings.rs
  • crates/biome_js_analyze/src/lib.rs

Walkthrough

Refactors embedded binding and value-reference storage from per-snippet hash maps to nested vectors of (TextRange, TokenText) tuples across analyzer and service crates. Updates APIs and tests to use the new shape, removes FxHashMap imports, and expands Svelte snippet default-parameter handling and tests to fix intermittent false positives for noUnusedVariables and noUndeclaredVariables in Svelte/Vue/Astro files.

Possibly related PRs

Suggested reviewers

  • dyc3
  • siketyan
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main fix: improving tracking of bindings and value references in the core system.
Description check ✅ Passed The description explains the two key bugs fixed, technical rationale for the changes, and notes test coverage, all directly relevant to the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/tracking-bindings-refs

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.changeset/clever-wombats-move.md:
- Line 5: Line 5 contains typos and awkward phrasing—change "occassionally" to
"occasionally", change "reference weren't" to "references weren't" (or rephrase
to "references weren't being tracked"), and trim repetition by rewriting the
sentence into a concise 1–2 sentence release note that mentions the root cause
and affected lint rules (e.g., "Fix occasional failure to track some bindings
and references that caused false positives in noUnusedVariables and
noUndeclaredVariables for Svelte, Vue, and Astro files.").

In
`@crates/biome_service/src/workspace/document/services/embedded_value_references.rs`:
- Around line 37-39: Add a collision regression test that constructs two
distinct snippets whose local TextRange values are identical (e.g., ranges for
"Foo;" and "Bar;") and calls register_reference for each (using TokenText for
the snippet text), then assert both entries survive in the storage (the
references Vec or the public accessor) so the previous FxHashMap-collision bug
can't reoccur; specifically, create a test that uses register_reference and
verifies that both TokenText values are present for the same TextRange rather
than one overwriting the other, and add it as a snapshot/unit test alongside the
other embedded-value-reference tests.
🪄 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

Run ID: a001ae29-efea-4cd5-89e0-558c83955679

📥 Commits

Reviewing files that changed from the base of the PR and between 7f67749 and c1d0844.

⛔ Files ignored due to path filters (2)
  • crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/invalid_svelte_snippet_defaults.svelte.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/valid_svelte_snippet_defaults.svelte.snap is excluded by !**/*.snap and included by **
📒 Files selected for processing (8)
  • .changeset/clever-wombats-move.md
  • crates/biome_js_analyze/src/lib.rs
  • crates/biome_js_analyze/src/services/embedded_bindings.rs
  • crates/biome_js_analyze/src/services/embedded_value_references.rs
  • crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/invalid_svelte_snippet_defaults.svelte
  • crates/biome_js_analyze/tests/specs/correctness/noUndeclaredVariables/valid_svelte_snippet_defaults.svelte
  • crates/biome_service/src/workspace/document/services/embedded_bindings.rs
  • crates/biome_service/src/workspace/document/services/embedded_value_references.rs

Comment thread .changeset/clever-wombats-move.md Outdated
@ematipico ematipico force-pushed the fix/tracking-bindings-refs branch from c1d0844 to 80e9b2e Compare April 9, 2026 15:18
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 9, 2026

Merging this PR will not alter performance

✅ 58 untouched benchmarks
⏩ 196 skipped benchmarks1


Comparing fix/tracking-bindings-refs (97cdc63) with main (7f67749)

Open in CodSpeed

Footnotes

  1. 196 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@dyc3 dyc3 merged commit e7c681e into main Apr 9, 2026
19 checks passed
@dyc3 dyc3 deleted the fix/tracking-bindings-refs branch April 9, 2026 15:36
@github-actions github-actions Bot mentioned this pull request Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Linter Area: linter A-Project Area: project L-JavaScript Language: JavaScript and super languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants