Skip to content

Commit da9b403

Browse files
Moktoclaudedyc3
authored
fix(lint): suppress false positives in noUnusedVariables for Svelte store subscriptions and $bindable() props (#10534)
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Carson McManus <carson.mcmanus1@gmail.com>
1 parent d83c66b commit da9b403

8 files changed

Lines changed: 214 additions & 5 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
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.

crates/biome_js_analyze/src/lint/correctness/no_unused_variables.rs

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ use biome_js_semantic::{ReferencesExtensions, SemanticModel};
99
use biome_js_syntax::binding_ext::{AnyJsBindingDeclaration, AnyJsIdentifierBinding};
1010
use biome_js_syntax::declaration_ext::is_in_ambient_context;
1111
use biome_js_syntax::{
12-
AnyJsExpression, JsClassExpression, JsForStatement, JsFunctionExpression,
12+
AnyJsExpression, JsCallExpression, JsClassExpression, JsForStatement, JsFunctionExpression,
1313
JsIdentifierExpression, JsModuleItemList, JsSequenceExpression, JsSyntaxKind, JsSyntaxNode,
14-
TsConditionalType, TsDeclarationModule, TsInferType, TsInterfaceDeclaration,
15-
TsTypeAliasDeclaration,
14+
JsVariableDeclarator, TsConditionalType, TsDeclarationModule, TsInferType,
15+
TsInterfaceDeclaration, TsTypeAliasDeclaration,
1616
};
1717
use biome_languages::JsFileSource;
1818
use biome_languages::javascript::JsEmbeddingKind;
@@ -424,7 +424,11 @@ impl Rule for NoUnusedVariables {
424424
| AnyJsBindingDeclaration::JsVariableDeclarator(_)
425425
)
426426
});
427-
let is_used_as_reference = embedded.is_used(binding_token_text);
427+
let is_used_as_reference = embedded.is_used(binding_token_text.clone())
428+
|| matches!(
429+
file_source.as_embedding_kind(),
430+
JsEmbeddingKind::Svelte { .. }
431+
) && embedded.is_svelte_store_used(binding_token_text);
428432

429433
if is_underscore_prefixed || is_defined_in_embedded_binding || is_used_as_reference {
430434
return None;
@@ -443,6 +447,16 @@ impl Rule for NoUnusedVariables {
443447
}
444448

445449
if is_unused(model, binding) {
450+
// In Svelte 5, assigning to a `$bindable()` prop reflects the value back to the
451+
// parent component. Such a variable may be write-only in the script block but is
452+
// still meaningful — suppress the diagnostic to avoid a false positive.
453+
if matches!(
454+
file_source.as_embedding_kind(),
455+
JsEmbeddingKind::Svelte { .. }
456+
) && is_svelte_bindable_prop(binding)
457+
{
458+
return None;
459+
}
446460
suggested_fix_if_unused(binding, ctx.options())
447461
} else {
448462
None
@@ -675,6 +689,57 @@ fn is_declaration_merged_with_used(
675689
}
676690
}
677691

692+
/// Returns `true` if `call` is a call to a simple identifier named `name`.
693+
fn is_call_to(call: &JsCallExpression, name: &str) -> bool {
694+
let Ok(AnyJsExpression::JsIdentifierExpression(ident)) = call.callee() else {
695+
return false;
696+
};
697+
ident.name().is_ok_and(|n| n.has_name(name))
698+
}
699+
700+
/// Returns `true` if the binding is a `$bindable()` shorthand property in a `$props()`
701+
/// destructuring in a Svelte 5 component.
702+
///
703+
/// In Svelte 5, assigning to a `$bindable()` prop reflects the new value back to the parent
704+
/// component. A variable that appears write-only in the script is therefore intentional and
705+
/// should not be flagged as unused.
706+
fn is_svelte_bindable_prop(binding: &AnyJsIdentifierBinding) -> bool {
707+
// The binding must be declared as a shorthand property in an object destructuring pattern.
708+
let Some(decl) = binding.declaration() else {
709+
return false;
710+
};
711+
let AnyJsBindingDeclaration::JsObjectBindingPatternShorthandProperty(shorthand) = decl else {
712+
return false;
713+
};
714+
715+
// The shorthand property must have a default initializer `= $bindable(...)`.
716+
let Some(init) = shorthand.init() else {
717+
return false;
718+
};
719+
let Ok(AnyJsExpression::JsCallExpression(call)) = init.expression() else {
720+
return false;
721+
};
722+
if !is_call_to(&call, "$bindable") {
723+
return false;
724+
}
725+
726+
// Walk up to find the enclosing `JsVariableDeclarator` whose rhs must be `$props()`.
727+
let Some(declarator) = shorthand
728+
.syntax()
729+
.ancestors()
730+
.find_map(JsVariableDeclarator::cast)
731+
else {
732+
return false;
733+
};
734+
let Some(declarator_init) = declarator.initializer() else {
735+
return false;
736+
};
737+
let Ok(AnyJsExpression::JsCallExpression(props_call)) = declarator_init.expression() else {
738+
return false;
739+
};
740+
is_call_to(&props_call, "$props")
741+
}
742+
678743
/// Returns `true` if `binding` is considered as unused.
679744
pub fn is_unused(model: &SemanticModel, binding: &AnyJsIdentifierBinding) -> bool {
680745
if matches!(binding, AnyJsIdentifierBinding::TsLiteralEnumMemberName(_)) {

crates/biome_js_analyze/src/services/embedded.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ use biome_workspace_db::embedded::bindings::{
55
InternedBindingText, InternedBindingTokenText, get_binding_by_name, get_binding_by_text,
66
};
77
use biome_workspace_db::embedded::references::{
8-
InternedReference, is_reference_used, is_type_reference_used, is_value_reference_used,
8+
InternedReference, is_reference_used, is_svelte_store_reference_used, is_type_reference_used,
9+
is_value_reference_used,
910
};
1011
use camino::Utf8PathBuf;
1112
use std::rc::Rc;
@@ -57,6 +58,16 @@ impl EmbeddedService {
5758
InternedReference::new(self.db.as_ref(), self.path.clone(), identifier),
5859
)
5960
}
61+
62+
/// Svelte stores are a special case. The `$` prefix is used to "dereference" the store and get its value.
63+
///
64+
/// See also: https://svelte.dev/docs/svelte/stores
65+
pub(crate) fn is_svelte_store_used(&self, identifier: TokenText) -> bool {
66+
is_svelte_store_reference_used(
67+
self.db.as_ref(),
68+
InternedReference::new(self.db.as_ref(), self.path.clone(), identifier),
69+
)
70+
}
6071
}
6172

6273
impl std::fmt::Debug for EmbeddedService {
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<!-- should not generate diagnostics -->
2+
<!-- Regression test: $bindable() props that are only written to in the script should -->
3+
<!-- not be flagged as unused. In Svelte 5, assigning to a $bindable() prop reflects -->
4+
<!-- the value back to the parent component, so write-only usage is intentional. -->
5+
<script>
6+
let {
7+
backButton = $bindable(),
8+
nextButton = $bindable(),
9+
} = $props();
10+
11+
// backButton is only written to — intentional for a $bindable() prop.
12+
backButton = { label: 'Back', onClick: () => {} };
13+
backButton = { label: 'Cancel', onClick: () => {} };
14+
</script>
15+
16+
{#if nextButton}
17+
<button onclick={nextButton.onClick}>{nextButton.label ?? 'Next'}</button>
18+
{/if}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
assertion_line: 149
4+
expression: valid-svelte-bindable-props.svelte
5+
---
6+
# Input
7+
```svelte
8+
<!-- should not generate diagnostics -->
9+
<!-- Regression test: $bindable() props that are only written to in the script should -->
10+
<!-- not be flagged as unused. In Svelte 5, assigning to a $bindable() prop reflects -->
11+
<!-- the value back to the parent component, so write-only usage is intentional. -->
12+
<script>
13+
let {
14+
backButton = $bindable(),
15+
nextButton = $bindable(),
16+
} = $props();
17+
18+
// backButton is only written to — intentional for a $bindable() prop.
19+
backButton = { label: 'Back', onClick: () => {} };
20+
backButton = { label: 'Cancel', onClick: () => {} };
21+
</script>
22+
23+
{#if nextButton}
24+
<button onclick={nextButton.onClick}>{nextButton.label ?? 'Next'}</button>
25+
{/if}
26+
27+
```
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<!-- should not generate diagnostics -->
2+
<!-- Regression test: $errors/$form in the template should suppress the -->
3+
<!-- "unused variable" diagnostic for the destructured `errors`/`form` -->
4+
<!-- bindings in the script, even when $-prefixed references only appear -->
5+
<!-- in the HTML template (not in the script block itself). -->
6+
<script lang="ts">
7+
import { superForm } from 'sveltekit-superforms';
8+
9+
const { form, errors, enhance } = superForm();
10+
</script>
11+
12+
<form use:enhance>
13+
<input bind:value={$form.email} />
14+
<span>{$errors.email}</span>
15+
</form>
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
expression: valid-svelte-store-subscription.svelte
4+
---
5+
# Input
6+
```svelte
7+
<!-- should not generate diagnostics -->
8+
<!-- Regression test: $errors/$form in the template should suppress the -->
9+
<!-- "unused variable" diagnostic for the destructured `errors`/`form` -->
10+
<!-- bindings in the script, even when $-prefixed references only appear -->
11+
<!-- in the HTML template (not in the script block itself). -->
12+
<script lang="ts">
13+
import { superForm } from 'sveltekit-superforms';
14+
15+
const { form, errors, enhance } = superForm();
16+
</script>
17+
18+
<form use:enhance>
19+
<input bind:value={$form.email} />
20+
<span>{$errors.email}</span>
21+
</form>
22+
23+
```

crates/biome_workspace_db/src/embedded/references.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,51 @@ pub fn is_reference_used(db: &dyn LanguageDb, reference: InternedReference<'_>)
7878
})
7979
}
8080

81+
/// Svelte stores are a special case. The `$` prefix is used to "dereference" the store and get its value.
82+
///
83+
/// See also: https://svelte.dev/docs/svelte/stores
84+
#[salsa::tracked]
85+
pub fn is_svelte_store_reference_used(
86+
db: &dyn LanguageDb,
87+
reference: InternedReference<'_>,
88+
) -> bool {
89+
let Some(parsed_source) = db.parsed_source_for_path(reference.path(db)) else {
90+
return false;
91+
};
92+
93+
embedded_references_from_source(db, parsed_source)
94+
.iter()
95+
.any(|refs| {
96+
refs.iter().any(|value_reference| {
97+
svelte_store_reference_name(value_reference.text.text()).is_some_and(
98+
|reference_store_name| reference_store_name == reference.name(db).text(),
99+
)
100+
})
101+
})
102+
}
103+
104+
fn svelte_store_reference_name(reference_name: &str) -> Option<&str> {
105+
// These are special Svelte runes that are not valid store names, so we should ignore them.
106+
const SVELTE_RUNES: [&str; 7] = [
107+
"$bindable",
108+
"$derived",
109+
"$effect",
110+
"$host",
111+
"$inspect",
112+
"$props",
113+
"$state",
114+
];
115+
116+
if SVELTE_RUNES.contains(&reference_name) {
117+
return None;
118+
}
119+
let store_name = reference_name.strip_prefix('$')?;
120+
if store_name.is_empty() || store_name.starts_with('$') {
121+
return None;
122+
}
123+
Some(store_name)
124+
}
125+
81126
#[cfg(test)]
82127
mod tests {
83128
use super::*;

0 commit comments

Comments
 (0)