feat(css): add support for the typed attr CSS function
#8255
Conversation
🦋 Changeset detectedLatest commit: e4bb941 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 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
ryan-m-walker
changed the title
attr CSS functionattr CSS function
WalkthroughAdds typed CSS attr() support across the codebase: new grammar nodes and lexer keywords for Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
CodSpeed Performance ReportMerging #8255 will not alter performanceComparing Summary
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
.changeset/proud-llamas-jog.md (1)
5-5: Consider a more user-facing description.Whilst the description is technically accurate, consider rephrasing to better communicate the user benefit. For example: "Added parser and formatter support for the typed CSS
attr()function as specified in CSS Values Level 5, allowing type constraints and fallback values."As per coding guidelines, changeset descriptions should be user-facing and use past tense for actions taken.
crates/biome_css_formatter/src/css/lists/attr_name_list.rs (1)
1-1: Unused import.
std::fmt::Debugappears to be unused — derive macros don't require explicit imports.-use std::fmt::Debug; -crates/biome_css_parser/tests/css_test_suite/error/function/attr.css (1)
1-9: Negative attr() coverage looks solidNice range of invalid
attr()forms here (missing attribute name, unbalanced parentheses, brokentype(<...>)syntax), which should exercise the new error paths well. If you ever feel like spoiling future archaeologists, a brief comment noting which line targets which failure mode could help, but it’s strictly optional.crates/biome_css_parser/src/syntax/value/function.rs (1)
6-7: Attr() integration into function parsing looks soundImporting
is_at_attr_function/parse_attr_functionand threading them throughis_at_any_functionandparse_any_functionkeeps the control flow consistent withurl()/if(). Checkingattrbefore the genericparse_functionbranch avoids misclassifying it as a simple function. The only tiny polish you might consider is updating theis_at_any_functiondoc comment to mentionattr()explicitly.Also applies to: 25-27, 40-46
xtask/codegen/css.ungram (1)
2078-2082: Minor formatting inconsistency.Line 2080 is missing a space after the
|character. All other variant options in the file use|for consistency.Apply this diff to fix the formatting:
AnyCssAttrType = CssRawStringDeclarator - |CssTypeFunction + | CssTypeFunction | CssNumberDeclarator | AnyCssAttrUnit
ryan-m-walker
changed the title
attr CSS functionattr CSS function
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
xtask/codegen/css.ungram (1)
2069-2071: Minor whitespace inconsistency.Extra leading space before
CssRegularAttrUnit.AnyCssAttrUnit = - CssRegularAttrUnit + CssRegularAttrUnit | CssUnknownAttrUnit
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
xtask/codegen/css.ungram(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.784Z
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("(")`)
📚 Learning: 2025-11-24T18:06:03.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.536Z
Learning: Applies to crates/biome_parser/**/*.ungram : Nodes for enclosing syntax errors must have the `Bogus` word, e.g., `HtmlBogusAttribute`, and must be part of a variant
Applied to files:
xtask/codegen/css.ungram
📚 Learning: 2025-11-24T18:06:03.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.536Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `ConditionalParsedSyntax` for syntax that is only valid in specific contexts (e.g., strict mode, file types, language versions) and call `or_invalid_to_bogus()` to convert to a bogus node if not supported
Applied to files:
xtask/codegen/css.ungram
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: End-to-end tests
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Documentation
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Validate rules documentation
- GitHub Check: autofix
- GitHub Check: Parser conformance
🔇 Additional comments (4)
xtask/codegen/css.ungram (4)
66-67: LGTM!Bogus nodes correctly follow the naming convention and provide error recovery for the new syntax types.
1803-1807: LGTM!Clean addition to the function union.
2048-2092: Well-structured attr() grammar.The spec comments are helpful, and the grammar correctly models the CSS Values 5
attr()function. Including the comma withinCssAttrFallbackValueis the right approach for optional fallback handling.
2094-2153: Solid syntax grammar implementation.The handling of
<syntax>follows the spec well:
CssUniversalSyntaxfor*CssSyntaxComponentListfor pipe-separated componentsCssSyntaxComponentWithoutMultiplierfor the<transform-list>special caseCssRegularSyntaxTypeName/CssUnknownSyntaxTypeNamepattern mirrors the dimension unit approach, preventing over-aggressive identifier parsing as noted in the PR description.
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
Wow, this is a fantastic feature! Well done! Overall, it looks good; however, there are things we should improve and address.
I spotted some problems regarding the recoverability of the parser, and we should add more tests, and possibly add more errors.
Also, the grammar file lacks the mandatory comments.
Another important thing: considering the size of the PR, shouldn't we land this in minor?
| let m = p.start(); | ||
|
|
||
| p.bump(T![<]); | ||
| parse_any_syntax_type_name(p).ok(); |
| a: attr(data-count; | ||
| a: attr(data-count type(<string>); | ||
| a: attr(data-count type(<string)); | ||
| a: attr(data-count type(string>)); |
There was a problem hiding this comment.
A test with missing string should be added
div {
a: attr(data-count type(<>));
}| 2: CSS_BOGUS@10..50 | ||
| 0: CSS_BOGUS_SUPPORTS_CONDITION@10..50 | ||
| 0: ATTR_KW@10..14 "attr" [] [] | ||
| 1: L_PAREN@14..15 "(" [] [] | ||
| 2: CSS_ATTR_NAME_LIST@15..26 | ||
| 0: CSS_IDENTIFIER@15..26 | ||
| 0: IDENT@15..26 "data-width" [] [Whitespace(" ")] | ||
| 3: CSS_BOGUS@26..49 | ||
| 0: TYPE_KW@26..30 "type" [] [] | ||
| 1: L_PAREN@30..31 "(" [] [] | ||
| 2: CSS_BOGUS@31..48 | ||
| 0: CSS_SYNTAX_COMPONENT_WITHOUT_MULTIPLIER@31..47 | ||
| 0: L_ANGLE@31..32 "<" [] [] | ||
| 1: IDENT@32..46 "transform-list" [] [] | ||
| 2: R_ANGLE@46..47 ">" [] [] | ||
| 1: CSS_BOGUS_SYNTAX_SINGLE_COMPONENT@47..48 |
There was a problem hiding this comment.
Not sure if you noticed, but here we have roughly five bogus nodes. This is usually a bug, it means that our parser isn't able to correctly recover from the first bogus node. At least, it doesn't recover how we want to. Also, having CSS_BOUGS -> <Named Bogus Node> is usually a bug. We usually want to have one NAME bogus node
I think the issue is because you didn't update this function. Added new bogus nodes, but we forgot the mapping. See if updating this function solves the problem
biome/crates/biome_css_syntax/src/lib.rs
Lines 107 to 136 in 98ca2ae
There was a problem hiding this comment.
thanks for clarifying this, I did miss this file
| @@ -0,0 +1,9 @@ | |||
| div { | |||
| a: attr(data-width type(<transform-list>+)); | |||
There was a problem hiding this comment.
We should add more tests.
For example:
- We added two new list nodes, but we didn't test if they recover at the right tokens
- We don't create a diagnostic if mandatory nodes are missing
There was a problem hiding this comment.
added quite a few more tests, let me know if you can think of any others i might have missed
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_css_parser/src/syntax/value/type.rs (1)
192-211: Consider movingis_at_*predicates to end of file.Per the codebase convention noted in past reviews,
is_at_*functions are typically placed at the end of the file. The predicatesis_at_syntax_multiplier(line 193),is_at_syntax_type(line 209),is_at_any_syntax_type_name(line 233), andis_at_valid_syntax_type_name(line 301) could be grouped together at the bottom.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
crates/biome_css_factory/src/generated/node_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_factory/src/generated/syntax_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_parser/tests/css_test_suite/error/function/attr.css.snapis excluded by!**/*.snapand included by**crates/biome_css_syntax/src/generated/kind.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_syntax/src/generated/macros.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_syntax/src/generated/nodes.rsis excluded by!**/generated/**,!**/generated/**and included by**
📒 Files selected for processing (14)
.changeset/proud-llamas-jog.md(1 hunks)crates/biome_css_formatter/src/css/any/attr_name.rs(1 hunks)crates/biome_css_formatter/src/css/any/mod.rs(2 hunks)crates/biome_css_formatter/src/css/bogus/bogus_attr_name.rs(1 hunks)crates/biome_css_formatter/src/css/bogus/mod.rs(2 hunks)crates/biome_css_formatter/src/css/lists/attr_name_list.rs(1 hunks)crates/biome_css_formatter/src/generated.rs(13 hunks)crates/biome_css_parser/src/syntax/value/attr.rs(1 hunks)crates/biome_css_parser/src/syntax/value/parse_error.rs(2 hunks)crates/biome_css_parser/src/syntax/value/type.rs(1 hunks)crates/biome_css_parser/tests/css_test_suite/error/function/attr.css(1 hunks)crates/biome_css_syntax/src/lib.rs(1 hunks)xtask/codegen/css.ungram(3 hunks)xtask/codegen/src/css_kinds_src.rs(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/biome_css_formatter/src/css/bogus/bogus_attr_name.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- crates/biome_css_formatter/src/css/any/mod.rs
- xtask/codegen/css.ungram
- crates/biome_css_parser/src/syntax/value/attr.rs
- xtask/codegen/src/css_kinds_src.rs
- .changeset/proud-llamas-jog.md
- crates/biome_css_formatter/src/css/lists/attr_name_list.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use the Rustdbg!()macro for debugging output during test execution, and pass the--show-outputflag tocargo testto display debug output.
Use snapshot testing with theinstacrate for testing in Rust projects. Accept or reject snapshots usingcargo insta accept,cargo insta reject, orcargo insta review.
Write doc comments as doc tests in Rust using code blocks with assertions that will be executed during the testing phase.
Use rustdoc inline documentation for rules, assists, and their options. Create corresponding documentation PRs for other documentation updates against thenextbranch of the website repository.
Set theversionmetadata field in linter rule implementations to'next'for newly created rules. Update this field to the new version number when releasing a minor or major version.
Files:
crates/biome_css_syntax/src/lib.rscrates/biome_css_formatter/src/css/bogus/mod.rscrates/biome_css_formatter/src/css/any/attr_name.rscrates/biome_css_parser/src/syntax/value/type.rscrates/biome_css_parser/src/syntax/value/parse_error.rscrates/biome_css_formatter/src/generated.rs
🧠 Learnings (53)
📓 Common learnings
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.784Z
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("(")`)
📚 Learning: 2025-11-24T18:06:03.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.536Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `ConditionalParsedSyntax` for syntax that is only valid in specific contexts (e.g., strict mode, file types, language versions) and call `or_invalid_to_bogus()` to convert to a bogus node if not supported
Applied to files:
crates/biome_css_syntax/src/lib.rscrates/biome_css_formatter/src/css/bogus/mod.rscrates/biome_css_parser/src/syntax/value/type.rscrates/biome_css_parser/src/syntax/value/parse_error.rscrates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:06:03.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.536Z
Learning: Applies to crates/biome_parser/**/*.ungram : Nodes for enclosing syntax errors must have the `Bogus` word, e.g., `HtmlBogusAttribute`, and must be part of a variant
Applied to files:
crates/biome_css_syntax/src/lib.rscrates/biome_css_formatter/src/css/bogus/mod.rscrates/biome_css_parser/src/syntax/value/type.rscrates/biome_css_parser/src/syntax/value/parse_error.rscrates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_css_analyze/lib/src/**/!(mod).rs : CSS rules that report non-standardized or unknown concepts should use the 'noUnknown<Concept>' naming convention (e.g., `noUnknownUnit`)
Applied to files:
crates/biome_css_syntax/src/lib.rscrates/biome_css_formatter/src/css/bogus/mod.rscrates/biome_css_formatter/src/css/any/attr_name.rscrates/biome_css_parser/src/syntax/value/parse_error.rscrates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:06:03.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.536Z
Learning: Applies to crates/biome_parser/**/language_kind.rs : Add a new variant to `LanguageKind` enum in `language_kind.rs` file and implement all methods for the new language variant
Applied to files:
crates/biome_css_syntax/src/lib.rs
📚 Learning: 2025-11-24T18:05:42.338Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.338Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Distinguish between `TypeData::Unknown` and `TypeData::UnknownKeyword` to measure inference effectiveness versus explicit user-provided unknown types
Applied to files:
crates/biome_css_syntax/src/lib.rscrates/biome_css_parser/src/syntax/value/type.rs
📚 Learning: 2025-11-24T18:06:03.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.536Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Implement error recovery in list parsing using `or_recover()` to wrap unparseable tokens in a `BOGUS_*` node and consume tokens until a recovery token is found
Applied to files:
crates/biome_css_syntax/src/lib.rscrates/biome_css_parser/src/syntax/value/type.rscrates/biome_css_parser/src/syntax/value/parse_error.rs
📚 Learning: 2025-11-24T18:05:42.338Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.338Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeData::Unknown` to indicate when type inference falls short or is not implemented
Applied to files:
crates/biome_css_syntax/src/lib.rscrates/biome_css_parser/src/syntax/value/type.rs
📚 Learning: 2025-11-24T18:06:03.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.536Z
Learning: Applies to crates/biome_parser/**/language_kind.rs : Add a new language prefix to the `LANGUAGE_PREFIXES` constant in `language_kind.rs` file
Applied to files:
crates/biome_css_syntax/src/lib.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Rules that ban functions or variables should use the semantic model to check if the variable is global before reporting, to avoid false positives on locally redeclared variables
Applied to files:
crates/biome_css_formatter/src/css/bogus/mod.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Code blocks in documentation can use 'ignore' property to exclude snippets from automatic validation (use sparingly)
Applied to files:
crates/biome_css_formatter/src/css/bogus/mod.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Rules that report redundant code should use the 'noRedundant<Concept>' naming convention (e.g., `noRedundantUseStrict`)
Applied to files:
crates/biome_css_formatter/src/css/bogus/mod.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Rules that overwhelmingly apply to a specific framework should be named using 'use<Framework>...' or 'no<Framework>...' prefix (e.g., `noVueReservedProps`)
Applied to files:
crates/biome_css_formatter/src/css/bogus/mod.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Rules that report valid but misleading code should use the 'noMisleading<Concept>' naming convention (e.g., `noMisleadingCharacterClass`)
Applied to files:
crates/biome_css_formatter/src/css/bogus/mod.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Rule documentation must include a '## Examples' header followed by '### Invalid' and '### Valid' sections, with '### Invalid' appearing first
Applied to files:
crates/biome_css_formatter/src/css/bogus/mod.rs
📚 Learning: 2025-11-24T18:05:42.337Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.337Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : No module may copy or clone data from another module in the module graph, not even behind an `Arc`
Applied to files:
crates/biome_css_formatter/src/css/bogus/mod.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Code blocks in rule documentation must specify a language and use 'expect_diagnostic' property for invalid snippets that should emit exactly one diagnostic
Applied to files:
crates/biome_css_formatter/src/css/bogus/mod.rscrates/biome_css_parser/src/syntax/value/parse_error.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Rules that report empty code should use the 'noEmpty<Concept>' naming convention (e.g., `noEmptyBlockStatements`)
Applied to files:
crates/biome_css_formatter/src/css/bogus/mod.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Rules that report runtime errors from mistyping should use 'noInvalid<Concept>' or 'useValid<Concept>' naming conventions (e.g., `noInvalidConstructorSuper`, `useValidTypeof`)
Applied to files:
crates/biome_css_formatter/src/css/bogus/mod.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Rules that report undefined entities should use the 'noUndeclared<Concept>' naming convention (e.g., `noUndeclaredVariables`)
Applied to files:
crates/biome_css_formatter/src/css/bogus/mod.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/lib/**/*.rs : The rule option serde configuration must include 'deny_unknown_fields' to raise errors for extraneous fields in configuration
Applied to files:
crates/biome_css_formatter/src/css/bogus/mod.rs
📚 Learning: 2025-11-24T18:06:03.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.536Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Parse rules must return `ParsedSyntax::Absent` if the rule can't predict by the next token(s) if they form the expected node, and must not progress the parser in this case
Applied to files:
crates/biome_css_formatter/src/css/bogus/mod.rscrates/biome_css_parser/src/syntax/value/type.rscrates/biome_css_parser/src/syntax/value/parse_error.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Rule names should follow the 'use<Concept>' prefix when a rule's sole intention is to mandate a single concept (e.g., `useValidLang` for valid HTML lang attribute values)
Applied to files:
crates/biome_css_formatter/src/css/bogus/mod.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/tests/specs/**/*.{js,ts,tsx,jsx,json,css,graphql} : Test files should use 'invalid' or 'valid' prefixes to indicate whether they contain code reported by the rule
Applied to files:
crates/biome_css_parser/tests/css_test_suite/error/function/attr.css
📚 Learning: 2025-11-24T18:05:27.784Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.784Z
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_css_formatter/src/css/any/attr_name.rscrates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:05:20.343Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.343Z
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_css_formatter/src/css/any/attr_name.rscrates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:05:20.343Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.343Z
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_css_formatter/src/css/any/attr_name.rscrates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:05:20.343Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.343Z
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_css_formatter/src/css/any/attr_name.rscrates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:05:20.343Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.343Z
Learning: Implement the `AsFormat<Context>` trait in `lib.rs` with generic implementations for references, `SyntaxResult`, and `Option` types as provided in the formatter boilerplate code
Applied to files:
crates/biome_css_formatter/src/css/any/attr_name.rscrates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:05:20.343Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.343Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/src/lib.rs : Define a type alias `<Language>Formatter<'buf>` as `Formatter<'buf, <Language>FormatContext>` in the main formatter crate
Applied to files:
crates/biome_css_formatter/src/css/any/attr_name.rscrates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:05:20.343Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.343Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/tests/language.rs : Implement `TestFormatLanguage` trait in `tests/language.rs` for the formatter's test language
Applied to files:
crates/biome_css_formatter/src/css/any/attr_name.rs
📚 Learning: 2025-11-24T18:05:20.343Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.343Z
Learning: Implement the `IntoFormat<Context>` trait in `lib.rs` with implementations for `SyntaxResult` and `Option` types as part of the formatter infrastructure
Applied to files:
crates/biome_css_formatter/src/css/any/attr_name.rscrates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:05:27.784Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.784Z
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_css_formatter/src/css/any/attr_name.rscrates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:05:20.343Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.343Z
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_css_formatter/src/css/any/attr_name.rscrates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:05:27.784Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.784Z
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_css_formatter/src/css/any/attr_name.rscrates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:05:42.337Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.337Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` instead of `Arc` for types that reference other types to avoid stale cache issues when modules are replaced
Applied to files:
crates/biome_css_parser/src/syntax/value/type.rs
📚 Learning: 2025-11-24T18:05:42.338Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.338Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` variants (`Qualifier`, `Resolved`, `Import`, `Unknown`) to represent different phases of type resolution
Applied to files:
crates/biome_css_parser/src/syntax/value/type.rs
📚 Learning: 2025-11-24T18:06:03.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.536Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Parse rules must take a mutable reference to the parser as their only parameter and return a `ParsedSyntax`
Applied to files:
crates/biome_css_parser/src/syntax/value/type.rscrates/biome_css_parser/src/syntax/value/parse_error.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Rule implementation should use 'Type Query = Ast<NodeType>' to query the AST/CST for specific node types
Applied to files:
crates/biome_css_parser/src/syntax/value/type.rscrates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:06:03.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.536Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Parse rule functions must be prefixed with `parse_` and use the name defined in the grammar file, e.g., `parse_for_statement` or `parse_expression`
Applied to files:
crates/biome_css_parser/src/syntax/value/type.rscrates/biome_css_parser/src/syntax/value/parse_error.rs
📚 Learning: 2025-11-24T18:06:03.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.536Z
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_css_parser/src/syntax/value/type.rscrates/biome_css_parser/src/syntax/value/parse_error.rs
📚 Learning: 2025-11-24T18:05:42.338Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.338Z
Learning: Applies to crates/biome_js_type_info/**/flattening.rs : Implement type flattening to simplify `TypeofExpression` variants once all component types are resolved
Applied to files:
crates/biome_css_parser/src/syntax/value/type.rs
📚 Learning: 2025-11-09T12:47:46.298Z
Learnt from: ematipico
Repo: biomejs/biome PR: 8031
File: crates/biome_html_parser/src/syntax/svelte.rs:140-147
Timestamp: 2025-11-09T12:47:46.298Z
Learning: In the Biome HTML parser, `expect` and `expect_with_context` consume the current token and then lex the next token. The context parameter in `expect_with_context` controls how the next token (after the consumed one) is lexed, not the current token being consumed. For example, in Svelte parsing, after `bump_with_context(T!["{:"], HtmlLexContext::Svelte)`, the next token is already lexed in the Svelte context, so `expect(T![else])` is sufficient unless the token after `else` also needs to be lexed in a specific context.
Applied to files:
crates/biome_css_parser/src/syntax/value/type.rs
📚 Learning: 2025-11-24T18:06:03.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.536Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `p.eat(token)` for optional tokens, `p.expect(token)` for required tokens, `parse_rule(p).ok(p)` for optional nodes, and `parse_rule(p).or_add_diagnostic(p, error)` for required nodes
Applied to files:
crates/biome_css_parser/src/syntax/value/type.rscrates/biome_css_parser/src/syntax/value/parse_error.rs
📚 Learning: 2025-11-24T18:04:57.290Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.290Z
Learning: Diagnostic should provide a way for the user to fix the issue through log advice, diff advice, or command advice. Add the FIXABLE tag to highlight actionable hints
Applied to files:
crates/biome_css_parser/src/syntax/value/type.rs
📚 Learning: 2025-11-24T18:06:03.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.536Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : A parser struct must implement the `Parser` trait and save the token source, parser context, and optional parser options
Applied to files:
crates/biome_css_parser/src/syntax/value/type.rs
📚 Learning: 2025-11-24T18:06:03.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.536Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Implement a token source struct that wraps the lexer and implements `TokenSourceWithBufferedLexer` and `LexerWithCheckpoint` for lookahead and re-lexing capabilities
Applied to files:
crates/biome_css_parser/src/syntax/value/type.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Code blocks in documentation marked with 'expect_diagnostic' must emit exactly one diagnostic for the build system to generate it automatically
Applied to files:
crates/biome_css_parser/src/syntax/value/parse_error.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Code blocks in documentation marked with 'options' should contain only rule-specific options in JSON/JSONC format, while 'full_options' contains complete biome.json configuration
Applied to files:
crates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Use 'declare_node_union!' macro to query multiple node types at once instead of repeating rule implementations for similar nodes
Applied to files:
crates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : The first paragraph of rule documentation must be a single line and serves as the brief description for the rule overview page
Applied to files:
crates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:05:20.343Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.343Z
Learning: Implement the `FormatLanguage` trait with `SyntaxLanguage`, `Context`, and `FormatRule` associated types for the language's formatter
Applied to files:
crates/biome_css_formatter/src/generated.rs
📚 Learning: 2025-11-24T18:05:20.343Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.343Z
Learning: Implement the `FormattedIterExt` trait and `FormattedIter` struct in `lib.rs` to provide iterator extensions for formatting
Applied to files:
crates/biome_css_formatter/src/generated.rs
🧬 Code graph analysis (3)
crates/biome_css_formatter/src/css/any/attr_name.rs (2)
crates/biome_css_formatter/src/css/lists/attr_name_list.rs (2)
fmt(13-26)fmt(35-50)crates/biome_css_formatter/src/generated.rs (14)
fmt(12-14)fmt(46-52)fmt(84-90)fmt(122-128)fmt(160-166)fmt(198-204)fmt(238-244)fmt(276-282)fmt(314-320)fmt(352-358)fmt(390-396)fmt(428-434)fmt(464-466)fmt(487-493)
crates/biome_css_parser/src/syntax/value/type.rs (2)
crates/biome_css_parser/src/syntax/mod.rs (1)
parse_regular_identifier(433-435)crates/biome_css_parser/src/syntax/value/parse_error.rs (3)
expected_any_syntax(38-40)expected_syntax_component(34-36)expected_syntax_type_name(42-44)
crates/biome_css_parser/src/syntax/value/parse_error.rs (1)
crates/biome_parser/src/diagnostic.rs (3)
expect_one_of(491-494)expected_any(486-488)expected_node(481-483)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Documentation
- GitHub Check: End-to-end tests
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Test Node.js API
- GitHub Check: autofix
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Parser conformance
🔇 Additional comments (13)
crates/biome_css_parser/tests/css_test_suite/error/function/attr.css (1)
1-30: Good error coverage for malformedattr()usages.The test cases cover a solid range of error scenarios including multiplier issues, missing tokens, invalid names, and bracket mismatches. Line 21 addresses the previously requested test for missing type name in
type(<>).One consideration: the past review comments mentioned testing that list nodes recover at the right tokens. You may want to verify the snapshot output confirms proper recovery behaviour for the list parsing edge cases (e.g., lines 9, 23, 25, 27).
crates/biome_css_formatter/src/css/bogus/mod.rs (1)
6-6: LGTM!New bogus module declarations are correctly placed in alphabetical order within the generated file.
Also applies to: 30-31
crates/biome_css_parser/src/syntax/value/parse_error.rs (1)
33-44: LGTM!The new diagnostic helpers follow the established pattern and provide clear, user-facing error messages for the new syntax components.
crates/biome_css_parser/src/syntax/value/type.rs (3)
26-41: LGTM on sorted array with verification test.The
KNOWN_SYNTAX_TYPE_NAMESarray is correctly sorted alphabetically, enabling efficientbinary_searchat line 302. The test at lines 312-315 ensures this ordering is maintained. Good practice.Also applies to: 305-316
268-298: LGTM onSyntaxComponentListimplementation.The
ParseSeparatedListimplementation correctly:
- Uses
|as separator (line 292)- Ends at
)(line 280)- Disallows empty lists (line 296)
- Delegates recovery to
SyntaxTypeListParseRecovery(line 288)Proper error recovery pattern.
145-177: Clever use of checkpoint fortransform-listedge case.The checkpoint/rewind pattern at lines 147-165 handles the special
<transform-list>case that doesn't allow multipliers, then falls back to normal parsing. Clear and correct implementation.crates/biome_css_syntax/src/lib.rs (1)
133-137: LGTM!New
to_bogusmappings correctly route syntax union types to their specific bogus variants, consistent with the existing pattern.crates/biome_css_formatter/src/css/any/attr_name.rs (1)
1-15: LGTM!Generated formatter correctly delegates to the concrete variant formatters for
AnyCssAttrName.crates/biome_css_formatter/src/generated.rs (5)
79-154: Typedattr()wiring looks sound
CssAttrFallbackValue,CssAttrFunction, the regular/unknown attr unit nodes,CssAttrNameList, and theAnyCssAttrName/Type/Unitunions all follow the existingFormatRule/AsFormat/IntoFormatpattern with the expected module paths. Nothing suspicious here.Also applies to: 5011-5048, 6253-6290, 7238-7262, 9282-9355
3686-3723: Declarator formatters fornumber/raw-stringare consistent
CssNumberDeclaratorandCssRawStringDeclaratorare wired up exactly like other auxiliary declarators: correct node type inFormatNodeRule, and matching auxiliary modules for the formatters. Looks good.Also applies to: 4973-5010
5087-5118: New syntax/type formatter glue matches established conventionsThe formatter impls for
CssRegularSyntaxTypeName,CssSyntaxComponent(+WithoutMultiplier),CssSyntaxMultiplier,CssSyntaxType,CssTypeFunction,CssUniversalSyntax,CssUnknownSyntaxTypeName,CssSyntaxComponentList, and theAnyCssSyntax*unions all delegate to the right formatter modules and node types, with bogus vs non‑bogus handled correctly where applicable. This is very boilerplate and very correct.Also applies to: 5790-5954, 6215-6251, 6367-6398, 7959-7983, 11202-11295
5993-6176: Unicode range / namespace / value nodes are wired correctly
CssUnicodeCodepoint, the variousCssUnicodeRange*nodes,CssUniversalNamespacePrefix, andAnyCssUnicodeValueall use the expected auxiliary/any formatter modules with matching node generics. Formatter behaviour should integrate cleanly with the existing unicode handling.Also applies to: 11296-11320
8124-8160: Bogus attr/syntax nodes correctly useFormatBogusNodeRule
CssBogusAttrName,CssBogusSyntax, andCssBogusSyntaxSingleComponentare all hooked up throughFormatBogusNodeRulewith the right bogus formatter modules, consistent with other bogus CSS nodes in this file.Also applies to: 9020-9088
|
@ematipico thanks for taking the time for the thorough review! went back and reexamined the recoverability and think I have a better grasp on the patterns now. Let me know if the updated error handling is more correct or if there are more improvements I could add. I think I addressed the rest of your comments as well, including updating the change to be a minor update. |
There was a problem hiding this comment.
Fantastic! Really, really amazing and well crafted PR! Now, here's a tricky part. Since this is a minor, we usually want to have the PR merged into the next branch, as per the contribution guidelines.
However, I noticed that there's a merge commit in the branch, so if you change the destination branch, you might end up with incorrect commits.
@dyc3 I recall you gave a good advice how to fix this problem easily. What was the right git command?
|
Tbh, I would try the easy route of just |
Co-authored-by: Emanuele Stoppa <[email protected]>
Co-authored-by: Emanuele Stoppa <[email protected]>
- add more error tests - add bogus types to `to_bogus` - use expect instead of bump
ryan-m-walker
force-pushed
the
feat/css-attr-function-parsing
branch
from
5924ecc to
a5e46f9
Compare
github-actions
bot
added
A-CLI
github-actions
bot
removed
A-Project
|
Oh yeah, my bad @ematipico @dyc3 rebased on next and updated the PR to be on that branch |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_css_syntax/src/lib.rs (1)
81-105: New bogus kinds should be flagged as bogus tooYou’ve taught
to_bogusaboutCSS_BOGUS_SYNTAX,CSS_BOGUS_SYNTAX_SINGLE_COMPONENT, andCSS_BOGUS_ATTR_NAME, butis_bogusdoesn’t list them yet. That means these nodes won’t be treated as bogus whereis_bogus()is consulted.Consider extending the
is_bogusmatch to keep things in sync, e.g.:fn is_bogus(&self) -> bool { matches!( self, CSS_BOGUS | CSS_BOGUS_RULE | CSS_BOGUS_SELECTOR | CSS_BOGUS_SUB_SELECTOR | CSS_BOGUS_BLOCK | CSS_BOGUS_PSEUDO_CLASS | CSS_BOGUS_PSEUDO_ELEMENT | CSS_BOGUS_AT_RULE | CSS_BOGUS_MEDIA_QUERY | CSS_BOGUS_KEYFRAMES_ITEM | CSS_BOGUS_PAGE_SELECTOR_PSEUDO | CSS_BOGUS_LAYER | CSS_BOGUS_SCOPE_RANGE | CSS_BOGUS_PROPERTY | CSS_BOGUS_PROPERTY_VALUE | CSS_BOGUS_DOCUMENT_MATCHER | CSS_BOGUS_KEYFRAMES_NAME | CSS_BOGUS_CUSTOM_IDENTIFIER | CSS_BOGUS_UNICODE_RANGE_VALUE | CSS_BOGUS_SUPPORTS_CONDITION + | CSS_BOGUS_SYNTAX + | CSS_BOGUS_SYNTAX_SINGLE_COMPONENT + | CSS_BOGUS_ATTR_NAME ) }Also applies to: 133-139
🧹 Nitpick comments (2)
crates/biome_css_analyze/src/lint/correctness/no_unknown_unit.rs (1)
93-183:x-unit scoping logic is sound, could reuse the tokenThe new handling for
xunits—only allowing them insideimage-setfunctions,image-resolution, or resolution media features—matches how that unit is meant to be used, and the ancestor walk is a reasonable way to detect these contexts.You already have
unit_tokenat hand; you could reuse it for the.ancestors()call instead of callingdimension.unit_token().ok()?again, which would avoid a small bit of duplication:- if unit == "x" { - let mut allow_x = false; - - for ancestor in dimension.unit_token().ok()?.ancestors() { + if unit == "x" { + let mut allow_x = false; + + for ancestor in unit_token.ancestors() {Purely a tidy‑up; behaviour is fine as is.
crates/biome_css_parser/src/syntax/value/attr.rs (1)
143-207: Fallback and attr-name list + recovery are well-shaped; one optional refinementThe fallback branch (
parse_attr_fallback_value) is straightforward and usingGenericComponentValueList.parse_list(p)after the comma matches how declaration values are handled elsewhere. TheAttrNameList+AttrNameListParseRecoverysetup looks good too: you get a dedicatedCSS_ATTR_NAME_LISTnode, recover cleanly at|,,,)and;, and delegate diagnostics viaexpected_identifier.If you ever want to mirror the
<attr-name>shape a bit more closely (namespace part optional, then a single required name), you could consider tighteningAttrNameListso it encodes that structure explicitly (e.g. at most two identifiers, or a dedicated node) rather than an unbounded|‑separated list. Not urgent, but might make downstream consumers’ lives easier.
| a { width: 8ic; } | ||
| a: { --test: attr(data-test rem); } | ||
| a: { --test: attr(data-test %); } | ||
| a: { --test: attr(data-test type(*)); } | ||
| a: { --test: attr(data-test type(length)); } | ||
| a: { --test: attr(data-test type(<color>)); } |
There was a problem hiding this comment.
Fix selector typo in new valid cases
The new attr() examples use a: as the selector:
a: { --test: attr(data-test rem); }etc. That’s invalid CSS and looks like a stray colon. It should almost certainly be a { ... } like the rest of the file so the fixture exercises a normal rule block.
🤖 Prompt for AI Agents
In crates/biome_css_analyze/tests/specs/correctness/noUnknownUnit/valid.css
around lines 72 to 77, the selector in several new test cases mistakenly uses
"a:" (e.g. "a: { --test: attr(data-test rem); }") which is invalid; change each
"a:" to the correct selector "a" so the blocks read "a { ... }" to match the
rest of the file and make the fixtures valid CSS.
3 participants
Summary
Adds support for the modern typed
attrfunction.Addresses #6183
Notes:
no_unknown_unitslint rule to also consider the units used in the type function typesCssRegularSyntaxTypeNameandCssUnknownSyntaxTypeNamerather than adding specific type name keywords to prevent over aggressive parsing on the specific keywords and causing issues for identifiers that use names that are the same as the type names. (For example, I originally added them as keywords but then this started parsingcoloras a keyword and was causing parsing issues). I based this off of the way units are being handled right now. Let me know if this seems like a good approach or not.Test Plan
Snapshot testing added for the parser and formatter
Docs
n/a
AI Usage Disclosure
Used Claude Code every once in a while help debug issues but pretty much all the code committed was written directly by me