feat(biome_js_analyze): implement noVueRefAsOperand#9063
Conversation
🦋 Changeset detectedLatest commit: e576dd0 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 |
|
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 a new Vue lint rule Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In @.changeset/fair-parks-taste.md:
- Line 5: Fix the grammar in the changelog sentence for the rule name
noVueRefAsOperand by replacing "disallow" with "disallows" so the sentence reads
that the rule "disallows cases where a ref is used as an operand"; update the
text in the .changeset entry where the rule is described accordingly.
In `@crates/biome_js_analyze/src/lint/nursery/no_vue_ref_as_operand.rs`:
- Around line 158-234: The match over parent.kind() in the function that checks
identifier usage (look for ident_expr, ident_binding, RuleState and the match
parent.kind() block) currently flags
if/switch/unary/binary/assignment/logical/conditional/template/static-member/call-argument
cases but misses loop-condition contexts; add JsSyntaxKind::JS_WHILE_STATEMENT,
JsSyntaxKind::JS_DO_WHILE_STATEMENT, and JsSyntaxKind::JS_FOR_STATEMENT as
additional arms (or include them with the existing JsSyntaxKind::JS_IF_STATEMENT
| JsSyntaxKind::JS_SWITCH_STATEMENT arm) so that those loop conditions return
Some(RuleState { range: ident_expr.range() }) just like the if/switch case.
- Around line 298-346: check_static_member_access incorrectly continues
validation after recognizing a valid toRefs access; modify it so when
ref_callee_name.text() == "toRefs" and
is_valid_static_member_wrapped_in_to_refs(...) returns true you immediately
return None (early exit) instead of falling through to is_valid_static_member.
Also fix is_valid_static_member_wrapped_in_to_refs: for the direct-ref case (the
branch that tries to validate refs.foo.value) advance one more parent hop before
extracting the member so you reach the outer "value" member (e.g., take
parent().parent() or get the parent as JsStaticMemberExpression of the outer
access) and then call .member().ok() and validate with is_valid_static_member;
keep the destructured-path behavior unchanged and continue to use
is_valid_static_member for the final check.
In
`@crates/biome_js_analyze/tests/specs/nursery/noVueRefAsOperand/invalid-conditional-operators.vue`:
- Around line 1-5: This test file is missing the standard marker comment; add
the comment /* should generate diagnostics */ to the top of the test (inside the
file containing the <script> block) so the invalid case with the ref variable
foo used as an operand (foo in the var a = foo ? x : y expression) is recognized
as an expected-diagnostic test; ensure the comment appears where other invalid
tests place it (e.g., immediately before the offending code in the <script>
block).
In
`@crates/biome_js_analyze/tests/specs/nursery/noVueRefAsOperand/invalid-defineemits.vue`:
- Around line 1-9: This file is missing the required test marker and uses the
wrong script type: add the comment "/* should generate diagnostics */" at the
top of the file and change the plain <script> block to a <script setup> block so
the compiler macro defineEmits is valid; ensure the refs/imports (ref,
defineEmits, count, update) remain unchanged so the test still triggers
diagnostics for using a Ref as an emit operand.
In
`@crates/biome_js_analyze/tests/specs/nursery/noVueRefAsOperand/invalid-definemodel-destructured`
copy.vue:
- Around line 1-12: The test file name includes an accidental duplicate suffix "
copy" (invalid-definemodel-destructured copy.vue); rename the file to remove the
space and suffix (e.g., invalid-definemodel-destructured.vue) and update any
test-suite references/fixtures that import or enumerate that file so the test
runner sees the intended filename; ensure the test contents (the <script setup>
block and the describe/spec harness that expects this invalid case) remain
unchanged and run after the rename.
In
`@crates/biome_js_analyze/tests/specs/nursery/noVueRefAsOperand/invalid-definemodel.vue`:
- Around line 3-11: The test uses the compiler macro defineModel() inside a
regular <script> block which is invalid; update the test so the macro is called
within a <script setup> block (move the existing let model = defineModel(); and
any top-level usage—e.g., console.log(model), the process() and update()
functions—into a <script setup>), or replace defineModel() with a non-macro stub
if you intend to keep a plain <script>; ensure defineModel() invocation and
related model assignment (in update) are located inside the <script setup>
context so the compiler macro is valid.
In
`@crates/biome_js_analyze/tests/specs/nursery/noVueRefAsOperand/valid-definemodel-destructured.vue`:
- Line 1: The file valid-definemodel-destructured.vue contains a contradictory
top-line comment "/* should generate diagnostics */" — either change that
comment to "/* should not generate diagnostics */" to match the valid-* naming
or rename the file to invalid-definemodel-destructured.vue if it is intended to
produce diagnostics; update whichever you choose so test conventions (file
prefix and the top comment) are consistent with the intended test outcome.
In
`@crates/biome_js_analyze/tests/specs/nursery/noVueRefAsOperand/valid-ignore-cases.vue`:
- Around line 34-43: The file defines const foo4 twice (shallowRef({}) and
ref(0)), causing a duplicate declaration SyntaxError; rename one of the
identifiers (e.g., change the second foo4 to foo5) everywhere it’s used in the
snippet (references in tag`${foo4}` or any subsequent use) so each const
(shallowRef and ref) has a unique name and the test remains valid.
🧹 Nitpick comments (4)
crates/biome_js_analyze/src/lint/nursery/no_vue_ref_as_operand.rs (4)
269-282: Consider returningboolinstead ofOption<bool>.
is_calling_a_refreturnsOption<bool>, and every call site does.is_some_and(|v| v)— effectively treatingNoneandSome(false)identically. Returningbool(with the callee?replaced by an earlyreturn Some(false)or usingand_theninternally) would simplify call sites from!is_calling_a_ref(...).is_some_and(|v| v)to just!is_calling_a_ref(...).
182-189: Fragile AST ancestry walk.
binding.declaration()→.syntax().grand_parent()assumes a fixed tree depth to reachJsVariableDeclaration. If the AST shape differs (e.g., TypeScript type-annotated destructuring), this could silently fail. Consider using.ancestors().find_map(JsVariableDeclaration::cast)— the same pattern you already use on line 144 — for consistency and robustness.♻️ Suggested change
- if let Some(decl) = binding.declaration() - && let Some(grand_parent) = decl.syntax().grand_parent() - && matches!(grand_parent.kind(), JsSyntaxKind::JS_VARIABLE_DECLARATION) - && let Some(decl) = grand_parent.cast::<JsVariableDeclaration>() - && (decl.is_const()) { + if let Some(decl) = binding.syntax().ancestors().find_map(JsVariableDeclaration::cast) + && decl.is_const() {
107-109: MoveAnyJsStaticMemberLikebelow theimpl Ruleblock.Per project convention, only the
Querynode union (NoVueRefAsOperandQuery) may live above theimpl Ruleblock. Helper unions likeAnyJsStaticMemberLikeshould be placed below it alongside other helper functions and types. Based on learnings: "all helper functions, structs, and enums must be placed below theimpl Ruleblock. The only exception is when declaring a node union to use in the rule's Query type."
223-232: Variablecall_exprshadows the outer ref-call binding.Line 226 introduces a
call_exprthat shadows the one from line 147 (the ref initializer). It's scoped, so no logic bug, but a name likeemit_call_exprwould make intent clearer at a glance.
| if let Some(parent) = expr.syntax().parent() { | ||
| match parent.kind() { | ||
| // if (refValue) | ||
| JsSyntaxKind::JS_IF_STATEMENT | | ||
| // switch (refValue) | ||
| JsSyntaxKind::JS_SWITCH_STATEMENT | | ||
| // -refValue, +refValue, !refValue, ~refValue, typeof refValue | ||
| JsSyntaxKind::JS_UNARY_EXPRESSION | | ||
| // refValue+1, refValue-1 | ||
| JsSyntaxKind::JS_BINARY_EXPRESSION | | ||
| // bar+=refValue, bar-=refValue | ||
| JsSyntaxKind::JS_ASSIGNMENT_EXPRESSION => { | ||
| return Some(RuleState { range: ident_expr.range() }) | ||
| } | ||
| // refValue || other, refValue && other. ignore: other || refValue | ||
| JsSyntaxKind::JS_LOGICAL_EXPRESSION => { | ||
| // Report only left | ||
| if let Some(logical_expr) = parent.cast::<JsLogicalExpression>() | ||
| && let Ok(left)= logical_expr.left() | ||
| && let Some(left) = left.as_js_identifier_expression() | ||
| && left != ident_expr { | ||
| return None | ||
| } | ||
|
|
||
| // Report only constants | ||
| if let Some(decl) = binding.declaration() | ||
| && let Some(grand_parent) = decl.syntax().grand_parent() | ||
| && matches!(grand_parent.kind(), JsSyntaxKind::JS_VARIABLE_DECLARATION) | ||
| && let Some(decl) = grand_parent.cast::<JsVariableDeclaration>() | ||
| && (decl.is_const()) { | ||
| return Some(RuleState { range: ident_expr.range() }) | ||
| } | ||
| } | ||
| // refValue ? x : y | ||
| JsSyntaxKind::JS_CONDITIONAL_EXPRESSION => { | ||
| // Report only test | ||
| if let Some(conditional_expr) = parent.cast::<JsConditionalExpression>() | ||
| && let Ok(test)= conditional_expr.test() | ||
| && let Some(expr) = test.as_js_identifier_expression() | ||
| && expr != ident_expr { | ||
| return None | ||
| } | ||
|
|
||
| return Some(RuleState { range: ident_expr.range() }) | ||
| } | ||
| // `${refValue}` | ||
| JsSyntaxKind::JS_TEMPLATE_ELEMENT => { | ||
| // Ignore tagged template | ||
| if let Some(template_ele) = parent.cast::<JsTemplateElement>() | ||
| && let Some(grand_parent) = template_ele.syntax().grand_parent() | ||
| && let Some(template_expr) = grand_parent.cast::<JsTemplateExpression>() | ||
| && template_expr.tag().is_some() { | ||
| return None | ||
| } | ||
|
|
||
| return Some(RuleState { range: ident_expr.range() }) | ||
| } | ||
| // refValue.x | ||
| JsSyntaxKind::JS_STATIC_MEMBER_ASSIGNMENT | JsSyntaxKind::JS_STATIC_MEMBER_EXPRESSION => { | ||
| if let Some(static_member_expr) = parent.cast::<AnyJsStaticMemberLike>() { | ||
| return check_static_member_access(ident_expr, ident_binding, &static_member_expr, call_expr) | ||
| } | ||
| } | ||
| // refValue in emit() in setup contexts: emit('event', refValue) or context.emit('event', refValue) | ||
| // refValue in emit() with defineEmits | ||
| JsSyntaxKind::JS_CALL_ARGUMENT_LIST => { | ||
| if let Some(call_argument_list) = parent.cast::<JsCallArgumentList>() | ||
| && let Some(grand_parent) = call_argument_list.syntax().grand_parent() | ||
| && let Some(call_expr) = grand_parent.cast::<JsCallExpression>() | ||
| && let Ok(callee) = call_expr.callee() | ||
| && (is_emit_call_in_setup(&callee, model) || is_emit_call_by_macro(&callee, model)) | ||
| { | ||
| return Some(RuleState { range: ident_expr.range() }); | ||
| } | ||
| } | ||
| _ => {} | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find test files related to no_vue_ref_as_operand
find . -type f -name "*.rs" | xargs grep -l "no_vue_ref_as_operand" | head -20Repository: biomejs/biome
Length of output: 230
🏁 Script executed:
#!/bin/bash
# Look for fixture/snapshot files
find . -type d -name "*no_vue_ref_as_operand*" 2>/dev/nullRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Search for test patterns in the rule file itself and nearby
find . -path "*nursery*" -name "*.rs" -o -name "*.snap" | grep -i "vue_ref" | head -20Repository: biomejs/biome
Length of output: 124
🌐 Web query:
eslint-plugin-vue no-ref-as-operand rule while statement do-while for loop
💡 Result:
vue/no-ref-as-operand reports when you use a Vue ref() itself in an expression where you meant its inner value. In JS, a Ref is an object, so you must read/write the inner value via .value. (eslint.vuejs.org)
while / do...while
Bad:
const done = ref(false)
while (!done) { // ❌ Ref used as operand
// ...
done = true // ❌ (also illegal reassignment if const)
}Good:
const done = ref(false)
while (!done.value) { // ✅
// ...
done.value = true // ✅
}(And similarly for do { ... } while (cond).)
for loops (init / test / update)
Bad:
const i = ref(0)
const n = ref(10)
for (; i < n; i++) { // ❌ i and n are refs; i++ updates the ref object
// ...
}Good (use .value everywhere inside the loop header):
const i = ref(0)
const n = ref(10)
for (; i.value < n.value; i.value++) { // ✅
// ...
}Or (often cleaner) don’t use refs for loop counters:
const n = ref(10)
for (let i = 0; i < n.value; i++) {
// ...
}Note: the rule is fixable in some cases with ESLint --fix. (eslint.vuejs.org)
Citations:
- 1: https://eslint.vuejs.org/rules/no-ref-as-operand.html?utm_source=openai
- 2: https://eslint.vuejs.org/rules/no-ref-as-operand.html?utm_source=openai
🏁 Script executed:
#!/bin/bash
# Find test/fixture files for this rule
find . -type f \( -name "*.snap" -o -name "*.test.*" -o -name "*.spec.*" \) | xargs grep -l "no_vue_ref_as_operand" 2>/dev/null | head -10Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Search for any fixture or snapshot directory
find . -type d -name "fixtures" -o -name "snapshots" | grep -i "nursery\|vue" | head -10Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Look for snapshot files that might contain test cases
fd -e snap | xargs grep -l "while\|do.*while\|for" | xargs grep -l "ref" 2>/dev/null | head -5Repository: biomejs/biome
Length of output: 697
🏁 Script executed:
#!/bin/bash
# Check the implementation for TODO or FIXME comments
rg -n "TODO|FIXME" crates/biome_js_analyze/src/lint/nursery/no_vue_ref_as_operand.rsRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Look at the full context of the file to understand test structure
wc -l crates/biome_js_analyze/src/lint/nursery/no_vue_ref_as_operand.rsRepository: biomejs/biome
Length of output: 126
🏁 Script executed:
#!/bin/bash
# Find the test specs directory
find crates/biome_js_analyze/tests -type d | head -20Repository: biomejs/biome
Length of output: 1214
🏁 Script executed:
#!/bin/bash
# Look for any files related to vue or ref
find crates/biome_js_analyze/tests -type f -name "*vue*" -o -name "*ref*" 2>/dev/null | head -20Repository: biomejs/biome
Length of output: 1665
🏁 Script executed:
#!/bin/bash
# Check the test directory structure
ls -la crates/biome_js_analyze/tests/specs/nursery/ 2>/dev/null | head -30Repository: biomejs/biome
Length of output: 2019
🏁 Script executed:
#!/bin/bash
# Search for noVueRefAsOperand test files
find . -type f -name "*noVueRefAsOperand*" 2>/dev/nullRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Let's look at the entire rule file to see test structure
tail -100 crates/biome_js_analyze/src/lint/nursery/no_vue_ref_as_operand.rsRepository: biomejs/biome
Length of output: 3657
🏁 Script executed:
#!/bin/bash
# Check if noVueRefAsOperand directory exists at all
ls -la crates/biome_js_analyze/tests/specs/nursery/ | grep -i "ref\|vue"Repository: biomejs/biome
Length of output: 936
🏁 Script executed:
#!/bin/bash
# Search more broadly for any test that mentions this rule
rg -l "noVueRefAsOperand" crates/biome_js_analyze/tests/Repository: biomejs/biome
Length of output: 1696
🏁 Script executed:
#!/bin/bash
# Check if there's a test or spec for this rule elsewhere
find crates/biome_js_analyze -type f \( -name "*.vue" -o -name "*.js" \) | xargs rg -l "while.*ref\|for.*ref\|do.*while.*ref" 2>/dev/null | head -10Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Let's check how the rule is imported/registered
rg "no_vue_ref_as_operand" crates/biome_js_analyze/src/lib.rsRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
# List all test files in the noVueRefAsOperand directory
ls -1 crates/biome_js_analyze/tests/specs/nursery/noVueRefAsOperand/ | grep -v snapRepository: biomejs/biome
Length of output: 876
🏁 Script executed:
#!/bin/bash
# Check the invalid test files for while/do-while/for patterns
find crates/biome_js_analyze/tests/specs/nursery/noVueRefAsOperand -name "invalid*.vue" -not -name "*.snap" | head -20Repository: biomejs/biome
Length of output: 1611
🏁 Script executed:
#!/bin/bash
# Search for while/do-while/for in the actual test files
find crates/biome_js_analyze/tests/specs/nursery/noVueRefAsOperand -name "*.vue" -not -name "*.snap" -exec cat {} \; 2>/dev/null | head -200Repository: biomejs/biome
Length of output: 3375
🏁 Script executed:
#!/bin/bash
# Get a clearer picture - check each test file
for file in crates/biome_js_analyze/tests/specs/nursery/noVueRefAsOperand/invalid-*.vue; do
echo "=== $(basename "$file") ==="
head -30 "$file"
echo ""
doneRepository: biomejs/biome
Length of output: 4172
Missing handling for while, do-while, and for loop conditions
The match arms cover if and switch but not loop contexts. Per the ESLint rule, while (refValue), do {} while (refValue), and for (; refValue; ) should all be flagged. Add JS_WHILE_STATEMENT, JS_DO_WHILE_STATEMENT, and JS_FOR_STATEMENT cases to keep parity with the upstream rule.
🤖 Prompt for AI Agents
In `@crates/biome_js_analyze/src/lint/nursery/no_vue_ref_as_operand.rs` around
lines 158 - 234, The match over parent.kind() in the function that checks
identifier usage (look for ident_expr, ident_binding, RuleState and the match
parent.kind() block) currently flags
if/switch/unary/binary/assignment/logical/conditional/template/static-member/call-argument
cases but misses loop-condition contexts; add JsSyntaxKind::JS_WHILE_STATEMENT,
JsSyntaxKind::JS_DO_WHILE_STATEMENT, and JsSyntaxKind::JS_FOR_STATEMENT as
additional arms (or include them with the existing JsSyntaxKind::JS_IF_STATEMENT
| JsSyntaxKind::JS_SWITCH_STATEMENT arm) so that those loop conditions return
Some(RuleState { range: ident_expr.range() }) just like the if/switch case.
There was a problem hiding this comment.
Original ESLint rule doesn't flag these cases, so keeping the same behavior for compatibility.
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.changeset/fair-parks-taste.md:
- Line 5: The changeset for the new lint rule noVueRefAsOperand lacks an
invalid-code example; update the .changeset entry to include a short snippet
that demonstrates using a Vue ref as a boolean/operand (e.g., using count where
count is created via ref(0)) and show the corrected form using count.value so
users see both the flagged pattern and the fix; reference the rule name
noVueRefAsOperand and include the suggested invalid line and its corrected
counterpart in the changeset text.
70ffe0f to
90449d8
Compare
dyc3
left a comment
There was a problem hiding this comment.
The impl looks good, just notes on the diagnostic pretty much.
|
@dyc3 Could you review again when you have a chance? Thanks. |
0f2d7c9 to
b3b070c
Compare
dyc3
left a comment
There was a problem hiding this comment.
Sorry for the wait! Looks good.
|
@dyc3 Thank you for reviewing again! I have addressed nit comments. |
Summary
vue/no-ref-as-operandfromeslint-plugin-vue#6307This PR ports
noVueRefAsOperandfrom eslint-plugin-vue. It disallows the use of value wrapped byref()as an operand because ref value must be accessed with.value.Test Plan
I added tests based on the original: https://github.com/vuejs/eslint-plugin-vue/blob/4b92896b8da0720052547942abcdee200acad4ed/tests/lib/rules/no-ref-as-operand.js
Docs