feat(css): add SCSS unary and binary expression#9222
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 SCSS binary and unary expression support across parser, lexer, formatter and tests. New AST nodes ScssBinaryExpression and ScssUnaryExpression and token 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}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/biome_css_parser/src/syntax/scss/expression/mod.rs (2)
56-62:parse_scss_inner_expression_untilis identical toparse_scss_expression_until.Both call
parse_scss_expression_with_options(p, end_ts, false). If the split is intentional for future differentiation, a brief comment would help readers understand why both exist. Otherwise, callers inside this module (lines 190, 223, 242, 281) could useparse_scss_expression_untildirectly.🤖 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 56 - 62, parse_scss_inner_expression_until is redundant with parse_scss_expression_until because both simply call parse_scss_expression_with_options(p, end_ts, false); either remove the duplicate or add a short explanatory comment. Locate the function parse_scss_inner_expression_until and either (A) delete it and update internal callers (the ones currently using parse_scss_inner_expression_until — referenced near the other callers around the parse_scss_expression_until usage) to call parse_scss_expression_until directly, or (B) keep it but add a one-line comment above parse_scss_inner_expression_until explaining the intended future differentiation, referencing parse_scss_expression_with_options to make the intent clear. Ensure no external API breakage by only changing callers inside the module.
103-128: The Pratt parser structure looks solid, but there's no test coverage for degenerate consecutive-operator input like$a + * $c.The concern about cascading operator consumption is actually correct error recovery behaviour: after bumping the
+operator and failing to parse the RHS, the loop re-enters and sees*as the next operator, which legitimately triggers another parsing attempt with its higher precedence. Each missing operand gets a diagnostic viaor_add_diagnostic, which is intentional. However, there's currently no explicit test case covering this scenario—consider adding one to the error test suite to ensure the diagnostic output remains user-friendly.🤖 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 103 - 128, Add a unit test covering the degenerate consecutive-operator case (e.g. "$a + * $c") to the parser error tests to ensure parse_scss_binary_expression produces readable diagnostics for each missing operand; specifically feed that input through the same test harness used for other error cases, assert that scss_binary_precedence-driven recovery yields diagnostics from or_add_diagnostic tied to expected_component_value for each missing RHS, and verify the final AST/diagnostic messages remain clear and not cryptic.
🤖 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 56-62: parse_scss_inner_expression_until is redundant with
parse_scss_expression_until because both simply call
parse_scss_expression_with_options(p, end_ts, false); either remove the
duplicate or add a short explanatory comment. Locate the function
parse_scss_inner_expression_until and either (A) delete it and update internal
callers (the ones currently using parse_scss_inner_expression_until — referenced
near the other callers around the parse_scss_expression_until usage) to call
parse_scss_expression_until directly, or (B) keep it but add a one-line comment
above parse_scss_inner_expression_until explaining the intended future
differentiation, referencing parse_scss_expression_with_options to make the
intent clear. Ensure no external API breakage by only changing callers inside
the module.
- Around line 103-128: Add a unit test covering the degenerate
consecutive-operator case (e.g. "$a + * $c") to the parser error tests to ensure
parse_scss_binary_expression produces readable diagnostics for each missing
operand; specifically feed that input through the same test harness used for
other error cases, assert that scss_binary_precedence-driven recovery yields
diagnostics from or_add_diagnostic tied to expected_component_value for each
missing RHS, and verify the final AST/diagnostic messages remain clear and not
cryptic.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/biome_css_parser/src/syntax/scss/expression/mod.rscrates/biome_css_parser/src/syntax/scss/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
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/expression/mod.rs`:
- Around line 151-154: The rustdoc above the SCSS binary operator precedence
function is a link-only doc comment and should be converted to either a plain
comment or a doctest-style rustdoc; update the triple-slash doc (the comment
starting "Returns the precedence level for the current SCSS binary operator
token.") to a normal line comment (//) or add a minimal doctest code block
showing expected behavior (e.g., an example call and assertion) so it satisfies
the crate's doctest guideline for the precedence-determining function in this
module.
| /// Returns the precedence level for the current SCSS binary operator token. | ||
| /// | ||
| /// Docs: https://sass-lang.com/documentation/operators/#order-of-operations | ||
| #[inline] |
There was a problem hiding this comment.
Rustdoc here should be doctest-formatted (or be a plain comment).
Guidelines ask for doctest-style rustdoc, but this is a link-only doc comment. Consider switching to a normal comment or adding a minimal doctest block.
💡 Minimal fix: convert to a non-rustdoc comment
-/// Returns the precedence level for the current SCSS binary operator token.
-///
-/// Docs: https://sass-lang.com/documentation/operators/#order-of-operations
+// Returns the precedence level for the current SCSS binary operator token.
+// Docs: https://sass-lang.com/documentation/operators/#order-of-operations📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Returns the precedence level for the current SCSS binary operator token. | |
| /// | |
| /// Docs: https://sass-lang.com/documentation/operators/#order-of-operations | |
| #[inline] | |
| // Returns the precedence level for the current SCSS binary operator token. | |
| // Docs: https://sass-lang.com/documentation/operators/#order-of-operations | |
| #[inline] |
🤖 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 151 -
154, The rustdoc above the SCSS binary operator precedence function is a
link-only doc comment and should be converted to either a plain comment or a
doctest-style rustdoc; update the triple-slash doc (the comment starting
"Returns the precedence level for the current SCSS binary operator token.") to a
normal line comment (//) or add a minimal doctest code block showing expected
behavior (e.g., an example call and assertion) so it satisfies the crate's
doctest guideline for the precedence-determining function in this module.
3 participants
Summary
New Scss syntax:
Test Plan
cargo test -p biome_css_parser
cargo test -p biome_css_formatter