feat(css): support SCSS qualified names in values and function calls#9096
feat(css): support SCSS qualified names in values and function calls#9096denbezrukov merged 3 commits intonextfrom
Conversation
|
github-actions
bot
added
A-Linter
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds SCSS qualified-name support and wires it through parser, formatter, lints, tests and GRIT mappings. Introduces ScssQualifiedName and AnyCssFunctionName grammar nodes, updates CSS function parsing to accept SCSS-qualified names, adjusts lint code to extract function-name tokens via identifier variants, generates formatters and generated.rs wiring for the new nodes, adds SCSS tests for qualified names, and updates kind-by-name mappings. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/biome_css_analyze/src/lint/correctness/no_unknown_function.rs (1)
81-84: SCSS qualified function names are silently skipped.When
node.name()returnsAnyCssFunctionName::ScssQualifiedName,as_css_identifier()returnsNone, so the rule bails out early. This is a sensible default (e.g.,math.divshouldn't be flagged as unknown), but it means all SCSS namespaced functions bypass the check — including genuinely misspelled ones likemath.divv(). Worth a follow-up to validate the member portion if you want tighter coverage.crates/biome_css_parser/src/syntax/scss/mod.rs (1)
65-72:parse_scss_function_nameshould returnParsedSyntaxto match parser conventions.Every other
parse_*function in the codebase returnsParsedSyntax—this is the only one that returns(). Even though the current call site doesn't use the result, it's an easy fix to keep the API consistent and future-proof.♻️ Suggested change
-pub(crate) fn parse_scss_function_name(p: &mut CssParser) { +pub(crate) fn parse_scss_function_name(p: &mut CssParser) -> ParsedSyntax { if is_at_scss_qualified_name(p) { - parse_scss_qualified_name(p).ok(); + parse_scss_qualified_name(p) } else { - parse_regular_identifier(p).ok(); + parse_regular_identifier(p) } }
Merging this PR will not alter performance
Comparing 
Footnotes
|
denbezrukov
force-pushed
the
db/scss-qualified-names-dedicated
branch
2 times, most recently
from
e619d77 to
24f3296
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/biome_css_parser/src/syntax/scss/mod.rs`:
- Around line 52-63: The branch handling a dollar-sign callable in
is_at_scss_qualified_name_function is dead/unnecessary; after confirming
is_at_scss_qualified_name(p) simply return p.nth_at(3, T!['(']) and remove the
p.nth_at(2, T![$]) conditional and the is_nth_at_identifier(p, 3) check (or
alternately keep the branch but add a clear comment about intentionally allowing
ns.$func() as tolerant error handling); update
is_at_scss_qualified_name_function to only perform the parentheses lookahead so
the redundant ns.$identifier handling is eliminated.
🧹 Nitpick comments (1)
crates/biome_css_parser/src/syntax/scss/mod.rs (1)
65-72:parse_scss_function_namereturns()rather thanParsedSyntax.This works as a fire-and-forget dispatch helper inside
parse_function's marker, but it diverges from the project convention that parse helpers returnParsedSyntax. If a future caller needs the result (e.g. for error recovery), it'll require a signature change. Low risk for now, but worth noting.Based on learnings: "Parse rules must take a mutable reference to the parser as their only parameter and return a
ParsedSyntax".♻️ Optional: return ParsedSyntax for consistency
-pub(crate) fn parse_scss_function_name(p: &mut CssParser) { +pub(crate) fn parse_scss_function_name(p: &mut CssParser) -> ParsedSyntax { if is_at_scss_qualified_name(p) { - parse_scss_qualified_name(p).ok(); + parse_scss_qualified_name(p) } else { - parse_regular_identifier(p).ok(); + parse_regular_identifier(p) } }Callers that discard the result can still call
.ok().
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/biome_css_parser/src/syntax/scss/mod.rs`:
- Around line 65-72: parse_scss_function_name currently returns () while all
other parse_... helpers return ParsedSyntax; update parse_scss_function_name to
return ParsedSyntax and propagate the results of the chosen parser so callers
can detect failures: call is_at_scss_qualified_name(p) and if true return
parse_scss_qualified_name(p) otherwise return parse_regular_identifier(p),
ensuring the function signature and return type use ParsedSyntax and not unit.
Reference: parse_scss_function_name, is_at_scss_qualified_name,
parse_scss_qualified_name, parse_regular_identifier, ParsedSyntax.
🧹 Nitpick comments (1)
crates/biome_css_analyze/src/lint/correctness/no_unknown_unit.rs (1)
117-139: Minor style inconsistency between the two extraction paths.Lines 121–122 use
.as_css_identifier().and_then(|name| name.value_token().ok())?while Lines 137–138 use.as_css_identifier()?.value_token().ok()?. Both are equivalent, but using the same style would be tidier.
denbezrukov
force-pushed
the
db/scss-qualified-names-dedicated
branch
from
24f3296 to
5b46088
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/biome_css_parser/src/syntax/scss/mod.rs (1)
52-55:is_at_scss_qualified_name_functiononly works for non-$members — this is fine, but worth a comment.The lookahead
p.nth_at(3, T!['('])correctly matchesmodule.func((3 tokens before(), but won't matchmodule.$func((4 tokens). Since$-prefixed SCSS variables aren't callable, this is correct behaviour. A brief inline comment would prevent future confusion, especially sinceis_at_scss_qualified_namedoes match the$form.Suggested comment
#[inline] pub(crate) fn is_at_scss_qualified_name_function(p: &mut CssParser) -> bool { + // Only matches `module.func(` — the `module.$var` form has 4 tokens + // before `(`, but $-prefixed SCSS variables are not callable anyway. is_at_scss_qualified_name(p) && p.nth_at(3, T!['(']) }crates/biome_css_parser/src/syntax/mod.rs (1)
305-306: Note: inconsistent feature-gating style betweenis_at_scss_identifierandis_at_scss_qualified_name.
is_at_scss_identifier(line 284) is not gated onCssSyntaxFeatures::Scss— it relies onparse_exclusive_syntaxat parse time (line 300). In contrast,is_at_scss_qualified_nameis gated at detection and parse time. Both approaches work, but the inconsistency could trip up future contributors. Not blocking — just flagging for awareness.
…for expected identifiers
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/biome_css_parser/src/syntax/scss/mod.rs`:
- Around line 31-33: The lookahead in is_at_scss_qualified_name currently allows
`ns.$` because p.nth_at(2, T![$]) doesn't ensure an identifier follows the `$`;
update the condition in is_at_scss_qualified_name so the `$` branch also
requires a trailing identifier (use is_nth_at_identifier with the next offset),
e.g. require p.nth_at(2, T![$]) && is_nth_at_identifier(p, 3), while keeping the
existing identifier branch (is_nth_at_identifier(p, 2)) and the initial
is_at_identifier(p) check.
…nd update diagnostics
2 participants
This PR was prepared with AI assistance (Codex/ChatGPT) for planning and PR text
drafting. I reviewed the final code and test changes manually.
Summary
Added support for SCSS qualified names in values and function names.
This slice introduces parsing/formatting support for patterns like:
math.div(10px, 2)map.get($map, a)map.$defaultI intentionally kept
ScssQualifiedNameseparate fromScssNamespacedIdentifierinsteadof reusing one node for both forms.
They represent different syntax roles:
ScssNamespacedIdentifiermodels declaration names likens.$var(SCSS declarationcontext,
$member only).ScssQualifiedNamemodels value/function qualified names likemath.div,map.get,and
map.$default(value/function context, member can be eitherCssIdentifierorScssIdentifier).Test Plan
Green CI