-
-
Notifications
You must be signed in to change notification settings - Fork 739
fix(parse/css): let @utility parse declarations and rules
#7314
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
|
Walkthrough
Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
xtask/codegen/css.ungram (1)
449-466: Regenerate grammar and refresh all generated code
You’ve adjusted the ungram rules but the oldAnyCssDeclarationBlock/CssDeclarationBlocktypes are still in the generated artifacts (syntax, parser tests, formatter, analyzer, etc.). Please:
- Rerun the CSS codegen (
xtask codegen) soAnyCssDeclarationOrRuleBlockandCssDeclarationOrRuleBlockreplace the old block types.- Commit the updated generated files under
crates/biome_css_syntax,crates/biome_css_parser,crates/biome_css_formatter, andcrates/biome_css_analyze.- Ensure there are no remaining references to
AnyCssDeclarationBlockorCssDeclarationBlockin the repo.This is essential to keep the AST and downstream tooling in sync.
🧹 Nitpick comments (5)
xtask/codegen/css.ungram (1)
1872-1878: Update the inline comment to reflect nested rules, not just “functional utilities”.The docstring still talks about functional utilities only. Let’s mention nested rules to avoid future confusion.
-// Enhanced @utility directive to support functional utilities -// @utility tab-* { tab-size: --value([integer]); } -// @utility tab-* { tab-size: --value("inherit", "initial", "unset"); } +// Enhanced @utility directive to support declarations and nested rules +// @utility tab-* { tab-size: --value([integer]); } +// @utility tab-* { tab-size: --value("inherit", "initial", "unset"); } +// @utility border-overlay-* { +// position: relative; +// &::after { border-width: 1px; } +// }crates/biome_css_parser/src/syntax/at_rule/tailwind.rs (1)
55-63: Edge-case check: functional utility detection.The
ident - *lookahead handlesfoo-*cleanly. Two quick follow-ups:
- Consider a test for a bare
@utility foo-* { &::after { ... } }(no declarations at top-level) to exercise rule-list-only blocks.- Add a negative test like
@utility -* {}to confirm graceful recovery.I can draft those spec tests if you like; shout and I’ll add them.
crates/biome_css_parser/tests/css_test_suite/ok/tailwind/utility/with-sub-block.css (1)
1-7: Good coverage for nested pseudo-element under @Utility.This “happy path” fixture validates the new grammar shape. Consider a companion fixture with only nested rules and no declarations to cover the pure rule-list case:
- File: crates/biome_css_parser/tests/css_test_suite/ok/tailwind/utility/only-rule-list.css
- Contents:
@utility border-overlay-* { &::after { border-width: 1px; } }crates/biome_css_parser/tests/quick_test.rs (2)
4-6: No need for pub on a test function.Minor tidy: test functions don’t need to be public.
-#[ignore] -#[test] -pub fn quick_test() { +#[ignore] +#[test] +fn quick_test() {
26-29: Assert on diagnostics and drop noisy dbg!.Make the guard stronger by asserting there are no parser errors; keep the bogus/empty-slot check.
dbg!can be noisy even when ignored tests are run manually.- dbg!(&syntax, root.diagnostics(), root.has_errors()); - if has_bogus_nodes_or_empty_slots(&syntax) { - panic!("modified tree has bogus nodes or empty slots:\n{syntax:#?} \n\n {syntax}") - } + assert!( + !root.has_errors(), + "Parser reported errors: {:#?}", + root.diagnostics() + ); + if has_bogus_nodes_or_empty_slots(&syntax) { + panic!("modified tree has bogus nodes or empty slots:\n{syntax:#?}\n\n{syntax}"); + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (15)
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_parser/tests/css_test_suite/error/tailwind/when-disabled/utility.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/tailwind/when-enabled/value-arbitrary.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/tailwind/when-enabled/value-incomplete.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/tailwind/simple.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/tailwind/utility/arbitrary-star.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/tailwind/utility/enhanced-value-function.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/tailwind/utility/modifier.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/tailwind/utility/simple-utility.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/tailwind/utility/value-literals.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/tailwind/utility/with-param.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/tailwind/utility/with-sub-block.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 (5)
crates/biome_css_parser/src/syntax/at_rule/tailwind.rs(2 hunks)crates/biome_css_parser/tests/css_test_suite/ok/tailwind/utility/with-sub-block.css(1 hunks)crates/biome_css_parser/tests/quick_test.rs(1 hunks)crates/biome_css_parser/tests/spec_test.rs(0 hunks)xtask/codegen/css.ungram(1 hunks)
💤 Files with no reviewable changes (1)
- crates/biome_css_parser/tests/spec_test.rs
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_css_parser/tests/quick_test.rscrates/biome_css_parser/src/syntax/at_rule/tailwind.rs
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/quick_test.rscrates/biome_css_parser/src/syntax/at_rule/tailwind.rscrates/biome_css_parser/tests/css_test_suite/ok/tailwind/utility/with-sub-block.css
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_css_parser/tests/quick_test.rscrates/biome_css_parser/src/syntax/at_rule/tailwind.rscrates/biome_css_parser/tests/css_test_suite/ok/tailwind/utility/with-sub-block.css
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_css_parser/tests/quick_test.rscrates/biome_css_parser/tests/css_test_suite/ok/tailwind/utility/with-sub-block.css
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
🧠 Learnings (7)
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Use `biome_js_analyze/tests/quick_test.rs` for quick, ad-hoc testing; un-ignore the test and adjust the rule filter as needed
Applied to files:
crates/biome_css_parser/tests/quick_test.rs
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/spec_test.rs : Create tests/spec_test.rs implementing the run(spec_input_file, _expected_file, test_directory, _file_type) function as shown and include!("language.rs")
Applied to files:
crates/biome_css_parser/tests/quick_test.rs
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/spec_tests.rs : Create tests/spec_tests.rs in the biome_html_formatter crate that generates tests via tests_macros::gen_tests! for all HTML files at tests/specs/html/**/*.html
Applied to files:
crates/biome_css_parser/tests/quick_test.rs
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/language.rs : Create tests/language.rs defining HtmlTestFormatLanguage and implement TestFormatLanguage for it
Applied to files:
crates/biome_css_parser/tests/quick_test.rs
📚 Learning: 2025-08-11T11:53:15.299Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/../biome_lsp/src/server.tests.rs : Keep end-to-end LSP tests in biome_lsp’s server.tests.rs
Applied to files:
crates/biome_css_parser/tests/quick_test.rs
📚 Learning: 2025-08-11T11:53:15.299Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Place watcher tests for workspace methods in src/workspace/watcher.tests.rs
Applied to files:
crates/biome_css_parser/tests/quick_test.rs
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Applied to files:
crates/biome_css_parser/tests/quick_test.rs
🧬 Code graph analysis (2)
crates/biome_css_parser/tests/quick_test.rs (2)
crates/biome_css_parser/src/lib.rs (1)
parse_css(27-30)crates/biome_test_utils/src/lib.rs (1)
has_bogus_nodes_or_empty_slots(384-399)
crates/biome_css_parser/src/syntax/at_rule/tailwind.rs (1)
crates/biome_css_parser/src/syntax/block/declaration_or_rule_list_block.rs (1)
parse_declaration_or_rule_list_block(19-21)
⏰ 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). (23)
- GitHub Check: Documentation
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: autofix
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Bench (biome_package)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Check Dependencies
- GitHub Check: Bench (biome_js_formatter)
🔇 Additional comments (2)
xtask/codegen/css.ungram (1)
1876-1879: Switching TwUtilityAtRule.block to AnyCssDeclarationOrRuleBlock is the right call.This aligns
@utilitywith nested selector use-cases and mirrors other Tailwind at-rules that accept rule lists. Nice one.crates/biome_css_parser/src/syntax/at_rule/tailwind.rs (1)
48-51: Using parse_declaration_or_rule_list_block unlocks nested selectors under @Utility.This is the core behavioural change and looks correct. It also aligns with
@theme/@varianthandling.
CodSpeed Performance ReportMerging #7314 will not alter performanceComparing Summary
|
Summary
This lets
@utilityat-rules parse both declarations and rules instead of just declarations.Fixes issue reported in this thread: #7223 (comment)
Test Plan
Added a snapshot test
Docs