feat(parse/tailwind): parse arbitrary values as css#10299
Merged
Conversation
|
github-actions
Bot
added
A-Parser
Contributor
Parser conformance results onjs/262
jsx/babel
markdown/commonmark
symbols/microsoft
ts/babel
ts/microsoft
|
Merging this PR will degrade performance by 30.42%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| 👁 | cached[arbitrary_classes.txt] |
249.1 µs | 335.3 µs | -25.69% |
| 👁 | cached[extreme_stress.txt] |
842.7 µs | 1,014.4 µs | -16.92% |
| 👁 | cached[stress.txt] |
2.2 ms | 2.3 ms | -6.6% |
| 👁 | uncached[stress.txt] |
2.8 ms | 3 ms | -6.96% |
| 👁 | uncached[arbitrary_classes.txt] |
311.3 µs | 447.5 µs | -30.42% |
| 👁 | uncached[extreme_stress.txt] |
1.1 ms | 1.3 ms | -19.68% |
Comparing dyc3/tw-parse-arbitrary-css-values (737e800) with main (91ed677)
Footnotes
-
246 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
dyc3
force-pushed
the
dyc3/tw-parse-arbitrary-css-values
branch
from
fcf4af6 to
b3f08bf
Compare
Contributor
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (34)
📒 Files selected for processing (20)
✅ Files skipped from review due to trivial changes (9)
🚧 Files skipped from review as they are similar to previous changes (8)
WalkthroughThis PR extends the Tailwind parser with structured CSS value parsing for arbitrary values and candidates. It replaces opaque Possibly related PRs
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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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_tailwind_parser/src/lexer/mod.rs`:
- Around line 162-177: In consume_css_url_raw_value, the loop currently stops on
')' or whitespace without honoring backslash escapes; update the loop inside
consume_css_url_raw_value to detect a backslash escape (e.g., when
current_byte() == b'\\'), consume the backslash and then consume the following
code point (so escaped ')' or whitespace are treated as part of the raw value),
and only treat unescaped ')' or whitespace as terminators; also add regression
tests covering bg-[url(foo\)bar)] and bg-[url(foo\ bar)] to validate the
escaped-paren and escaped-space behavior.
- Around line 453-460: The loop in consume_bracketed_thing fails to ignore
bracket characters inside quoted strings, so modify consume_bracketed_thing to
track quote state and escapes (mirroring the logic in consume_css_string): add
variables for current quote char and escape flag, update those inside the while
loop before bracket checks (toggle quote on unescaped quote chars, set/clear
escape on backslashes) and only consider dispatched == BTO/BTC or looking_for
when not inside a quote and when escape is not active; keep existing
bracket_depth logic otherwise. After implementing, add a regression test that
parses inputs like `[data-label="]"]` and `[data-label='\\'\\\"]']` to verify
brackets inside quoted attribute values do not prematurely terminate
tokenization.
In `@crates/biome_tailwind_parser/src/syntax/css_value.rs`:
- Around line 279-283: The recovery token set passed to
parsed_element.or_recover_with_token_set currently stops on commas and ')' only,
allowing recovery to continue past ']' inside constructs like calc(...); update
the ParseRecoveryTokenSet::new call (the one using CSS_BOGUS_PROPERTY_VALUE and
token_set![T![,], T![')']]) to also include the closing bracket token T![']]' so
recovery will stop at ']' as well.
- Around line 315-321: The current code treats every binary operator as
right-associative by immediately parsing the right-hand side with
parse_any_expression when is_at_binary_operator is true, producing incorrect
trees for mixed precedence (e.g. parse_unary_expression_operand -> binary
operator -> parse_any_expression makes 1 * 2 + 3 parse as 1 * (2 + 3)). Change
this to a simple precedence-climbing approach: after parsing the left operand
with parse_unary_expression_operand, check is_at_binary_operator and
loop/recursively consume subsequent binary operators by distinguishing
low-precedence (+/-) vs high-precedence (*/%) operators (using
BINARY_OPERATION_TOKEN and TailwindLexContext::CssValue to read the operator),
build left-associative CSS_BINARY_EXPRESSION nodes by creating a binary node
from the current left and the next parsed right operand (call
parse_unary_expression_operand for higher-precedence RHS when needed or parse
the next operand and then continue), and only call parse_any_expression for
full-expression contexts where appropriate (ensure parse_any_expression use is
replaced so mixed-precedence expressions produce (1 * 2) + 3 rather than 1 * (2
+ 3)).
- Around line 327-350: The unary operator token set is wrong: remove '*' from
UNARY_OPERATION_TOKEN so only '+' and '-' are treated as unary; update the
token_set declaration for UNARY_OPERATION_TOKEN to token_set![T![+], T![-]] so
is_at_unary_operator, parse_unary_expression, and any callers (e.g.,
parse_any_expression) no longer accept `*foo` as a CSS_UNARY_EXPRESSION; no
other changes to BINARY_OPERATION_TOKEN or
COMPONENT_VALUE_EXPRESSION_RECOVERY_SET are needed.
In `@xtask/codegen/tailwind.ungram`:
- Around line 280-287: The field types are too narrow: update
CssUnaryExpression's argument from AnyCssValue to
CssListOfComponentValuesExpression, and update CssParenthesizedExpression's
expression from CssComponentValueList to CssListOfComponentValuesExpression so
the generated accessors match the parser output and won't return None for valid
parse trees.
🪄 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: d62904ef-e76b-44a1-847f-77f849759540
⛔ Files ignored due to path filters (34)
crates/biome_tailwind_factory/src/generated/node_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_tailwind_factory/src/generated/syntax_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_tailwind_parser/tests/tailwind_specs/error/arbitrary-candidate/missing-value-in-arbitrary.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/error/css-arbitrary-values.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/error/css-number-exponents.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-functions.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-math-functions.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-number-exponents.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-urls.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-values.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/gradient.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/image-url.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/inset.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/misc-xy.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/shadow.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/arbitrary-candidate-0.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/arbitrary-candidate-1.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/arbitrary-candidate-2.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/arbitrary-candidate-3.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/arbitrary-candidate-css-function.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/css-arbitrary-candidates.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/group-data.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/simple/arbitrary-value-0.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/simple/arbitrary-value-1.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/simple/base-has-dash-1.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/simple/base-has-dash-2.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/stress/stress-1.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/variants/arbitrary-selector-functional-candidate.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/variants/consecutive-arbitrary-variants.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/variants/functional-arbirary-param.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_syntax/src/generated/kind.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_tailwind_syntax/src/generated/macros.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_tailwind_syntax/src/generated/nodes.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_tailwind_syntax/src/generated/nodes_mut.rsis excluded by!**/generated/**,!**/generated/**and included by**
📒 Files selected for processing (19)
crates/biome_tailwind_parser/src/lexer/mod.rscrates/biome_tailwind_parser/src/lexer/tests.rscrates/biome_tailwind_parser/src/syntax/css_value.rscrates/biome_tailwind_parser/src/syntax/mod.rscrates/biome_tailwind_parser/src/syntax/value.rscrates/biome_tailwind_parser/src/token_source.rscrates/biome_tailwind_parser/tests/tailwind_specs/error/css-arbitrary-values.txtcrates/biome_tailwind_parser/tests/tailwind_specs/error/css-number-exponents.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-functions.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-math-functions.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-number-exponents.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-urls.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-values.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/arbitrary-candidate-css-function.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/css-arbitrary-candidates.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/variants/arbitrary-selector-functional-candidate.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/variants/consecutive-arbitrary-variants.txtxtask/codegen/src/tailwind_kinds_src.rsxtask/codegen/tailwind.ungram
dyc3
force-pushed
the
dyc3/tw-parse-arbitrary-css-values
branch
from
b3f08bf to
6b164c8
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_tailwind_parser/src/syntax/mod.rs (1)
179-186:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEmit error only if closing bracket is found.
The error on line 180 is emitted before validating the
]token. Ifparse_css_generic_component_value_listreturnsfalseand the closing bracket is missing, the rewind on line 185 doesn't roll back that diagnostic—it survives as a spurious error.Whilst the existing test
[width:]passes because the bracket is present, an edge case like[width:at EOF would trigger this leak. Move the error emission after the successfulexpect(T![']'])or use a diagnostic guard approach to avoid it.🤖 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_tailwind_parser/src/syntax/mod.rs` around lines 179 - 186, The diagnostic is emitted unconditionally when parse_css_generic_component_value_list(p) returns false, which leaks the error if the closing bracket is missing and the parser rewinds; move the p.error(expected_value(p, p.cur_range())) call so it only runs after confirming the closing bracket with p.expect(T![']']), or alternatively wrap the error emission in a guard that is only committed when m is not abandoned; update the block around parse_css_generic_component_value_list, p.expect, m.abandon, p.rewind, checkpoint and Absent accordingly so errors are only recorded on successful consumption of the trailing ']' token.
🧹 Nitpick comments (1)
crates/biome_tailwind_parser/src/syntax/mod.rs (1)
188-196: 💤 Low valueDead condition — the second
if p.at(T![/])is alwaystruehere.After the
if !p.at(T![/]) { return … }early-return on Line 188, execution only continues whenp.at(T![/])is alreadytrue. The second guard is dead code.✂️ Suggested simplification
- if !p.at(T![/]) { - return Present(m.complete(p, TW_ARBITRARY_CANDIDATE)); - } - - if p.at(T![/]) { - parse_modifier(p).or_add_diagnostic(p, expected_modifier); - } + if p.at(T![/]) { + parse_modifier(p).or_add_diagnostic(p, expected_modifier); + }🤖 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_tailwind_parser/src/syntax/mod.rs` around lines 188 - 196, The second conditional checking p.at(T![/]) is dead because the earlier if !p.at(T![/]) returns; remove that redundant guard and directly call parse_modifier(p).or_add_diagnostic(p, expected_modifier) before returning Present(m.complete(p, TW_ARBITRARY_CANDIDATE)); update the block around m.complete/Present so the flow is: if not p.at('/'){ return Present(m.complete(...)) } parse_modifier(...).or_add_diagnostic(...); Present(m.complete(...)); keeping the same symbols m.complete, TW_ARBITRARY_CANDIDATE, parse_modifier, expected_modifier and Present.
🤖 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 @.claude/skills/parser-development/SKILL.md:
- Around line 199-200: Update the misleading comment so it matches the parser
logic: replace the comment "Stop at array closing bracket or file end" with a
precise note like "Stop at array closing bracket" (referencing the parser check
p.at(T![']']) in the same block) — or alternatively, if the intent was to
include EOF, modify the condition to check p.at(T![']']) || p.eof(); ensure the
comment and the code (the line using p.at(T![']'])) stay consistent.
In `@xtask/codegen/tailwind.ungram`:
- Around line 257-260: AnyCssUrlValue currently includes CssParameterList which
incorrectly allows constructs like url(expr, expr) into the AST; change the
grammar so AnyCssUrlValue only accepts CssUrlValueRaw or CssString and remove
CssParameterList from its alternatives, and ensure parsing of non-URL/raw/string
content falls back to CssBogusPropertyValue (update the parser rule that
produces AnyCssUrlValue and the error/recovery path so invalid parameter lists
are emitted as CssBogusPropertyValue rather than as CssParameterList).
---
Outside diff comments:
In `@crates/biome_tailwind_parser/src/syntax/mod.rs`:
- Around line 179-186: The diagnostic is emitted unconditionally when
parse_css_generic_component_value_list(p) returns false, which leaks the error
if the closing bracket is missing and the parser rewinds; move the
p.error(expected_value(p, p.cur_range())) call so it only runs after confirming
the closing bracket with p.expect(T![']']), or alternatively wrap the error
emission in a guard that is only committed when m is not abandoned; update the
block around parse_css_generic_component_value_list, p.expect, m.abandon,
p.rewind, checkpoint and Absent accordingly so errors are only recorded on
successful consumption of the trailing ']' token.
---
Nitpick comments:
In `@crates/biome_tailwind_parser/src/syntax/mod.rs`:
- Around line 188-196: The second conditional checking p.at(T![/]) is dead
because the earlier if !p.at(T![/]) returns; remove that redundant guard and
directly call parse_modifier(p).or_add_diagnostic(p, expected_modifier) before
returning Present(m.complete(p, TW_ARBITRARY_CANDIDATE)); update the block
around m.complete/Present so the flow is: if not p.at('/'){ return
Present(m.complete(...)) } parse_modifier(...).or_add_diagnostic(...);
Present(m.complete(...)); keeping the same symbols m.complete,
TW_ARBITRARY_CANDIDATE, parse_modifier, expected_modifier and Present.
🪄 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: e33d8002-0db2-406c-83e6-3f75ab63436e
⛔ Files ignored due to path filters (34)
crates/biome_tailwind_factory/src/generated/node_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_tailwind_factory/src/generated/syntax_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_tailwind_parser/tests/tailwind_specs/error/arbitrary-candidate/missing-value-in-arbitrary.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/error/css-arbitrary-values.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/error/css-number-exponents.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-functions.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-math-functions.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-number-exponents.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-urls.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-values.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/gradient.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/image-url.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/inset.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/misc-xy.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/shadow.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/arbitrary-candidate-0.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/arbitrary-candidate-1.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/arbitrary-candidate-2.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/arbitrary-candidate-3.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/arbitrary-candidate-css-function.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/css-arbitrary-candidates.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/group-data.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/simple/arbitrary-value-0.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/simple/arbitrary-value-1.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/simple/base-has-dash-1.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/simple/base-has-dash-2.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/stress/stress-1.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/variants/arbitrary-selector-functional-candidate.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/variants/consecutive-arbitrary-variants.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/variants/functional-arbirary-param.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_syntax/src/generated/kind.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_tailwind_syntax/src/generated/macros.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_tailwind_syntax/src/generated/nodes.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_tailwind_syntax/src/generated/nodes_mut.rsis excluded by!**/generated/**,!**/generated/**and included by**
📒 Files selected for processing (20)
.claude/skills/parser-development/SKILL.mdcrates/biome_tailwind_parser/src/lexer/mod.rscrates/biome_tailwind_parser/src/lexer/tests.rscrates/biome_tailwind_parser/src/syntax/css_value.rscrates/biome_tailwind_parser/src/syntax/mod.rscrates/biome_tailwind_parser/src/syntax/value.rscrates/biome_tailwind_parser/src/token_source.rscrates/biome_tailwind_parser/tests/tailwind_specs/error/css-arbitrary-values.txtcrates/biome_tailwind_parser/tests/tailwind_specs/error/css-number-exponents.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-functions.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-math-functions.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-number-exponents.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-urls.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-values.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/arbitrary-candidate-css-function.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/css-arbitrary-candidates.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/variants/arbitrary-selector-functional-candidate.txtcrates/biome_tailwind_parser/tests/tailwind_specs/ok/variants/consecutive-arbitrary-variants.txtxtask/codegen/src/tailwind_kinds_src.rsxtask/codegen/tailwind.ungram
✅ Files skipped from review due to trivial changes (8)
- crates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/arbitrary-candidate-css-function.txt
- crates/biome_tailwind_parser/tests/tailwind_specs/error/css-number-exponents.txt
- crates/biome_tailwind_parser/tests/tailwind_specs/ok/variants/arbitrary-selector-functional-candidate.txt
- crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-number-exponents.txt
- crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-urls.txt
- crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/css-functions.txt
- crates/biome_tailwind_parser/tests/tailwind_specs/ok/candidates/css-arbitrary-candidates.txt
- crates/biome_tailwind_parser/tests/tailwind_specs/ok/variants/consecutive-arbitrary-variants.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_tailwind_parser/src/syntax/value.rs
Comment on lines
+257
to
+260
| AnyCssUrlValue = | ||
| CssUrlValueRaw | ||
| | CssString | ||
| | CssParameterList |
Contributor
There was a problem hiding this comment.
CssParameterList is not a valid URL value form.
AnyCssUrlValue should only ever be a raw literal or a quoted string. Listing CssParameterList as a third alternative makes url(expr, expr) structurally valid in the AST, which isn't a real CSS URL form. Error content that doesn't lex as a raw URL or string body should fall back to CssBogusPropertyValue, not be parsed as a comma-separated expression list.
🛠️ Suggested fix
AnyCssUrlValue =
CssUrlValueRaw
| CssString
- | CssParameterList
+ | CssBogusPropertyValue📝 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.
Suggested change
| AnyCssUrlValue = | |
| CssUrlValueRaw | |
| | CssString | |
| | CssParameterList | |
| AnyCssUrlValue = | |
| CssUrlValueRaw | |
| | CssString | |
| | CssBogusPropertyValue |
🤖 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 `@xtask/codegen/tailwind.ungram` around lines 257 - 260, AnyCssUrlValue
currently includes CssParameterList which incorrectly allows constructs like
url(expr, expr) into the AST; change the grammar so AnyCssUrlValue only accepts
CssUrlValueRaw or CssString and remove CssParameterList from its alternatives,
and ensure parsing of non-URL/raw/string content falls back to
CssBogusPropertyValue (update the parser rule that produces AnyCssUrlValue and
the error/recovery path so invalid parameter lists are emitted as
CssBogusPropertyValue rather than as CssParameterList).
ematipico
approved these changes
May 8, 2026
dyc3
force-pushed
the
dyc3/tw-parse-arbitrary-css-values
branch
from
6b164c8 to
7caea11
Compare
dyc3
force-pushed
the
dyc3/tw-parse-arbitrary-css-values
branch
from
7caea11 to
737e800
Compare
jiwon79
added a commit
to jiwon79/biome
that referenced
this pull request
May 10, 2026
NamedTyped only carries Number/Percentage/Ratio in the preset, so delete the 12 dormant Color/Length/Angle/Image/etc. predicates and their helpers. ValueType::matches now dispatches only those three and returns false for the rest. ArbitraryTyped/Arbitrary branches stay in the preset for the follow-up PR that wires them up via the parser nodes from biomejs#10299.
jiwon79
added a commit
to jiwon79/biome
that referenced
this pull request
May 10, 2026
NamedTyped only carries Number/Percentage/Ratio in the preset, so delete the 12 dormant Color/Length/Angle/Image/etc. predicates and their helpers. ValueType::matches now dispatches only those three and returns false for the rest. ArbitraryTyped/Arbitrary branches stay in the preset for the follow-up PR that wires them up via the parser nodes from biomejs#10299.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This implements more precise parsing of arbitrary values in tailwind candidates.
There's 3 important decisions I made:
Cssprefix instead of using theTwbecause its more clear what is tailwind syntax and css syntax.The performance regressions are expected to a certain extent because we are now doing more complicated parsing, where previously we were doing effectively nothing.
Implemented initially by gpt 5.5, but i had to make significant changes to it.
related: #10291
Test Plan
snapshots and new tests
Docs