fix(lint): suppress false positives in noUnusedVariables for Svelte store subscriptions and $bindable() props - #10534
Conversation
🦋 Changeset detectedLatest commit: 2e2bc63 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
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 |
✅ Organic activityNo automation signals detected in the analyzed events. This is an automated analysis by AgentScan |
285021a to
d6a8610
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
👮 Files not reviewed due to content moderation or server errors (3)
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
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 @.changeset/fix-svelte-store-bindable.md:
- Line 5: The changeset description in the markdown file needs the required
bugfix prefix format. Update the opening sentence so it starts with Fixed
[`#NUMBER`](issue link): followed by the summary, while keeping the existing
noUnusedVariables rule link in the rest of the text. Make sure the description
also includes the issue link and preserves the references to the Svelte store
subscription and $bindable() false-positive fix.
In `@crates/biome_js_analyze/src/lint/correctness/no_unused_variables.rs`:
- Around line 449-456: The $bindable() suppression in no_unused_variables
currently hides all bindable props from $props(), even when they are never used,
so tighten the check to only return None for write-only bindings. Update the
logic around is_svelte_bindable_prop(binding) in the unused-variable diagnostic
path to require an explicit write-only usage condition before suppressing, and
mirror the same fix anywhere else the same suppression is applied. Add a
regression test covering a $bindable() prop that is neither read nor written so
it still reports unused.
🪄 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: d5d8f8da-8aaa-4a25-8ca6-94e766914e70
⛔ Files ignored due to path filters (2)
crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/valid-svelte-bindable-props.svelte.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/valid-svelte-store-subscription.svelte.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (6)
.changeset/fix-svelte-store-bindable.mdcrates/biome_js_analyze/src/lint/correctness/no_unused_variables.rscrates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/valid-svelte-bindable-props.sveltecrates/biome_js_analyze/tests/specs/correctness/noUnusedVariables/valid-svelte-store-subscription.sveltecrates/biome_service/src/workspace/document/services/embedded_value_references.rscrates/biome_service/src/workspace/server.rs
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Fixed [`noUnusedVariables`](https://biomejs.dev/linter/rules/no-unused-variables/) false positives in Svelte files: Svelte store subscriptions (`$store` references in templates now keep the underlying `store` binding from being flagged), and `$bindable()` props that are only written to in the script block (write-only is intentional for bindable props) are no longer reported as unused. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the required bugfix prefix with an issue link.
For bug fixes, this should start with Fixed [#NUMBER](issue link): ...; keep the rule link in the rest of the sentence.
As per coding guidelines, "for bug fixes start with 'Fixed [#NUMBER](issue link): ...'" and "include issue links, rule links, and assist links in changeset descriptions." Based on learnings, Biome changesets require the exact Fixed [#NUMBER](issue link): ... prefix format.
🤖 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 @.changeset/fix-svelte-store-bindable.md at line 5, The changeset description
in the markdown file needs the required bugfix prefix format. Update the opening
sentence so it starts with Fixed [`#NUMBER`](issue link): followed by the summary,
while keeping the existing noUnusedVariables rule link in the rest of the text.
Make sure the description also includes the issue link and preserves the
references to the Svelte store subscription and $bindable() false-positive fix.
Sources: Coding guidelines, Learnings
| // In Svelte 5, assigning to a `$bindable()` prop reflects the value back to the | ||
| // parent component. Such a variable may be write-only in the script block but is | ||
| // still meaningful — suppress the diagnostic to avoid a false positive. | ||
| if matches!(file_source.as_embedding_kind(), JsEmbeddingKind::Svelte { .. }) | ||
| && is_svelte_bindable_prop(binding) | ||
| { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Gate $bindable() suppression to write-only bindings only.
This currently suppresses any unused $bindable() prop from $props(), including truly untouched ones, which can create false negatives. Please require a write-only usage condition before returning None, and add a regression case where a $bindable() prop is never read or written.
Also applies to: 703-738
🤖 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 `@crates/biome_js_analyze/src/lint/correctness/no_unused_variables.rs` around
lines 449 - 456, The $bindable() suppression in no_unused_variables currently
hides all bindable props from $props(), even when they are never used, so
tighten the check to only return None for write-only bindings. Update the logic
around is_svelte_bindable_prop(binding) in the unused-variable diagnostic path
to require an explicit write-only usage condition before suppressing, and mirror
the same fix anywhere else the same suppression is applied. Add a regression
test covering a $bindable() prop that is neither read nor written so it still
reports unused.
Merging this PR will not alter performance
Comparing Footnotes
|
|
Hey @Mokto can you rebase the branch and address linting? |
…in templates
When a Svelte template expression like `{$errors.email}` references a
store subscription, only the `$errors` identifier was registered in the
embedded value references, so `is_used_as_value("errors")` returned
false and the bound variable `const { errors } = superForm(...)` was
incorrectly flagged as unused.
Fix: when building value references from Svelte non-source snippets, any
`$store`-style identifier that is not a Svelte rune also registers the
un-prefixed store name. The `$` prefix is stripped using
`TokenText::slice`, keeping the reference as a zero-copy view into the
same underlying token.
The server-side snippet collector is updated to use a Svelte-aware
builder for Svelte non-source snippets.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ble() props in Svelte 5 In Svelte 5, assigning to a prop declared with $bindable() as its default reflects the new value back to the parent component. Such props may be write-only in the script block yet serve a real purpose, so flagging them as unused variables is a false positive. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
d6a8610 to
b334de7
Compare
b334de7 to
ac05664
Compare
ac05664 to
2e2bc63
Compare
This PR contains the following updates: | Package | Type | Update | Change | Pending | |---|---|---|---|---| | [@biomejs/biome](https://biomejs.dev) ([source](https://github.com/biomejs/biome/tree/HEAD/packages/@biomejs/biome)) | imports | patch | [`2.5.2` -> `2.5.3`](https://renovatebot.com/diffs/npm/@biomejs%2fbiome/2.5.2/2.5.3) | `2.5.4` | --- ### Release Notes <details> <summary>biomejs/biome (@​biomejs/biome)</summary> ### [`v2.5.3`](https://github.com/biomejs/biome/blob/HEAD/packages/@​biomejs/biome/CHANGELOG.md#253) [Compare Source](https://github.com/biomejs/biome/compare/@biomejs/biome@2.5.2...@biomejs/biome@2.5.3) ##### Patch Changes - [#​10815](biomejs/biome#10815) [`86613d5`](biomejs/biome@86613d5) Thanks [@​WaterWhisperer](https://github.com/WaterWhisperer)! - Fixed a parser panic reported in [#​10708](biomejs/biome#10708): Biome now recovers when unsupported CSS Modules `@value` rules or scoped `@keyframes` names end at EOF. - [#​10534](biomejs/biome#10534) [`da9b403`](biomejs/biome@da9b403) Thanks [@​Mokto](https://github.com/Mokto)! - Fixed [`noUnusedVariables`](https://biomejs.dev/linter/rules/no-unused-variables/) false positives in Svelte files: Svelte store subscriptions (`$store` references in templates now keep the underlying `store` binding from being flagged), and `$bindable()` props that are only written to in the script block (write-only is intentional for bindable props) are no longer reported as unused. - [#​10827](biomejs/biome#10827) [`098ba41`](biomejs/biome@098ba41) Thanks [@​Aqu1bp](https://github.com/Aqu1bp)! - Fixed [#​10698](biomejs/biome#10698): The [`noUnsafeOptionalChaining`](https://biomejs.dev/linter/rules/no-unsafe-optional-chaining/) rule now reports unsafe optional chains wrapped in TypeScript `as`, `satisfies`, type assertion, and instantiation expressions, such as `new (value?.constructor as Constructor)()`. - [#​10773](biomejs/biome#10773) [`3c6513d`](biomejs/biome@3c6513d) Thanks [@​otkrickey](https://github.com/otkrickey)! - Fixed [#​10772](biomejs/biome#10772): [`useVueValidVOn`](https://biomejs.dev/linter/rules/use-vue-valid-v-on/) no longer reports a missing handler for v-on directives using a verb modifier (`.stop` / `.prevent`) without an expression, e.g. `<div @​click.stop></div>`. The rule also accepts the arg-less object syntax `<div v-on="$listeners"></div>` instead of reporting a missing event name. - [#​10721](biomejs/biome#10721) [`d83c66b`](biomejs/biome@d83c66b) Thanks [@​minseong0324](https://github.com/minseong0324)! - Improved type-aware lint rule inference for built-in globals and indexed function calls. Biome now resolves `Error(...)`, `new Error(...)`, optional `Error#stack`, and calls through indexed function values such as `handlers[0]()` more accurately. - [#​10865](biomejs/biome#10865) [`6450276`](biomejs/biome@6450276) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed [#​10845](biomejs/biome#10845). Biome Language Server no longer goes in deadlock when the scanner is enabled. - [#​10853](biomejs/biome#10853) [`93d8e53`](biomejs/biome@93d8e53) Thanks [@​Netail](https://github.com/Netail)! - Fixed [#​10840](biomejs/biome#10840): Astro shorthand attribute syntax is now correctly being parsed from embedded nodes. - [#​10820](biomejs/biome#10820) [`bba3092`](biomejs/biome@bba3092) Thanks [@​JamBalaya56562](https://github.com/JamBalaya56562)! - Fixed [#​10619](biomejs/biome#10619): [`noProcessEnv`](https://biomejs.dev/linter/rules/no-process-env/) now also reports computed (bracket) member access. Previously only dot access was checked, so `process["env"]` and `env["NODE_ENV"]` (where `env` is imported from `node:process`) were missed. Both static and computed accesses are now reported. - [#​10835](biomejs/biome#10835) [`3447b2f`](biomejs/biome@3447b2f) Thanks [@​dyc3](https://github.com/dyc3)! - Fixed [#​10824](biomejs/biome#10824): [`useDomQuerySelector`](https://biomejs.dev/linter/rules/use-dom-query-selector/) now supports an `ignore` option for receiver identifiers that should not be reported. - [#​10875](biomejs/biome#10875) [`b12e486`](biomejs/biome@b12e486) Thanks [@​dyc3](https://github.com/dyc3)! - Fixed [#​10795](biomejs/biome#10795): `--profile-rules` now reports timings for each plugin separately as `plugin/<pluginName>`, matching the naming used by plugin suppressions, instead of aggregating all plugins under a single `plugin/plugin` entry. - [#​10877](biomejs/biome#10877) [`d6bc447`](biomejs/biome@d6bc447) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed [biome-zed#164](biomejs/biome-zed#164): Biome no longer inserts stray whitespace when format-on-type runs after closing delimiters such as `)`, `]`, and `}`. - [#​10867](biomejs/biome#10867) [`a21463e`](biomejs/biome@a21463e) Thanks [@​dyc3](https://github.com/dyc3)! - Fixed [#​10864](biomejs/biome#10864): Biome no longer crashes when checking or linting HTML files with unquoted attribute values such as `<textarea rows=4></textarea>`. </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yNTIuMyIsInVwZGF0ZWRJblZlciI6IjQzLjI1Mi4zIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Reviewed-on: https://git.oirnoir.dev/OIRNOIR/YouTube-Helper-Client/pulls/12
This PR contains the following updates: | Package | Type | Update | Change | Pending | |---|---|---|---|---| | [@biomejs/biome](https://biomejs.dev) ([source](https://github.com/biomejs/biome/tree/HEAD/packages/@biomejs/biome)) | imports | patch | [`2.5.2` -> `2.5.3`](https://renovatebot.com/diffs/npm/@biomejs%2fbiome/2.5.2/2.5.3) | `2.5.4` | --- ### Release Notes <details> <summary>biomejs/biome (@​biomejs/biome)</summary> ### [`v2.5.3`](https://github.com/biomejs/biome/blob/HEAD/packages/@​biomejs/biome/CHANGELOG.md#253) [Compare Source](https://github.com/biomejs/biome/compare/@biomejs/biome@2.5.2...@biomejs/biome@2.5.3) ##### Patch Changes - [#​10815](biomejs/biome#10815) [`86613d5`](biomejs/biome@86613d5) Thanks [@​WaterWhisperer](https://github.com/WaterWhisperer)! - Fixed a parser panic reported in [#​10708](biomejs/biome#10708): Biome now recovers when unsupported CSS Modules `@value` rules or scoped `@keyframes` names end at EOF. - [#​10534](biomejs/biome#10534) [`da9b403`](biomejs/biome@da9b403) Thanks [@​Mokto](https://github.com/Mokto)! - Fixed [`noUnusedVariables`](https://biomejs.dev/linter/rules/no-unused-variables/) false positives in Svelte files: Svelte store subscriptions (`$store` references in templates now keep the underlying `store` binding from being flagged), and `$bindable()` props that are only written to in the script block (write-only is intentional for bindable props) are no longer reported as unused. - [#​10827](biomejs/biome#10827) [`098ba41`](biomejs/biome@098ba41) Thanks [@​Aqu1bp](https://github.com/Aqu1bp)! - Fixed [#​10698](biomejs/biome#10698): The [`noUnsafeOptionalChaining`](https://biomejs.dev/linter/rules/no-unsafe-optional-chaining/) rule now reports unsafe optional chains wrapped in TypeScript `as`, `satisfies`, type assertion, and instantiation expressions, such as `new (value?.constructor as Constructor)()`. - [#​10773](biomejs/biome#10773) [`3c6513d`](biomejs/biome@3c6513d) Thanks [@​otkrickey](https://github.com/otkrickey)! - Fixed [#​10772](biomejs/biome#10772): [`useVueValidVOn`](https://biomejs.dev/linter/rules/use-vue-valid-v-on/) no longer reports a missing handler for v-on directives using a verb modifier (`.stop` / `.prevent`) without an expression, e.g. `<div @​click.stop></div>`. The rule also accepts the arg-less object syntax `<div v-on="$listeners"></div>` instead of reporting a missing event name. - [#​10721](biomejs/biome#10721) [`d83c66b`](biomejs/biome@d83c66b) Thanks [@​minseong0324](https://github.com/minseong0324)! - Improved type-aware lint rule inference for built-in globals and indexed function calls. Biome now resolves `Error(...)`, `new Error(...)`, optional `Error#stack`, and calls through indexed function values such as `handlers[0]()` more accurately. - [#​10865](biomejs/biome#10865) [`6450276`](biomejs/biome@6450276) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed [#​10845](biomejs/biome#10845). Biome Language Server no longer goes in deadlock when the scanner is enabled. - [#​10853](biomejs/biome#10853) [`93d8e53`](biomejs/biome@93d8e53) Thanks [@​Netail](https://github.com/Netail)! - Fixed [#​10840](biomejs/biome#10840): Astro shorthand attribute syntax is now correctly being parsed from embedded nodes. - [#​10820](biomejs/biome#10820) [`bba3092`](biomejs/biome@bba3092) Thanks [@​JamBalaya56562](https://github.com/JamBalaya56562)! - Fixed [#​10619](biomejs/biome#10619): [`noProcessEnv`](https://biomejs.dev/linter/rules/no-process-env/) now also reports computed (bracket) member access. Previously only dot access was checked, so `process["env"]` and `env["NODE_ENV"]` (where `env` is imported from `node:process`) were missed. Both static and computed accesses are now reported. - [#​10835](biomejs/biome#10835) [`3447b2f`](biomejs/biome@3447b2f) Thanks [@​dyc3](https://github.com/dyc3)! - Fixed [#​10824](biomejs/biome#10824): [`useDomQuerySelector`](https://biomejs.dev/linter/rules/use-dom-query-selector/) now supports an `ignore` option for receiver identifiers that should not be reported. - [#​10875](biomejs/biome#10875) [`b12e486`](biomejs/biome@b12e486) Thanks [@​dyc3](https://github.com/dyc3)! - Fixed [#​10795](biomejs/biome#10795): `--profile-rules` now reports timings for each plugin separately as `plugin/<pluginName>`, matching the naming used by plugin suppressions, instead of aggregating all plugins under a single `plugin/plugin` entry. - [#​10877](biomejs/biome#10877) [`d6bc447`](biomejs/biome@d6bc447) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed [biome-zed#164](biomejs/biome-zed#164): Biome no longer inserts stray whitespace when format-on-type runs after closing delimiters such as `)`, `]`, and `}`. - [#​10867](biomejs/biome#10867) [`a21463e`](biomejs/biome@a21463e) Thanks [@​dyc3](https://github.com/dyc3)! - Fixed [#​10864](biomejs/biome#10864): Biome no longer crashes when checking or linting HTML files with unquoted attribute values such as `<textarea rows=4></textarea>`. </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yNTIuMyIsInVwZGF0ZWRJblZlciI6IjQzLjI1Mi4zIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Reviewed-on: https://git.oirnoir.dev/OIRNOIR/YouTube-Helper-Server/pulls/23
Summary
Two false-positive patterns in
noUnusedVariablesfor Svelte files, both building on the cross-script reference tracking introduced in #10473.1. Svelte store auto-subscription
When a variable is declared in
<script>and only used in the template via Svelte's$storeauto-subscription syntax, it was incorrectly flagged as unused:Fix: when collecting template references in
EmbeddedValueReferencesBuilder, in Svelte mode,$-prefixed identifiers that are not runes ($state,$derived, etc.) also register the store name without the$prefix.2. Svelte 5
$bindable()propsVariables declared as
= $bindable()in a$props()destructuring and only assigned to (never read) were flagged as unused. In Svelte 5, assigning to a$bindable()prop reflects the value back to the parent — the write IS the observable use.Fix: in
no_unused_variables.rs, when the binding is a shorthand property whose default initializer is$bindable()and whose enclosing declarator is initialized from$props(), suppress the diagnostic.Test Plan
New fixtures
valid-svelte-store-subscription.svelteandvalid-svelte-bindable-props.svelteadded tonoUnusedVariablestest specs. All existing tests pass.Docs
N/A