feat(css): add unary expression parsing#9093
Conversation
|
github-actions
bot
added
A-Parser
Merging this PR will not alter performance
Comparing 
Footnotes
|
denbezrukov
force-pushed
the
db/scss-unary-expression
branch
from
59a12ab to
62ca41c
Compare
denbezrukov
changed the title
denbezrukov
force-pushed
the
db/scss-unary-expression
branch
from
62ca41c to
ef11701
Compare
denbezrukov
force-pushed
the
db/scss-unary-expression
branch
from
ef11701 to
279da92
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughAdds first-class support for CSS unary expressions across the repo: grammar (xtask codegen) introduces 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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
crates/biome_css_formatter/tests/specs/css/unary-precedence.css (1)
1-5: Consider expanding test coverage for*unary operator.The PR description mentions
*1%as a supported unary expression, but there's no test case exercising*as a unary operator here. The static analysis hint about spacing on line 2 is expected — these are intentionally malformatted inputs for the formatter to normalise.🤖 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/unary-precedence.css` around lines 1 - 5, Add a test case exercising the unary "*" operator inside the .unary-precedence rule: update the selector block that currently contains the calc examples (the lines with width:calc(-1 +2), width:calc(+var(--x)+1px), width:calc(- var(--y)*2)) by adding at least one calc expression using the unary * form mentioned in the PR (e.g. a calc containing *1% with malformatted spacing variants) so the formatter/lexer handles "*" as a unary operator; ensure the new test mirrors the other intentionally-misformatted inputs to validate normalization.crates/biome_css_parser/src/syntax/value/function.rs (1)
282-291: Minor: consider adding a brief rustdoc comment.The sibling functions (
is_at_binary_operator,parse_parenthesized_expression, etc.) all have doc comments. A one-liner here would keep things consistent. As per coding guidelines: "Use inline rustdoc documentation for rules, assists, and their options."🤖 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.rs` around lines 282 - 291, Add a one-line rustdoc comment to parse_unary_expression explaining its role (e.g., "Parse a unary CSS expression starting at a unary operator"), mirroring siblings like is_at_binary_operator and parse_parenthesized_expression; place the comment immediately above the pub(crate) fn parse_unary_expression(...) signature and mention that it returns ParsedSyntax after consuming the unary operator and parsing the operand via parse_unary_expression_operand.
🤖 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/value/function.rs`:
- Around line 276-279: The function is_at_unary_operator should be gated to
SCSS-only: change its implementation to return true only when the parser is in
SCSS mode and the current token matches UNARY_OPERATION_TOKEN (e.g., implement
as something like if p.is_scss() && p.at_ts(UNARY_OPERATION_TOKEN) return true);
update the is_at_unary_operator function in CssParser accordingly and ensure you
reference UNARY_OPERATION_TOKEN; also move or remove the existing plain-CSS test
(unary-precedence.css) to the SCSS test suite since the syntax is SCSS-specific.
In `@xtask/codegen/css.ungram`:
- Around line 2102-2105: The grammar rule CssUnaryExpression currently includes
'*' as a unary operator which is invalid; edit the CssUnaryExpression operator
list to remove '*' (keep '+' and '-') and, if you intend to support historical
Sass forms, optionally add '/' and 'not' per language spec; after updating the
operator tokens in the CssUnaryExpression rule, run the grammar generator with
`just gen-grammar css` to regenerate artifacts before pushing the PR.
---
Nitpick comments:
In `@crates/biome_css_formatter/tests/specs/css/unary-precedence.css`:
- Around line 1-5: Add a test case exercising the unary "*" operator inside the
.unary-precedence rule: update the selector block that currently contains the
calc examples (the lines with width:calc(-1 +2), width:calc(+var(--x)+1px),
width:calc(- var(--y)*2)) by adding at least one calc expression using the unary
* form mentioned in the PR (e.g. a calc containing *1% with malformatted spacing
variants) so the formatter/lexer handles "*" as a unary operator; ensure the new
test mirrors the other intentionally-misformatted inputs to validate
normalization.
In `@crates/biome_css_parser/src/syntax/value/function.rs`:
- Around line 282-291: Add a one-line rustdoc comment to parse_unary_expression
explaining its role (e.g., "Parse a unary CSS expression starting at a unary
operator"), mirroring siblings like is_at_binary_operator and
parse_parenthesized_expression; place the comment immediately above the
pub(crate) fn parse_unary_expression(...) signature and mention that it returns
ParsedSyntax after consuming the unary operator and parsing the operand via
parse_unary_expression_operand.
| #[inline] | ||
| pub(crate) fn is_at_unary_operator(p: &mut CssParser) -> bool { | ||
| p.at_ts(UNARY_OPERATION_TOKEN) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check for SCSS feature gates in this specific file
rg -n 'CssSyntaxFeatures::Scss' crates/biome_css_parser/src/syntax/value/function.rs
# Check the surrounding context to understand how this file handles SCSS features
rg -n 'is_supported' crates/biome_css_parser/src/syntax/value/function.rs -B 2 -A 2Repository: biomejs/biome
Length of output: 605
🏁 Script executed:
# Search for unary operator tests to understand current test coverage
fd . crates/biome_css_parser/tests -type f -name '*.rs' | xargs rg -l 'unary'
# Look at the actual token definition to understand what's in UNARY_OPERATION_TOKEN
rg -n 'UNARY_OPERATION_TOKEN' crates/biome_css_parser/src/Repository: biomejs/biome
Length of output: 6433
🏁 Script executed:
# Check CSS spec: in standard CSS calc(), do we allow unary operators?
# And check how the parser handles plain CSS vs SCSS
rg -n 'BINARY_OPERATION_TOKEN' crates/biome_css_parser/src/syntax/value/function.rs -B 2 -A 2
# Check if there are any guards in parse_any_expression
sed -n '240,260p' crates/biome_css_parser/src/syntax/value/function.rsRepository: biomejs/biome
Length of output: 1237
🏁 Script executed:
# Look at what calls is_at_unary_operator to understand the impact
rg -n 'is_at_unary_operator' crates/biome_css_parser/src/ -B 1 -A 1Repository: biomejs/biome
Length of output: 1507
🏁 Script executed:
# Check the unary precedence test file to understand what's being tested
cat crates/biome_css_parser/tests/css_test_suite/ok/function/unary-precedence.css
# Also check what kind of values/functions are involved
head -50 crates/biome_css_parser/tests/css_test_suite/ok/function/unary-precedence.cssRepository: biomejs/biome
Length of output: 272
🏁 Script executed:
# Check if there's any calc() context handling in the parser
rg -n 'calc\|CALC' crates/biome_css_parser/src/syntax/value/function.rs
# Check the parse_any_expression usage to understand contexts where it's called
rg -n 'parse_any_expression' crates/biome_css_parser/src/ -B 2 -A 1 | head -40Repository: biomejs/biome
Length of output: 1490
🏁 Script executed:
# Verify CSS spec: check comments or docs about unary operator support
rg -n 'unary|precedence' crates/biome_css_parser/src/syntax/value/function.rs -B 1 -A 1Repository: biomejs/biome
Length of output: 1009
🏁 Script executed:
# Check if parse_function_parameter is specific to calc() or general function parameters
rg -n 'parse_function_parameter' crates/biome_css_parser/src/ -B 3 -A 2
# Check calc() function handling specifically
rg -n 'fn parse_' crates/biome_css_parser/src/syntax/value/ | grep -i calcRepository: biomejs/biome
Length of output: 2162
🏁 Script executed:
# Verify CSS spec for unary operators in calc()
web_search "CSS calc() unary operators specification"Repository: biomejs/biome
Length of output: 107
🏁 Script executed:
# Look for calc function parsing directly
rg -n 'calc' crates/biome_css_parser/src/syntax/value/function.rs
# Check if parse_any_expression is only in specific value contexts
rg -n 'parse_any_expression\|parse_function_parameter' crates/biome_css_parser/src/syntax/value/Repository: biomejs/biome
Length of output: 139
🌐 Web query:
CSS calc() function unary operators + - specification standard
💡 Result:
In the CSS standard, calc() (and other math functions) do not define separate unary + / - operators in the expression grammar. The only + / - in the grammar are the binary add/subtract operators in <calc-sum> (and they require whitespace on both sides). [1]
Unary sign is instead part of the numeric token itself: CSS Syntax defines <number-token>, <percentage-token>, and <dimension-token> as including a preceding + or - as part of the same token (so -5px or +5px are single dimensions). [2]
So:
- Valid:
calc(10px + -5px)(binary+, then a negative dimension token) [1] - Valid:
calc(+5px)/calc(-5px)(signed dimension token) [2] - Invalid:
calc(10px+-5px)orcalc(10px-5px)(missing required whitespace around binary+/-) [1] - Not a thing in the grammar:
calc(+ 5px)as a unary+operator; that’s just a+delimiter followed by a value, which doesn’t match<calc-value>. [1]
Sources
[1] W3C, CSS Values and Units Module Level 4 — calc grammar + required whitespace around +/- in math functions. (w3.org)
[2] W3C, CSS Syntax Module Level 3 — numeric tokens include the leading +/- sign as part of the token. (w3.org)
Citations:
🏁 Script executed:
# Check if there are other unary operator tests and their file extensions
find crates/biome_css_parser/tests -name '*unary*' -o -name '*precedence*' | xargs ls -la
# Verify what the intended behavior is - check if there are SCSS tests
find crates/biome_css_parser/tests -name '*.scss' | xargs grep -l 'unary\|+\|-' 2>/dev/null | head -5Repository: biomejs/biome
Length of output: 565
🏁 Script executed:
# Check git history or comments around this feature to understand intent
rg -n 'unary' crates/biome_css_parser/ -B 2 -A 2 --type rust | head -60Repository: biomejs/biome
Length of output: 3019
Unary operators should be gated to SCSS, consistent with other SCSS-specific features in this file.
The W3C CSS specification does not define unary +/- operators; signs are part of numeric tokens (e.g., -5px). The current code allows them globally without gating, inconsistent with SCSS feature guards elsewhere in the file (lines 74, 116).
The proposed fix is correct, but note the existing unary-precedence.css test (plain CSS suite, marked "ok") will fail—it will need to move to the SCSS test suite or be removed if this syntax is SCSS-only.
Proposed fix
#[inline]
pub(crate) fn is_at_unary_operator(p: &mut CssParser) -> bool {
- p.at_ts(UNARY_OPERATION_TOKEN)
+ CssSyntaxFeatures::Scss.is_supported(p) && p.at_ts(UNARY_OPERATION_TOKEN)
}🤖 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.rs` around lines 276 - 279,
The function is_at_unary_operator should be gated to SCSS-only: change its
implementation to return true only when the parser is in SCSS mode and the
current token matches UNARY_OPERATION_TOKEN (e.g., implement as something like
if p.is_scss() && p.at_ts(UNARY_OPERATION_TOKEN) return true); update the
is_at_unary_operator function in CssParser accordingly and ensure you reference
UNARY_OPERATION_TOKEN; also move or remove the existing plain-CSS test
(unary-precedence.css) to the SCSS test suite since the syntax is SCSS-specific.
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/value/function.rs`:
- Line 248: UNARY_OPERATION_TOKEN currently includes T![*], which is incorrect
for CSS/SCSS unary operators; update the TokenSet declaration (the
TokenSet<CssSyntaxKind> constant named UNARY_OPERATION_TOKEN created with
token_set!) to only include T![+] and T![-] by removing T![*] so that '*' is
parsed exclusively as a binary multiplication operator.
---
Duplicate comments:
In `@crates/biome_css_parser/src/syntax/value/function.rs`:
- Around line 260-263: The helper is_at_unary_operator currently treats +/− as
unary without checking feature flags; update it to only return true when the
SCSS feature flag is enabled on the parser (e.g., check
p.features.contains(Scss) or the parser method that exposes scss_enabled), so
plain CSS still parses signed numbers correctly. Modify the function
(is_at_unary_operator) to consult the SCSS enablement boolean before calling
p.at_ts(UNARY_OPERATION_TOKEN) and return false when SCSS is disabled; ensure
any callers relying on this behavior continue to work with the new conditional.
(cherry picked from commit a1976dd)
2 participants
Summary
This PR adds unary expression support in CSS/SCSS expression parsing and formatting as
part of the SCSS work.
We need unary expressions, syntax like:
inside functions, for example:
Test Plan