feat(css): improve SCSS expression parsing#9396
Merged
denbezrukov merged 6 commits intomainfrom Mar 8, 2026
Merged
Conversation
|
github-actions
bot
added
A-Parser
Contributor
Parser conformance results onjs/262
jsx/babel
markdown/commonmark
symbols/microsoft
ts/babel
ts/microsoft
|
Merging this PR will not alter performance
Comparing 
Footnotes
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR modularises and extends SCSS parsing and formatting across the css crates: it splits SCSS expression parsing into submodules (list, map, precedence, primary), adds SCSS identifier/qualified-name and function-name parsing, introduces precedence-based unary/binary parsing, implements dedicated SCSS variable‑modifier parsing and recovery, centralises SCSS token sets and diagnostics, wires Possibly related PRs
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
crates/biome_css_parser/tests/css_test_suite/ok/scss/value/function-call.scss (1)
22-24: Minor: trailing blank lines.There are a few empty lines at the end of the file. Not a blocker, but you might tidy these up if you're touching the file anyway.
🤖 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/value/function-call.scss` around lines 22 - 24, Remove the trailing blank lines at the end of function-call.scss (the test file for scss value function-call) so the file ends with a single newline only; open function-call.scss, delete any extra empty lines at EOF and ensure there is exactly one terminating newline character, then save and commit the tidy change.crates/biome_css_parser/src/syntax/scss/expression/primary.rs (1)
11-19: Docstring example could be more illustrative.The example
foo($x: 1, $y: 2)demonstrates keyword arguments rather than the primary expression constructs this function handles (parenthesised values, maps,!important, fallback values). Consider updating to something like(key: value)or!important.🤖 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/primary.rs` around lines 11 - 19, Update the docstring in primary.rs so the illustrative example matches the kinds of SCSS primaries this parser handles: replace the current foo($x: 1, $y: 2) example with one or more examples showing a parenthesized value and a map and an !important/fallback usage (e.g. a parenthesized map like (key: value), and a declaration using !important or the SCSS fallback syntax) so readers immediately see the constructs parsed by this module.crates/biome_css_parser/src/syntax/scss/expression/mod.rs (1)
69-75: Clarify the hardcoded)check.The function always treats
)as an expression end, regardless ofoptions.end_ts. If this is intentional (e.g., for nested parenthesised contexts), a brief inline comment would clarify the design decision.💡 Optional clarification
#[inline] pub(super) fn is_at_scss_expression_end( p: &mut crate::parser::CssParser, options: ScssExpressionOptions, ) -> bool { + // `)` always ends an expression—parenthesised sub-expressions must not + // consume the closing paren. p.at_ts(options.end_ts) || p.at(T![')']) }🤖 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 69 - 75, The helper is_at_scss_expression_end currently treats a closing parenthesis ')' as an end unconditionally which can be confusing; either add a concise inline comment explaining that ')' must always terminate SCSS expressions (e.g., to handle nested parenthesized contexts) or change the logic to respect options.end_ts (for example only treat ')' as end when options.allow_paren_end is true). Update the function is_at_scss_expression_end to include the chosen behavior and reference options.end_ts and the ')' check in your comment so future readers know why ')' is handled specially.
🤖 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/syntax/scss/declaration/variable.rs`:
- Around line 50-52: Replace the permissive bump call with a required expect for
the colon: in the code that calls parse_scss_declaration_name(p) then
p.bump(T![:]), change p.bump(T![:]) to p.expect(T![:]) so the parser emits a
diagnostic when the required ':' is missing (this affects the variable
declaration parsing flow around parse_scss_declaration_name and the parser
instance `p`).
In `@crates/biome_css_parser/src/syntax/scss/parse_error.rs`:
- Line 62: The hint currently uses p.cur_text() which only targets the current
token; instead produce a hint that targets the entire modifier tail (the full
`!…` range). Update the call site using with_hint(format!(...)) so it formats
and inserts the modifier-level text/span (the slice/span covering the whole
modifier tail) rather than p.cur_text(); locate the code that builds the
modifier error (the with_hint call that uses p.cur_text()) and replace
p.cur_text() with the parser method or slice that returns the full modifier
text/span for the error hint.
In `@crates/biome_css_parser/src/syntax/scss/token_sets.rs`:
- Around line 12-27: SCSS statement-start recovery set is missing the percent
token; update the SCSS_STATEMENT_START_SET token_set! to include T![%] so
modifier-tail resynchronization properly stops at a % (e.g., in `$x: 1 !oops
%placeholder {}`) and doesn't swallow the following rule—locate the
SCSS_STATEMENT_START_SET constant and add T![%] to the list of tokens inside the
token_set! macro.
In `@crates/biome_css_parser/src/syntax/value/function/call.rs`:
- Around line 81-83: The lookahead using p.nth_at(2, T![style]) || p.nth_at(2,
T![media]) || p.nth_at(2, T![supports]) is too permissive; change the check to
require the opening parenthesis after those identifiers so only function calls
like style(...), media(...), supports(...) match. Update the condition that uses
p.nth_at with T![style], T![media], T![supports] to also assert the next token
is an open paren (the '(' token) — i.e., only treat these as CSS functions when
the identifier is immediately followed by '('.
In `@crates/biome_css_parser/tests/css_test_suite/ok/scss/comment/multiline.scss`:
- Around line 41-46: The inner block comment inside the commented `@mixin` (the
nested /* Comment inside mixin */ within the `@mixin` test-mixin block)
prematurely closes the outer block comment and turns the trailing `}` into live
invalid SCSS; fix by replacing the inner block comment with a line comment (use
// Comment inside mixin) or remove the inner comment entirely so the outer /*
TODO... */ remains intact and the fixture stays valid.
In `@crates/biome_css_parser/tests/css_test_suite/ok/scss/value/list.scss`:
- Line 5: The double-slash TODO comment "// TODO(interpolation support):
$no-whitespaces: $variable#{something};" triggers
scss/double-slash-comment-empty-line-before; fix by inserting a blank line
before that TODO (or place the TODO on its own separated line) so the comment is
separated from the preceding declaration; locate the TODO comment referencing
$no-whitespaces and ensure a single empty line precedes it.
---
Nitpick comments:
In `@crates/biome_css_parser/src/syntax/scss/expression/mod.rs`:
- Around line 69-75: The helper is_at_scss_expression_end currently treats a
closing parenthesis ')' as an end unconditionally which can be confusing; either
add a concise inline comment explaining that ')' must always terminate SCSS
expressions (e.g., to handle nested parenthesized contexts) or change the logic
to respect options.end_ts (for example only treat ')' as end when
options.allow_paren_end is true). Update the function is_at_scss_expression_end
to include the chosen behavior and reference options.end_ts and the ')' check in
your comment so future readers know why ')' is handled specially.
In `@crates/biome_css_parser/src/syntax/scss/expression/primary.rs`:
- Around line 11-19: Update the docstring in primary.rs so the illustrative
example matches the kinds of SCSS primaries this parser handles: replace the
current foo($x: 1, $y: 2) example with one or more examples showing a
parenthesized value and a map and an !important/fallback usage (e.g. a
parenthesized map like (key: value), and a declaration using !important or the
SCSS fallback syntax) so readers immediately see the constructs parsed by this
module.
In
`@crates/biome_css_parser/tests/css_test_suite/ok/scss/value/function-call.scss`:
- Around line 22-24: Remove the trailing blank lines at the end of
function-call.scss (the test file for scss value function-call) so the file ends
with a single newline only; open function-call.scss, delete any extra empty
lines at EOF and ensure there is exactly one terminating newline character, then
save and commit the tidy change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 54e320ff-a7dc-4369-b369-a73ba8461cce
⛔ Files ignored due to path filters (29)
crates/biome_css_formatter/tests/specs/css/scss/expression/edge-cases.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/expression/formatting.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/scss/declaration/duplicate-modifier-recovery.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/scss/declaration/invalid-modifier.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/scss/declaration/modifier-recovery.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/scss/expression/list-map-paren.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/scss/value/colon-value.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/scss/value/if-disambiguation-recovery.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/comment/multiline.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/expression/complex-expressions.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/expression/fallback-values.scss.snapis excluded by!**/*.snapand 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/recovery/list-space-trailing-comma.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/binary-operators.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/builtin-modules.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/function-call.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/functions-advanced.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/if-disambiguation.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/list-advanced.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/list-empty-elements.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/list-important-item.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/list-space.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/list.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/map-advanced.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/map-expression-key.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/map-list-access.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/map.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/url.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_syntax/src/generated/nodes.rsis excluded by!**/generated/**,!**/generated/**and included by**
📒 Files selected for processing (44)
crates/biome_css_formatter/src/scss/any/expression_item.rscrates/biome_css_parser/src/lib.rscrates/biome_css_parser/src/syntax/at_rule/tailwind.rscrates/biome_css_parser/src/syntax/parse_error.rscrates/biome_css_parser/src/syntax/scss/declaration/mod.rscrates/biome_css_parser/src/syntax/scss/declaration/nesting.rscrates/biome_css_parser/src/syntax/scss/declaration/variable.rscrates/biome_css_parser/src/syntax/scss/declaration/variable_modifier.rscrates/biome_css_parser/src/syntax/scss/expression/list.rscrates/biome_css_parser/src/syntax/scss/expression/map.rscrates/biome_css_parser/src/syntax/scss/expression/mod.rscrates/biome_css_parser/src/syntax/scss/expression/precedence.rscrates/biome_css_parser/src/syntax/scss/expression/primary.rscrates/biome_css_parser/src/syntax/scss/function_name.rscrates/biome_css_parser/src/syntax/scss/identifiers/identifier.rscrates/biome_css_parser/src/syntax/scss/identifiers/mod.rscrates/biome_css_parser/src/syntax/scss/identifiers/qualified_name.rscrates/biome_css_parser/src/syntax/scss/mod.rscrates/biome_css_parser/src/syntax/scss/parse_error.rscrates/biome_css_parser/src/syntax/scss/token_sets.rscrates/biome_css_parser/src/syntax/scss/value.rscrates/biome_css_parser/src/syntax/value/function/call.rscrates/biome_css_parser/tests/css_test_suite/error/scss/declaration/invalid-modifier.scsscrates/biome_css_parser/tests/css_test_suite/error/scss/declaration/modifier-recovery.scsscrates/biome_css_parser/tests/css_test_suite/error/scss/expression/list-map-paren.scsscrates/biome_css_parser/tests/css_test_suite/error/scss/value/colon-value.scsscrates/biome_css_parser/tests/css_test_suite/error/scss/value/if-disambiguation-recovery.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/comment/multiline.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/expression/keyword-argument-context.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/value/builtin-modules.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/value/function-call.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/value/functions-advanced.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/value/if-disambiguation.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/value/list-advanced.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/value/list-empty-elements.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/value/list-important-item.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/value/list-space.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/value/list.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/value/map-advanced.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/value/map-expression-key.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/value/map-list-access.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/value/map.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/value/url.scssxtask/codegen/css.ungram
💤 Files with no reviewable changes (3)
- crates/biome_css_parser/src/syntax/parse_error.rs
- crates/biome_css_parser/tests/css_test_suite/error/scss/value/colon-value.scss
- crates/biome_css_parser/tests/css_test_suite/error/scss/expression/list-map-paren.scss
crates/biome_css_parser/tests/css_test_suite/ok/scss/comment/multiline.scss
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/biome_css_parser/src/syntax/value/function/call.rs (1)
81-86:⚠️ Potential issue | 🟠 MajorCSS
if()lookahead needs to verify the opening paren.The checks for
style,media, andsupportsat position 2 don't confirm the following(at position 3. This means Sass-styleif(media, a, b)(wheremediais just a value) would incorrectly be routed as CSSif().Suggested fix
- p.nth_at(2, T![style]) - || p.nth_at(2, T![media]) - || p.nth_at(2, T![supports]) + (p.nth_at(2, T![style]) && p.nth_at(3, T!['('])) + || (p.nth_at(2, T![media]) && p.nth_at(3, T!['('])) + || (p.nth_at(2, T![supports]) && p.nth_at(3, T!['('])) || p.nth_at(2, T![not]) || p.nth_at(2, T![else]) || p.nth_at(2, T!['('])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_parser/src/syntax/value/function/call.rs` around lines 81 - 86, The lookahead in the function handling CSS if() calls is matching tokens like p.nth_at(2, T![style|media|supports]) without ensuring an opening paren follows, so values like `if(media, a, b)` get misclassified; update the condition around p.nth_at to require that when p.nth_at(2, T![style|media|supports|not|else]) is true it also checks p.nth_at(3, T!['(']) (i.e., only treat those keywords as the CSS if() form when the next token is '('), leaving the existing direct p.nth_at(2, T!['(']) case unchanged; adjust the combined boolean expression in the call detection logic (the p.nth_at checks in call.rs) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/biome_css_parser/src/syntax/value/function/call.rs`:
- Around line 81-86: The lookahead in the function handling CSS if() calls is
matching tokens like p.nth_at(2, T![style|media|supports]) without ensuring an
opening paren follows, so values like `if(media, a, b)` get misclassified;
update the condition around p.nth_at to require that when p.nth_at(2,
T![style|media|supports|not|else]) is true it also checks p.nth_at(3, T!['('])
(i.e., only treat those keywords as the CSS if() form when the next token is
'('), leaving the existing direct p.nth_at(2, T!['(']) case unchanged; adjust
the combined boolean expression in the call detection logic (the p.nth_at checks
in call.rs) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b30e6a84-bf89-4038-803d-f1427ede1716
📒 Files selected for processing (6)
crates/biome_css_parser/src/syntax/scss/declaration/variable_modifier.rscrates/biome_css_parser/src/syntax/scss/expression/list.rscrates/biome_css_parser/src/syntax/scss/expression/precedence.rscrates/biome_css_parser/src/syntax/scss/mod.rscrates/biome_css_parser/src/syntax/scss/parse_error.rscrates/biome_css_parser/src/syntax/value/function/call.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_css_parser/src/syntax/scss/declaration/variable_modifier.rs
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_css_parser/src/syntax/scss/declaration/variable_modifier.rs (1)
52-57: Consider simplifying withp.bump_any().Since you've already confirmed the token is in
SCSS_VARIABLE_MODIFIER_TOKEN_SET, the explicit if-else isn't strictly necessary here. However, the current explicit approach is perfectly readable and makes the intent clear.🔧 Optional simplification
if p.at_ts(SCSS_VARIABLE_MODIFIER_TOKEN_SET) { - if p.at(T![default]) { - p.bump(T![default]); - } else { - p.bump(T![global]); - } + p.bump_any(); } else {🤖 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/declaration/variable_modifier.rs` around lines 52 - 57, The branch that checks p.at_ts(SCSS_VARIABLE_MODIFIER_TOKEN_SET) then distinguishes T![default] vs T![global] can be simplified: once p.at_ts(SCSS_VARIABLE_MODIFIER_TOKEN_SET) is true, call p.bump_any() instead of the explicit if/else that checks p.at(T![default]) and p.bump(T![default]) or p.bump(T![global]); replace that inner conditional with p.bump_any() to consume whichever modifier token is present (leave the outer p.at_ts(...) guard intact).
🤖 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/declaration/variable_modifier.rs`:
- Around line 52-57: The branch that checks
p.at_ts(SCSS_VARIABLE_MODIFIER_TOKEN_SET) then distinguishes T![default] vs
T![global] can be simplified: once p.at_ts(SCSS_VARIABLE_MODIFIER_TOKEN_SET) is
true, call p.bump_any() instead of the explicit if/else that checks
p.at(T![default]) and p.bump(T![default]) or p.bump(T![global]); replace that
inner conditional with p.bump_any() to consume whichever modifier token is
present (leave the outer p.at_ts(...) guard intact).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bb7a100f-27ea-45f4-af50-2edf446e86d5
⛔ Files ignored due to path filters (3)
crates/biome_css_parser/tests/css_test_suite/error/scss/declaration/invalid-modifier.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/scss/declaration/missing-modifier.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/scss/declaration/modifier-recovery.scss.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (4)
crates/biome_css_parser/src/syntax/scss/declaration/variable_modifier.rscrates/biome_css_parser/src/syntax/scss/mod.rscrates/biome_css_parser/src/syntax/scss/parse_error.rscrates/biome_css_parser/src/syntax/scss/token_sets.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/syntax/scss/declaration/variable_modifier.rs`:
- Around line 87-89: Remove the erroneous newline-based short-circuit from the
modifier-list boundary logic: in is_at_list_end (and any other checks using
has_preceding_line_break for variable modifiers) stop treating
has_preceding_line_break() as a termination condition and rely solely on
SCSS_VARIABLE_MODIFIER_LIST_END_SET (which already includes ;, }, EOF) to
determine list end; update the function is_at_list_end to only check
p.at_ts(SCSS_VARIABLE_MODIFIER_LIST_END_SET) and remove or refactor any uses of
has_preceding_line_break() in the variable modifier parsing path so wrapped
declarations like `$x: 1\n!default;` parse correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4446bfdf-c7c6-4aab-8c78-27b50383d02d
📒 Files selected for processing (1)
crates/biome_css_parser/src/syntax/scss/declaration/variable_modifier.rs
crates/biome_css_parser/src/syntax/scss/declaration/variable_modifier.rs
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/biome_css_parser/src/syntax/scss/declaration/variable_modifier.rs (1)
114-117:⚠️ Potential issue | 🟠 MajorRemove
has_preceding_line_break()from recovery logic.This mirrors the earlier fix applied to
is_at_list_endat line 89. SCSS doesn't treat newlines as statement terminators, so a wrapped declaration like$x: 1\n!default;should still parse the modifier correctly. The existingSCSS_VARIABLE_MODIFIER_LIST_END_SETandis_at_scss_statement_boundarychecks already encode proper boundaries.Suggested fix
fn is_at_recovered(&self, p: &mut Self::Parser<'_>) -> bool { - p.at(T![!]) || p.at_ts(SCSS_VARIABLE_MODIFIER_LIST_END_SET) || p.has_preceding_line_break() - || is_at_scss_statement_boundary(p) + p.at(T![!]) || p.at_ts(SCSS_VARIABLE_MODIFIER_LIST_END_SET) + || is_at_scss_statement_boundary(p) }🤖 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/declaration/variable_modifier.rs` around lines 114 - 117, In is_at_recovered (function) remove the p.has_preceding_line_break() check from the recovery condition so it becomes: check for T![!] OR SCSS_VARIABLE_MODIFIER_LIST_END_SET OR is_at_scss_statement_boundary(p); this mirrors the fix in is_at_list_end and ensures a newline (e.g. between value and !default) does not break modifier parsing while keeping the existing T![!] token and SCSS_VARIABLE_MODIFIER_LIST_END_SET / is_at_scss_statement_boundary checks intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/biome_css_parser/src/syntax/scss/declaration/variable_modifier.rs`:
- Around line 114-117: In is_at_recovered (function) remove the
p.has_preceding_line_break() check from the recovery condition so it becomes:
check for T![!] OR SCSS_VARIABLE_MODIFIER_LIST_END_SET OR
is_at_scss_statement_boundary(p); this mirrors the fix in is_at_list_end and
ensures a newline (e.g. between value and !default) does not break modifier
parsing while keeping the existing T![!] token and
SCSS_VARIABLE_MODIFIER_LIST_END_SET / is_at_scss_statement_boundary checks
intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 66221eca-8942-4868-a5cf-d9846bbe807b
📒 Files selected for processing (3)
crates/biome_css_parser/src/syntax/scss/declaration/variable_modifier.rscrates/biome_css_parser/src/syntax/scss/mod.rscrates/biome_css_parser/src/syntax/scss/token_sets.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_css_parser/src/syntax/scss/token_sets.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Improves SCSS expression parsing and recovery. Its more about splitting and refactorning.
Biome now parses SCSS lists, maps, module-qualified function calls, and mixed expression forms
more consistently, including sparse comma-separated lists like
1,,3. It also improves SCSSvariable declaration recovery for malformed identifiers and modifier tails, and adds formatter
support for
!importantwhen it appears as an SCSS expression item.Test Plan
New tests.
cargo test -p biome_css_parser/formatter