Fixed invalid document#8437#8729
Conversation
|
github-actions
bot
added
A-Linter
WalkthroughThis PR encompasses improvements across HTML and JavaScript components. The HTML formatter refactors multiline element list formatting to differentiate between root and non-root nodes, adjusting how Fill and NoFill layouts are emitted. A new Angular HTML template test case with control-flow bindings is introduced. The JavaScript linter adds a guard to restrict JSX-key-in-iterable checks to JSX contexts only, and accompanying test files are updated to move non-JSX cases to valid test scenarios. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rs:
- Around line 130-135: The check using node.parent::<JsxExpressionChild>() only
inspects the immediate parent and misses cases like parenthesised arrays (e.g.,
JsParenthesizedExpression) inside JSX; change the logic that sets is_inside_jsx
to walk ancestors instead (use node.ancestors() and detect any
JsxExpressionChild ancestor) so arrays wrapped in parentheses are correctly
recognized as being inside JSX; update the code around the is_inside_jsx
computation in use_jsx_key_in_iterable.rs to use an ancestors() search for
JsxExpressionChild rather than parent::<JsxExpressionChild>().
🧹 Nitpick comments (1)
crates/biome_html_formatter/src/html/lists/element_list.rs (1)
879-970: Non-root Fill fix is solid; a small simplification would reduce moving parts.The “Fill tags outside,
block_indentinside” approach is exactly what the bug description calls for. The only nit: theelements_formatclosure is a bit ceremony-heavy—if there’s a helper in the formatter to indent/write an interned element directly, using it would make this harder to break later.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
crates/biome_html_formatter/src/html/lists/element_list.rscrates/biome_html_formatter/tests/specs/prettier/html/angular-control-flow.htmlcrates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rscrates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/invalid.jsxcrates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/valid.jsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use inline rustdoc documentation for rules, assists, and their options
Use thedbg!()macro for debugging output in Rust tests and code
Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests
Files:
crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rscrates/biome_html_formatter/src/html/lists/element_list.rs
🧠 Learnings (16)
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use helper functions like `map`, `filter`, and `and_then` to avoid excessive nested `if let` statements
Applied to files:
crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rs
📚 Learning: 2025-12-22T09:26:56.943Z
Learnt from: ematipico
Repo: biomejs/biome PR: 8537
File: crates/biome_js_analyze/src/lint/nursery/no_leaked_render.rs:167-210
Timestamp: 2025-12-22T09:26:56.943Z
Learning: When defining lint rules (declare_lint_rule!), only specify fix_kind if the rule implements an action(...) function. Rules that only emit diagnostics without a code fix should omit fix_kind. This applies to all Rust lint rule definitions under crates/.../src/lint (e.g., crates/biome_js_analyze/src/lint/...).
Applied to files:
crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Prefer using `Box<[_]>` over `Vec<_>` for signal collections to reduce memory usage
Applied to files:
crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use `declare_node_union!` macro to query multiple node types together to avoid redundant traversal passes
Applied to files:
crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rscrates/biome_html_formatter/src/html/lists/element_list.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use `.ok()?` to transform `Result` types into `Option` or `let else` pattern to handle errors when the function returns `Vec`
Applied to files:
crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Store type data in linear vectors instead of using recursive data structures with `Arc` for improved data locality and performance
Applied to files:
crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/* : Create test files with `invalid` and `valid` prefixes to represent code that should and should not trigger the rule
Applied to files:
crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/invalid.jsx
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/src/lib.rs : Expose a public `format_node` function that accepts formatting options and a root syntax node, returning a `FormatResult<Formatted<Context>>` with appropriate documentation
Applied to files:
crates/biome_html_formatter/src/html/lists/element_list.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the `FormatNode` trait and implement it for your Node when creating formatters in biome_js_formatter
Applied to files:
crates/biome_html_formatter/src/html/lists/element_list.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `ParseSeparatedList` and `ParseNodeList` for parsing lists with error recovery to avoid infinite loops
Applied to files:
crates/biome_html_formatter/src/html/lists/element_list.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/src/context.rs : Define `<Language>FormatContext` struct in a `context.rs` file containing `comments` and `source_map` fields, implementing `FormatContext` and `CstFormatContext` traits
Applied to files:
crates/biome_html_formatter/src/html/lists/element_list.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/*.ungram : Nodes that represent a list must end with the postfix `List`, e.g., `HtmlAttributeList`, and lists are mandatory (not optional) but empty by default
Applied to files:
crates/biome_html_formatter/src/html/lists/element_list.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : When formatting AST nodes, use mandatory tokens from the AST instead of hardcoding token strings (e.g., use `node.l_paren_token().format()` instead of `token("(")`)
Applied to files:
crates/biome_html_formatter/src/html/lists/element_list.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: The formatter foundation relies on using the generic `Format` trait and `FormatNode` for nodes, with creation of an intermediate IR via a series of helpers
Applied to files:
crates/biome_html_formatter/src/html/lists/element_list.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Define `FormatHtmlSyntaxNode` struct in a `cst.rs` file implementing `FormatRule<HtmlSyntaxNode>`, `AsFormat<HtmlFormatContext>`, and `IntoFormat<HtmlFormatContext>` traits using the provided boilerplate code
Applied to files:
crates/biome_html_formatter/src/html/lists/element_list.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Implement the `FormatNodeRule<N>` trait with `fmt_fields` as the only required method; default implementations of `fmt`, `is_suppressed`, `fmt_leading_comments`, `fmt_dangling_comments`, and `fmt_trailing_comments` are provided
Applied to files:
crates/biome_html_formatter/src/html/lists/element_list.rs
🧬 Code graph analysis (1)
crates/biome_html_formatter/src/html/lists/element_list.rs (1)
crates/biome_formatter/src/builders.rs (3)
format_once(2381-2389)format_with(2304-2312)block_indent(1189-1194)
🔇 Additional comments (4)
crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/valid.jsx (1)
111-129: Test additions align with the new “only warn in JSX-render context” behaviour.These cases clearly lock in the intended regression fix (no diagnostics for top-level JSX arrays). Nice.
crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/invalid.jsx (1)
19-21: Spec note is clear and helps future readers.crates/biome_html_formatter/tests/specs/prettier/html/angular-control-flow.html (1)
1-48: Great regression target; please confirm the fixture is intentionally valid (and minimal enough).This is exactly the sort of gnarly input that flushes out tag-stack issues—just make sure the braces/control-flow blocks are balanced and that this file actually triggered the original formatter panic before the fix (so we don’t end up testing “random template noise” forever).
crates/biome_html_formatter/src/html/lists/element_list.rs (1)
845-877: Root special-casing may still risk Fill/Indent tag mismatch without validation.Root
Fillbypassesblock_indentwrapping that non-root uses explicitly to prevent nested Indent tag mismatches. Whilst root-level elements shouldn't require indentation, there's no code documentation confirming this invariant, and no regression test for root Fill with nested indent-producing content. If interned root elements can emit Indent tags at all, you'd reintroduce the original mismatch.Either add a targeted test for root multiline Fill with nested indentation, or document why root elements provably cannot generate nested Indent tags.
| let is_inside_jsx = node.parent::<JsxExpressionChild>().is_some(); | ||
| // Only check for keys when the array is inside JSX context (used for rendering) | ||
| // Arrays outside JSX context (e.g., const arr = [<Hello />]) don't need keys | ||
| if !is_inside_jsx { | ||
| return vec![]; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the biome_rowan crate and understand how parent::<T>() works
fd -t f "rowan" --type d | head -5Repository: biomejs/biome
Length of output: 76
🏁 Script executed:
# Search for parent method definition in the codebase
rg -t rs "fn parent" --max-count 20 | grep -E "(pub|fn)\s+parent"Repository: biomejs/biome
Length of output: 83
🏁 Script executed:
# Look at the test cases in the file to see what's covered
cat -n crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rs | head -200Repository: biomejs/biome
Length of output: 7995
🏁 Script executed:
# Find the AstNode trait definition in biome_rowan
fd -path "*/biome_rowan/*" -type f -name "*.rs" | xargs rg -l "fn parent"Repository: biomejs/biome
Length of output: 1115
🏁 Script executed:
# Search for parent method signature specifically
rg "fn parent" crates/biome_rowan/ -A 2Repository: biomejs/biome
Length of output: 2064
🏁 Script executed:
# Check if there are tests in the use_jsx_key_in_iterable file that cover wrapped arrays
tail -n +200 crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rs | head -300Repository: biomejs/biome
Length of output: 10131
🏁 Script executed:
# Look for test cases in the file
tail -n +500 crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rs | head -500Repository: biomejs/biome
Length of output: 974
🏁 Script executed:
# Check the AstNode trait definition in biome_rowan/src/ast/mod.rs
sed -n '1,50p' crates/biome_rowan/src/ast/mod.rsRepository: biomejs/biome
Length of output: 1842
🏁 Script executed:
# Search for the unwrap_parenthesis function to understand how parentheses are handled elsewhere
rg "fn unwrap_parenthesis" crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rs -A 5Repository: biomejs/biome
Length of output: 340
🏁 Script executed:
# Search for test cases at the end of the file
tail -n +850 crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rsRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Look at the AstNode trait's parent method more carefully - check if there's an ancestors method
rg "fn ancestors" crates/biome_rowan/src/ast/mod.rs -A 3Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Check if SyntaxNode has an ancestors method
rg "fn ancestors" crates/biome_rowan/src/syntax/node.rs -A 3Repository: biomejs/biome
Length of output: 177
🏁 Script executed:
# Get test cases at the end of the file
wc -l crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rsRepository: biomejs/biome
Length of output: 132
🏁 Script executed:
# Get the test section
tail -n 200 crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rsRepository: biomejs/biome
Length of output: 6216
🏁 Script executed:
# Get the full test section at the end of the file
tail -n +330 crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rsRepository: biomejs/biome
Length of output: 6156
🏁 Script executed:
# Check line 11 to confirm JsxExpressionChild is imported correctly
sed -n '1,15p' crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rsRepository: biomejs/biome
Length of output: 873
🏁 Script executed:
# Find test cases for this rule
rg "use_jsx_key_in_iterable" --type f --include="*.rs" | grep -i testRepository: biomejs/biome
Length of output: 498
🏁 Script executed:
# Look at the test file
fd "use_jsx_key_in_iterable" -path "*test*" -o -path "*tests*" | head -5Repository: biomejs/biome
Length of output: 225
🏁 Script executed:
# Search for test cases in the codebase
rg "use_jsx_key_in_iterable" | grep -i "test\|check"Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Look for test directories related to lints
find crates/biome_js_analyze -type d -name "*test*" -o -name "*tests*"Repository: biomejs/biome
Length of output: 351
🏁 Script executed:
# Check if tests are in the same file or a separate directory
ls -la crates/biome_js_analyze/src/lint/correctness/Repository: biomejs/biome
Length of output: 4396
🏁 Script executed:
# Search for test specifications or snapshots
find crates/biome_js_analyze/tests -type f -name "*.json" | xargs grep -l "use_jsx_key_in_iterable" 2>/dev/null | head -5Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Look for how rules are tested
ls -la crates/biome_js_analyze/tests/Repository: biomejs/biome
Length of output: 546
🏁 Script executed:
# Check the spec_tests.rs file
head -50 crates/biome_js_analyze/tests/spec_tests.rsRepository: biomejs/biome
Length of output: 2132
🏁 Script executed:
# Find test specifications for use_jsx_key_in_iterable
find crates/biome_js_analyze/tests/specs -type f | xargs grep -l "use_jsx_key_in_iterable" 2>/dev/nullRepository: biomejs/biome
Length of output: 41
🏁 Script executed:
# Look for jsx-key or similar test files
ls -la crates/biome_js_analyze/tests/specs/*/Repository: biomejs/biome
Length of output: 26782
🏁 Script executed:
# Search for test files related to JSX keys
find crates/biome_js_analyze/tests/specs -type f -name "*jsx*key*" -o -name "*key*"Repository: biomejs/biome
Length of output: 231
🏁 Script executed:
# List test files for useJsxKeyInIterable
ls -la crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/Repository: biomejs/biome
Length of output: 660
🏁 Script executed:
# Check one of the test files to understand the format
head -100 crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/invalid.jsxRepository: biomejs/biome
Length of output: 2006
🏁 Script executed:
# Check the valid.jsx test file
cat -n crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/valid.jsxRepository: biomejs/biome
Length of output: 5522
🏁 Script executed:
# Now let me check the AstNode implementation more carefully - search for how parent::<T>() is defined
cat -n crates/biome_rowan/src/ast/mod.rs | sed -n '1,100p'Repository: biomejs/biome
Length of output: 4311
🏁 Script executed:
# Search for the actual parent::<T> method implementation in the AstNode trait
rg "fn parent" crates/biome_rowan/src/ast/mod.rs -A 5Repository: biomejs/biome
Length of output: 366
🏁 Script executed:
# Search for uses of .ancestors() in the js_analyze crate to see if there's a pattern for this
rg "\.ancestors\(" crates/biome_js_analyze/src/lint/ | head -10Repository: biomejs/biome
Length of output: 1133
🏁 Script executed:
# Check if there's a test for parenthesized array in JSX expression
rg "\{\(\[" crates/biome_js_analyze/tests/specs/correctness/useJsxKeyInIterable/Repository: biomejs/biome
Length of output: 39
Parenthesised arrays inside JSX won't be checked for keys.
node.parent::<JsxExpressionChild>() checks immediate parent only. For {([<Hello />])}, the array's parent is JsParenthesizedExpression, not JsxExpressionChild, so the rule skips the check. Use .ancestors() instead to walk up the tree. This pattern is already used elsewhere in the analyser for similar cases.
Suggested fix
- let is_inside_jsx = node.parent::<JsxExpressionChild>().is_some();
+ let is_inside_jsx = node
+ .syntax()
+ .ancestors()
+ .any(|ancestor| JsxExpressionChild::can_cast(ancestor.kind()));📝 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.
| let is_inside_jsx = node.parent::<JsxExpressionChild>().is_some(); | |
| // Only check for keys when the array is inside JSX context (used for rendering) | |
| // Arrays outside JSX context (e.g., const arr = [<Hello />]) don't need keys | |
| if !is_inside_jsx { | |
| return vec![]; | |
| } | |
| let is_inside_jsx = node | |
| .syntax() | |
| .ancestors() | |
| .any(|ancestor| JsxExpressionChild::can_cast(ancestor.kind())); | |
| // Only check for keys when the array is inside JSX context (used for rendering) | |
| // Arrays outside JSX context (e.g., const arr = [<Hello />]) don't need keys | |
| if !is_inside_jsx { | |
| return vec![]; | |
| } |
🤖 Prompt for AI Agents
In @crates/biome_js_analyze/src/lint/correctness/use_jsx_key_in_iterable.rs
around lines 130 - 135, The check using node.parent::<JsxExpressionChild>() only
inspects the immediate parent and misses cases like parenthesised arrays (e.g.,
JsParenthesizedExpression) inside JSX; change the logic that sets is_inside_jsx
to walk ancestors instead (use node.ancestors() and detect any
JsxExpressionChild ancestor) so arrays wrapped in parentheses are correctly
recognized as being inside JSX; update the code around the is_inside_jsx
computation in use_jsx_key_in_iterable.rs to use an ancestors() search for
JsxExpressionChild rather than parent::<JsxExpressionChild>().
|
We don't support angular right now. Plus, this is pure AI slop. |
2 participants
Fix HTML Formatter Fill/Indent Tag Mismatch Bug
Problem
When formatting HTML files containing Angular control flow syntax (
@if,@switch,@case), Biome's HTML formatter throws an error: "Expected end tag of kind Fill but found Indent". This indicates a mismatch in the formatter's internal document structure where aStartFilltag is opened but anEndIndenttag is encountered before the matchingEndFill.Root Cause Analysis
The issue is in
crates/biome_html_formatter/src/html/lists/element_list.rs, specifically in theFormatMultilineChildren::fmtmethod (lines 843-900). When usingMultilineLayout::Fill, the formatter writes:StartFilltagelements(which may contain nested Indent tags)EndFilltagThe problem occurs when the interned
elementscontain Indent tags that get closed before the Fill tag is closed, causing a tag stack mismatch.