feat(css): support scss nesting declarations in declaration lists#9135
feat(css): support scss nesting declarations in declaration lists#9135denbezrukov merged 2 commits intonextfrom
Conversation
|
github-actions
bot
added
A-Linter
Merging this PR will not alter performance
Comparing 
Footnotes
|
WalkthroughThis pull request introduces support for SCSS nesting declarations throughout the Biome CSS toolchain. It adds a new ScssNestingDeclaration AST node defined in the grammar, implements parsing logic with detection and recovery mechanisms for SCSS nested property syntax, adds formatting support for the new node type across multiple formatter matchers, updates the CSS lexer with whitespace-tracking state, and propagates 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 (8)
crates/biome_css_analyze/src/assist/source/use_sorted_properties.rs (1)
246-246: LGTM —UnknownKindis a safe choice;NestedRuleOrAtRuleis worth considering.Consistent with the
ScssDeclarationarm immediately above. Mapping toUnknownKindis conservative and risk-free: ties are broken by source position, so relative order of SCSS nesting declarations is preserved.One optional thought: a SCSS nesting declaration (
font: { size: 10px; }) is semantically a nested block, soNestedRuleOrAtRulecould be argued as the more precise bucket — sorting it alongside.child {}rather than after it. Up to you whether that matters for the intended UX.♻️ Alternative: treat as a nested rule rather than unknown
- AnyCssDeclarationOrRule::ScssNestingDeclaration(_) => NodeKindOrder::UnknownKind, + AnyCssDeclarationOrRule::ScssNestingDeclaration(_) => NodeKindOrder::NestedRuleOrAtRule,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_analyze/src/assist/source/use_sorted_properties.rs` at line 246, The match arm mapping AnyCssDeclarationOrRule::ScssNestingDeclaration currently returns NodeKindOrder::UnknownKind; if you prefer nested SCSS blocks to sort with rules rather than after them, change that arm to return NodeKindOrder::NestedRuleOrAtRule instead (update the match in use_sorted_properties.rs where ScssNestingDeclaration is handled). Ensure the symbol names AnyCssDeclarationOrRule::ScssNestingDeclaration and NodeKindOrder::NestedRuleOrAtRule are used so the declaration is categorized as a nested rule.crates/biome_css_formatter/tests/specs/css/scss/declaration/ambiguous-selector-vs-nesting.scss (1)
1-6: Missing test case for the double-colon (::) pseudo-element path.The PR description calls out
font::beforeas a canonical example of the "no-whitespace" disambiguation — it should be parsed as a nested selector, not a nesting declaration. But no test here exercises that code path.Consider adding:
+font::before { + content: ""; +}inside
.testto confirm that a double-colon pseudo-element is correctly recognised as a nested rule rather than silently misclassified as a nesting declaration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_formatter/tests/specs/css/scss/declaration/ambiguous-selector-vs-nesting.scss` around lines 1 - 6, Add a test line inside the .test block that exercises the double-colon pseudo-element disambiguation (e.g., use "font::before{color:green;}" or similar) so the parser treats "font::before" as a nested selector rather than a nesting declaration; update the test file's .test block alongside existing cases (selectors like "label:hover", "font:bold", "font:12px", "font: bold") to include the "font::before" nested rule to verify the no-whitespace double-colon path is handled.crates/biome_css_parser/tests/quick_test.rs (1)
22-23: Unrelated options in quick test.
allow_metavariables()andallow_tailwind_directives()are not exercised by the current snippet. Trim them so the test stays focused on what's actually being debugged.✂️ Suggested trim
CssParserOptions::default() .allow_wrong_line_comments() .allow_css_modules() - .allow_metavariables() - .allow_tailwind_directives(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_parser/tests/quick_test.rs` around lines 22 - 23, The test config is enabling unrelated options allow_metavariables() and allow_tailwind_directives() that aren’t used by the snippet; remove those method calls from the parser/config builder in quick_test.rs (i.e., drop allow_metavariables() and allow_tailwind_directives() from the chained configuration) so the test only sets options relevant to the behavior being exercised.crates/biome_css_parser/src/syntax/declaration.rs (1)
25-31: Theparse_exclusive_syntaxerror callback is unreachable.
is_at_scss_nesting_declarationalready returnsfalsewhenCssSyntaxFeatures::Scssis not supported, so this branch is only entered when SCSS is supported — meaning thescss_only_syntax_errorcallback can never fire.This is the opposite design to
is_at_scss_declaration(which deliberately omits the feature check, lettingparse_exclusive_syntaxhandle the non-SCSS case). Since the feature check must stay insideis_at_scss_nesting_declaration(to prevent false positives on regular CSS identifier +:tokens), you could simplifyparse_elementto callparse_scss_nesting_declaration(p)directly:♻️ Suggested simplification
fn parse_element(&mut self, p: &mut Self::Parser<'_>) -> ParsedSyntax { if is_at_scss_nesting_declaration(p) { - CssSyntaxFeatures::Scss.parse_exclusive_syntax( - p, - parse_scss_nesting_declaration, - |p, marker| scss_only_syntax_error(p, "SCSS nesting declarations", marker.range(p)), - ) + parse_scss_nesting_declaration(p) } else { parse_any_declaration_with_semicolon(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/declaration.rs` around lines 25 - 31, The parse_exclusive_syntax error callback is unreachable because is_at_scss_nesting_declaration already guards for CssSyntaxFeatures::Scss; update the branch so it calls parse_scss_nesting_declaration(p) directly (instead of CssSyntaxFeatures::Scss.parse_exclusive_syntax(..., scss_only_syntax_error, ...)), keeping the feature check inside is_at_scss_nesting_declaration to avoid false positives and removing the now-unnecessary scss_only_syntax_error callback in this path.crates/biome_css_parser/src/syntax/block/declaration_or_rule_list_block.rs (1)
111-143: Three-phase disambiguation is correct but complex — consider a clarifying comment.The logic is:
- Speculative declaration parse: if the ambiguous
name:identform resolves to a semicolon-terminatedCSS_DECLARATION_WITH_SEMICOLON, accept it.- Speculative strict selector parse: if the form opens a
{ }block as a qualified rule, accept it.- Fallback: parse as
ScssNestingDeclarationnon-speculatively.This matches Sass's declaration-first strategy. One nit: inside the
try_parseclosure (lines 122–129),SCSS_NESTING_DECLARATIONis explicitly rejected because the block form is ambiguous and should prefer the selector path. A one-line comment on line 122 explaining why the block form is rejected here would save the next reader a few minutes.💡 Suggested comment
match declaration.kind(p) { - Some(SCSS_NESTING_DECLARATION) => Err(()), + // Block-form is ambiguous; reject so we can try selector parsing next. + Some(SCSS_NESTING_DECLARATION) => Err(()),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_parser/src/syntax/block/declaration_or_rule_list_block.rs` around lines 111 - 143, Add a one-line clarifying comment inside the try_parse closure in declaration_or_rule_list_block.rs (around the match arm rejecting SCSS_NESTING_DECLARATION) explaining that a block-form SCSS_NESTING_DECLARATION is rejected in the speculative declaration phase because the three-phase disambiguation prefers treating ambiguous block forms as selectors (qualified rules) rather than declarations; reference the match on declaration.kind(p) and the SCSS_NESTING_DECLARATION symbol so readers immediately understand why that arm returns Err(()).crates/biome_css_parser/src/syntax/scss/declaration/nesting.rs (2)
83-99:complete_declaration_with_semicolonduplicates logic fromparse_scss_declaration.The semicolon-handling pattern (
if !at('}'),if nth_at(1, '}'), eat-or-expect) is repeated fromparse_scss_declarationinvariable.rs(lines 57–64 in the relevant snippets). A shared helper could reduce this duplication — though I won't insist if you'd rather keep the two paths independent for now.🤖 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/nesting.rs` around lines 83 - 99, The semicolon-handling logic in complete_declaration_with_semicolon is duplicated from parse_scss_declaration (variable.rs); extract the shared pattern into a small helper (e.g., consume_trailing_semicolon or ensure_semicolon_if_not_closing) that accepts &mut CssParser and performs: if !p.at('}') then if p.nth_at(1, '}') { p.eat(';') } else { p.expect(';') }; then replace the duplicated blocks in complete_declaration_with_semicolon and parse_scss_declaration to call this helper so both sites reuse the same logic while leaving the surrounding marker handling (declaration.precede / m.complete) unchanged.
101-111: Refactor to eliminate duplication:parse_declaration_importantappears in both files.The function in
nesting.rs(lines 101–111) duplicates the logic indeclaration.rs(lines 149–157). Although thedeclaration.rsversion uses a helper function (is_at_declaration_important) whilenesting.rsinlines the condition, the logic is identical. Extractparse_declaration_importantfromdeclaration.rs, make itpub(crate), and import it innesting.rsto avoid duplication.🤖 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/nesting.rs` around lines 101 - 111, Extract the existing parse_declaration_important implementation from declaration.rs, mark it pub(crate) so it can be reused, and remove the duplicate implementation in nesting.rs; update nesting.rs to import and call declaration::parse_declaration_important instead of inlining the condition (also reuse the existing helper is_at_declaration_important if present), ensuring symbols parse_declaration_important and is_at_declaration_important are referenced to locate the code to change.crates/biome_css_parser/src/syntax/mod.rs (1)
288-297: Abandoned marker inside speculative parse — safe but worth noting.When
disable_selector_recoveryistrueand the selector doesn't reach{, the markermis abandoned at line 295 before returningNone. Since this path is always called insidetry_parse(viatry_parse_nested_qualified_rule_without_selector_recovery), the checkpoint rewind will restore the event list, so the abandoned marker is harmless. Just flagging this coupling for awareness — callingparse_nested_qualified_rule_with_selector_recovery(p, true)outside atry_parsewould leak consumed tokens.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_parser/src/syntax/mod.rs` around lines 288 - 297, The code abandons marker m inside parse_nested_qualified_rule_with_selector_recovery when disable_selector_recovery is true and the selector doesn't reach `{`, which is safe only because callers (try_parse_nested_qualified_rule_without_selector_recovery) currently call it inside try_parse; change the implementation to avoid abandoning the marker here—either remove the m.abandon(p) and simply return None so the caller controls marker rollback, or guard the abandon with an explicit check/assert that we are inside a speculative try_parse context; update parse_nested_qualified_rule_with_selector_recovery (and document its contract) so callers like try_parse_nested_qualified_rule_without_selector_recovery remain valid and external callers are not able to leak consumed tokens.
🤖 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_analyze/src/assist/source/use_sorted_properties.rs`:
- Line 246: The match arm mapping
AnyCssDeclarationOrRule::ScssNestingDeclaration currently returns
NodeKindOrder::UnknownKind; if you prefer nested SCSS blocks to sort with rules
rather than after them, change that arm to return
NodeKindOrder::NestedRuleOrAtRule instead (update the match in
use_sorted_properties.rs where ScssNestingDeclaration is handled). Ensure the
symbol names AnyCssDeclarationOrRule::ScssNestingDeclaration and
NodeKindOrder::NestedRuleOrAtRule are used so the declaration is categorized as
a nested rule.
In
`@crates/biome_css_formatter/tests/specs/css/scss/declaration/ambiguous-selector-vs-nesting.scss`:
- Around line 1-6: Add a test line inside the .test block that exercises the
double-colon pseudo-element disambiguation (e.g., use
"font::before{color:green;}" or similar) so the parser treats "font::before" as
a nested selector rather than a nesting declaration; update the test file's
.test block alongside existing cases (selectors like "label:hover", "font:bold",
"font:12px", "font: bold") to include the "font::before" nested rule to verify
the no-whitespace double-colon path is handled.
In `@crates/biome_css_parser/src/syntax/block/declaration_or_rule_list_block.rs`:
- Around line 111-143: Add a one-line clarifying comment inside the try_parse
closure in declaration_or_rule_list_block.rs (around the match arm rejecting
SCSS_NESTING_DECLARATION) explaining that a block-form SCSS_NESTING_DECLARATION
is rejected in the speculative declaration phase because the three-phase
disambiguation prefers treating ambiguous block forms as selectors (qualified
rules) rather than declarations; reference the match on declaration.kind(p) and
the SCSS_NESTING_DECLARATION symbol so readers immediately understand why that
arm returns Err(()).
In `@crates/biome_css_parser/src/syntax/declaration.rs`:
- Around line 25-31: The parse_exclusive_syntax error callback is unreachable
because is_at_scss_nesting_declaration already guards for
CssSyntaxFeatures::Scss; update the branch so it calls
parse_scss_nesting_declaration(p) directly (instead of
CssSyntaxFeatures::Scss.parse_exclusive_syntax(..., scss_only_syntax_error,
...)), keeping the feature check inside is_at_scss_nesting_declaration to avoid
false positives and removing the now-unnecessary scss_only_syntax_error callback
in this path.
In `@crates/biome_css_parser/src/syntax/mod.rs`:
- Around line 288-297: The code abandons marker m inside
parse_nested_qualified_rule_with_selector_recovery when
disable_selector_recovery is true and the selector doesn't reach `{`, which is
safe only because callers
(try_parse_nested_qualified_rule_without_selector_recovery) currently call it
inside try_parse; change the implementation to avoid abandoning the marker
here—either remove the m.abandon(p) and simply return None so the caller
controls marker rollback, or guard the abandon with an explicit check/assert
that we are inside a speculative try_parse context; update
parse_nested_qualified_rule_with_selector_recovery (and document its contract)
so callers like try_parse_nested_qualified_rule_without_selector_recovery remain
valid and external callers are not able to leak consumed tokens.
In `@crates/biome_css_parser/src/syntax/scss/declaration/nesting.rs`:
- Around line 83-99: The semicolon-handling logic in
complete_declaration_with_semicolon is duplicated from parse_scss_declaration
(variable.rs); extract the shared pattern into a small helper (e.g.,
consume_trailing_semicolon or ensure_semicolon_if_not_closing) that accepts &mut
CssParser and performs: if !p.at('}') then if p.nth_at(1, '}') { p.eat(';') }
else { p.expect(';') }; then replace the duplicated blocks in
complete_declaration_with_semicolon and parse_scss_declaration to call this
helper so both sites reuse the same logic while leaving the surrounding marker
handling (declaration.precede / m.complete) unchanged.
- Around line 101-111: Extract the existing parse_declaration_important
implementation from declaration.rs, mark it pub(crate) so it can be reused, and
remove the duplicate implementation in nesting.rs; update nesting.rs to import
and call declaration::parse_declaration_important instead of inlining the
condition (also reuse the existing helper is_at_declaration_important if
present), ensuring symbols parse_declaration_important and
is_at_declaration_important are referenced to locate the code to change.
In `@crates/biome_css_parser/tests/quick_test.rs`:
- Around line 22-23: The test config is enabling unrelated options
allow_metavariables() and allow_tailwind_directives() that aren’t used by the
snippet; remove those method calls from the parser/config builder in
quick_test.rs (i.e., drop allow_metavariables() and allow_tailwind_directives()
from the chained configuration) so the test only sets options relevant to the
behavior being exercised.
) (cherry picked from commit 7a36937)
3 participants
Summary
rules.
:as an ambiguity signal because Sass does:font:bold,font::before) can be selector-like and isspeculatively parsed as a nested rule (strict selector parse, recovery disabled),
font: bold,font:\n bold) is treated as a nesteddeclaration/value.
Test Plan
cargo test -p biome_css_parsercargo test -p biome_css_formatter