fix(css): preserve CSS unicode escapes in noUselessEscapeInString#9390
fix(css): preserve CSS unicode escapes in noUselessEscapeInString#9390xvchris wants to merge 2 commits intobiomejs:mainfrom
Conversation
The rule incorrectly flagged CSS unicode escape sequences (e.g. \e7bb, \e644) as useless escapes because it only recognized hex digits 0-7 after a backslash. In CSS, \ followed by 1-6 hex digits (0-9, a-f, A-F) is a valid unicode escape sequence. Expanded the hex digit range from 0-7 to the full set (0-9, a-f, A-F). The previously separate b and f entries are now covered by the a-f range. Fixes biomejs#9385
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe CSS 🚥 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)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_css_analyze/tests/specs/suspicious/noUselessEscapeInString/valid.css (1)
8-19: Add one single-digit hex escape to pin the edge case.These cases cover multi-digit escapes nicely, but they do not prove that lone hex escapes like
\a,\b, or\fstay valid after dropping the explicit branches. One tiny fixture here would keep that regression firmly boxed in.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_analyze/tests/specs/suspicious/noUselessEscapeInString/valid.css` around lines 8 - 19, Add a single-digit hex escape fixture to this test by adding a new rule (e.g., .icon-single-digit:before) near the other examples and set its content to a one-digit unicode escape like "\a" (so the rule looks like .icon-single-digit:before { content: "\a" }). This pins the edge case for lone hex escapes without changing existing multi-digit examples such as .icon-digit:before or .icon-hex:before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_css_analyze/src/lint/suspicious/no_useless_escape_in_string.rs`:
- Around line 112-114: Update the rustdoc example in the docstring of the lint
in no_useless_escape_in_string.rs so it uses a truly useless escape (e.g. "\z")
instead of "\a" which is now considered a meaningful hex-related escape; locate
the example in the doc comment for the lint (the rustdoc block around the rule
implementation) and replace the invalid example string, and run cargo test /
rustdoc checks to ensure the documentation example matches the new hex-handling
behavior.
---
Nitpick comments:
In
`@crates/biome_css_analyze/tests/specs/suspicious/noUselessEscapeInString/valid.css`:
- Around line 8-19: Add a single-digit hex escape fixture to this test by adding
a new rule (e.g., .icon-single-digit:before) near the other examples and set its
content to a one-digit unicode escape like "\a" (so the rule looks like
.icon-single-digit:before { content: "\a" }). This pins the edge case for lone
hex escapes without changing existing multi-digit examples such as
.icon-digit:before or .icon-hex:before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 18916fd3-d430-44cd-980a-391ca4d76110
⛔ Files ignored due to path filters (2)
crates/biome_css_analyze/tests/specs/suspicious/noUselessEscapeInString/invalid.css.snapis excluded by!**/*.snapand included by**crates/biome_css_analyze/tests/specs/suspicious/noUselessEscapeInString/valid.css.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_css_analyze/src/lint/suspicious/no_useless_escape_in_string.rscrates/biome_css_analyze/tests/specs/suspicious/noUselessEscapeInString/invalid.csscrates/biome_css_analyze/tests/specs/suspicious/noUselessEscapeInString/valid.css
crates/biome_css_analyze/src/lint/suspicious/no_useless_escape_in_string.rs
Show resolved
Hide resolved
Merging this PR will not alter performance
Comparing Footnotes
|
\a is now recognized as a valid CSS hex escape (a is a hex digit), so it no longer triggers the diagnostic. Use \z which is genuinely useless as z is not a hex digit.
Summary
Fixes #9385
The
noUselessEscapeInStringrule incorrectly flagged CSS unicode escape sequences (e.g.\e7bb,\e644) as useless escapes and its auto-fix stripped the backslash, breaking iconfontcontentdeclarations.Root Cause
In
next_useless_escape(), the hex digit range after a backslash wasb'0'..=b'7', which only covers octal-range digits. CSS unicode escapes use the full hex range:\followed by 1-6 hex digits (0-9,a-f,A-F). Characters like\e,\a,\8,\9,\c,\dwere incorrectly treated as useless escapes.Fix
Expanded the recognized hex digit range to
b'0'..=b'9' | b'a'..=b'f' | b'A'..=b'F'. The previously separateb'b'andb'f'entries (for\band\f) are now subsumed by thea-frange — this is correct since in CSS strings,\band\fare unicode escapes for U+000B and U+000F respectively, not C-style control character escapes.Test Plan
\z(truly useless) instead of\a(valid hex escape)\e7bb,\e644,\AB12,\39Docs
N/A — This is a bugfix to an existing lint rule. No new rules, actions, or options are introduced.