feat(css): add scss keyword and arbitrary arguments#9227
Conversation
|
github-actions
bot
added
A-Parser
Parser conformance results onjs/262
jsx/babel
markdown/commonmark
symbols/microsoft
ts/babel
ts/microsoft
|
Merging this PR will not alter performance
Comparing 
Footnotes
|
WalkthroughAdds parsing and formatting support for two new SCSS constructs: 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
xtask/codegen/src/css_kinds_src.rs (1)
575-576: Minor ordering nit —SCSS_KEYWORD_ARGUMENTis listed beforeSCSS_ARBITRARY_ARGUMENT, which is the reverse of alphabetical order. The SCSS section here isn't strictly sorted, so no functional impact, but the rest of the codebase (e.g.generated_mappings.rs,expression_item.rs) listsArbitrarybeforeKeyword. Worth flipping for consistency, but totally optional.🔀 Suggested order
- "SCSS_KEYWORD_ARGUMENT", - "SCSS_ARBITRARY_ARGUMENT", + "SCSS_ARBITRARY_ARGUMENT", + "SCSS_KEYWORD_ARGUMENT",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@xtask/codegen/src/css_kinds_src.rs` around lines 575 - 576, Minor ordering mismatch: swap "SCSS_KEYWORD_ARGUMENT" and "SCSS_ARBITRARY_ARGUMENT" in the SCSS list so that SCSS_ARBITRARY_ARGUMENT comes before SCSS_KEYWORD_ARGUMENT; locate the SCSS enum/array in css_kinds_src.rs (the entries "SCSS_KEYWORD_ARGUMENT" and "SCSS_ARBITRARY_ARGUMENT") and reorder those two entries to match the rest of the codebase's alphabetical/Arbitrary-before-Keyword convention.crates/biome_css_parser/src/syntax/scss/expression/mod.rs (1)
136-168: Subtle CST concern on the standalone...error path.Lines 145–150: when
...appears without a preceding expression, the token is bumped andAbsentis returned. The bumped...token won't be wrapped in any child node — it becomes a bare token insideSCSS_EXPRESSION_ITEM_LIST. The recovery layer then runs onAbsentbut the token is already consumed.This is fine for error recovery (parser advances, diagnostic emitted), but the unattached token could confuse downstream consumers of the CST. Consider wrapping it in a bogus node instead:
Possible approach
Absent => { if p.at(T![...]) { + let bogus = p.start(); let range = p.cur_range(); p.error(scss_ellipsis_not_allowed(p, range)); p.bump(T![...]); + return Present(bogus.complete(p, CSS_BOGUS_PROPERTY_VALUE)); } return Absent; }🤖 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/expression/mod.rs` around lines 136 - 168, When `T![...]` appears with no preceding expression in parse_scss_expression_item, don't just bump and return Absent (which leaves the token unattached); instead, create and complete a small bogus node that contains the `...` token so the token is represented in the CST. Concretely, in parse_scss_expression_item where the match arm currently does p.error(...); p.bump(T![...]); return Absent;, use the parser node API (start/precede + complete) to wrap the bumped `T![...]` into a completed node (e.g., SCSS_BOGUS_EXPRESSION or a clearly named bogus/placeholder node) and return Present of that node while still emitting the same scss_ellipsis_not_allowed error; ensure you use the same parser helpers used elsewhere (precede/complete or start/complete) so the node is attached under SCSS_EXPRESSION_ITEM_LIST.
🤖 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/syntax/scss/expression/mod.rs`:
- Around line 136-168: When `T![...]` appears with no preceding expression in
parse_scss_expression_item, don't just bump and return Absent (which leaves the
token unattached); instead, create and complete a small bogus node that contains
the `...` token so the token is represented in the CST. Concretely, in
parse_scss_expression_item where the match arm currently does p.error(...);
p.bump(T![...]); return Absent;, use the parser node API (start/precede +
complete) to wrap the bumped `T![...]` into a completed node (e.g.,
SCSS_BOGUS_EXPRESSION or a clearly named bogus/placeholder node) and return
Present of that node while still emitting the same scss_ellipsis_not_allowed
error; ensure you use the same parser helpers used elsewhere (precede/complete
or start/complete) so the node is attached under SCSS_EXPRESSION_ITEM_LIST.
In `@xtask/codegen/src/css_kinds_src.rs`:
- Around line 575-576: Minor ordering mismatch: swap "SCSS_KEYWORD_ARGUMENT" and
"SCSS_ARBITRARY_ARGUMENT" in the SCSS list so that SCSS_ARBITRARY_ARGUMENT comes
before SCSS_KEYWORD_ARGUMENT; locate the SCSS enum/array in css_kinds_src.rs
(the entries "SCSS_KEYWORD_ARGUMENT" and "SCSS_ARBITRARY_ARGUMENT") and reorder
those two entries to match the rest of the codebase's
alphabetical/Arbitrary-before-Keyword convention.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
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/ok/scss/expression/keyword-argument-context.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/keyword-args-ellipsis.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 (16)
crates/biome_css_formatter/src/generated.rscrates/biome_css_formatter/src/scss/any/expression.rscrates/biome_css_formatter/src/scss/any/expression_item.rscrates/biome_css_formatter/src/scss/auxiliary/arbitrary_argument.rscrates/biome_css_formatter/src/scss/auxiliary/keyword_argument.rscrates/biome_css_formatter/src/scss/auxiliary/mod.rscrates/biome_css_parser/src/lexer/mod.rscrates/biome_css_parser/src/syntax/parse_error.rscrates/biome_css_parser/src/syntax/scss/expression/mod.rscrates/biome_css_parser/src/syntax/scss/mod.rscrates/biome_css_parser/src/syntax/value/function.rscrates/biome_css_parser/tests/css_test_suite/ok/scss/expression/keyword-argument-context.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/value/keyword-args-ellipsis.scsscrates/biome_grit_patterns/src/grit_target_language/css_target_language/generated_mappings.rsxtask/codegen/css.ungramxtask/codegen/src/css_kinds_src.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_css_parser/src/lexer/mod.rs (1)
1145-1204: Consider not gating//lexing on parser options.Right now
allow_wrong_line_commentschanges token emission, which can hide invalid syntax and reduce diagnostic quality. Consider always lexing line comments and surfacing a diagnostic when not permitted.Based on learnings, "In the Biome CSS parser, lexer token emission should not be gated behind parser options like
is_tailwind_directives_enabled(). The lexer must emit correct tokens regardless of parser options to enable accurate diagnostics and error messages when the syntax is used incorrectly."
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_css_parser/src/lexer/mod.rs`:
- Around line 251-277: The rustdoc examples in the comments for the methods
is_scss and is_line_comment_enabled are plain code blocks and must be converted
into doctest format; change the examples to fenced ```rust blocks containing
small, compilable assertions (e.g., create a string like let scss = "@if $a ==
$b { `@include` foo($args...); }"; assert!(scss.contains("...")); and let scss =
"// overrides\n$color: red;"; assert!(scss.starts_with("//"));), so the doc
comments become runnable doctests and satisfy the repo guideline.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/biome_css_parser/src/lexer/mod.rscrates/biome_css_parser/src/syntax/value/function.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_css_parser/src/syntax/value/function.rs
| /// Returns true when lexing SCSS so SCSS-only tokens (`==`, `!=`, `...`, `//`) are enabled. | ||
| /// | ||
| /// Example: | ||
| /// ```scss | ||
| /// @if $a == $b { @include foo($args...); } | ||
| /// ``` | ||
| /// | ||
| /// Docs: https://sass-lang.com/documentation/operators | ||
| #[inline] | ||
| fn is_scss(&self) -> bool { | ||
| self.source_type.is_scss() | ||
| } | ||
|
|
||
| /// Enables `//` line-comment lexing only for SCSS (or permissive mode), since | ||
| /// plain CSS treats `//` as two delimiters, not a comment. | ||
| /// | ||
| /// Example: | ||
| /// ```scss | ||
| /// // overrides | ||
| /// $color: red; | ||
| /// ``` | ||
| /// | ||
| /// Docs: https://sass-lang.com/documentation/syntax/comments | ||
| #[inline] | ||
| fn is_line_comment_enabled(&self) -> bool { | ||
| self.options.allow_wrong_line_comments || self.is_scss() | ||
| } |
There was a problem hiding this comment.
Convert the rustdoc examples to doctest format.
These code blocks are in rustdoc but aren’t in doctest form. This breaks the repo guideline and leaves the examples unverified.
💡 Proposed update
- /// ```scss
- /// `@if` $a == $b { `@include` foo($args...); }
- /// ```
+ /// ```rust
+ /// let scss = "@if $a == $b { `@include` foo($args...); }";
+ /// assert!(scss.contains("..."));
+ /// ```- /// ```scss
- /// // overrides
- /// $color: red;
- /// ```
+ /// ```rust
+ /// let scss = "// overrides\n$color: red;";
+ /// assert!(scss.starts_with("//"));
+ /// ```As per coding guidelines, "Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests".
🤖 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` around lines 251 - 277, The rustdoc
examples in the comments for the methods is_scss and is_line_comment_enabled are
plain code blocks and must be converted into doctest format; change the examples
to fenced ```rust blocks containing small, compilable assertions (e.g., create a
string like let scss = "@if $a == $b { `@include` foo($args...); }";
assert!(scss.contains("...")); and let scss = "// overrides\n$color: red;";
assert!(scss.starts_with("//"));), so the doc comments become runnable doctests
and satisfy the repo guideline.
2 participants
Summary
Adds SCSS keyword and arbitrary argument support in function call argument context.
$name: valueand$args...as SCSS expression items in arguments.Test Plan
cargo test -p biome_css_parser -p biome_css_formatter