feat(parse/html/vue): nested destructuring in v-for#10136
Conversation
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Parser conformance results onjs/262
jsx/babel
markdown/commonmark
symbols/microsoft
ts/babel
ts/microsoft
|
WalkthroughAdds a new Vue v-for AST node Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 (1)
crates/biome_html_parser/src/syntax/vue.rs (1)
321-351:⚠️ Potential issue | 🟠 Major
property: bindingis now accepted in array destructuring too.Line 328 routes every
HTML_LITERALbinding-list element through object-property parsing, and the same list parser is reused for both{...}and[...]. That means invalid array syntax such asv-for="([a:b]) in items"can be parsed as valid.💡 Suggested fix (gate object-property parsing to object bindings only)
-fn parse_vue_v_for_binding_list_element(p: &mut HtmlParser) -> ParsedSyntax { +fn parse_vue_v_for_binding_list_element( + p: &mut HtmlParser, + allow_object_property_binding: bool, +) -> ParsedSyntax { if p.at(T![...]) { parse_vue_v_for_rest_binding(p) } else { match p.cur() { T!['{'] => parse_vue_v_for_object_binding(p), T!['['] => parse_vue_v_for_array_binding(p), - HTML_LITERAL => parse_vue_v_for_object_property_or_identifier_binding(p), + HTML_LITERAL if allow_object_property_binding => { + parse_vue_v_for_object_property_or_identifier_binding(p) + } + HTML_LITERAL => parse_vue_v_for_identifier_binding(p), _ => Absent, } } }struct VueVForBindingList { end: HtmlSyntaxKind, + allow_object_property_binding: bool, } impl VueVForBindingList { - fn new(end: HtmlSyntaxKind) -> Self { - Self { end } + fn new(end: HtmlSyntaxKind, allow_object_property_binding: bool) -> Self { + Self { + end, + allow_object_property_binding, + } } }- VueVForBindingList::new(T!['}']).parse_list(p); + VueVForBindingList::new(T!['}'], true).parse_list(p); ... - VueVForBindingList::new(T![']']).parse_list(p); + VueVForBindingList::new(T![']'], false).parse_list(p); ... - parse_vue_v_for_binding_list_element(p) + parse_vue_v_for_binding_list_element(p, self.allow_object_property_binding)Also applies to: 374-396, 494-496
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_html_parser/src/syntax/vue.rs` around lines 321 - 351, The HTML_LITERAL branch in parse_vue_v_for_binding_list_element incorrectly sends every literal through parse_vue_v_for_object_property_or_identifier_binding (allowing object-property syntax in arrays); change the logic so HTML_LITERAL is routed to parse_vue_v_for_object_property_or_identifier_binding only when we are actually parsing an object binding (i.e., when the current binding list was opened by '{' / object context), otherwise call parse_vue_v_for_identifier_binding directly; update parse_vue_v_for_binding_list_element to detect object vs array context and call the appropriate parser so inputs like v-for="([a:b]) in items" are rejected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@crates/biome_html_parser/src/syntax/vue.rs`:
- Around line 321-351: The HTML_LITERAL branch in
parse_vue_v_for_binding_list_element incorrectly sends every literal through
parse_vue_v_for_object_property_or_identifier_binding (allowing object-property
syntax in arrays); change the logic so HTML_LITERAL is routed to
parse_vue_v_for_object_property_or_identifier_binding only when we are actually
parsing an object binding (i.e., when the current binding list was opened by '{'
/ object context), otherwise call parse_vue_v_for_identifier_binding directly;
update parse_vue_v_for_binding_list_element to detect object vs array context
and call the appropriate parser so inputs like v-for="([a:b]) in items" are
rejected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4e684003-4278-480f-a5e6-becb5324ed14
⛔ Files ignored due to path filters (10)
crates/biome_html_factory/src/generated/node_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_html_factory/src/generated/syntax_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_html_parser/tests/html_specs/error/vue/v-for/invalid-destructuring.vue.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/error/vue/v-for/invalid-nested-destructuring.vue.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/error/vue/v-for/invalid-tuples.vue.snapis excluded by!**/*.snapand included by**crates/biome_html_parser/tests/html_specs/ok/vue/v-for/nested-destructuring.vue.snapis excluded by!**/*.snapand included by**crates/biome_html_syntax/src/generated/kind.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_html_syntax/src/generated/macros.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_html_syntax/src/generated/nodes.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_html_syntax/src/generated/nodes_mut.rsis excluded by!**/generated/**,!**/generated/**and included by**
📒 Files selected for processing (11)
crates/biome_html_formatter/src/generated.rscrates/biome_html_formatter/src/vue/any/v_for_binding_list_element.rscrates/biome_html_formatter/src/vue/auxiliary/mod.rscrates/biome_html_formatter/src/vue/auxiliary/v_for_object_property_binding.rscrates/biome_html_parser/src/lexer/mod.rscrates/biome_html_parser/src/syntax/parse_error.rscrates/biome_html_parser/src/syntax/vue.rscrates/biome_html_parser/tests/html_specs/error/vue/v-for/invalid-nested-destructuring.vuecrates/biome_html_parser/tests/html_specs/ok/vue/v-for/nested-destructuring.vuextask/codegen/html.ungramxtask/codegen/src/html_kinds_src.rs
Merging this PR will not alter performance
Comparing 
Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_html_parser/src/syntax/vue.rs`:
- Around line 328-350: The parser currently treats every HTML_LITERAL as
potentially an object-property binding by always calling
parse_vue_v_for_object_property_or_identifier_binding, which accepts forms like
`a: b` even inside array destructuring; modify
parse_vue_v_for_object_property_or_identifier_binding to take a boolean
parameter (e.g., allow_object_property_binding) and only accept the `identifier
':' binding` branch when that flag is true, otherwise fall back to treating the
token as a simple identifier binding; update callers (notably
VueVForBindingList::parse_element) to pass allow_object_property_binding =
(self.end == T!['}']) so object-property syntax is only allowed when parsing
object-binding lists.
- Around line 422-424: Replace the direct token consumption p.eat(T![')']) with
the context-preserving consumer p.bump_v_for(...) so the VueVForValue context is
maintained; specifically, when handling the tuple binding closing parenthesis in
the tuple binding parsing code, call p.bump_v_for(T![')']) (or the appropriate
p.bump_v_for overload) and keep the existing error path that calls
expected_vue_v_for_tuple_binding_end(p, p.cur_range()) if the token is missing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b331cb54-aed6-40cb-a681-6536441ec47f
📒 Files selected for processing (2)
crates/biome_html_formatter/src/vue/auxiliary/v_for_object_property_binding.rscrates/biome_html_parser/src/syntax/vue.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_html_formatter/src/vue/auxiliary/v_for_object_property_binding.rs
| HTML_LITERAL => parse_vue_v_for_object_property_or_identifier_binding(p), | ||
| _ => Absent, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn parse_vue_v_for_object_property_or_identifier_binding(p: &mut HtmlParser) -> ParsedSyntax { | ||
| if !p.at(HTML_LITERAL) { | ||
| return Absent; | ||
| } | ||
|
|
||
| let Some(property) = parse_vue_v_for_identifier_binding(p).ok() else { | ||
| return Absent; | ||
| }; | ||
|
|
||
| if !p.at(T![:]) { | ||
| return Present(property); | ||
| } | ||
|
|
||
| let m = property.precede(p); | ||
| p.bump_v_for(T![:]); | ||
| parse_vue_v_for_binding(p).or_add_diagnostic(p, expected_vue_v_for_binding); | ||
| Present(m.complete(p, VUE_V_FOR_OBJECT_PROPERTY_BINDING)) |
There was a problem hiding this comment.
Don’t parse a: b as valid inside array destructuring.
Routing every HTML_LITERAL through parse_vue_v_for_object_property_or_identifier_binding means invalid array forms like [a: b] can be accepted without an error. This should only be enabled for object-binding lists.
Suggested direction
-fn parse_vue_v_for_binding_list_element(p: &mut HtmlParser) -> ParsedSyntax {
+fn parse_vue_v_for_binding_list_element(
+ p: &mut HtmlParser,
+ allow_object_property_binding: bool,
+) -> ParsedSyntax {
if p.at(T![...]) {
parse_vue_v_for_rest_binding(p)
} else {
match p.cur() {
T!['{'] => parse_vue_v_for_object_binding(p),
T!['['] => parse_vue_v_for_array_binding(p),
- HTML_LITERAL => parse_vue_v_for_object_property_or_identifier_binding(p),
+ HTML_LITERAL if allow_object_property_binding => {
+ parse_vue_v_for_object_property_or_identifier_binding(p)
+ }
+ HTML_LITERAL => parse_vue_v_for_identifier_binding(p),
_ => Absent,
}
}
}Then call it with allow_object_property_binding = self.end == T!['}'] from VueVForBindingList::parse_element.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_html_parser/src/syntax/vue.rs` around lines 328 - 350, The
parser currently treats every HTML_LITERAL as potentially an object-property
binding by always calling parse_vue_v_for_object_property_or_identifier_binding,
which accepts forms like `a: b` even inside array destructuring; modify
parse_vue_v_for_object_property_or_identifier_binding to take a boolean
parameter (e.g., allow_object_property_binding) and only accept the `identifier
':' binding` branch when that flag is true, otherwise fall back to treating the
token as a simple identifier binding; update callers (notably
VueVForBindingList::parse_element) to pass allow_object_property_binding =
(self.end == T!['}']) so object-property syntax is only allowed when parsing
object-binding lists.
| if !p.eat(T![')']) { | ||
| p.error(expected_vue_v_for_tuple_binding_end(p, p.cur_range())); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the specific file and context around lines 422-424
cat -n crates/biome_html_parser/src/syntax/vue.rs | sed -n '410,435p'Repository: biomejs/biome
Length of output: 972
🏁 Script executed:
# Search for p.eat definition in the parser crate
rg -A 5 'fn eat\(' crates/biome_parser/src/parser.rs | head -30Repository: biomejs/biome
Length of output: 131
🏁 Script executed:
# Search for p.bump_v_for definition
rg -A 5 'fn bump_v_for' crates/biome_html_parser/src/Repository: biomejs/biome
Length of output: 532
🏁 Script executed:
# Search for bump_with_context usage patterns in HTML parser to understand lex context handling
rg 'bump_with_context.*V[ui]e' crates/biome_html_parser/src/syntax/ -A 2 -B 2Repository: biomejs/biome
Length of output: 1753
🏁 Script executed:
# Check if expect_with_context is used in vue.rs for similar context preservation
rg 'expect_with_context' crates/biome_html_parser/src/syntax/vue.rs -B 2 -A 2Repository: biomejs/biome
Length of output: 841
🏁 Script executed:
# Search for the eat function definition in the HTML parser
rg -A 10 'fn eat\(' crates/biome_html_parser/src/parser.rsRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Also check the base parser trait/implementation
rg -B 5 -A 10 'fn eat\(' crates/biome_parser/ --type rs | head -40Repository: biomejs/biome
Length of output: 84
🏁 Script executed:
# Look at the context around the closing paren - what comes after it?
cat -n crates/biome_html_parser/src/syntax/vue.rs | sed -n '400,445p'Repository: biomejs/biome
Length of output: 1647
🏁 Script executed:
# Search for expect_v_for to see if there's a similar context-aware method
rg 'expect_v_for' crates/biome_html_parser/src/Repository: biomejs/biome
Length of output: 409
🏁 Script executed:
# Check what happens after the tuple binding is parsed - what token should be lexed with what context?
rg -B 5 -A 15 'parse_vue_v_for_tuple_binding' crates/biome_html_parser/src/syntax/vue.rs | head -50Repository: biomejs/biome
Length of output: 1422
🏁 Script executed:
# Find the eat method implementation
rg -A 5 'pub fn eat' crates/biome_html_parser/src/parser.rsRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Check how bump_with_context is implemented
rg -B 2 -A 8 'fn bump_with_context' crates/biome_html_parser/src/parser.rsRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Look at the full flow - what happens after parse_vue_v_for_tuple_binding returns?
rg -B 3 -A 15 'fn parse_vue_v_for_value' crates/biome_html_parser/src/syntax/vue.rs | head -60Repository: biomejs/biome
Length of output: 685
🏁 Script executed:
# Search for where parse_vue_v_for_tuple_binding is called to understand the context flow
rg -B 5 -A 10 'parse_vue_v_for_binding\(' crates/biome_html_parser/src/syntax/vue.rs | grep -A 10 'parse_vue_v_for_tuple_binding'Repository: biomejs/biome
Length of output: 425
🏁 Script executed:
# Search in the parser base files
find crates -name "*.rs" -path "*parser*" -type f | head -20Repository: biomejs/biome
Length of output: 1081
🏁 Script executed:
# Look for eat implementation more broadly
rg 'fn eat' crates/biome_html_parser/ -A 3Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Check the parent parser trait or implementation
cat crates/biome_html_parser/src/parser.rs | head -100Repository: biomejs/biome
Length of output: 3770
🏁 Script executed:
# Look at what comes after parse_vue_v_for_tuple_binding in the calling context
rg -B 5 'parse_vue_v_for_binding\(\)' crates/biome_html_parser/src/syntax/vue.rs | grep -A 15 'parse_vue_v_for_value'Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Get the complete parse_vue_v_for_value function
rg -A 50 'pub fn parse_vue_v_for_value' crates/biome_html_parser/src/syntax/vue.rsRepository: biomejs/biome
Length of output: 1728
🏁 Script executed:
# Search for 'eat' in biome_parser crate
find crates/biome_parser -name "*.rs" | xargs grep -l "fn eat" | head -5Repository: biomejs/biome
Length of output: 125
🏁 Script executed:
# Check the parser trait definition
cat crates/biome_parser/src/parser.rs | head -200Repository: biomejs/biome
Length of output: 123
🏁 Script executed:
# Find the parser implementation
cat crates/biome_parser/src/lib.rs | grep -A 5 "eat"Repository: biomejs/biome
Length of output: 3475
🏁 Script executed:
# Look for ParserContext implementation
rg -B 3 -A 8 'fn eat' crates/biome_parser/src/ -t rustRepository: biomejs/biome
Length of output: 3904
🏁 Script executed:
# Check what eat does in relation to expect
rg -B 2 -A 10 'fn expect\(' crates/biome_parser/src/ -t rust | head -40Repository: biomejs/biome
Length of output: 1906
🏁 Script executed:
# Look at the Parser trait itself
rg 'trait Parser' crates/biome_parser/src/ -A 20 -t rust | head -60Repository: biomejs/biome
Length of output: 1489
🏁 Script executed:
# More directly - look at the prelude which HtmlParser likely uses
cat crates/biome_parser/src/prelude.rsRepository: biomejs/biome
Length of output: 448
🏁 Script executed:
# Let me check how bump_with_context is implemented and what do_bump does
rg -B 5 -A 15 'fn do_bump' crates/biome_parser/src/lib.rsRepository: biomejs/biome
Length of output: 1141
🏁 Script executed:
# Check if there's a default do_bump vs do_bump_with_context difference
rg -A 10 'fn do_bump' crates/biome_parser/src/context.rsRepository: biomejs/biome
Length of output: 132
🏁 Script executed:
# Let's see how similar closing delimiters are handled in Vue object/array bindings
rg -B 3 -A 5 'parse_vue_v_for_object_binding\|parse_vue_v_for_array_binding' crates/biome_html_parser/src/syntax/vue.rs | head -100Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Check what happens after tuple binding is parsed - does the operator need VueVForValue context?
cat -n crates/biome_html_parser/src/syntax/vue.rs | sed -n '354,380p'Repository: biomejs/biome
Length of output: 952
🏁 Script executed:
# See the closing patterns for object and array bindings
cat -n crates/biome_html_parser/src/syntax/vue.rs | sed -n '374,442p'Repository: biomejs/biome
Length of output: 2377
🏁 Script executed:
# Also check where parse_vue_v_for_operator is called - what context should be set before 'in'/'of'?
cat -n crates/biome_html_parser/src/syntax/vue.rs | sed -n '444,470p'Repository: biomejs/biome
Length of output: 1140
🏁 Script executed:
# Check if expect_v_for is used elsewhere for similar delimiters
rg 'expect_v_for' crates/biome_html_parser/src/syntax/vue.rs -B 2 -A 2Repository: biomejs/biome
Length of output: 498
🏁 Script executed:
# Verify that expect_v_for would lose the custom error message
rg -A 3 'fn expect_with_context' crates/biome_parser/src/lib.rsRepository: biomejs/biome
Length of output: 191
🏁 Script executed:
# Confirm the custom error is used consistently for similar cases
rg 'expected_vue_v_for.*_end' crates/biome_html_parser/src/syntax/vue.rsRepository: biomejs/biome
Length of output: 200
Use p.bump_v_for() to preserve VueVForValue context when consuming the closing ).
The tuple binding inconsistently uses p.eat(T![')']) instead of p.expect_v_for(T![')']) like object and array bindings do. This causes the next token to be lexed without the intended VueVForValue context.
Consistent context-preserving approach
- if !p.eat(T![')']) {
- p.error(expected_vue_v_for_tuple_binding_end(p, p.cur_range()));
- }
+ if p.at(T![')']) {
+ p.bump_v_for(T![')']);
+ } else {
+ p.error(expected_vue_v_for_tuple_binding_end(p, p.cur_range()));
+ }📝 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.
| if !p.eat(T![')']) { | |
| p.error(expected_vue_v_for_tuple_binding_end(p, p.cur_range())); | |
| } | |
| if p.at(T![')']) { | |
| p.bump_v_for(T![')']); | |
| } else { | |
| p.error(expected_vue_v_for_tuple_binding_end(p, p.cur_range())); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_html_parser/src/syntax/vue.rs` around lines 422 - 424, Replace
the direct token consumption p.eat(T![')']) with the context-preserving consumer
p.bump_v_for(...) so the VueVForValue context is maintained; specifically, when
handling the tuple binding closing parenthesis in the tuple binding parsing
code, call p.bump_v_for(T![')']) (or the appropriate p.bump_v_for overload) and
keep the existing error path that calls expected_vue_v_for_tuple_binding_end(p,
p.cur_range()) if the token is missing.
Summary
v-for parsing now handles nested destructuring
follow up to #8620
Test Plan
Docs