fix(css): preserve CSS unicode escapes in noUselessEscapeInString#9389
fix(css): preserve CSS unicode escapes in noUselessEscapeInString#9389xvchris wants to merge 3 commits intobiomejs:mainfrom
noUselessEscapeInString#9389Conversation
The rule incorrectly treated CSS unicode escapes (e.g. \e7bb, \a, \AB12CD) as useless escapes and removed the backslash during auto-fix, breaking iconfont and other unicode content. In CSS, a backslash followed by 1-6 hex digits is a valid unicode escape sequence (e.g. \e7bb represents U+E7BB). The rule only recognized hex digits 0-7 as meaningful, missing 8-9 and a-f/A-F. This fix extends the meaningful escape check to cover all hex digits, matching the CSS specification for unicode escape sequences. Closes biomejs#9385
🦋 Changeset detectedLatest commit: 1b6eaad The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 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 |
|
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 ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR fixes the noUselessEscapeInString CSS lint rule. The lint now recognises CSS hex escapes (a backslash followed by 1–6 hex digits, e.g. \e7bb) as meaningful and no longer strips them from string literals. Tests and examples were updated to include valid hex-escape cases and adjust invalid examples accordingly. Suggested reviewers
🚥 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.
🧹 Nitpick comments (2)
crates/biome_css_analyze/tests/specs/suspicious/noUselessEscapeInString/valid.css (1)
8-22: Please add a case starting with8or9as well.These fixtures cover escapes beginning with
a–f/A–F, but none start with8or9, which was the other half of the bug. A tiny sample likecontent: "\89"would lock that branch down too.🧪 Minimal addition
.g::after { content: "\AB12CD" } +.h::after { + content: "\89" +}🤖 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 - 22, Add a test case for a CSS unicode escape that starts with 8 or 9 to cover the missing branch; e.g., add a new rule similar to .g::after (or a new .h::after) with content: "\89" (or "\8A" etc.) alongside the existing .c/.d/.e/.f/.g cases so the parser branch handling escapes starting with 8/9 is exercised..changeset/fix-css-unicode-escape.md (1)
5-5: Keep the changeset user-facing.The second sentence dips into matcher internals; I’d trim it to the broken behaviour users actually saw.
As per coding guidelines, changeset descriptions must be written for end users (explain impact and what changed), not developers (avoid implementation details).✍️ Possible rewrite
-Fixed [`#9385`](https://github.com/biomejs/biome/issues/9385): the [`noUselessEscapeInString`](https://biomejs.dev/linter/rules/no-useless-escape-in-string/) rule no longer removes valid CSS unicode escape sequences (e.g. `\e7bb`, `\e644`) from string literals. Previously, backslashes followed by hex digits `8`–`9` and `a`–`f`/`A`–`F` were incorrectly treated as useless escapes, breaking iconfont `content` properties. +Fixed [`#9385`](https://github.com/biomejs/biome/issues/9385): the [`noUselessEscapeInString`](https://biomejs.dev/linter/rules/no-useless-escape-in-string/) rule no longer strips valid CSS unicode escapes from string literals. This fixes broken iconfont `content` values caused by the autofix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/fix-css-unicode-escape.md at line 5, Update the changeset text in .changeset/fix-css-unicode-escape.md to be user-facing: remove the internal matcher detail about hex digit ranges and instead concisely state the observable behavior and impact—e.g., that the noUselessEscapeInString rule no longer strips valid CSS unicode escapes like “\e7bb”/“\e644”, which previously broke iconfont content properties—so the description explains the user-facing fix and its effect without implementation details.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.changeset/fix-css-unicode-escape.md:
- Line 5: Update the changeset text in .changeset/fix-css-unicode-escape.md to
be user-facing: remove the internal matcher detail about hex digit ranges and
instead concisely state the observable behavior and impact—e.g., that the
noUselessEscapeInString rule no longer strips valid CSS unicode escapes like
“\e7bb”/“\e644”, which previously broke iconfont content properties—so the
description explains the user-facing fix and its effect without implementation
details.
In
`@crates/biome_css_analyze/tests/specs/suspicious/noUselessEscapeInString/valid.css`:
- Around line 8-22: Add a test case for a CSS unicode escape that starts with 8
or 9 to cover the missing branch; e.g., add a new rule similar to .g::after (or
a new .h::after) with content: "\89" (or "\8A" etc.) alongside the existing
.c/.d/.e/.f/.g cases so the parser branch handling escapes starting with 8/9 is
exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 01aa2474-3e25-48b4-9736-3cbb613c1a89
⛔ 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 (4)
.changeset/fix-css-unicode-escape.mdcrates/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
.changeset/fix-css-unicode-escape.md
Outdated
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Fixed [#9385](https://github.com/biomejs/biome/issues/9385): the [`noUselessEscapeInString`](https://biomejs.dev/linter/rules/no-useless-escape-in-string/) rule no longer removes valid CSS unicode escape sequences (e.g. `\e7bb`, `\e644`) from string literals. Previously, backslashes followed by hex digits `8`–`9` and `a`–`f`/`A`–`F` were incorrectly treated as useless escapes, breaking iconfont `content` properties. |
There was a problem hiding this comment.
oh good, the ai figured out it was hex sequence that's allowed.
There was a problem hiding this comment.
Simplified the changeset to be more user-facing as suggested. Also added test cases for \89, \eezaf, and \edk to cover the edge cases you and @dyc3 brought up.
crates/biome_css_analyze/tests/specs/suspicious/noUselessEscapeInString/invalid.css
Show resolved
Hide resolved
crates/biome_css_analyze/tests/specs/suspicious/noUselessEscapeInString/invalid.css
Show resolved
Hide resolved
Merging this PR will not alter performance
Comparing Footnotes
|
|
@dyc3 Thanks for the review! I've addressed all the feedback and added the extra test cases. Could you re-approve when you get a chance? 🙏 |
Summary
Fixes #9385
The
noUselessEscapeInStringrule incorrectly identified CSS unicode escape sequences (e.g.\e7bb,\e644,\a) as useless escapes and stripped the backslash during auto-fix, breaking iconfont and other unicode content.What changed
In CSS,
\followed by 1–6 hex digits is a valid unicode escape sequence (e.g.\e7bb→ U+E7BB). Thenext_useless_escapefunction only recognized hex digits0–7as meaningful after a backslash, missing8,9, anda–f/A–F.Before:
content: "\e7bb"→ auto-fixed tocontent: "e7bb"(broken)After:
content: "\e7bb"→ no diagnostic (correct)Changes
next_useless_escapeto cover all hex digits (0–9,a–f,A–F)\g(a genuinely useless escape) instead of\a(valid hex escape)\e7bb,\e644,\a,\0a,\AB12CD\ginstead of\aTest plan
All 109 CSS analyzer spec tests pass.
AI Disclosure
This PR was authored with AI assistance (Claude). The bug was identified and fix was implemented with AI help.