fix(lint/noUselessEscapeInString): avoid false positive for $\{ escape in template literals#7624
Conversation
…e in template literals
🦋 Changeset detectedLatest commit: 1d52943 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 |
WalkthroughThe noUselessEscapeInString rule was changed to correctly recognise escapes inside template literals: it treats Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (24)
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)
.changeset/real-comics-build.md (1)
1-5: Changeset follows guidelines well.The entry correctly uses past tense, links to the issue, includes the rule documentation link, and ends with a full stop.
Consider adding a minimal code block to demonstrate the fix, as per the coding guidelines for changesets. For example:
+Fixed [#7595](https://github.com/biomejs/biome/issues/7595): [`noUselessEscapeInString`](https://biomejs.dev/linter/rules/no-useless-escape-in-string/) no longer reports `$\{` escape in template literals. + +```js +// Now correctly recognised as valid: +const s = `$\{var}`; +```As per coding guidelines.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
crates/biome_js_analyze/tests/specs/suspicious/noUselessEscapeInString/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/suspicious/noUselessEscapeInString/valid.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (4)
.changeset/real-comics-build.md(1 hunks)crates/biome_js_analyze/src/lint/suspicious/no_useless_escape_in_string.rs(2 hunks)crates/biome_js_analyze/tests/specs/suspicious/noUselessEscapeInString/invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/suspicious/noUselessEscapeInString/valid.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
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_js_analyze/tests/specs/suspicious/noUselessEscapeInString/invalid.jscrates/biome_js_analyze/src/lint/suspicious/no_useless_escape_in_string.rscrates/biome_js_analyze/tests/specs/suspicious/noUselessEscapeInString/valid.js
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/suspicious/noUselessEscapeInString/invalid.jscrates/biome_js_analyze/src/lint/suspicious/no_useless_escape_in_string.rscrates/biome_js_analyze/tests/specs/suspicious/noUselessEscapeInString/valid.js
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/suspicious/noUselessEscapeInString/invalid.jscrates/biome_js_analyze/tests/specs/suspicious/noUselessEscapeInString/valid.js
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: In changeset files, only use #### or ##### headers; avoid other header levels
Changeset descriptions should use past tense for what you did (e.g., "Added...")
Describe current Biome behavior in present tense within changesets (e.g., "Biome now supports...")
For bug fixes in changesets, start with a link to the issue (e.g., "Fixed #1234: ...")
When referencing rules or assists in changesets, include links to their documentation pages
Include a minimal code block in the changeset when applicable to demonstrate the change
End every sentence in the changeset description with a period
Files:
.changeset/real-comics-build.md
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_js_analyze/src/lint/suspicious/no_useless_escape_in_string.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). (24)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Test Node.js API
- GitHub Check: autofix
- GitHub Check: Documentation
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
🔇 Additional comments (3)
crates/biome_js_analyze/tests/specs/suspicious/noUselessEscapeInString/valid.js (1)
7-9: LGTM! Comprehensive test coverage for the fix.The new test cases properly cover both
$\{(the primary issue from #7595) and$\{}(with content after the brace). The trailing comma addition maintains consistency.crates/biome_js_analyze/src/lint/suspicious/no_useless_escape_in_string.rs (2)
43-47: Documentation updated appropriately.The doc comment now correctly describes both
\${and$\{as valid escapes in template literals.
182-187: Correct implementation for$\{escape handling.The logic properly checks that the backslash precedes
{and that$precedes the backslash. The index arithmetic is sound:ipoints to the backslash, soi-1correctly identifies the preceding character.
CodSpeed Performance ReportMerging #7624 will not alter performanceComparing Summary
|
…e in template literals (#7624)
Summary
Fixes #7595, which incorrectly flagged
$\{escapes in template literals.Test Plan
$\{escapes and additional invalid cases.Docs