-
-
Notifications
You must be signed in to change notification settings - Fork 918
fix(noVueDuplicateKeys): fix false positives with toRefs patterns
#9072
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Fixed [#9068](https://github.com/biomejs/biome/issues/9068): The `noVueDuplicateKeys` rule now correctly handles `toRefs(props)` patterns and no longer produces false positives when destructuring props, particularly in `<script setup>` blocks. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,6 @@ | ||
| use crate::frameworks::vue::vue_call::{is_vue_api_reference, is_vue_compiler_macro_call}; | ||
| use crate::frameworks::vue::vue_call::{ | ||
| is_to_refs_call, is_vue_api_reference, is_vue_compiler_macro_call, | ||
| }; | ||
| use crate::services::semantic::Semantic; | ||
| use biome_js_semantic::SemanticModel; | ||
| use biome_js_syntax::{ | ||
|
|
@@ -743,6 +745,10 @@ declare_node_union! { | |
| } | ||
|
|
||
| impl AnyVueSetupDeclaration { | ||
| /// Checks if this setup declaration is assigned directly from `defineProps`. | ||
| /// | ||
| /// This handles cases like `const { foo } = defineProps<{ foo: string }>()` where | ||
| /// the destructured property comes from the props definition. | ||
| pub fn is_assigned_to_props(&self, model: &SemanticModel) -> bool { | ||
|
Comment on lines
+748
to
752
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move the defineProps example into a rustdoc code block. ✍️ Suggested rustdoc tweak-/// This handles cases like `const { foo } = defineProps<{ foo: string }>()` where
-/// the destructured property comes from the props definition.
+/// This handles cases like:
+/// ```js
+/// const { foo } = defineProps<{ foo: string }>();
+/// ```
+/// where the destructured property comes from the props definition.As per coding guidelines, "Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests". 🤖 Prompt for AI Agents |
||
| if let Self::JsIdentifierBinding(binding) = self | ||
| && let Some(declarator) = binding | ||
|
|
@@ -760,6 +766,65 @@ impl AnyVueSetupDeclaration { | |
| } | ||
| false | ||
| } | ||
|
|
||
| /// Checks if this setup declaration is assigned from `toRefs(props)`. | ||
| /// | ||
| /// This handles cases like `const { foo } = toRefs(props)` where the destructured | ||
| /// property comes from converting props to refs. These should not be flagged as | ||
| /// duplicates of the props themselves. | ||
| /// | ||
| /// Only returns true if the argument to `toRefs` is either: | ||
| /// - A direct `defineProps()` call | ||
| /// - An identifier whose binding resolves to a variable initialized from `defineProps` | ||
| pub fn is_assigned_to_to_refs(&self, model: &SemanticModel) -> bool { | ||
| if let Self::JsIdentifierBinding(binding) = self | ||
| && let Some(declarator) = binding | ||
| .syntax() | ||
| .ancestors() | ||
| .find_map(|syntax| JsVariableDeclarator::try_cast(syntax).ok()) | ||
| && let Some(initializer) = declarator.initializer() | ||
| && let Some(expression) = initializer | ||
| .expression() | ||
| .ok() | ||
| .and_then(|expression| expression.inner_expression()) | ||
| && let AnyJsExpression::JsCallExpression(call) = expression | ||
| && is_to_refs_call(&call, model) | ||
| { | ||
| // Check that the first argument to `toRefs` is the props source | ||
| if let Some(Ok(first_arg)) = call | ||
| .arguments() | ||
| .ok() | ||
| .and_then(|args| args.args().iter().next()) | ||
| && let Some(arg_expr) = first_arg.as_any_js_expression() | ||
| && let Some(arg_expr) = arg_expr.inner_expression() | ||
| { | ||
| // Case 1: Direct defineProps() call: `toRefs(defineProps(...))` | ||
| if let AnyJsExpression::JsCallExpression(arg_call) = &arg_expr | ||
| && is_vue_compiler_macro_call(arg_call, model, "defineProps") | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| // Case 2: Identifier bound to defineProps: `const props = defineProps(...); toRefs(props)` | ||
| if let Some(ident_ref) = arg_expr.as_js_reference_identifier() | ||
| && let Some(binding) = model.binding(&ident_ref) | ||
| && let Some(declarator) = binding | ||
| .syntax() | ||
| .ancestors() | ||
| .find_map(|syntax| JsVariableDeclarator::try_cast(syntax).ok()) | ||
| && let Some(decl_initializer) = declarator.initializer() | ||
| && let Some(decl_expr) = decl_initializer | ||
| .expression() | ||
| .ok() | ||
| .and_then(|expr| expr.inner_expression()) | ||
| && let AnyJsExpression::JsCallExpression(decl_call) = decl_expr | ||
| { | ||
| return is_vue_compiler_macro_call(&decl_call, model, "defineProps"); | ||
| } | ||
| } | ||
| } | ||
| false | ||
| } | ||
| } | ||
|
|
||
| impl VueDeclarationName for AnyVueSetupDeclaration { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| <!-- should emit diagnostics --> | ||
|
|
||
| <script setup> | ||
| import { toRefs, reactive } from 'vue'; | ||
| defineProps<{ toast: string; number?: number }>(); | ||
| const obj = reactive({ toast: 'toast' }); | ||
| const { toast } = toRefs(obj) | ||
| </script> | ||
|
|
||
| <template> | ||
| <!-- Which `toast` this refers to is ambiguous, because defineProps makes the props available in the template area --> | ||
| {{ toast }} | ||
| </template> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| --- | ||
| source: crates/biome_js_analyze/tests/spec_tests.rs | ||
| expression: invalid-script-setup-to-refs-no-assign.vue | ||
| --- | ||
| # Input | ||
| ```ts | ||
| import { toRefs, reactive } from 'vue'; | ||
| defineProps<{ toast: string; number?: number }>(); | ||
| const obj = reactive({ toast: 'toast' }); | ||
| const { toast } = toRefs(obj) | ||
|
|
||
| ``` | ||
|
|
||
| # Diagnostics | ||
| ``` | ||
| invalid-script-setup-to-refs-no-assign.vue:2:15 lint/correctness/noVueDuplicateKeys ━━━━━━━━━━━━━━━━ | ||
|
|
||
| × Duplicate key toast found in Vue component. | ||
|
|
||
| 1 │ import { toRefs, reactive } from 'vue'; | ||
| > 2 │ defineProps<{ toast: string; number?: number }>(); | ||
| │ ^^^^^ | ||
| 3 │ const obj = reactive({ toast: 'toast' }); | ||
| 4 │ const { toast } = toRefs(obj) | ||
|
|
||
| i Key toast is also defined here. | ||
|
|
||
| 2 │ defineProps<{ toast: string; number?: number }>(); | ||
| 3 │ const obj = reactive({ toast: 'toast' }); | ||
| > 4 │ const { toast } = toRefs(obj) | ||
| │ ^^^^^ | ||
| 5 │ | ||
|
|
||
| i Keys defined in different Vue component options (props, data, methods, computed) can conflict when accessed in the template. Rename the key to avoid conflicts. | ||
|
|
||
|
|
||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| <!-- should not emit diagnostics --> | ||
|
|
||
| <script setup lang="ts"> | ||
| import { toRefs } from 'vue'; | ||
| interface Props { | ||
| videoUrl: string; | ||
| thumbnail?: string; | ||
| } | ||
| const props = defineProps<Props>() | ||
| const { videoUrl, thumbnail } = toRefs(props) | ||
| </script> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| --- | ||
| source: crates/biome_js_analyze/tests/spec_tests.rs | ||
| expression: valid-script-setup-to-refs-interface.vue | ||
| --- | ||
| # Input | ||
| ```ts | ||
| import { toRefs } from 'vue'; | ||
| interface Props { | ||
| videoUrl: string; | ||
| thumbnail?: string; | ||
| } | ||
| const props = defineProps<Props>() | ||
| const { videoUrl, thumbnail } = toRefs(props) | ||
|
|
||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| <!-- should not emit diagnostics --> | ||
|
|
||
| <script setup> | ||
| import { toRefs } from 'vue'; | ||
| const props = defineProps<{ toast: string; number?: number }>() | ||
| const { toast } = toRefs(props) | ||
| </script> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| --- | ||
| source: crates/biome_js_analyze/tests/spec_tests.rs | ||
| expression: valid-script-setup-to-refs.vue | ||
| --- | ||
| # Input | ||
| ```ts | ||
| import { toRefs } from 'vue'; | ||
| const props = defineProps<{ toast: string; number?: number }>() | ||
| const { toast } = toRefs(props) | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a fenced rustdoc code block for the example.
Inline snippets make the docs harder to validate.
✍️ Suggested rustdoc tweak
As per coding guidelines, "Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests".
📝 Committable suggestion
🤖 Prompt for AI Agents