Skip to content
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

feat(js/noUselessEscapeInString): add lint rule #5229

Merged
merged 1 commit into from
Mar 2, 2025

Conversation

Conaclos
Copy link
Member

@Conaclos Conaclos commented Mar 1, 2025

Summary

Close #1090
This PR implements noUselessEscapeInString.

Test Plan

I added some tests.

@github-actions github-actions bot added A-Project Area: project A-Linter Area: linter L-JavaScript Language: JavaScript and super languages A-Diagnostic Area: diagnostocis labels Mar 1, 2025
Copy link

codspeed-hq bot commented Mar 1, 2025

CodSpeed Performance Report

Merging #5229 will not alter performance

Comparing conaclos/noUselessEscapeInString (aeac7cb) with main (9b2d5db)

Summary

✅ 97 untouched benchmarks

@Conaclos Conaclos force-pushed the conaclos/noUselessEscapeInString branch 2 times, most recently from 4ca6630 to 94b2dee Compare March 1, 2025 21:07
@Conaclos
Copy link
Member Author

Conaclos commented Mar 1, 2025

I completely missed the fact that we should add the same rule for JSON and CSS.

Copy link
Contributor

@arendjr arendjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

I agree the same rule would be nice to have in CSS and JSON too, but I would consider that out of scope for this PR :) Ideally it would have the same name too then, so users only need to configure one rule.

Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some reservations about the severity of the rule

@Conaclos Conaclos force-pushed the conaclos/noUselessEscapeInString branch from 94b2dee to 4da8666 Compare March 2, 2025 12:16
@Conaclos Conaclos requested a review from ematipico March 2, 2025 12:16
@Conaclos Conaclos changed the title feat(noUselessEscapeInString): add lint rule feat(js/noUselessEscapeInString): add lint rule Mar 2, 2025
@Conaclos Conaclos force-pushed the conaclos/noUselessEscapeInString branch from 4da8666 to aeac7cb Compare March 2, 2025 12:22
@Conaclos Conaclos merged commit 3ab73ff into main Mar 2, 2025
14 checks passed
@Conaclos Conaclos deleted the conaclos/noUselessEscapeInString branch March 2, 2025 13:52
@Conaclos
Copy link
Member Author

Conaclos commented Mar 2, 2025

We don't need a such rule for JSON. Because JSON accepts a curated list of escaped characters. Other escapes are errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Diagnostic Area: diagnostocis A-Linter Area: linter A-Project Area: project L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

📎 Implement nursery/noUselessEscape - eslint/no-useless-escape
3 participants