Skip to content

Commit 071c700

Browse files
dyc3coderabbitai[bot]ematipico
authored
fix(noUndeclaredVariables): fix false positives for functions and variables defined in vue <script setup> components (#9283)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Emanuele Stoppa <[email protected]>
1 parent 6b01778 commit 071c700

6 files changed

Lines changed: 300 additions & 7 deletions

File tree

‎.changeset/better-pets-judge.md‎

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 [`noUndeclaredVariables`](https://biomejs.dev/linter/rules/no-undeclared-variables/) erroneously flagging functions and variables defined in the `<script setup>` section of Vue SFCs.

‎crates/biome_cli/tests/cases/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,4 @@ mod suppressions;
4747
mod tailwind_directives;
4848
mod unknown_files;
4949
mod vcs_ignored_files;
50+
mod vue_cross_language_rules;

‎crates/biome_cli/tests/cases/vue_cross_language_rules.rs‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
use crate::run_cli;
2+
use crate::snap_test::{SnapshotPayload, assert_cli_snapshot};
3+
use biome_console::BufferConsole;
4+
use biome_fs::MemoryFileSystem;
5+
use bpaf::Args;
6+
use camino::Utf8Path;
7+
8+
const BIOME_CONFIG_HTML_FULL_SUPPORT: &str =
9+
r#"{ "html": { "linter": {"enabled": true}, "experimentalFullSupportEnabled": true } }"#;
10+
11+
#[test]
12+
fn no_undeclared_variables_script_setup_with_functions_and_vars() {
13+
let fs = MemoryFileSystem::default();
14+
let mut console = BufferConsole::default();
15+
fs.insert(
16+
"biome.json".into(),
17+
BIOME_CONFIG_HTML_FULL_SUPPORT.as_bytes(),
18+
);
19+
let file = Utf8Path::new("file.vue");
20+
fs.insert(
21+
file.into(),
22+
r#"
23+
<template>
24+
<v-text-field
25+
:class="copySuccess ? 'text-success' : ''"
26+
ref="inviteLinkText"
27+
:value="inviteLink"
28+
:append-icon="mdiClipboardOutline"
29+
:messages="copySuccess ? 'Copied' : ''"
30+
@focus="onFocusHighlightText"
31+
@click:append="copyInviteLink"
32+
/>
33+
</template>
34+
35+
<script lang="ts" setup>
36+
import { mdiClipboardOutline } from "@mdi/js";
37+
import { ref, computed } from "vue";
38+
import { useStore } from "@/store";
39+
import { useCopyFromTextbox } from "./composables";
40+
41+
function buildInviteLink(
42+
currentLocation: string,
43+
roomName: string,
44+
shortUrl: string | undefined
45+
): string {
46+
if (shortUrl !== undefined) {
47+
return `https://${shortUrl}/${roomName}`;
48+
}
49+
return currentLocation.split("?")[0].toLowerCase();
50+
}
51+
52+
const store = useStore();
53+
const inviteLinkText = ref();
54+
55+
function getInviteLink() {
56+
return buildInviteLink(window.location.href, store.state.room.name, store.state.shortUrl);
57+
}
58+
const inviteLink = computed(getInviteLink);
59+
60+
function onFocusHighlightText(e) {
61+
e.target.select();
62+
}
63+
64+
const { copy: copyInviteLink, copySuccess } = useCopyFromTextbox(inviteLink, inviteLinkText);
65+
</script>
66+
"#
67+
.as_bytes(),
68+
);
69+
let (fs, result) = run_cli(
70+
fs,
71+
&mut console,
72+
Args::from(["lint", "--only=noUndeclaredVariables", file.as_str()].as_slice()),
73+
);
74+
assert!(result.is_ok(), "run_cli returned {result:?}");
75+
76+
assert_cli_snapshot(SnapshotPayload::new(
77+
module_path!(),
78+
"no_undeclared_variables_script_setup_with_functions_and_vars",
79+
fs,
80+
console,
81+
result,
82+
));
83+
}

‎crates/biome_cli/tests/snapshots/main_cases_vue_cross_language_rules/no_undeclared_variables_script_setup_with_functions_and_vars.snap‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
---
2+
source: crates/biome_cli/tests/snap_test.rs
3+
expression: redactor(content)
4+
---
5+
## `biome.json`
6+
7+
```json
8+
{
9+
"html": {
10+
"linter": { "enabled": true },
11+
"experimentalFullSupportEnabled": true
12+
}
13+
}
14+
```
15+
16+
## `file.vue`
17+
18+
```vue
19+
20+
<template>
21+
<v-text-field
22+
:class="copySuccess ? 'text-success' : ''"
23+
ref="inviteLinkText"
24+
:value="inviteLink"
25+
:append-icon="mdiClipboardOutline"
26+
:messages="copySuccess ? 'Copied' : ''"
27+
@focus="onFocusHighlightText"
28+
@click:append="copyInviteLink"
29+
/>
30+
</template>
31+
32+
<script lang="ts" setup>
33+
import { mdiClipboardOutline } from "@mdi/js";
34+
import { ref, computed } from "vue";
35+
import { useStore } from "@/store";
36+
import { useCopyFromTextbox } from "./composables";
37+
38+
function buildInviteLink(
39+
currentLocation: string,
40+
roomName: string,
41+
shortUrl: string | undefined
42+
): string {
43+
if (shortUrl !== undefined) {
44+
return `https://${shortUrl}/${roomName}`;
45+
}
46+
return currentLocation.split("?")[0].toLowerCase();
47+
}
48+
49+
const store = useStore();
50+
const inviteLinkText = ref();
51+
52+
function getInviteLink() {
53+
return buildInviteLink(window.location.href, store.state.room.name, store.state.shortUrl);
54+
}
55+
const inviteLink = computed(getInviteLink);
56+
57+
function onFocusHighlightText(e) {
58+
e.target.select();
59+
}
60+
61+
const { copy: copyInviteLink, copySuccess } = useCopyFromTextbox(inviteLink, inviteLinkText);
62+
</script>
63+
64+
```
65+
66+
# Emitted Messages
67+
68+
```block
69+
Checked 1 file in <TIME>. No fixes applied.
70+
```

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,25 @@ impl Rule for NoUnusedVariables {
339339

340340
// Ignore name prefixed with `_`
341341
let is_underscore_prefixed = binding_name.starts_with('_');
342-
let is_defined_in_embedded_binding = embedded_bindings.contains_binding(binding_name);
342+
// Only suppress noUnusedVariables for imports and variable declarations in
343+
// embedded script blocks. Function/class/type declarations should still be
344+
// flagged unless they are actually referenced in the template
345+
// (handled by is_used_as_reference below).
346+
// Eventually, we should probably not ignore bindings in embedded blocks, because they might be genuinely unused.
347+
let is_defined_in_embedded_binding = embedded_bindings.contains_binding(binding_name)
348+
&& binding
349+
.declaration()
350+
.map(|d| d.parent_binding_pattern_declaration().unwrap_or(d))
351+
.is_some_and(|d| {
352+
matches!(
353+
d,
354+
AnyJsBindingDeclaration::JsShorthandNamedImportSpecifier(_)
355+
| AnyJsBindingDeclaration::JsNamedImportSpecifier(_)
356+
| AnyJsBindingDeclaration::JsDefaultImportSpecifier(_)
357+
| AnyJsBindingDeclaration::JsNamespaceImportSpecifier(_)
358+
| AnyJsBindingDeclaration::JsVariableDeclarator(_)
359+
)
360+
});
343361
let is_used_as_reference = embedded_references.is_used_as_value(binding_name);
344362

345363
if is_underscore_prefixed || is_defined_in_embedded_binding || is_used_as_reference {

‎crates/biome_service/src/workspace/document/services/embedded_bindings.rs‎

Lines changed: 122 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use biome_js_syntax::{
2-
AnyJsArrayBindingPatternElement, AnyJsBindingPattern, AnyJsImportClause, AnyJsModuleItem,
3-
AnyJsObjectBindingPatternMember, AnyJsRoot, JsImport, JsModuleItemList, JsVariableStatement,
2+
AnyJsArrayBindingPatternElement, AnyJsBindingPattern, AnyJsModuleItem,
3+
AnyJsObjectBindingPatternMember, AnyJsRoot, AnyJsStatement, AnyTsIdentifierBinding, JsImport,
4+
JsModuleItemList, JsVariableStatement,
45
};
56
use biome_rowan::{AstNode, AstSeparatedList, TextRange, TokenText, WalkEvent};
67
use rustc_hash::FxHashMap;
@@ -52,11 +53,30 @@ impl EmbeddedBuilder {
5253
fn visit_module_item_list(&mut self, list: JsModuleItemList) {
5354
for item in list {
5455
match item {
55-
AnyJsModuleItem::AnyJsStatement(statement) => {
56-
if let Some(variable_statement) = statement.as_js_variable_statement() {
56+
AnyJsModuleItem::AnyJsStatement(statement) => match statement {
57+
AnyJsStatement::JsVariableStatement(variable_statement) => {
5758
self.visit_js_variable_statement(variable_statement.clone());
5859
}
59-
}
60+
AnyJsStatement::JsFunctionDeclaration(decl) => {
61+
self.register_js_binding(decl.id());
62+
}
63+
AnyJsStatement::JsClassDeclaration(decl) => {
64+
self.register_js_binding(decl.id());
65+
}
66+
AnyJsStatement::TsEnumDeclaration(decl) => {
67+
self.register_js_binding(decl.id());
68+
}
69+
AnyJsStatement::TsInterfaceDeclaration(decl) => {
70+
self.register_ts_identifier_binding(decl.id());
71+
}
72+
AnyJsStatement::TsTypeAliasDeclaration(decl) => {
73+
self.register_ts_identifier_binding(decl.binding_identifier());
74+
}
75+
AnyJsStatement::TsDeclareFunctionDeclaration(decl) => {
76+
self.register_js_binding(decl.id());
77+
}
78+
_ => {}
79+
},
6080
AnyJsModuleItem::JsExport(_) => {}
6181
AnyJsModuleItem::JsImport(import) => {
6282
self.visit_js_import(import);
@@ -85,7 +105,8 @@ impl EmbeddedBuilder {
85105
}
86106
}
87107

88-
if let AnyJsImportClause::JsImportDefaultClause(import) = clause {
108+
// Handle default clause using accessors generated by the syntax crate.
109+
if let Some(import) = clause.as_js_import_default_clause() {
89110
let name = import.default_specifier().ok()?;
90111
let name = name.local_name().ok()?;
91112
let name = name.as_js_identifier_binding()?;
@@ -94,6 +115,45 @@ impl EmbeddedBuilder {
94115
.insert(name.text_trimmed_range(), name.token_text_trimmed());
95116
}
96117

118+
// Namespace imports: `import * as Foo from "bar"` should register `Foo`.
119+
if let Some(import) = clause.as_js_import_namespace_clause() {
120+
let specifier = import.namespace_specifier().ok()?;
121+
let name = specifier.local_name().ok()?;
122+
let name = name.as_js_identifier_binding()?;
123+
let name = name.name_token().ok()?;
124+
self.js_bindings
125+
.insert(name.text_trimmed_range(), name.token_text_trimmed());
126+
}
127+
128+
Some(())
129+
}
130+
131+
/// Registers the name binding from a `SyntaxResult<AnyJsBinding>`.
132+
/// Used for `JsFunctionDeclaration::id()`, `JsClassDeclaration::id()`,
133+
/// `TsEnumDeclaration::id()`, and `TsDeclareFunctionDeclaration::id()`.
134+
fn register_js_binding(
135+
&mut self,
136+
result: biome_rowan::SyntaxResult<biome_js_syntax::AnyJsBinding>,
137+
) -> Option<()> {
138+
let binding = result.ok()?;
139+
let identifier = binding.as_js_identifier_binding()?;
140+
let token = identifier.name_token().ok()?;
141+
self.js_bindings
142+
.insert(token.text_trimmed_range(), token.token_text_trimmed());
143+
Some(())
144+
}
145+
/// Registers the name binding from a `SyntaxResult<AnyTsIdentifierBinding>`.
146+
/// Used for `TsInterfaceDeclaration::id()` and
147+
/// `TsTypeAliasDeclaration::binding_identifier()`.
148+
fn register_ts_identifier_binding(
149+
&mut self,
150+
result: biome_rowan::SyntaxResult<AnyTsIdentifierBinding>,
151+
) -> Option<()> {
152+
let binding = result.ok()?;
153+
let identifier = binding.as_ts_identifier_binding()?;
154+
let token = identifier.name_token().ok()?;
155+
self.js_bindings
156+
.insert(token.text_trimmed_range(), token.token_text_trimmed());
97157
Some(())
98158
}
99159

@@ -312,4 +372,60 @@ let lorem = "";
312372
assert!(contains_binding(&service, "Alas2"));
313373
assert!(contains_binding(&service, "lorem"));
314374
}
375+
376+
#[test]
377+
fn tracks_function_declarations() {
378+
let source = r#"
379+
function buildLink(base: string, path: string): string { return base + path; }
380+
async function fetchData() {}
381+
function* generator() {}
382+
"#;
383+
let mut service = EmbeddedExportedBindings::default();
384+
let mut builder = service.builder();
385+
visit_js_root(&mut builder, &parse_js(source));
386+
service.finish(builder);
387+
assert!(contains_binding(&service, "buildLink"));
388+
assert!(contains_binding(&service, "fetchData"));
389+
assert!(contains_binding(&service, "generator"));
390+
}
391+
392+
#[test]
393+
fn tracks_class_declarations() {
394+
let source = r#"
395+
class MyService {}
396+
abstract class BaseHandler {}
397+
"#;
398+
let mut service = EmbeddedExportedBindings::default();
399+
let mut builder = service.builder();
400+
visit_js_root(&mut builder, &parse_js(source));
401+
service.finish(builder);
402+
assert!(contains_binding(&service, "MyService"));
403+
assert!(contains_binding(&service, "BaseHandler"));
404+
}
405+
406+
#[test]
407+
fn tracks_typescript_declarations() {
408+
let source = r#"
409+
type UserId = string;
410+
interface UserProfile { name: string }
411+
enum Direction { Up, Down }
412+
"#;
413+
let mut service = EmbeddedExportedBindings::default();
414+
let mut builder = service.builder();
415+
visit_js_root(&mut builder, &parse_js(source));
416+
service.finish(builder);
417+
assert!(contains_binding(&service, "UserId"));
418+
assert!(contains_binding(&service, "UserProfile"));
419+
assert!(contains_binding(&service, "Direction"));
420+
}
421+
422+
#[test]
423+
fn tracks_namespace_imports() {
424+
let source = r#"import * as Vue from "vue";"#;
425+
let mut service = EmbeddedExportedBindings::default();
426+
let mut builder = service.builder();
427+
visit_js_root(&mut builder, &parse_js(source));
428+
service.finish(builder);
429+
assert!(contains_binding(&service, "Vue"));
430+
}
315431
}

0 commit comments

Comments
 (0)