-
-
Notifications
You must be signed in to change notification settings - Fork 762
feat(css): add support for parsing and formatting the CSS if function #8111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 4ac9b9b The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds comprehensive support for CSS Values 5 if-notation: grammar and codegen entries; lexer keywords for Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)crates/biome_css_parser/src/syntax/value/if.rs (5)
⏰ 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)
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/biome_css_formatter/src/css/auxiliary/if_supports_identifier_test.rs (1)
20-20: Consider adding a space after the colon for consistency.The
if_branch.rsformatter explicitly addsspace()aftercolon_token, whilst this formatter does not. CSS convention typically includes a space after the colon in property declarations (e.g.,color: value).Apply this diff if explicit spacing is needed:
- write!(f, [ident.format(), colon_token.format(), value.format()]) + write!(f, [ident.format(), colon_token.format(), space(), value.format()])If tokens already preserve spacing, please verify the formatter output matches the expected CSS style guide conventions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
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_formatter/tests/specs/css/if.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/function/if.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/function/if.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**crates/biome_css_syntax/src/generated/nodes_mut.rsis excluded by!**/generated/**,!**/generated/**and included by**
📒 Files selected for processing (46)
.changeset/seven-words-trade.md(1 hunks)crates/biome_css_formatter/src/css/any/function.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_branch.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_condition.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_media_test_query.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_supports_test_condition.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_test.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_test_boolean_and_combinable_expr.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_test_boolean_expr.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_test_boolean_expr_group.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_test_boolean_or_combinable_expr.rs(1 hunks)crates/biome_css_formatter/src/css/any/mod.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/else_keyword.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_branch.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_function.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_media_test.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_style_test.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_supports_identifier_test.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_supports_test.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_and_expr.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_expr_in_parens.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_not_expr.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_or_expr.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/mod.rs(1 hunks)crates/biome_css_formatter/src/css/bogus/bogus_if_branch.rs(1 hunks)crates/biome_css_formatter/src/css/bogus/bogus_if_test.rs(1 hunks)crates/biome_css_formatter/src/css/bogus/mod.rs(1 hunks)crates/biome_css_formatter/src/css/lists/if_branch_list.rs(1 hunks)crates/biome_css_formatter/src/css/lists/mod.rs(1 hunks)crates/biome_css_formatter/src/generated.rs(5 hunks)crates/biome_css_formatter/tests/specs/css/if.css(1 hunks)crates/biome_css_parser/src/lexer/mod.rs(1 hunks)crates/biome_css_parser/src/syntax/at_rule/container/mod.rs(4 hunks)crates/biome_css_parser/src/syntax/at_rule/media.rs(2 hunks)crates/biome_css_parser/src/syntax/at_rule/mod.rs(1 hunks)crates/biome_css_parser/src/syntax/at_rule/supports/mod.rs(1 hunks)crates/biome_css_parser/src/syntax/property/mod.rs(2 hunks)crates/biome_css_parser/src/syntax/value/function.rs(3 hunks)crates/biome_css_parser/src/syntax/value/if.rs(1 hunks)crates/biome_css_parser/src/syntax/value/mod.rs(1 hunks)crates/biome_css_parser/src/syntax/value/parse_error.rs(1 hunks)crates/biome_css_parser/tests/css_test_suite/error/function/if.css(1 hunks)crates/biome_css_parser/tests/css_test_suite/ok/function/if.css(1 hunks)crates/biome_grit_patterns/src/grit_target_language/css_target_language/generated_mappings.rs(2 hunks)xtask/codegen/css.ungram(3 hunks)xtask/codegen/src/css_kinds_src.rs(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-20T16:24:59.781Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7266
File: crates/biome_js_type_info/src/type.rs:94-102
Timestamp: 2025-08-20T16:24:59.781Z
Learning: In crates/biome_js_type_info/src/type.rs, the flattened_union_variants() method returns TypeReference instances that already have the correct module IDs applied to them. These references should be used directly with resolver.resolve_reference() without applying additional module ID transformations, as variant references may originate from nested unions in different modules.
Applied to files:
crates/biome_css_parser/src/syntax/value/mod.rs
🧬 Code graph analysis (24)
crates/biome_css_formatter/src/css/auxiliary/if_media_test.rs (3)
crates/biome_css_formatter/src/css/auxiliary/if_style_test.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_supports_test.rs (1)
fmt_fields(9-28)crates/biome_formatter/src/builders.rs (1)
soft_block_indent(1260-1265)
crates/biome_css_formatter/src/css/any/if_test.rs (2)
crates/biome_css_formatter/src/generated.rs (16)
fmt(12-14)fmt(46-52)fmt(84-90)fmt(124-130)fmt(162-168)fmt(200-206)fmt(238-244)fmt(276-282)fmt(314-320)fmt(350-352)fmt(373-379)fmt(411-417)fmt(449-455)fmt(481-487)fmt(519-525)fmt(559-565)packages/@biomejs/js-api/src/common.ts (1)
FormatResult(30-39)
crates/biome_css_formatter/src/css/lists/if_branch_list.rs (2)
crates/biome_css_formatter/src/generated.rs (16)
fmt(12-14)fmt(46-52)fmt(84-90)fmt(124-130)fmt(162-168)fmt(200-206)fmt(238-244)fmt(276-282)fmt(314-320)fmt(350-352)fmt(373-379)fmt(411-417)fmt(449-455)fmt(481-487)fmt(519-525)fmt(559-565)crates/biome_formatter/src/builders.rs (1)
soft_line_break_or_space(188-190)
crates/biome_css_formatter/src/css/auxiliary/if_supports_test.rs (3)
crates/biome_css_formatter/src/css/auxiliary/if_media_test.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_style_test.rs (1)
fmt_fields(9-28)crates/biome_formatter/src/builders.rs (1)
soft_block_indent(1260-1265)
crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_or_expr.rs (1)
crates/biome_formatter/src/builders.rs (1)
space(606-608)
crates/biome_css_formatter/src/css/auxiliary/if_supports_identifier_test.rs (5)
crates/biome_css_formatter/src/css/auxiliary/if_media_test.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/else_keyword.rs (1)
fmt_fields(9-13)crates/biome_css_formatter/src/css/auxiliary/if_branch.rs (1)
fmt_fields(9-25)crates/biome_css_formatter/src/css/auxiliary/if_function.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_supports_test.rs (1)
fmt_fields(9-28)
crates/biome_css_parser/src/syntax/value/parse_error.rs (1)
crates/biome_parser/src/diagnostic.rs (1)
expected_any(486-488)
crates/biome_css_formatter/src/css/any/if_supports_test_condition.rs (1)
crates/biome_css_formatter/src/generated.rs (16)
fmt(12-14)fmt(46-52)fmt(84-90)fmt(124-130)fmt(162-168)fmt(200-206)fmt(238-244)fmt(276-282)fmt(314-320)fmt(350-352)fmt(373-379)fmt(411-417)fmt(449-455)fmt(481-487)fmt(519-525)fmt(559-565)
crates/biome_css_formatter/src/css/any/if_test_boolean_expr.rs (1)
crates/biome_css_formatter/src/generated.rs (16)
fmt(12-14)fmt(46-52)fmt(84-90)fmt(124-130)fmt(162-168)fmt(200-206)fmt(238-244)fmt(276-282)fmt(314-320)fmt(350-352)fmt(373-379)fmt(411-417)fmt(449-455)fmt(481-487)fmt(519-525)fmt(559-565)
crates/biome_css_parser/src/syntax/value/function.rs (1)
crates/biome_css_parser/src/syntax/value/if.rs (2)
is_at_if_function(30-32)parse_if_function(93-108)
crates/biome_css_formatter/src/css/any/if_branch.rs (1)
crates/biome_css_formatter/src/generated.rs (16)
fmt(12-14)fmt(46-52)fmt(84-90)fmt(124-130)fmt(162-168)fmt(200-206)fmt(238-244)fmt(276-282)fmt(314-320)fmt(350-352)fmt(373-379)fmt(411-417)fmt(449-455)fmt(481-487)fmt(519-525)fmt(559-565)
crates/biome_css_formatter/src/css/any/if_condition.rs (1)
crates/biome_css_formatter/src/generated.rs (16)
fmt(12-14)fmt(46-52)fmt(84-90)fmt(124-130)fmt(162-168)fmt(200-206)fmt(238-244)fmt(276-282)fmt(314-320)fmt(350-352)fmt(373-379)fmt(411-417)fmt(449-455)fmt(481-487)fmt(519-525)fmt(559-565)
crates/biome_css_formatter/src/css/any/if_media_test_query.rs (1)
crates/biome_css_formatter/src/generated.rs (16)
fmt(12-14)fmt(46-52)fmt(84-90)fmt(124-130)fmt(162-168)fmt(200-206)fmt(238-244)fmt(276-282)fmt(314-320)fmt(350-352)fmt(373-379)fmt(411-417)fmt(449-455)fmt(481-487)fmt(519-525)fmt(559-565)
crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_expr_in_parens.rs (5)
crates/biome_css_formatter/src/css/auxiliary/if_media_test.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_style_test.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_function.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_supports_test.rs (1)
fmt_fields(9-28)crates/biome_formatter/src/builders.rs (1)
soft_block_indent(1260-1265)
crates/biome_css_formatter/src/css/any/if_test_boolean_and_combinable_expr.rs (2)
crates/biome_css_formatter/src/generated.rs (16)
fmt(12-14)fmt(46-52)fmt(84-90)fmt(124-130)fmt(162-168)fmt(200-206)fmt(238-244)fmt(276-282)fmt(314-320)fmt(350-352)fmt(373-379)fmt(411-417)fmt(449-455)fmt(481-487)fmt(519-525)fmt(559-565)packages/@biomejs/js-api/src/common.ts (1)
FormatResult(30-39)
crates/biome_css_formatter/src/css/auxiliary/if_style_test.rs (4)
crates/biome_css_formatter/src/css/auxiliary/if_media_test.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_function.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_supports_test.rs (1)
fmt_fields(9-28)crates/biome_formatter/src/builders.rs (1)
soft_block_indent(1260-1265)
crates/biome_css_formatter/src/css/any/if_test_boolean_expr_group.rs (1)
crates/biome_css_formatter/src/generated.rs (16)
fmt(12-14)fmt(46-52)fmt(84-90)fmt(124-130)fmt(162-168)fmt(200-206)fmt(238-244)fmt(276-282)fmt(314-320)fmt(350-352)fmt(373-379)fmt(411-417)fmt(449-455)fmt(481-487)fmt(519-525)fmt(559-565)
crates/biome_css_formatter/src/css/auxiliary/if_function.rs (6)
crates/biome_css_formatter/src/css/auxiliary/if_media_test.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_style_test.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_branch.rs (1)
fmt_fields(9-25)crates/biome_css_formatter/src/css/auxiliary/if_supports_test.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_expr_in_parens.rs (1)
fmt_fields(9-28)crates/biome_formatter/src/builders.rs (1)
soft_block_indent(1260-1265)
crates/biome_css_formatter/src/css/any/if_test_boolean_or_combinable_expr.rs (1)
crates/biome_css_formatter/src/generated.rs (16)
fmt(12-14)fmt(46-52)fmt(84-90)fmt(124-130)fmt(162-168)fmt(200-206)fmt(238-244)fmt(276-282)fmt(314-320)fmt(350-352)fmt(373-379)fmt(411-417)fmt(449-455)fmt(481-487)fmt(519-525)fmt(559-565)
crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_not_expr.rs (5)
crates/biome_css_formatter/src/css/auxiliary/else_keyword.rs (1)
fmt_fields(9-13)crates/biome_css_formatter/src/css/auxiliary/if_branch.rs (1)
fmt_fields(9-25)crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_and_expr.rs (1)
fmt_fields(9-26)crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_expr_in_parens.rs (1)
fmt_fields(9-28)crates/biome_formatter/src/builders.rs (1)
space(606-608)
crates/biome_css_parser/src/syntax/value/if.rs (5)
crates/biome_css_parser/src/syntax/at_rule/container/mod.rs (2)
parse_any_container_style_query(321-333)is_at_recovered(430-441)crates/biome_css_parser/src/syntax/at_rule/media.rs (3)
is_at_any_media_condition(95-97)parse_any_media_condition(100-126)recover(62-72)crates/biome_css_parser/src/syntax/at_rule/supports/mod.rs (2)
parse_any_supports_condition(104-114)is_at_recovered(89-100)crates/biome_css_parser/src/syntax/mod.rs (2)
is_at_declaration(204-206)parse_declaration(208-219)crates/biome_css_parser/src/syntax/value/parse_error.rs (3)
expected_if_branch(23-25)expected_if_test_boolean_expr(27-33)expected_if_test_boolean_expr_group(35-40)
crates/biome_css_formatter/src/css/auxiliary/if_branch.rs (2)
crates/biome_css_formatter/src/css/auxiliary/if_supports_identifier_test.rs (1)
fmt_fields(9-21)crates/biome_formatter/src/builders.rs (1)
space(606-608)
crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_and_expr.rs (3)
crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_expr_in_parens.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_not_expr.rs (1)
fmt_fields(9-16)crates/biome_formatter/src/builders.rs (1)
space(606-608)
crates/biome_css_formatter/src/generated.rs (6)
crates/biome_css_formatter/src/css/any/if_test_boolean_expr.rs (1)
fmt(9-19)crates/biome_css_formatter/src/css/any/if_test_boolean_or_combinable_expr.rs (1)
fmt(9-22)crates/biome_css_formatter/src/css/any/if_branch.rs (1)
fmt(9-14)crates/biome_css_formatter/src/css/any/if_condition.rs (1)
fmt(9-14)crates/biome_css_formatter/src/css/lists/if_branch_list.rs (1)
fmt(10-17)crates/biome_css_formatter/src/css/any/function.rs (1)
fmt(9-15)
🔇 Additional comments (44)
crates/biome_css_parser/src/syntax/property/mod.rs (1)
212-212: Visibility changes confirmed appropriate.GenericComponentValueList is imported and used in the new CSS if() parsing module (
crates/biome_css_parser/src/syntax/value/if.rs), validating the visibility change. The tight coupling between both exposed items remains sound for internal crate reuse.crates/biome_css_formatter/src/css/bogus/mod.rs (1)
12-13: LGTM!The new bogus formatter modules follow the established pattern for error recovery nodes.
crates/biome_css_formatter/src/css/lists/if_branch_list.rs (1)
1-18: LGTM!The formatter follows the standard list formatting pattern with appropriate semicolon separators and trailing separator policy for CSS if-branch lists.
crates/biome_css_parser/src/syntax/at_rule/media.rs (1)
95-95: LGTM!Appropriate visibility expansion to
pub(crate)enables reuse of media condition parsing for the new if-notation without exposing these functions publicly.Also applies to: 100-100, 248-248
crates/biome_css_formatter/src/css/any/if_test_boolean_or_combinable_expr.rs (1)
1-23: LGTM!Standard generated formatter with proper delegation pattern for boolean OR combinable expressions.
crates/biome_css_formatter/src/css/any/if_branch.rs (1)
1-15: LGTM!Generated formatter correctly delegates to both valid and bogus if-branch variants.
crates/biome_css_parser/src/lexer/mod.rs (1)
915-916: LGTM!Keyword mappings for
ifandelseappropriately extend the lexer's token recognition for the CSS if() function.crates/biome_css_parser/src/syntax/value/mod.rs (1)
3-3: LGTM!Correct use of raw identifier syntax (
r#if) to declare a module for a Rust keyword, with appropriate crate-level visibility..changeset/seven-words-trade.md (1)
9-13: Syntax is correct per CSS Values Level 5 specification.The if() function format is
if(<if-test>: <value>; ... else: <value>;?)with semicolon-separated clauses, and your example follows this precisely. The style() test against custom properties is a supported if-test type, so the changeset is good to merge.crates/biome_css_formatter/src/css/any/function.rs (1)
10-14: LGTM!The CssIfFunction integration follows the established pattern perfectly. Generated code is consistent with existing variants.
crates/biome_css_formatter/src/css/lists/mod.rs (1)
15-15: LGTM!Module addition maintains alphabetical ordering as expected for generated code.
crates/biome_css_parser/src/syntax/at_rule/supports/mod.rs (1)
82-82: LGTM!The visibility change to
pub(crate)sensibly enables cross-module usage for if-notation parsing whilst maintaining crate encapsulation.crates/biome_css_parser/src/syntax/value/function.rs (1)
1-1: LGTM!The if-function integration follows the established url-function pattern precisely. The parsing dispatch order is correct: url → if → general function.
Also applies to: 6-6, 25-25, 41-42
crates/biome_css_formatter/src/css/auxiliary/if_supports_test.rs (1)
1-29: LGTM!Formatter implementation mirrors the pattern from if_media_test and if_style_test consistently. The use of soft_block_indent is appropriate for the nested test content.
crates/biome_css_parser/tests/css_test_suite/error/function/if.css (1)
1-83: LGTM!Excellent error-case coverage spanning 27 distinct scenarios. The test cases are well-organised with descriptive names, providing comprehensive validation of the parser's error-handling paths.
crates/biome_css_formatter/src/css/bogus/bogus_if_branch.rs (1)
1-5: LGTM!Standard bogus-node formatter following the established pattern.
crates/biome_grit_patterns/src/grit_target_language/css_target_language/generated_mappings.rs (1)
54-54: LGTM!Generated mappings for CssElseKeyword and the ten CssIf* variants follow the established pattern and maintain alphabetical ordering.
Also applies to: 67-78
crates/biome_css_formatter/src/css/any/if_test.rs (1)
1-17: LGTM!The generated formatter correctly delegates to all four test variants. The implementation follows the established pattern for Any* formatters.
crates/biome_css_formatter/src/css/bogus/bogus_if_test.rs (1)
1-5: LGTM!Standard bogus formatter implementation—empty impl is correct for error recovery nodes.
crates/biome_css_formatter/src/css/auxiliary/if_branch.rs (1)
8-25: LGTM!The formatting sequence (condition, colon, space, value) produces readable output consistent with CSS conventions. All fields are properly handled.
crates/biome_css_formatter/src/css/auxiliary/if_media_test.rs (1)
8-28: LGTM!Excellent consistency—this formatter mirrors the pattern used in
if_style_test.rsandif_supports_test.rs. The grouped soft-block indentation appropriately handles nested test expressions.crates/biome_css_formatter/src/css/auxiliary/else_keyword.rs (1)
8-13: LGTM!Straightforward keyword formatter—correctly renders the else token without unnecessary decoration.
crates/biome_css_formatter/src/css/any/if_media_test_query.rs (1)
1-15: LGTM!Generated formatter correctly delegates to both media condition and query feature variants.
crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_not_expr.rs (1)
8-16: LGTM!The space after
notmaintains consistency with the boolean operator formatting pattern used inif_test_boolean_and_expr.rsand improves readability.crates/biome_css_parser/src/syntax/at_rule/mod.rs (1)
3-22: LGTM!Sensible visibility changes—exposing these modules as
pub(crate)enables internal code reuse whilst maintaining encapsulation of the public API.crates/biome_css_formatter/tests/specs/css/if.css (1)
1-429: Excellent test coverage!This test suite comprehensively covers the CSS
if()function syntax with basic cases, logical operators, nesting, edge cases, and real-world scenarios. The intentional spacing variations will ensure the formatter handles diverse input correctly.crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_or_expr.rs (1)
1-27: LGTM!The formatter correctly handles boolean OR expressions with proper spacing around the operator, consistent with the codebase patterns.
crates/biome_css_formatter/src/css/any/if_condition.rs (1)
1-15: LGTM!Generated formatter correctly delegates to the appropriate inner formatters for each condition variant.
crates/biome_css_formatter/src/css/auxiliary/if_style_test.rs (1)
1-29: LGTM!The formatter follows the same pattern as
if_media_testandif_supports_test, using grouped soft indentation for the parenthesised test content. Implementation is consistent with the codebase conventions.crates/biome_css_parser/src/syntax/at_rule/container/mod.rs (1)
1-1: LGTM!Exposing these container query parsing helpers as
pub(crate)enables reuse within the crate for the if-notation implementation without affecting the public API.Also applies to: 217-217, 299-299, 321-321
crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_and_expr.rs (1)
1-27: LGTM!The formatter correctly handles boolean AND expressions with appropriate spacing around the operator, following the same pattern as the OR and NOT expression formatters.
crates/biome_css_formatter/src/css/any/if_supports_test_condition.rs (1)
1-19: LGTM!Generated formatter correctly delegates to the appropriate inner formatters for each supports test condition variant.
crates/biome_css_formatter/src/css/auxiliary/if_function.rs (1)
1-29: LGTM! Consistent formatter implementation.The implementation follows the same pattern as the related
if_media_test,if_style_test, andif_supports_testformatters, with appropriate use ofgroup()andsoft_block_indent()for the branch list.xtask/codegen/src/css_kinds_src.rs (1)
96-97: LGTM! Comprehensive if-notation support.The additions include the necessary keywords, node types, and error variants for complete CSS if() function support. The placement follows the existing organization patterns.
Also applies to: 389-400, 564-565
crates/biome_css_formatter/src/css/any/if_test_boolean_expr_group.rs (1)
1-17: LGTM! Generated formatter with proper delegation.The formatter correctly delegates to inner node formatters for both variants. The implementation follows the standard pattern for
Any*formatters.crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_expr_in_parens.rs (1)
1-29: LGTM! Appropriate formatting for parenthesized expressions.The implementation correctly uses
soft_block_indent()without thegroup()wrapper, which is appropriate since there's no leading token (unlikeif_media_test,if_style_test, etc., which wrap their parens ingroup()because they include a preceding token).crates/biome_css_formatter/src/css/any/if_test_boolean_expr.rs (1)
1-20: LGTM! Standard generated formatter.The implementation properly delegates formatting to the three inner variant types. The pattern is consistent with other
Any*formatters in the codebase.crates/biome_css_parser/src/syntax/value/parse_error.rs (1)
23-40: LGTM! Helpful error diagnostics.The three new error helpers follow the established pattern and provide clear, descriptive labels for the if-notation syntax elements. The error messages will aid developers in understanding parsing failures.
crates/biome_css_parser/tests/css_test_suite/ok/function/if.css (1)
1-435: Excellent test coverage!This test file provides comprehensive coverage of the CSS if() function, including:
- Basic conditional tests (style, media, supports)
- Logical operators (and, or, not) with complex combinations
- Nested conditionals and multiple branches
- Integration with CSS functions (calc, clamp, var)
- Edge cases (whitespace handling, optional semicolons, special characters)
- Real-world scenarios (responsive typography, grid layouts, accessibility, print styles)
The thoroughness here will help ensure robust parsing and formatting support.
crates/biome_css_formatter/src/css/any/mod.rs (1)
32-40: LGTM! Complete module structure.The nine new if-related modules provide the necessary formatter infrastructure for the CSS if() function support. The organization is consistent with the existing module structure.
crates/biome_css_formatter/src/generated.rs (4)
1344-1381: Solid plumbing for if() syntax.The FormatRule/AsFormat/IntoFormat triplets for the new if() nodes are consistent, and AnyCssFunction already accounts for CssIfFunction. All good here.
Also applies to: 1810-1843, 1844-1881, 1882-1919, 1920-1957, 1958-1989, 1990-2027, 2028-2067, 2068-2099, 2100-2139, 2140-2177
7481-7556: Bogus if nodes wired correctly.*Error-path coverage for CssBogusIfBranch and CssBogusIfTest looks aligned with existing bogus patterns.
8948-9134: All AnyCssIf wrappers verified as complete and consistent.*The script confirms the implementation is exhaustive:
- All 9 AnyCssIf* formatter modules exist and have corresponding match arms
- Generated code shows proper AsFormat and IntoFormat implementations for each variant
- CssIfFunction is correctly handled in AnyCssFunction
No gaps or missing coverage detected.
6705-6729: The semicolon separator is correct—the review comment misunderstands CSS if() syntax.The CSS if() function uses semicolon-separated branches where each branch is a condition and value pair. The formatter is already compliant. No change needed.
Likely an incorrect or invalid review comment.
7a70653 to
68f0fcc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_css_parser/src/syntax/value/function.rs (1)
19-22: Consider updating documentation to mentionif()function.The doc comments describe checking for "URL functions and simple functions", but now also support
if()functions. Mentioning this would improve clarity for future maintainers.Also applies to: 28-32
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
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_formatter/tests/specs/css/if.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/function/if.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/function/if.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**crates/biome_css_syntax/src/generated/nodes_mut.rsis excluded by!**/generated/**,!**/generated/**and included by**
📒 Files selected for processing (44)
.changeset/seven-words-trade.md(1 hunks)crates/biome_css_formatter/src/css/any/function.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_branch.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_condition.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_media_test_query.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_supports_test_condition.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_test.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_test_boolean_and_combinable_expr.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_test_boolean_expr.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_test_boolean_expr_group.rs(1 hunks)crates/biome_css_formatter/src/css/any/if_test_boolean_or_combinable_expr.rs(1 hunks)crates/biome_css_formatter/src/css/any/mod.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/else_keyword.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_branch.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_function.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_media_test.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_style_test.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_supports_identifier_test.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_supports_test.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_and_expr.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_expr_in_parens.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_not_expr.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_or_expr.rs(1 hunks)crates/biome_css_formatter/src/css/auxiliary/mod.rs(2 hunks)crates/biome_css_formatter/src/css/bogus/bogus_if_branch.rs(1 hunks)crates/biome_css_formatter/src/css/bogus/bogus_if_test.rs(1 hunks)crates/biome_css_formatter/src/css/bogus/mod.rs(1 hunks)crates/biome_css_formatter/src/css/lists/if_branch_list.rs(1 hunks)crates/biome_css_formatter/src/css/lists/mod.rs(1 hunks)crates/biome_css_formatter/src/generated.rs(5 hunks)crates/biome_css_formatter/tests/specs/css/if.css(1 hunks)crates/biome_css_parser/src/lexer/mod.rs(1 hunks)crates/biome_css_parser/src/syntax/at_rule/container/mod.rs(4 hunks)crates/biome_css_parser/src/syntax/at_rule/media.rs(2 hunks)crates/biome_css_parser/src/syntax/at_rule/mod.rs(1 hunks)crates/biome_css_parser/src/syntax/at_rule/supports/mod.rs(1 hunks)crates/biome_css_parser/src/syntax/property/mod.rs(2 hunks)crates/biome_css_parser/src/syntax/value/function.rs(3 hunks)crates/biome_css_parser/src/syntax/value/if.rs(1 hunks)crates/biome_css_parser/src/syntax/value/mod.rs(1 hunks)crates/biome_css_parser/src/syntax/value/parse_error.rs(1 hunks)crates/biome_css_parser/tests/css_test_suite/error/function/if.css(1 hunks)crates/biome_css_parser/tests/css_test_suite/ok/function/if.css(1 hunks)crates/biome_grit_patterns/src/grit_target_language/css_target_language/generated_mappings.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (25)
- crates/biome_grit_patterns/src/grit_target_language/css_target_language/generated_mappings.rs
- crates/biome_css_formatter/src/css/auxiliary/if_media_test.rs
- crates/biome_css_parser/src/syntax/at_rule/supports/mod.rs
- crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_expr_in_parens.rs
- crates/biome_css_formatter/src/css/auxiliary/if_branch.rs
- crates/biome_css_formatter/src/css/bogus/mod.rs
- crates/biome_css_formatter/src/css/lists/if_branch_list.rs
- crates/biome_css_parser/src/syntax/value/parse_error.rs
- crates/biome_css_formatter/src/css/any/if_test_boolean_expr_group.rs
- crates/biome_css_formatter/src/css/bogus/bogus_if_branch.rs
- crates/biome_css_formatter/src/css/any/if_branch.rs
- crates/biome_css_formatter/src/css/any/if_test_boolean_expr.rs
- crates/biome_css_parser/src/syntax/value/mod.rs
- crates/biome_css_formatter/src/css/auxiliary/if_supports_identifier_test.rs
- crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_not_expr.rs
- crates/biome_css_parser/tests/css_test_suite/ok/function/if.css
- crates/biome_css_parser/src/lexer/mod.rs
- .changeset/seven-words-trade.md
- crates/biome_css_parser/src/syntax/value/if.rs
- crates/biome_css_formatter/src/css/any/if_condition.rs
- crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_or_expr.rs
- crates/biome_css_formatter/src/css/auxiliary/mod.rs
- crates/biome_css_formatter/src/css/auxiliary/if_function.rs
- crates/biome_css_formatter/src/css/any/if_test.rs
- crates/biome_css_formatter/src/generated.rs
🧰 Additional context used
🧬 Code graph analysis (6)
crates/biome_css_formatter/src/css/any/if_media_test_query.rs (1)
crates/biome_css_formatter/src/generated.rs (16)
fmt(12-14)fmt(46-52)fmt(84-90)fmt(122-128)fmt(162-168)fmt(200-206)fmt(238-244)fmt(276-282)fmt(314-320)fmt(352-358)fmt(388-390)fmt(411-417)fmt(444-444)fmt(462-468)fmt(500-506)fmt(532-538)
crates/biome_css_formatter/src/css/any/if_test_boolean_or_combinable_expr.rs (1)
crates/biome_css_formatter/src/generated.rs (16)
fmt(12-14)fmt(46-52)fmt(84-90)fmt(122-128)fmt(162-168)fmt(200-206)fmt(238-244)fmt(276-282)fmt(314-320)fmt(352-358)fmt(388-390)fmt(411-417)fmt(444-444)fmt(462-468)fmt(500-506)fmt(532-538)
crates/biome_css_formatter/src/css/auxiliary/if_supports_test.rs (4)
crates/biome_css_formatter/src/css/auxiliary/if_media_test.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_style_test.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_supports_identifier_test.rs (1)
fmt_fields(9-21)crates/biome_formatter/src/builders.rs (1)
soft_block_indent(1260-1265)
crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_and_expr.rs (3)
crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_expr_in_parens.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_not_expr.rs (1)
fmt_fields(9-16)crates/biome_formatter/src/builders.rs (1)
space(606-608)
crates/biome_css_parser/src/syntax/value/function.rs (1)
crates/biome_css_parser/src/syntax/value/if.rs (2)
is_at_if_function(30-32)parse_if_function(93-108)
crates/biome_css_formatter/src/css/auxiliary/if_style_test.rs (4)
crates/biome_css_formatter/src/css/auxiliary/if_media_test.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_function.rs (1)
fmt_fields(9-28)crates/biome_css_formatter/src/css/auxiliary/if_supports_test.rs (1)
fmt_fields(9-28)crates/biome_formatter/src/builders.rs (1)
soft_block_indent(1260-1265)
⏰ 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). (9)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: autofix
🔇 Additional comments (21)
crates/biome_css_formatter/src/css/bogus/bogus_if_test.rs (1)
1-5: LGTM! Clean boilerplate.This follows the standard pattern for bogus node formatters—minimal imports, appropriate visibility, standard derives, and an empty trait implementation that delegates to default formatting behaviour. Exactly what's needed.
crates/biome_css_formatter/src/css/auxiliary/if_supports_test.rs (3)
1-3: LGTM!Standard imports for a formatter implementation. All are used and necessary.
5-6: LGTM!Standard formatter struct with appropriate derives and visibility.
8-29: Excellent consistency with sibling formatters.The implementation follows the exact same pattern as
if_media_test.rsandif_style_test.rs, correctly formatting the test token, parentheses, and soft-indented test content. The use ofsoft_block_indentandgroupensures appropriate line breaking for nested expressions.crates/biome_css_parser/src/syntax/property/mod.rs (1)
212-212: Visibility changes look good.Exposing
GenericComponentValueListandparse_generic_component_valueaspub(crate)enables internal reuse whilst maintaining proper encapsulation. Well-scoped change for the CSSif()support.Also applies to: 247-247
crates/biome_css_parser/src/syntax/at_rule/mod.rs (1)
3-3: Submodule visibility adjustments approved.Making these at-rule modules
pub(crate)is appropriate for enabling cross-module parser coordination. Clean structural change.Also applies to: 6-7, 14-14, 22-22
crates/biome_css_parser/src/syntax/at_rule/container/mod.rs (1)
1-1: Container query parser visibility updates look sound.Exposing the error module and key container query parsing functions as
pub(crate)enables internal reuse. Well-considered scope for the CSSif()feature work.Also applies to: 229-229, 311-311, 333-333
crates/biome_css_parser/src/syntax/at_rule/media.rs (1)
109-109: Media query parser visibility changes approved.Making these media condition and in-parens parsers
pub(crate)follows the established pattern and enables appropriate internal reuse. Nicely scoped.Also applies to: 114-114, 262-262
crates/biome_css_parser/src/syntax/value/function.rs (1)
1-1: No issues found; the implementation is correct as-is.The concern was unfounded. Since
is_nth_at_functionexplicitly requiresis_nth_at_identifier(p, n)to be true, andifis a keyword token (not an identifier),is_at_functioncannot matchif(by definition. The design correctly prevents collision by checking for identifiers only, which makes the explicit exclusion ofis_at_if_functionfromis_at_functionunnecessary but harmless.crates/biome_css_formatter/src/css/any/function.rs (1)
12-12: LGTM!The new match arm for
CssIfFunctionfollows the established pattern and correctly delegates formatting to the node's formatter.crates/biome_css_parser/tests/css_test_suite/error/function/if.css (1)
1-83: Excellent error test coverage!The test file covers a comprehensive range of error scenarios including syntax errors, missing tokens, invalid queries, operator precedence issues, and edge cases. Well done!
crates/biome_css_formatter/src/css/auxiliary/else_keyword.rs (1)
8-14: LGTM!Simple and correct formatter implementation for the
elsekeyword.crates/biome_css_formatter/src/css/auxiliary/if_style_test.rs (1)
8-29: LGTM!The formatter implementation is consistent with the related
if_media_testandif_supports_testformatters, using proper grouping and soft indentation.crates/biome_css_formatter/src/css/auxiliary/if_test_boolean_and_expr.rs (1)
8-27: LGTM!Proper formatting of boolean AND expressions with correct spacing around the operator, consistent with the
if_test_boolean_not_exprformatter.crates/biome_css_formatter/src/css/any/if_test_boolean_and_combinable_expr.rs (1)
7-25: LGTM!Standard generated formatter that correctly delegates to the inner node formatters.
crates/biome_css_formatter/src/css/lists/mod.rs (1)
15-15: LGTM!Module declaration properly added in alphabetical order.
crates/biome_css_formatter/src/css/any/if_media_test_query.rs (1)
10-13: Unable to verify enum variants against grammar definition.The match statement is exhaustive (Rust would fail to compile otherwise, and the code is in a compilable PR), confirming it covers all enum variants:
AnyCssMediaConditionandAnyCssQueryFeature. However, I couldn't locate the grammar definition inxtask/codegen/css.ungramthrough automated search. Please manually confirm these variants match theif_media_test_querydefinition in the grammar.crates/biome_css_formatter/tests/specs/css/if.css (1)
1-421: Excellent comprehensive test coverage!The test suite thoroughly exercises the CSS if() function across a wide range of use cases, edge cases, and formatting variations. Well done on covering basic queries, logical operators, nesting, integration with other CSS functions, and real-world scenarios.
crates/biome_css_formatter/src/css/any/if_supports_test_condition.rs (1)
1-19: LGTM!Generated formatter follows the expected pattern and correctly delegates formatting to the inner variants.
crates/biome_css_formatter/src/css/any/if_test_boolean_or_combinable_expr.rs (1)
1-23: LGTM!Generated formatter correctly implements the OR-combinable expression formatting, mirroring the pattern used for AND-combinable expressions.
crates/biome_css_formatter/src/css/any/mod.rs (1)
33-41: LGTM!The nine new module declarations are correctly positioned and maintain crate-private visibility, properly wiring the if-notation formatter implementations.
CodSpeed Performance ReportMerging #8111 will not alter performanceComparing Summary
Footnotes
|
| - ( <style-query> ) | ||
| - ( <style-feature> ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are not formatted like other diagnostics are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these are inherited from reuse of the style in container nodes (similar existing example here, coming from here).
| h1::before{ | ||
| content:if( | ||
| style(--scheme: ice):"D "; | ||
| style(--scheme: fire):"=% "; | ||
| else:""; | ||
| ); | ||
| } | ||
|
|
||
| .responsive-typography { | ||
| font-size:if( | ||
| media(width >= 1920px):clamp(1.25rem, 1.5vw, 2rem); | ||
| media(width >= 1024px):clamp(1.125rem, 1.25vw, 1.5rem); | ||
| media(width >= 768px):clamp(1rem, 1.1vw, 1.25rem); | ||
| else:clamp(0.875rem, 3vw, 1rem); | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This formatting looks a bit wonky. We should try to match prettier. Idk if they fully support this syntax yet, but prettier seems to handle it fine: https://biomejs.dev/playground/?code=YQByAHQAaQBjAGwAZQAgAHsACgAgACAALQAtAGMAbwBsAG8AcgAxADoAaQBmACgACgAgACAAIAAgAHMAdAB5AGwAZQAoAC0ALQBzAGMAaABlAG0AZQA6ACAAaQBjAGUAKQA6ACMAMAAzADAANAA1AGUAOwAKACAAIAAgACAAIAAgAHMAdAB5AGwAZQAoAC0ALQBzAGMAaABlAG0AZQA6ACAAZgBpAHIAZQApADoAIAAgACMANgAyADEANwAwADgAOwAKAGUAbABzAGUAOgBiAGwAYQBjAGsAOwAKACAAIAApADsACgAgACAALQAtAGMAbwBsAG8AcgAyADoAaQBmACgACgBzAHQAeQBsAGUAKAAtAC0AcwBjAGgAZQBtAGUAOgAgAGkAYwBlACkAOgAjAGMAYQBmADAAZgA4ADsACgAgACAAIAAgAHMAdAB5AGwAZQAoAC0ALQBzAGMAaABlAG0AZQA6ACAAZgBpAHIAZQApADoAIwBmAGYAYwA5ADcAMQA7AAoAIAAgACAAIAAgACAAZQBsAHMAZQA6AHcAaABpAHQAZQA7AAoAKQA7AAoACgAgACAAYwBvAGwAbwByADoAdgBhAHIAKAAtAC0AYwBvAGwAbwByADEAKQA7AAoAIAAgAGIAbwByAGQAZQByADoAMwBwAHgAIABzAG8AbABpAGQAIAB2AGEAcgAoAC0ALQBjAG8AbABvAHIAMQApADsACgAgACAAYgBhAGMAawBnAHIAbwB1AG4AZAA6AHYAYQByACgALQAtAGMAbwBsAG8AcgAyACkAOwAKAH0A&language=css
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm it's weird, prettier seems to handle your example pretty gracefully but then seems to break down a bit in similar examples (weird indents, space before semicolons). I notice that prettier seems to utilize more of the character width where the formatter here seems more likely to add line breaks. I'll see if i can tweak that to make it behave a bit more like prettier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, yeah i see that. I don't think Prettier fully supports this syntax yet, so use your best judgement for now. We can patch to align with Prettier later if needed.
Co-authored-by: Carson McManus <[email protected]>
Co-authored-by: Carson McManus <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_css_parser/src/syntax/value/if.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/biome_css_parser/src/syntax/value/if.rs (6)
crates/biome_css_parser/src/syntax/at_rule/container/mod.rs (2)
parse_any_container_style_query(333-345)is_at_recovered(442-453)crates/biome_css_parser/src/syntax/at_rule/media.rs (5)
is_at_any_media_condition(109-111)parse_any_media_condition(114-140)parse_element(68-70)is_at_list_end(72-74)recover(76-86)crates/biome_css_parser/src/syntax/at_rule/supports/mod.rs (2)
parse_any_supports_condition(116-126)is_at_recovered(101-112)crates/biome_css_parser/src/syntax/mod.rs (2)
is_at_declaration(204-206)parse_declaration(208-219)crates/biome_css_parser/src/syntax/value/parse_error.rs (3)
expected_if_branch(23-25)expected_if_test_boolean_expr(27-33)expected_if_test_boolean_expr_group(35-40)crates/biome_css_parser/src/syntax/property/mod.rs (7)
is_at_recovered(153-156)parse_element(129-131)parse_element(219-221)is_at_list_end(133-135)is_at_list_end(223-225)recover(137-143)recover(227-238)
⏰ 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). (5)
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: autofix
🔇 Additional comments (7)
crates/biome_css_parser/src/syntax/value/if.rs (7)
1-32: LGTM on imports and detection helper.The imports cover all necessary parser utilities, and the
is_at_if_functionhelper is appropriately simplified.
93-108: Solid entry point.The implementation correctly follows the standard parser pattern: validate position, mark, consume tokens, parse children, and complete.
222-251: Dispatcher and group parser look correct.The test dispatcher tries each variant in sequence, and the group parser properly handles both parenthesised expressions and direct tests.
253-276: NOT expression handling is correct.Standard unary operator implementation with appropriate child parsing.
376-399: Condition and branch parsing look solid.The condition parser properly handles both
elseand boolean expressions, and the branch parser follows the expected structure with appropriate recovery.
401-475: Recovery and list implementation are well structured.The recovery points are sensible (operator boundaries, separators, structural tokens, line breaks), and the branch list correctly implements a semicolon-separated list with trailing separator support.
291-318: Disregard the precedence concern—the CSS spec requires explicit parentheses for mixed operators.The test suite confirms that unpacked mixed operators like
a and b or care invalid syntax (see error file line 66). All valid mixed-operator expressions in theok/tests use explicit parentheses. The parser doesn't need precedence logic because the spec forbids unpacked mixed operators outright—they simply aren't allowed.The code is correct.
Likely an incorrect or invalid review comment.
Summary
Closes #6725
This PR adds parsing and formatting support for the new CSS if function.
Note:
These two PRs were merged earlier and are somewhat related. They were required to support the growing number of parsing nodes needed for this work:
Test Plan
Formatting and parsing spec tests added:
crates/biome_css_formatter/tests/specs/css/if.csscrates/biome_css_parser/tests/css_test_suite/ok/function/if.csscrates/biome_css_parser/tests/css_test_suite/error/function/if.cssAI Assistance Disclosure:
I use Claude Code off and on. It was mostly used early on to research the codebase and draft out initial plans to approach the work but I pretty much scrapped almost all of it's code and rewrote from the ground up. It was used every once in a while again for investigating and validating things as well as generating test examples. The vast majority of the final code was written by me.