feat(css): add support for the @function CSS at rule#8839
feat(css): add support for the @function CSS at rule#8839ematipico merged 20 commits intobiomejs:nextfrom
@function CSS at rule#8839Conversation
🦋 Changeset detectedLatest commit: d50037d The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 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 |
github-actions
bot
added
A-Parser
WalkthroughAdds first-class support for the CSS 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
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: 1
🤖 Fix all issues with AI agents
In `@crates/biome_css_parser/src/syntax/at_rule/function.rs`:
- Around line 215-224: The recovery predicate in
CssFunctionParameterNameParseRecovery::is_at_recovered currently stops on
commas, `)`, `:` and when is_at_type_function/is_at_syntax_type match, but it
misses identifier-style types allowed by is_at_syntax_single_component which
lets the parser swallow a following type/default; update is_at_recovered to also
return true when is_at_syntax_single_component(p) (or a more specific
"identifier type" predicate if available) so identifier types are preserved, and
add a regression test exercising an invalid parameter name followed by an
identifier-type/default to ensure recovery no longer swallows the type.
🧹 Nitpick comments (1)
xtask/codegen/css.ungram (1)
1765-1773: Consider allowing bogus blocks for@function. Most at‑rules that own declaration blocks use theAny*wrapper to preserve recovery; using it here would keep error recovery consistent when the block is malformed.♻️ Suggested tweak
CssFunctionAtRule = declarator: CssFunctionAtRuleDeclarator - block: CssDeclarationOrAtRuleBlock + block: AnyCssDeclarationOrAtRuleBlock
| struct CssFunctionParameterNameParseRecovery; | ||
|
|
||
| impl ParseRecovery for CssFunctionParameterNameParseRecovery { | ||
| type Kind = CssSyntaxKind; | ||
| type Parser<'source> = CssParser<'source>; | ||
| const RECOVERED_KIND: Self::Kind = CSS_BOGUS; | ||
|
|
||
| fn is_at_recovered(&self, p: &mut Self::Parser<'_>) -> bool { | ||
| p.at_ts(token_set![T![,], T![')'], T![:]]) || is_at_type_function(p) || is_at_syntax_type(p) | ||
| } |
There was a problem hiding this comment.
Potential recovery gap for identifier types after an invalid name.
If a parameter name is invalid (e.g., missing --) and the next token is an identifier type (allowed by is_at_syntax_single_component), CssFunctionParameterNameParseRecovery won’t stop and may swallow the type/default, reducing recovery quality. Consider adding a regression test for this scenario and, if it reproduces, extending the recovery stop condition to preserve identifier types.
🤖 Prompt for AI Agents
In `@crates/biome_css_parser/src/syntax/at_rule/function.rs` around lines 215 -
224, The recovery predicate in
CssFunctionParameterNameParseRecovery::is_at_recovered currently stops on
commas, `)`, `:` and when is_at_type_function/is_at_syntax_type match, but it
misses identifier-style types allowed by is_at_syntax_single_component which
lets the parser swallow a following type/default; update is_at_recovered to also
return true when is_at_syntax_single_component(p) (or a more specific
"identifier type" predicate if available) so identifier types are preserved, and
add a regression test exercising an invalid parameter name followed by an
identifier-type/default to ensure recovery no longer swallows the type.
Merging this PR will not alter performance
Comparing 
Footnotes
|
dyc3
left a comment
dyc3
left a comment
There was a problem hiding this comment.
At a glance, looks good. How does this interact with lint rules like noUnknownProperty? Would like to avoid introducing false positives if possible.
Co-authored-by: Carson McManus <dyc3@users.noreply.github.com>
ryan-m-walker
force-pushed
the
feat/css-function-at-rule
branch
from
f755c9b to
85d7397
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@crates/biome_css_parser/src/syntax/value/function.rs`:
- Around line 318-331: Rename the function parse_coma_separated_value to
parse_comma_separated_value and update all call sites to use the new name;
specifically change the function signature and body declaration
(parse_coma_separated_value -> parse_comma_separated_value) and replace any
invocations of parse_coma_separated_value (e.g., the call referenced near the
is_at_comma_separated_value check) to parse_comma_separated_value so the
identifier spelling is consistent across the module.
In `@crates/biome_css_syntax/src/lib.rs`:
- Around line 139-140: The match in is_bogus() must include the new bogus kinds
added to to_bogus(): add CSS_BOGUS_FUNCTION_PARAMETER and CSS_BOGUS_TYPE to the
is_bogus() pattern so those branches return true; locate the is_bogus() function
and update its match arm (the same enum/const checks that currently include
other CSS_BOGUS_* variants) to also match CSS_BOGUS_FUNCTION_PARAMETER and
CSS_BOGUS_TYPE, keeping consistency with to_bogus() which uses
AnyCssFunctionParameter::can_cast(*) and AnyCssType::can_cast(*).
🧹 Nitpick comments (2)
crates/biome_css_formatter/src/css/auxiliary/function_parameter.rs (1)
17-28: Unnecessaryformat_withwrapper.The
format_withclosure adds indirection without benefit here. Other formatters in this PR (e.g.,function_parameter_default_value.rs,returns_statement.rs) use straightforwardwrite!calls. Consider simplifying for consistency.♻️ Suggested simplification
- write!( - f, - [&format_with(|f| { - write!(f, [name.format()])?; - - if let Some(ty) = &ty { - write!(f, [space(), ty.format()])?; - } - - write!(f, [default_value.format()]) - })] - ) + write!(f, [name.format()])?; + + if let Some(ty) = &ty { + write!(f, [space(), ty.format()])?; + } + + write!(f, [default_value.format()])crates/biome_css_formatter/src/css/lists/function_parameter_list.rs (1)
11-19: Optional: add the joiner rationale comment for consistency.
This mirrorsparameter_list.rs, so a brief comment helps keep the intent uniform.♻️ Suggested tweak
- fn fmt(&self, node: &CssFunctionParameterList, f: &mut CssFormatter) -> FormatResult<()> { - let separator = soft_line_break_or_space(); + fn fmt(&self, node: &CssFunctionParameterList, f: &mut CssFormatter) -> FormatResult<()> { + // Using `join_with` instead of `join_nodes_with_soft_line` to avoid + // preserving empty lines from the input source (see FormatCssSelectorList). + let separator = soft_line_break_or_space(); let mut joiner = f.join_with(&separator);
There was a problem hiding this comment.
Thank you! Waiting to merge until #8839 (review) is answered by @ryan-m-walker
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@crates/biome_css_analyze/src/lint/correctness/no_unknown_property.rs`:
- Around line 131-133: The check that skips the in-function-at-rule descriptor
uses the wrong name: change the literal comparison in the no_unknown_property
logic from property_name_lower == "return" to property_name_lower == "result" so
the in_function_at_rule branch correctly recognizes the CSS "result" descriptor;
update any nearby comment text if present to reflect "result" instead of
"return".
In
`@crates/biome_css_analyze/tests/specs/correctness/noUnknownProperty/invalid.css`:
- Around line 13-17: The implementation in no_unknown_property.rs currently
treats the function-body descriptor as "return" but the spec and tests use
"result"; update the logic that checks property names inside `@function` blocks
(the branch that compares to the string "return") to accept "result" (or both
"return" and "result") so that "result: 1;" is not flagged as unknown inside
`@function` bodies; modify the check in no_unknown_property.rs accordingly and run
the existing test to confirm the failure is resolved.
- Around line 13-17: The `@function` at-rule handler in no_unknown_property.rs is
checking for the wrong property name ("return") when validating properties
inside a CSS `@function`; update the check to look for "result" instead of
"return" so the validator accepts the spec-correct property used in the tests
(change the literal/condition that compares against "return" to "result" in the
function that validates properties inside `@function` at-rules).
🧹 Nitpick comments (2)
crates/biome_css_analyze/tests/specs/correctness/noUnknownProperty/invalid.css (1)
9-11: Questionable test case forreturnproperty.This tests that
returnoutside a function at-rule is flagged as unknown. However, per the CSS Functions specification, the correct descriptor name isresult, notreturn. If the implementation is corrected, this test case may no longer serve its intended purpose—unless the intent is simply to confirmreturnis not a valid CSS property anywhere.crates/biome_css_analyze/src/lint/correctness/no_unknown_property.rs (1)
120-126: Simplify the closure.The closure body can be reduced to a single expression.
♻️ Suggested simplification
- let in_function_at_rule = node.syntax().ancestors().skip(1).any(|ancestor| { - if CssFunctionAtRule::can_cast(ancestor.kind()) { - return true; - } - - false - }); + let in_function_at_rule = node + .syntax() + .ancestors() + .skip(1) + .any(|ancestor| CssFunctionAtRule::can_cast(ancestor.kind()));
crates/biome_css_analyze/src/lint/correctness/no_unknown_property.rs
Outdated
Show resolved
Hide resolved
crates/biome_css_analyze/tests/specs/correctness/noUnknownProperty/invalid.css
Show resolved
Hide resolved
|
I don't see the reason why someone would use a regular prop in a |
3 participants
Summary
Adds support for the
@functionCSS at rule.Addresses #8184
Note: This PR also adds support for coma separated values which isn't specific to
@functionbut is needed for calling functions.Test Plan
Snapshot testing added for the parser and formatter
Docs
n/a
AI Usage Disclosure
The vast majority of the written code was by me. Mostly just used Claude code for minor debugging, test generation and small tasks.