[WIP - waiting for 10473] fix(lint): suppress false positives in noUnusedVariables for Svelte store subscriptions and $bindable() props#10534
Draft
Mokto wants to merge 17 commits into
Conversation
…ript bindings in Svelte/Vue/Astro The two rules consult `EmbeddedExportedBindings` to avoid false positives on imports/vars referenced only in templates. But under `html.experimentalFullSupportEnabled`, the source `<script>` block's own top-level bindings get registered into that set via `visit_js_source_snippet`, so the membership check at no_unused_variables.rs:357 and no_unused_imports.rs:686 self-suppresses every import and let/const in the script — including truly unused ones (biomejs#8590). Gate the embedded-binding suppression on `!file_source.is_embedded_source()`: for `{@const}` / Vue v-for / Svelte snippet params we keep the existing behavior, but for the `<script>` block itself rely on `EmbeddedValueReferences` (precise per-name template references) instead. `noUnusedImports` now consults that service too, mirroring `useImportType`/`noUnusedVariables`. Also extracts `HtmlSpreadAttribute` arguments as non-source snippets so `<input {...props} />` properly registers `props` as a template reference. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ation, directives, bind shorthand, snippet params
Removing the source-script over-suppression (previous commit) exposed several
Svelte template constructs whose references the extractor never collected, each
becoming a false positive on noUnusedImports/noUnusedVariables:
- `{expr}` inside quoted attribute values (`style="top: {top}px"`): the parser
stores these as one opaque string token, so a brace/string-aware scanner
(`svelte_interpolations`) pulls the expressions out and parses them as JS.
- Directive names `use:action` / `transition:fn` / `in:` / `out:` / `animate:`
reference imported values; the name token is now registered.
- Shorthand `bind:open` (no initializer) reads the local `open`.
- `{#snippet foo(param)}` params used only in the snippet body — noUnused
FunctionParameters now consults EmbeddedValueReferences like the other rules.
Verified against a real SvelteKit codebase: 70 false positives → 0, while
genuinely-unused script bindings are still reported.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…port rules
An import used only as a type inside a template snippet — e.g. `icon: IconType`
in a `{#snippet}` parameter type — was flagged as unused. `EmbeddedValueReferences`
dropped type-only references (so `useImportType` could distinguish value vs type
usage), leaving them invisible to noUnusedImports/noUnusedVariables.
Track type-only references in a parallel channel. `useImportType` keeps using the
value-only set (unchanged); the unused-* rules now consult `is_used` (value OR
type). A genuinely-unused type import is still reported.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…h on directives, move tests
- Move attribute-string `{...}` interpolation extraction into parse_embedded_nodes
so each is a tracked snippet, instead of re-parsing and discarding it in the
references visitor.
- Replace the directive if/else chain with an exhaustive match on AnySvelteDirective.
- visit_html_root takes &HtmlFileSource instead of an is_svelte bool.
- Rename EmbeddedValueReferences fields to values/types, drop the contains helper.
- Move the Svelte CLI cases to analyzer spec fixtures.
- Trim comments and the changeset.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…UnusedVariables Verifies that genuinely-unused script bindings are still flagged after the embedded-reference tracking fix; previously only valid (clean) cases existed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ro templates
When an imported name appears only in type position inside a template
snippet (e.g. `{#snippet card(p: { icon: IconType })}`), the semantic
model of the <script> block has no references to it, so is_only_used_as_type
returned false and the rule stayed silent. Now it falls back to checking
the embedded type_references channel, correctly suggesting `import type`.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers the case where `import type { T }` is never referenced in either
the script or the template — it should still be reported by noUnusedImports.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…value_references.rs Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
…olation test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e interpolation
Verifies that undeclared variables used in attribute interpolations like
{foo}px are flagged by noUndeclaredVariables.
…nt, add bind: tests - Rename EmbeddedValueReferences::references to value_references (pairs cleaner with type_references) - Restore the detailed doc comment on visit_html_root with examples - Add unit tests for bind: shorthand (registers reference) and bind: with initializer (does not register directive name as reference)
…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>
🦋 Changeset detectedLatest commit: 285021a 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 |
Contributor
✅ Organic activityNo automation signals detected in the analyzed events. This is an automated analysis by AgentScan |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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