-
-
Notifications
You must be signed in to change notification settings - Fork 781
fix(parse/css): fix parsing --*: initial; with tailwindDirectives enabled
#7852
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: f2ba2a6 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 |
Parser conformance results onjs/262
jsx/babel
symbols/microsoft
ts/babel
ts/microsoft
|
CodSpeed Performance ReportMerging #7852 will not alter performanceComparing Summary
Footnotes
|
WalkthroughThis PR updates the CSS parser to accept Tailwind-style internal variable patterns such as Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
[pre_merge_check_pass] The code changes directly address the requirements from issue #7843. The PR modifies the CSS lexer and parser to correctly handle the [pre_merge_check_pass] All changes are directly scoped to fixing the ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_css_parser/src/lexer/mod.rs (1)
1292-1340: Don’t broaden ident-start globally; gate the--*start behind Tailwind.
MUL => truechanges tokenisation for non‑Tailwind users. Guard it with the option to avoid regressions.Apply this diff:
- MUL => true, + MUL if self.options.is_tailwind_directives_enabled() => true,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
crates/biome_css_factory/src/generated/node_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_parser/tests/css_test_suite/ok/tailwind/theme/custom-theme.css.snapis excluded by!**/*.snapand included by**crates/biome_css_syntax/src/generated/nodes.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_syntax/src/generated/nodes_mut.rsis excluded by!**/generated/**,!**/generated/**and included by**
📒 Files selected for processing (6)
.changeset/itchy-aliens-ask.md(1 hunks)crates/biome_css_parser/src/lexer/mod.rs(2 hunks)crates/biome_css_parser/src/syntax/property/mod.rs(2 hunks)crates/biome_css_parser/tests/css_test_suite/ok/tailwind/theme/custom-theme.css(1 hunks)crates/biome_parser/src/lexer.rs(1 hunks)xtask/codegen/css.ungram(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_css_parser/tests/css_test_suite/ok/tailwind/theme/custom-theme.csscrates/biome_css_parser/src/syntax/property/mod.rscrates/biome_css_parser/src/lexer/mod.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_css_parser/tests/css_test_suite/ok/tailwind/theme/custom-theme.csscrates/biome_parser/src/lexer.rscrates/biome_css_parser/src/syntax/property/mod.rscrates/biome_css_parser/src/lexer/mod.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_css_parser/tests/css_test_suite/ok/tailwind/theme/custom-theme.css
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: In changesets, only use #### or ##### headers; other header levels are not allowed
Changesets should cover user-facing changes only; internal changes do not need changesets
Use past tense for what you did and present tense for current Biome behavior in changesets
When fixing a bug in a changeset, start with an issue link (e.g., “Fixed #1234: …”)
When referencing a rule or assist in a changeset, include a link to its page on the website
Include code blocks in changesets when applicable to illustrate changes
End every sentence in a changeset with a period
Files:
.changeset/itchy-aliens-ask.md
xtask/codegen/*.ungram
📄 CodeRabbit inference engine (CLAUDE.md)
Define and modify language grammars in .ungram files; ASTs are generated from these
Files:
xtask/codegen/css.ungram
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Format Rust files before committing (e.g., viajust fwhich formats Rust)
Document rules, assists, and options with inline rustdoc in source
Files:
crates/biome_parser/src/lexer.rscrates/biome_css_parser/src/syntax/property/mod.rscrates/biome_css_parser/src/lexer/mod.rs
🧠 Learnings (1)
📚 Learning: 2025-10-15T09:24:31.042Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:24:31.042Z
Learning: Lexer must implement the biome_parser::Lexer trait
Applied to files:
crates/biome_parser/src/lexer.rs
🧬 Code graph analysis (1)
crates/biome_css_parser/src/syntax/property/mod.rs (2)
crates/biome_parser/src/lib.rs (2)
cur(183-185)cur_text(198-200)crates/biome_css_parser/src/syntax/mod.rs (2)
is_at_dashed_identifier(499-501)parse_dashed_identifier(507-515)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Check Dependencies
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Parser conformance
- GitHub Check: autofix
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_json_formatter)
🔇 Additional comments (3)
xtask/codegen/css.ungram (1)
1876-1885: Codegen verified and ASTs in sync.Generated files correctly reflect the optional dash:
minus_tokenisOption<SyntaxToken>in nodes.rs, the formatter handles it properly, and test snapshots confirm the parser works as expected. All good.crates/biome_parser/src/lexer.rs (1)
212-219: Additive Lexer API is solid—already in use.Verification confirms
prev_byte()is a safe default method with no breaking implications. The CSS lexer is already leveraging it at line 1013 for the look-behind logic you mentioned. All in-repo implementations inherit the default without override, and no trait object assumptions are at risk. No issues found.crates/biome_css_parser/tests/css_test_suite/ok/tailwind/theme/custom-theme.css (1)
1-5: Good snapshot test for the Tailwind--*: initial;fix.Minimal, well-placed test that directly addresses issue #7843. The comment provides helpful context from Tailwind's documentation. This should serve as an effective verification that the parser now handles this pattern correctly.
fc0d8a9 to
aa36270
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
crates/biome_css_factory/src/generated/node_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_parser/tests/css_test_suite/ok/tailwind/theme/custom-theme.css.snapis excluded by!**/*.snapand included by**crates/biome_css_syntax/src/generated/nodes.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_syntax/src/generated/nodes_mut.rsis excluded by!**/generated/**,!**/generated/**and included by**
📒 Files selected for processing (6)
.changeset/itchy-aliens-ask.md(1 hunks)crates/biome_css_parser/src/lexer/mod.rs(2 hunks)crates/biome_css_parser/src/syntax/property/mod.rs(2 hunks)crates/biome_css_parser/tests/css_test_suite/ok/tailwind/theme/custom-theme.css(1 hunks)crates/biome_parser/src/lexer.rs(1 hunks)xtask/codegen/css.ungram(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- .changeset/itchy-aliens-ask.md
- xtask/codegen/css.ungram
- crates/biome_css_parser/src/syntax/property/mod.rs
- crates/biome_parser/src/lexer.rs
- crates/biome_css_parser/tests/css_test_suite/ok/tailwind/theme/custom-theme.css
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_css_parser/src/lexer/mod.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_css_parser/src/lexer/mod.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Format Rust files before committing (e.g., viajust fwhich formats Rust)
Document rules, assists, and options with inline rustdoc in source
Files:
crates/biome_css_parser/src/lexer/mod.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Parser conformance
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Documentation
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: autofix
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_css_parser)
🔇 Additional comments (1)
crates/biome_css_parser/src/lexer/mod.rs (1)
1010-1021: LGTM – clean handling of the Tailwind edge case.The logic correctly distinguishes between
--*(consume the second dash) and--colour-*(stop at the identifier boundary). Properly option-gated and the previous commented-out code has been removed as requested.
ematipico
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add one more test
crates/biome_css_parser/tests/css_test_suite/ok/tailwind/theme/custom-theme.css
Show resolved
Hide resolved
aa36270 to
f2ba2a6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/biome_css_parser/src/lexer/mod.rs (1)
1314-1314: Unconditionally allowingMULfor--*ident-start is correct.Keeping this ungated ensures stable tokenisation and better diagnostics when Tailwind is disabled. Looks good. Based on learnings.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
crates/biome_css_factory/src/generated/node_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_parser/tests/css_test_suite/error/tailwind/when-disabled/custom-theme.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/tailwind/theme/custom-theme.css.snapis excluded by!**/*.snapand included by**crates/biome_css_syntax/src/generated/nodes.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_css_syntax/src/generated/nodes_mut.rsis excluded by!**/generated/**,!**/generated/**and included by**
📒 Files selected for processing (7)
.changeset/itchy-aliens-ask.md(1 hunks)crates/biome_css_parser/src/lexer/mod.rs(2 hunks)crates/biome_css_parser/src/syntax/property/mod.rs(3 hunks)crates/biome_css_parser/tests/css_test_suite/error/tailwind/when-disabled/custom-theme.css(1 hunks)crates/biome_css_parser/tests/css_test_suite/ok/tailwind/theme/custom-theme.css(1 hunks)crates/biome_parser/src/lexer.rs(1 hunks)xtask/codegen/css.ungram(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- .changeset/itchy-aliens-ask.md
- crates/biome_parser/src/lexer.rs
- xtask/codegen/css.ungram
- crates/biome_css_parser/tests/css_test_suite/ok/tailwind/theme/custom-theme.css
🧰 Additional context used
📓 Path-based instructions (4)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_css_parser/tests/css_test_suite/error/tailwind/when-disabled/custom-theme.csscrates/biome_css_parser/src/lexer/mod.rscrates/biome_css_parser/src/syntax/property/mod.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_css_parser/tests/css_test_suite/error/tailwind/when-disabled/custom-theme.csscrates/biome_css_parser/src/lexer/mod.rscrates/biome_css_parser/src/syntax/property/mod.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_css_parser/tests/css_test_suite/error/tailwind/when-disabled/custom-theme.css
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Format Rust files before committing (e.g., viajust fwhich formats Rust)
Document rules, assists, and options with inline rustdoc in source
Files:
crates/biome_css_parser/src/lexer/mod.rscrates/biome_css_parser/src/syntax/property/mod.rs
🧠 Learnings (1)
📚 Learning: 2025-10-25T07:22:18.511Z
Learnt from: ematipico
PR: biomejs/biome#7852
File: crates/biome_css_parser/src/syntax/property/mod.rs:161-168
Timestamp: 2025-10-25T07:22:18.511Z
Learning: In the Biome CSS parser, lexer token emission should not be gated behind parser options like `is_tailwind_directives_enabled()`. The lexer must emit correct tokens regardless of parser options to enable accurate diagnostics and error messages when the syntax is used incorrectly.
Applied to files:
crates/biome_css_parser/src/lexer/mod.rscrates/biome_css_parser/src/syntax/property/mod.rs
🧬 Code graph analysis (1)
crates/biome_css_parser/src/syntax/property/mod.rs (2)
crates/biome_css_parser/src/syntax/parse_error.rs (1)
tailwind_disabled(236-241)crates/biome_css_parser/src/syntax/mod.rs (5)
is_at_dashed_identifier(499-501)parse_any_value(336-370)parse_custom_identifier_with_keywords(482-496)parse_dashed_identifier(507-515)parse_regular_identifier(433-435)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: autofix
- GitHub Check: Documentation
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: End-to-end tests
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Check Dependencies
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Parser conformance
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_graphql_formatter)
🔇 Additional comments (3)
crates/biome_css_parser/tests/css_test_suite/error/tailwind/when-disabled/custom-theme.css (1)
1-5: Nice negative test coverage!This properly complements the positive test by verifying that
--*: initial;fails to parse when tailwindDirectives are disabled, which confirms the fix is scoped correctly to Tailwind-enabled contexts only.crates/biome_css_parser/src/syntax/property/mod.rs (2)
163-170: Broadened lookahead for--*:and--color-*:is spot on.Recognises both forms without option-gating; the exclusive Tailwind path handles diagnostics when disabled. Nice.
183-199: Lexer correctly preserves*token—verification complete.The concern has been validated: the lexer's
consume_ident_partmethod (line 1014 inmod.rs) explicitly handles the--*sequence by only advancing past the dash, leaving the asterisk as a separateSTARtoken for the parser. The parser then correctly consumes it at line 193. Tailwind-exclusive wrapping and the "disabled" diagnostic path are well-shaped.
| // HACK: handle `--*` | ||
| if self.prev_byte() == Some(b'-') { | ||
| self.advance(1); | ||
| return Some(current as char); | ||
| } | ||
| // otherwise, handle cases like `--color-*` | ||
| return None; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t swallow the * in --* during ident lexing.
This advances past *, removing it from the token stream. The parser then can’t see *, breaking the new lookaheads (--*: / --color-*:) and the Tailwind-exclusive wrapper that expects *. Let the ident consume -- and stop before * so * remains a separate token.
Fix: remove this special-case entirely and rely on the existing logic (plus the MUL start tweak).
Apply this diff:
- if self.options.is_tailwind_directives_enabled()
- && current == b'-'
- && self.peek_byte() == Some(b'*')
- {
- // HACK: handle `--*`
- if self.prev_byte() == Some(b'-') {
- self.advance(1);
- return Some(current as char);
- }
- // otherwise, handle cases like `--color-*`
- return None;
- }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/biome_css_parser/src/lexer/mod.rs around lines 1014 to 1020, the ident
lexing special-case that checks for prev_byte() == Some(b'-') and calls
advance(1) is swallowing a '*' in sequences like `--*` and `--color-*`; remove
this special-case entirely so the ident consumes `--` but stops before `*`,
allowing the `*` to remain as its own token and preserving downstream lookaheads
(`--*:` / `--color-*:`) and Tailwind wrapper handling; rely on the existing
ident logic (and the existing MUL start tweak) instead of advancing past `*`.
Summary
fixes #7843
I wanted to keep the parsing of things like
--color-*to remain the same where--coloris the identifier, but if I just blindly did that for the--*case, then the identifier would be-, which would be confusing for lint rule authors.Test Plan
added a snapshot test
Docs