feat(css): add support for SCSS @mixin at-rule#9423
Conversation
…in parameter defaults
|
Parser conformance results onjs/262
jsx/babel
markdown/commonmark
symbols/microsoft
ts/babel
ts/microsoft
|
Merging this PR will not alter performance
Comparing Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds full SCSS Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
crates/biome_css_parser/src/lexer/mod.rs (1)
843-843: Please add one regression wheremixinis not an at-rule keyword.This now lexes as a dedicated keyword everywhere, so one parser case using
mixinin an ordinary identifier/value position would guard against contextual-keyword fallout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_parser/src/lexer/mod.rs` at line 843, Add a regression test that ensures the lexer/parser treats "mixin" as an ordinary identifier/value (not an at-rule keyword) in non-@-rule positions: create a test that feeds input containing "mixin" in a declaration or selector context (e.g., as a property name or identifier token) and assert that the produced token kind is IDENT or value token rather than MIXIN_KW; update or add the test near existing lexer/parser tests that exercise keyword vs identifier behavior so changes around the match arm "b\"mixin\" => MIXIN_KW" will be caught if it becomes a dedicated keyword in all contexts.crates/biome_css_parser/tests/css_test_suite/ok/scss/at-rule/mixin.scss (1)
1-11: Please add an explicit@mixin foo()fixture as well.The happy-path coverage already hits omitted parentheses and non-empty parameters, but not the zero-arg
()spelling. That is a handy regression case for this rule.💡 Tiny fixture addition
`@mixin` button { border-radius: 4px; } +@mixin empty() {} + `@mixin` size($width, $height: $width, $args...) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_parser/tests/css_test_suite/ok/scss/at-rule/mixin.scss` around lines 1 - 11, Add an explicit zero-arg mixin fixture to the same test file to cover the `@mixin foo()` spelling; currently the file only contains `@mixin button` (no parens) and `@mixin size($width, ...)` (non-empty params). Add a new mixin declaration using the empty-parentheses form—e.g., `@mixin foo()`—so the parser/regression for zero-arg `()` is exercised.crates/biome_css_parser/src/syntax/scss/at_rule/mixin_at_rule.rs (1)
73-93: Minor: Consider swapping ellipsis and default value parsing order.In SCSS, variadic parameters (
$args...) cannot have default values. The current order parses default value before ellipsis, but if you ever want to add validation that flags$args: default...as invalid, having them in the order they appear in valid syntax might be clearer.That said, this works correctly for valid SCSS, so it's a very minor point.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_parser/src/syntax/scss/at_rule/mixin_at_rule.rs` around lines 73 - 93, In parse_scss_parameter, swap the order of parsing the variadic ellipsis and the default value so the parser checks for and consumes the ellipsis (p.at(T![...]) / p.bump(T![...])) before attempting to parse a default via is_at_scss_parameter_default_value and parse_scss_parameter_default_value; ensure the default value is only parsed when the ellipsis is not present to match SCSS semantics (refer to functions/guards parse_scss_parameter, is_at_scss_parameter_default_value, parse_scss_parameter_default_value, and the p.at/p.bump handling of T![...]).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_css_parser/src/lexer/mod.rs`:
- Line 843: Add a regression test that ensures the lexer/parser treats "mixin"
as an ordinary identifier/value (not an at-rule keyword) in non-@-rule
positions: create a test that feeds input containing "mixin" in a declaration or
selector context (e.g., as a property name or identifier token) and assert that
the produced token kind is IDENT or value token rather than MIXIN_KW; update or
add the test near existing lexer/parser tests that exercise keyword vs
identifier behavior so changes around the match arm "b\"mixin\" => MIXIN_KW"
will be caught if it becomes a dedicated keyword in all contexts.
In `@crates/biome_css_parser/src/syntax/scss/at_rule/mixin_at_rule.rs`:
- Around line 73-93: In parse_scss_parameter, swap the order of parsing the
variadic ellipsis and the default value so the parser checks for and consumes
the ellipsis (p.at(T![...]) / p.bump(T![...])) before attempting to parse a
default via is_at_scss_parameter_default_value and
parse_scss_parameter_default_value; ensure the default value is only parsed when
the ellipsis is not present to match SCSS semantics (refer to functions/guards
parse_scss_parameter, is_at_scss_parameter_default_value,
parse_scss_parameter_default_value, and the p.at/p.bump handling of T![...]).
In `@crates/biome_css_parser/tests/css_test_suite/ok/scss/at-rule/mixin.scss`:
- Around line 1-11: Add an explicit zero-arg mixin fixture to the same test file
to cover the `@mixin foo()` spelling; currently the file only contains `@mixin
button` (no parens) and `@mixin size($width, ...)` (non-empty params). Add a new
mixin declaration using the empty-parentheses form—e.g., `@mixin foo()`—so the
parser/regression for zero-arg `()` is exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eb79cc88-5c2a-4f28-9051-5139226d3645
⛔ 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/scss/at-rule/mixin.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/scss/at-rule/mixin.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/at-rule/mixin.scss.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 (25)
crates/biome_css_formatter/src/css/any/at_rule.rscrates/biome_css_formatter/src/generated.rscrates/biome_css_formatter/src/scss/any/mod.rscrates/biome_css_formatter/src/scss/any/parameter.rscrates/biome_css_formatter/src/scss/auxiliary/mod.rscrates/biome_css_formatter/src/scss/auxiliary/parameter.rscrates/biome_css_formatter/src/scss/auxiliary/parameter_default_value.rscrates/biome_css_formatter/src/scss/auxiliary/parameter_list.rscrates/biome_css_formatter/src/scss/lists/mod.rscrates/biome_css_formatter/src/scss/lists/parameter_item_list.rscrates/biome_css_formatter/src/scss/statements/mixin_at_rule.rscrates/biome_css_formatter/src/scss/statements/mod.rscrates/biome_css_formatter/tests/specs/css/scss/at-rule/mixin.scsscrates/biome_css_parser/src/lexer/mod.rscrates/biome_css_parser/src/lexer/tests.rscrates/biome_css_parser/src/syntax/at_rule/mod.rscrates/biome_css_parser/src/syntax/scss/at_rule/if_at_rule.rscrates/biome_css_parser/src/syntax/scss/at_rule/mixin_at_rule.rscrates/biome_css_parser/src/syntax/scss/at_rule/mod.rscrates/biome_css_parser/src/syntax/scss/mod.rscrates/biome_css_parser/tests/css_test_suite/error/scss/at-rule/mixin.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/at-rule/mixin.scsscrates/biome_grit_patterns/src/grit_target_language/css_target_language/generated_mappings.rsxtask/codegen/css.ungramxtask/codegen/src/css_kinds_src.rs
Summary
Adds SCSS
@mixinat-rule support to the CSS parser and formatter.Test Plan
@mixin@mixincargo test -p biome_css_parsercargo test -p biome_css_formatter