fix(css_parser): gate SCSS syntax in CSS#10456
Conversation
|
✅ Organic activityNo automation signals detected in the analyzed events. This is an automated analysis by AgentScan |
WalkthroughRefactors SCSS interpolation handling: removes SCSS feature gates from detection predicates, introduces selector- and value-specific interpolation helpers, and routes SCSS-only parses through CssSyntaxFeatures::Scss.parse_exclusive_syntax with scss_only_syntax_error. Updates at-rule, declaration/block, selector, property, value and URL parsing to use the new helpers and adds test fixtures covering SCSS-exclusive forms. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/biome_css_parser/tests/css_test_suite/error/property/scss_exclusive_values.css (1)
40-42: 💤 Low valueClarify the intent of the malformed case.
The space between
#and{value}breaks SCSS interpolation syntax. Is this testing:
- Whitespace rejection in interpolation detection?
- General malformed syntax error handling?
- Something else?
A comment explaining the test intent would help future maintainers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_css_parser/tests/css_test_suite/error/property/scss_exclusive_values.css` around lines 40 - 42, Add a one-line comment above the .malformed-css test case clarifying the exact intent: explicitly state whether this case is testing "whitespace-breaking SCSS interpolation" (i.e., space between '#' and '{value}' should cause interpolation rejection), "general malformed syntax handling" (parser should emit a syntax error), or another behavior; reference the selector .malformed-css and the property "color: calc(# {value});" so maintainers can immediately see which scenario is covered.crates/biome_css_parser/src/syntax/scss/property.rs (1)
32-39: 💤 Low valueConsider adding whitespace constraint for pattern 3.
For
margin-#{$side}, the interpolation should immediately follow the identifier with no whitespace. Other similar predicates (e.g.,is_nth_at_scss_selector_identifier_suffix) include!p.has_nth_preceding_whitespace(n + 1). Without it,margin- #{$side}(with space) might be detected as a valid interpolated property name when it arguably shouldn't be.That said, if the parsing layer or lexer already handles this edge case, feel free to disregard.
♻️ Suggested fix
// `margin-#{$side}: 1px;` - || (is_nth_at_identifier(p, n) && is_nth_at_scss_interpolation(p, n + 1)) + || (is_nth_at_identifier(p, n) + && !p.has_nth_preceding_whitespace(n + 1) + && is_nth_at_scss_interpolation(p, n + 1))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_css_parser/src/syntax/scss/property.rs` around lines 32 - 39, The third branch of is_nth_at_scss_interpolated_property_name can falsely match "margin- #{$side}" because it doesn't ensure the interpolation immediately follows the identifier; update the predicate in is_nth_at_scss_interpolated_property_name to also require that there is no preceding whitespace before the interpolation by adding a check like !p.has_nth_preceding_whitespace(n + 1) alongside the existing is_nth_at_identifier(p, n) && is_nth_at_scss_interpolation(p, n + 1) so the pattern only matches when the interpolation directly follows the identifier.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@crates/biome_css_parser/tests/css_test_suite/error/at_rule/scss_exclusive_values.css`:
- Around line 65-87: Add positive SCSS fixtures mirroring the negative cases so
the parser has bidirectional coverage: update
ok/scss/at-rule/exclusive-value-gates.scss to include valid SCSS examples for
interpolated media queries and ranges (e.g. `@media` #{$query} { ... }, `@media`
(#{$min} <= width) { ... }, `@media` (#{$min} <= width <= #{$max}) { ... }),
keyframe percentages using interpolation (e.g. `@keyframes` loader { #{$i}% { ...
} }), and interpolated at-rule names/blocks (e.g. @#{$rule-name} #{$value}; and
@#{$block-rule} #{$value} { color: red; }) so the ok fixture matches the error
fixture constructs.
---
Nitpick comments:
In `@crates/biome_css_parser/src/syntax/scss/property.rs`:
- Around line 32-39: The third branch of
is_nth_at_scss_interpolated_property_name can falsely match "margin- #{$side}"
because it doesn't ensure the interpolation immediately follows the identifier;
update the predicate in is_nth_at_scss_interpolated_property_name to also
require that there is no preceding whitespace before the interpolation by adding
a check like !p.has_nth_preceding_whitespace(n + 1) alongside the existing
is_nth_at_identifier(p, n) && is_nth_at_scss_interpolation(p, n + 1) so the
pattern only matches when the interpolation directly follows the identifier.
In
`@crates/biome_css_parser/tests/css_test_suite/error/property/scss_exclusive_values.css`:
- Around line 40-42: Add a one-line comment above the .malformed-css test case
clarifying the exact intent: explicitly state whether this case is testing
"whitespace-breaking SCSS interpolation" (i.e., space between '#' and '{value}'
should cause interpolation rejection), "general malformed syntax handling"
(parser should emit a syntax error), or another behavior; reference the selector
.malformed-css and the property "color: calc(# {value});" so maintainers can
immediately see which scenario is covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7579ed28-f33a-446c-8df0-c2eaec91f5bf
⛔ Files ignored due to path filters (7)
crates/biome_css_parser/tests/css_test_suite/error/at_rule/scss_exclusive_values.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/property/scss_exclusive_values.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/scss/declaration/scss-variable-declaration-in-css.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/selector/scss_exclusive_values.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/at_rule/scss-named-at-rules-unknown.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/at-rule/exclusive-value-gates.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/value/exclusive-value-gates.scss.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (45)
crates/biome_css_parser/src/syntax/at_rule/feature.rscrates/biome_css_parser/src/syntax/at_rule/keyframes.rscrates/biome_css_parser/src/syntax/at_rule/media.rscrates/biome_css_parser/src/syntax/at_rule/mod.rscrates/biome_css_parser/src/syntax/at_rule/supports/mod.rscrates/biome_css_parser/src/syntax/at_rule/unknown.rscrates/biome_css_parser/src/syntax/block/declaration_or_at_rule_list_block.rscrates/biome_css_parser/src/syntax/block/declaration_or_rule_list_block.rscrates/biome_css_parser/src/syntax/declaration.rscrates/biome_css_parser/src/syntax/mod.rscrates/biome_css_parser/src/syntax/property/mod.rscrates/biome_css_parser/src/syntax/scss/at_rule/keyframes.rscrates/biome_css_parser/src/syntax/scss/declaration/nesting.rscrates/biome_css_parser/src/syntax/scss/expression/interpolation.rscrates/biome_css_parser/src/syntax/scss/expression/operand.rscrates/biome_css_parser/src/syntax/scss/identifiers/interpolated_dashed.rscrates/biome_css_parser/src/syntax/scss/identifiers/interpolated_identifier.rscrates/biome_css_parser/src/syntax/scss/identifiers/interpolated_regular.rscrates/biome_css_parser/src/syntax/scss/identifiers/interpolated_selector.rscrates/biome_css_parser/src/syntax/scss/identifiers/mod.rscrates/biome_css_parser/src/syntax/scss/mod.rscrates/biome_css_parser/src/syntax/scss/property.rscrates/biome_css_parser/src/syntax/scss/selector/attribute.rscrates/biome_css_parser/src/syntax/scss/selector/interpolated_pseudo.rscrates/biome_css_parser/src/syntax/scss/selector/parent_selector.rscrates/biome_css_parser/src/syntax/scss/selector/placeholder.rscrates/biome_css_parser/src/syntax/scss/selector/pseudo_class_nth.rscrates/biome_css_parser/src/syntax/scss/value/any.rscrates/biome_css_parser/src/syntax/scss/value/interpolated_string.rscrates/biome_css_parser/src/syntax/scss/value/interpolated_value.rscrates/biome_css_parser/src/syntax/scss/value/mod.rscrates/biome_css_parser/src/syntax/scss/value/parent_selector.rscrates/biome_css_parser/src/syntax/selector/attribute.rscrates/biome_css_parser/src/syntax/selector/mod.rscrates/biome_css_parser/src/syntax/selector/nested_selector.rscrates/biome_css_parser/src/syntax/selector/pseudo_class/function_nth.rscrates/biome_css_parser/src/syntax/selector/pseudo_class/function_value_list.rscrates/biome_css_parser/src/syntax/value/url.rscrates/biome_css_parser/tests/css_test_suite/error/at_rule/scss_exclusive_values.csscrates/biome_css_parser/tests/css_test_suite/error/property/scss_exclusive_values.csscrates/biome_css_parser/tests/css_test_suite/error/scss/declaration/scss-variable-declaration-in-css.csscrates/biome_css_parser/tests/css_test_suite/error/selector/scss_exclusive_values.csscrates/biome_css_parser/tests/css_test_suite/ok/at_rule/scss-named-at-rules-unknown.csscrates/biome_css_parser/tests/css_test_suite/ok/scss/at-rule/exclusive-value-gates.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/value/exclusive-value-gates.scss
| @media #{$query} { | ||
| .media-query {} | ||
| } | ||
|
|
||
| @media (#{$min} <= width) { | ||
| .media-reverse-range {} | ||
| } | ||
|
|
||
| @media (#{$min} <= width <= #{$max}) { | ||
| .media-interval-range {} | ||
| } | ||
|
|
||
| @keyframes loader { | ||
| #{$i}% { | ||
| opacity: 1; | ||
| } | ||
| } | ||
|
|
||
| @#{$rule-name} #{$value}; | ||
|
|
||
| @#{$block-rule} #{$value} { | ||
| color: red; | ||
| } |
There was a problem hiding this comment.
Test coverage gap: missing SCSS positive cases.
These SCSS-exclusive features (interpolated media query, range queries, keyframes percentage, and interpolated at-rule names) lack corresponding positive test cases in ok/scss/at-rule/exclusive-value-gates.scss. If these are valid SCSS syntax, please add them to the SCSS ok fixture to ensure bidirectional coverage.
🧰 Tools
🪛 Stylelint (17.11.1)
[error] 66-66: Empty block (block-no-empty)
(block-no-empty)
[error] 70-70: Empty block (block-no-empty)
(block-no-empty)
[error] 74-74: Empty block (block-no-empty)
(block-no-empty)
[error] 83-83: Unexpected unknown at-rule "@#{$rule-name}" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
[error] 85-85: Unexpected unknown at-rule "@#{$block-rule}" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@crates/biome_css_parser/tests/css_test_suite/error/at_rule/scss_exclusive_values.css`
around lines 65 - 87, Add positive SCSS fixtures mirroring the negative cases so
the parser has bidirectional coverage: update
ok/scss/at-rule/exclusive-value-gates.scss to include valid SCSS examples for
interpolated media queries and ranges (e.g. `@media` #{$query} { ... }, `@media`
(#{$min} <= width) { ... }, `@media` (#{$min} <= width <= #{$max}) { ... }),
keyframe percentages using interpolation (e.g. `@keyframes` loader { #{$i}% { ...
} }), and interpolated at-rule names/blocks (e.g. @#{$rule-name} #{$value}; and
@#{$block-rule} #{$value} { color: red; }) so the ok fixture matches the error
fixture constructs.
Merging this PR will degrade performance by 6.18%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
Don't merge till 2.5
Summary
Adds parser coverage and SCSS feature gating for interpolation syntax that was previously parsed too permissively in plain CSS contexts.
Biome now reports SCSS-only diagnostics for CSS inputs such as:
The SCSS parser path remains supported for the same syntax in .scss files, including value interpolation, query features, selector names, property names, keyframes, and interpolated unknown at-rules.
Test Plan
cargo test -p biome_css_parser