fix(noVueDuplicateKeys): fix false positives with toRefs patterns#9072
fix(noVueDuplicateKeys): fix false positives with toRefs patterns#9072
toRefs patterns#9072Conversation
🦋 Changeset detectedLatest commit: b845eca 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 |
github-actions
bot
added
A-Linter
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Vue-specific API detection and handling for toRefs(props). Introduces Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@crates/biome_js_analyze/src/frameworks/vue/vue_component.rs`:
- Around line 770-789: The is_assigned_to_to_refs method is too permissive: it
treats any toRefs(...) call as coming from props; narrow it by verifying the
toRefs call argument is actually the props source (a defineProps() call or an
identifier bound to defineProps). Update is_assigned_to_to_refs so after
discovering the JsCallExpression via AnyJsExpression::JsCallExpression(call) it
inspects call.arguments() to get the first argument and returns true only when
that argument is either a defineProps call (e.g., passes an expression that
satisfies whatever existing is_define_props_call helper or pattern matches
JsCallExpression with callee defineProps) or is a JsIdentifier whose binding
resolves (via model/semantic lookup) to a variable initialised from defineProps;
keep using the existing is_to_refs_call check but AND it with the new validation
of the call's argument source.
In
`@crates/biome_js_analyze/tests/specs/correctness/noVueDuplicateKeys/valid-script-setup-to-refs.vue`:
- Around line 2-3: The code calls toRefs(props) but never captures defineProps()
into a variable; change the usage so you assign the result of defineProps() to a
local symbol (e.g., props = defineProps<{ toast: string; number?: number }>()),
then pass that props variable into toRefs(props) so toRefs has the actual props
object to destructure; update references to the returned prop refs accordingly
(symbols: defineProps, props, toRefs).
...s/biome_js_analyze/tests/specs/correctness/noVueDuplicateKeys/valid-script-setup-to-refs.vue
Outdated
Show resolved
Hide resolved
Merging this PR will not alter performance
Comparing 
Footnotes
|
dyc3
force-pushed
the
dyc3/fix-no-vue-dupe-keys-false-positive
branch
2 times, most recently
from
2ed931b to
b8f456e
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/biome_js_analyze/src/frameworks/vue/vue_call.rs (1)
36-69:⚠️ Potential issue | 🟡 MinorAdd a doctest block to the new rustdoc.
These new rustdoc comments lack a doctest code block; please add a minimal example with a passing assertion.As per coding guidelines:
**/*.rs: Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests.crates/biome_js_analyze/src/frameworks/vue/vue_component.rs (1)
747-828:⚠️ Potential issue | 🟡 MinorRustdoc needs a doctest example here too.
Please add a small doctest code block (with a passing assertion) to the newis_assigned_to_to_refsdocs.As per coding guidelines:
**/*.rs: Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests.
dyc3
force-pushed
the
dyc3/fix-no-vue-dupe-keys-false-positive
branch
from
b8f456e to
1568093
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@crates/biome_js_analyze/src/frameworks/vue/vue_call.rs`:
- Around line 51-69: Update the rustdoc for is_to_refs_call to use a fenced code
block in doctest format: replace the inline example comment with a
triple-backtick fenced block specifying the language (js) and include the
example line(s) inside (e.g., const { foo, bar } = toRefs(props);) followed by
closing triple backticks so the example becomes a proper rustdoc code block that
can be validated as a doctest.
In `@crates/biome_js_analyze/src/frameworks/vue/vue_component.rs`:
- Around line 748-752: The rustdoc for the method is_assigned_to_props should
put the example usage into a doctest code block: replace the inline example
"const { foo } = defineProps<{ foo: string }>()" with a fenced code block using
triple backticks and the js language tag (/// ```js ... /// ```), so the comment
becomes a proper rustdoc code block that will be picked up by doctests and keep
the surrounding explanatory sentence intact; update the doc comment above pub fn
is_assigned_to_props accordingly.
- Around line 770-825: Update the rustdoc above the is_assigned_to_to_refs
method to replace the inline snippet `const { foo } = toRefs(props)` with a
fenced doctest code block (use triple backticks and an optional language tag
like "js") so the example is rendered/parsed as a rustdoc code block; ensure the
surrounding explanatory lines remain and the fenced block is properly closed to
satisfy doctest-style formatting.
| /// Checks if the given call expression is a call to `toRefs` from the Vue package. | ||
| /// | ||
| /// `toRefs` is used to convert a reactive object to a plain object where each property | ||
| /// is a ref pointing to the corresponding property of the original object. This is commonly | ||
| /// used with `defineProps` to destructure props while maintaining reactivity. | ||
| /// | ||
| /// This function handles both imported `toRefs` and auto-imported `toRefs` in Vue `<script setup>`. | ||
| /// | ||
| /// Example: `const { foo, bar } = toRefs(props)` | ||
| pub fn is_to_refs_call(call: &JsCallExpression, model: &SemanticModel) -> bool { | ||
| let Some(callee) = call.callee().ok() else { | ||
| return false; | ||
| }; | ||
| let Some(expression) = callee.inner_expression() else { | ||
| return false; | ||
| }; | ||
|
|
||
| is_vue_api_reference(&expression, model, "toRefs") | ||
| } |
There was a problem hiding this comment.
Use a fenced rustdoc code block for the example.
Inline snippets make the docs harder to validate.
✍️ Suggested rustdoc tweak
-/// Example: `const { foo, bar } = toRefs(props)`
+/// Example:
+/// ```js
+/// const { foo, bar } = toRefs(props);
+/// ```As per coding guidelines, "Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Checks if the given call expression is a call to `toRefs` from the Vue package. | |
| /// | |
| /// `toRefs` is used to convert a reactive object to a plain object where each property | |
| /// is a ref pointing to the corresponding property of the original object. This is commonly | |
| /// used with `defineProps` to destructure props while maintaining reactivity. | |
| /// | |
| /// This function handles both imported `toRefs` and auto-imported `toRefs` in Vue `<script setup>`. | |
| /// | |
| /// Example: `const { foo, bar } = toRefs(props)` | |
| pub fn is_to_refs_call(call: &JsCallExpression, model: &SemanticModel) -> bool { | |
| let Some(callee) = call.callee().ok() else { | |
| return false; | |
| }; | |
| let Some(expression) = callee.inner_expression() else { | |
| return false; | |
| }; | |
| is_vue_api_reference(&expression, model, "toRefs") | |
| } | |
| /// Checks if the given call expression is a call to `toRefs` from the Vue package. | |
| /// | |
| /// `toRefs` is used to convert a reactive object to a plain object where each property | |
| /// is a ref pointing to the corresponding property of the original object. This is commonly | |
| /// used with `defineProps` to destructure props while maintaining reactivity. | |
| /// | |
| /// This function handles both imported `toRefs` and auto-imported `toRefs` in Vue `<script setup>`. | |
| /// | |
| /// Example: | |
| /// |
🤖 Prompt for AI Agents
In `@crates/biome_js_analyze/src/frameworks/vue/vue_call.rs` around lines 51 - 69,
Update the rustdoc for is_to_refs_call to use a fenced code block in doctest
format: replace the inline example comment with a triple-backtick fenced block
specifying the language (js) and include the example line(s) inside (e.g., const
{ foo, bar } = toRefs(props);) followed by closing triple backticks so the
example becomes a proper rustdoc code block that can be validated as a doctest.
| /// 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 { |
There was a problem hiding this comment.
Move the defineProps example into a rustdoc code block.
Keeps docs consistent with doctest formatting expectations.
✍️ 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
In `@crates/biome_js_analyze/src/frameworks/vue/vue_component.rs` around lines 748
- 752, The rustdoc for the method is_assigned_to_props should put the example
usage into a doctest code block: replace the inline example "const { foo } =
defineProps<{ foo: string }>()" with a fenced code block using triple backticks
and the js language tag (/// ```js ... /// ```), so the comment becomes a proper
rustdoc code block that will be picked up by doctests and keep the surrounding
explanatory sentence intact; update the doc comment above pub fn
is_assigned_to_props accordingly.
| /// 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 |
There was a problem hiding this comment.
Use a fenced code block for the toRefs example.
Inline snippets don’t meet the doctest-style doc format.
✍️ Suggested rustdoc tweak
-/// 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.
+/// This handles cases like:
+/// ```js
+/// 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.As per coding guidelines, "Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests".
🤖 Prompt for AI Agents
In `@crates/biome_js_analyze/src/frameworks/vue/vue_component.rs` around lines 770
- 825, Update the rustdoc above the is_assigned_to_to_refs method to replace the
inline snippet `const { foo } = toRefs(props)` with a fenced doctest code block
(use triple backticks and an optional language tag like "js") so the example is
rendered/parsed as a rustdoc code block; ensure the surrounding explanatory
lines remain and the fenced block is properly closed to satisfy doctest-style
formatting.
1 participant
Summary
implemented by minimax 2.5
fixes #9068
Test Plan
snapshots
Docs