fix(parse/css): handle bogus declarations slightly more gracefully - #11321
Conversation
🦋 Changeset detectedLatest commit: 3bae583 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Organic activityNo automation signals detected in the analyzed events. This is an automated analysis by AgentScan |
Parser conformance results onjs/262
jsx/babel
markdown/commonmark
symbols/microsoft
ts/babel
ts/microsoft
|
Merging this PR will not alter performance
Comparing Footnotes
|
57f110a to
3bae583
Compare
WalkthroughThe CSS grammar now represents malformed declarations with Suggested labels: Suggested reviewers: Mergeability Score: 🟡 Moderate · up to This change improves recovery for invalid CSS declarations, but the current implementation can lose the declaration-specific syntax node for some invalid declarations, which may cause incorrect downstream parsing or formatting behavior. Merge should wait until that mapping is added. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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_syntax/src/lib.rs`:
- Line 110: Update the AnyCssDeclaration mapping in to_bogus to add a
CSS_BOGUS_DECLARATION arm, preserving that declaration-specific bogus node when
converting invalid declarations. Keep the existing CSS_BOGUS handling for other
node kinds and ensure the mapping aligns with the CSS_BOGUS_DECLARATION case
already recognized by is_bogus.
🪄 Autofix
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: a1c79adf-12bf-4d05-afce-2469f2bb2edb
⛔ Files ignored due to path filters (10)
crates/biome_css_factory/src/generated/node_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_factory/src/generated/syntax_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_formatter/tests/specs/prettier/scss/math/3945.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/at_rule/at_rule_keyframes/at_rule_keyframes.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/at_rule/at_rule_keyframes/at_rule_keyframes_1.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/declaration/bogus_declaration_recovery.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/tailwind/when-enabled/plugin-with-invalid-options-2.css.snapis excluded by!**/*.snapand included by**crates/biome_css_syntax/src/generated/kind.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_syntax/src/generated/macros.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_syntax/src/generated/nodes.rsis excluded by!**/generated/**,!**/generated/**and included by**
📒 Files selected for processing (12)
.changeset/bright-animals-sniff.md.changeset/quick-books-join.mdcrates/biome_css_formatter/src/css/any/declaration.rscrates/biome_css_formatter/src/css/bogus/bogus_declaration.rscrates/biome_css_formatter/src/css/bogus/mod.rscrates/biome_css_formatter/src/generated.rscrates/biome_css_parser/src/syntax/declaration.rscrates/biome_css_parser/tests/css_test_suite/error/declaration/bogus_declaration_recovery.csscrates/biome_css_syntax/src/lib.rscrates/biome_service/src/file_handlers/html/parse_embedded_nodes.tests.rsxtask/codegen/css.ungramxtask/codegen/src/css_kinds_src.rs
| matches!( | ||
| self, | ||
| CSS_BOGUS | ||
| | CSS_BOGUS_DECLARATION |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add the declaration mapping to to_bogus.
is_bogus now recognises CSS_BOGUS_DECLARATION, but to_bogus has no AnyCssDeclaration arm. When an invalid declaration uses this conversion path, the match falls through to CSS_BOGUS. The grammar does not include CSS_BOGUS in AnyCssDeclaration, so the declaration-specific node cannot be preserved.
Proposed mapping
match self {
+ kind if AnyCssDeclaration::can_cast(*kind) => CSS_BOGUS_DECLARATION,
kind if AnyCssSubSelector::can_cast(*kind) => CSS_BOGUS_SUB_SELECTOR,Based on learnings, invalid parser syntax uses or_invalid_to_bogus() and must preserve the matching bogus node kind.
📝 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.
| | CSS_BOGUS_DECLARATION | |
| match self { | |
| kind if AnyCssDeclaration::can_cast(*kind) => CSS_BOGUS_DECLARATION, | |
| kind if AnyCssSubSelector::can_cast(*kind) => CSS_BOGUS_SUB_SELECTOR, |
🤖 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_syntax/src/lib.rs` at line 110, Update the AnyCssDeclaration
mapping in to_bogus to add a CSS_BOGUS_DECLARATION arm, preserving that
declaration-specific bogus node when converting invalid declarations. Keep the
existing CSS_BOGUS handling for other node kinds and ensure the mapping aligns
with the CSS_BOGUS_DECLARATION case already recognized by is_bogus.
Source: Learnings
Summary
This adds a CssBogusDeclaration node, and makes the parser recovery a little more graceful for declaration lists. This affects top level declaration lists, like in html style attributes.
fixes #11315
Test Plan
snapshots
Docs