-
-
Notifications
You must be signed in to change notification settings - Fork 781
fix(parse/tailwind): fix lexing inset-x, border-slate-500, and others
#7975
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
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
WalkthroughThis PR enhances the Tailwind lexer's base name handling by implementing boundary validation after matched bases. When a base name is found in Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
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.
If possible, I would remove the stress file from the usual testing suite for performance reasons
crates/biome_tailwind_parser/tests/tailwind_specs/ok/stress/stress-3.txt.snap
Outdated
Show resolved
Hide resolved
564d878 to
13dd0b9
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: 0
🧹 Nitpick comments (1)
crates/biome_tailwind_parser/src/lexer/mod.rs (1)
143-151: Consider combining the duplicate branches.Both arms perform identical operations. You could simplify slightly:
match self.byte_at(base_name.len()) { - Some(b'-') | None => { - self.advance(base_name.len()); - return TW_BASE; - } - Some(b) if b.is_ascii_whitespace() => { + Some(b'-') | None | Some(b) if b.is_ascii_whitespace() => { self.advance(base_name.len()); return TW_BASE; } _ => {} }Though the current explicit form is perfectly clear.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
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/simple/basic-2.txt.snapis excluded by!**/*.snapand included by**crates/biome_tailwind_parser/tests/tailwind_specs/ok/simple/border.txt.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (6)
crates/biome_tailwind_parser/src/lexer/mod.rs(1 hunks)crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/inset.txt(1 hunks)crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/misc-xy.txt(1 hunks)crates/biome_tailwind_parser/tests/tailwind_specs/ok/simple/basic-2.txt(1 hunks)crates/biome_tailwind_parser/tests/tailwind_specs/ok/simple/border.txt(1 hunks)crates/biome_tailwind_syntax/src/metadata.rs(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/inset.txt
- crates/biome_tailwind_syntax/src/metadata.rs
- crates/biome_tailwind_parser/tests/tailwind_specs/ok/brackets/misc-xy.txt
- crates/biome_tailwind_parser/tests/tailwind_specs/ok/simple/border.txt
- crates/biome_tailwind_parser/tests/tailwind_specs/ok/simple/basic-2.txt
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-15T09:24:31.042Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:24:31.042Z
Learning: Applies to crates/biome_parser/crates/**/src/lexer/mod.rs : Create a lexer module at crates/<parser_crate>/src/lexer/mod.rs
Applied to files:
crates/biome_tailwind_parser/src/lexer/mod.rs
📚 Learning: 2025-10-15T09:22:46.002Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:22:46.002Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Do not attempt to fix code; if a mandatory token/node is missing, return `None` instead
Applied to files:
crates/biome_tailwind_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). (7)
- GitHub Check: Documentation
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: autofix
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: Test Node.js API
🔇 Additional comments (2)
crates/biome_tailwind_parser/src/lexer/mod.rs (2)
132-165: Solid boundary validation fix.The implementation correctly solves the false-match problem described in the PR. The fallthrough to generic logic ensures
border-slate-500parses asborder(notborder-s), whilst dash-containing bases likeinset-xare properly recognised when followed by valid delimiters.
142-153: No action required—byte_atexists via theLexertrait.The
byte_atmethod is defined as a default implementation in theLexertrait, whichTailwindLexerimplements. The boundary validation logic is sound and correctly prevents false matches likeborder-sinborder-slate-500.
…hers (biomejs#7975) <!-- IMPORTANT!! If you generated this PR with the help of any AI assistance, please disclose it in the PR. https://github.com/biomejs/biome/blob/main/CONTRIBUTING.md#ai-assistance-notice --> <!-- Thanks for submitting a Pull Request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your PR. Once created, your PR will be automatically labeled according to changed files. Learn more about contributing: https://github.com/biomejs/biome/blob/main/CONTRIBUTING.md --> ## Summary <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve?--> Fixes misc lexing bugs - some base names like `inset-x` and `inset-y` were missing - some candidates like `border-slate-500` would get aggressively interpreted as `border-s` as the base name, when it should be `border`. <!-- Link any relevant issues if necessary or include a transcript of any Discord discussion. --> <!-- If you create a user-facing change, please write a changeset: https://github.com/biomejs/biome/blob/main/CONTRIBUTING.md#writing-a-changeset (your changeset is often a good starting point for this summary as well) --> ## Test Plan <!-- What demonstrates that your implementation is correct? --> added tests ## Docs <!-- If you're submitting a new rule or action (or an option for them), the documentation is part of the code. Make sure rules and actions have example usages, and that all options are documented. --> <!-- For other features, please submit a documentation PR to the `next` branch of our website: https://github.com/biomejs/website/. Link the PR here once it's ready. -->

Summary
Fixes misc lexing bugs
inset-xandinset-ywere missingborder-slate-500would get aggressively interpreted asborder-sas the base name, when it should beborder.Test Plan
added tests
Docs