-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
Fix: allow escaped backreferences in no-useless-escape
(fixes #7472)
#7474
Conversation
@not-an-aardvark, thanks for your PR! By analyzing the history of the files in this pull request, we identified @kaicataldo, @onurtemizkan and @vitorbal to be potential reviewers. |
LGTM |
@@ -36,6 +36,8 @@ const VALID_STRING_ESCAPES = [ | |||
"\r" | |||
]; | |||
|
|||
const DIGITS = Array.from(Array(10).keys(), String); |
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.
I have to admit, I think const DIGITS = "0123456789".split("");
is a bit more readable.
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.
Yeah, maybe using Array#keys is overkill for 10 elements. I'll use that instead.
7977659
to
75bc55f
Compare
LGTM |
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.
LGTM, thanks for turning this around so quickly!
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.
LGTM, thank you for fast fix!
@@ -53,7 +55,8 @@ const REGEX_GENERAL_ESCAPES = new Set([ | |||
"W", | |||
"x", | |||
"u" | |||
]); | |||
].concat(DIGITS)); |
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.
FWIW, I think this can be written as new Set("\\0123456789bcdDfnrsStvwWxu")
since a string is iterable.
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.
Good idea, I updated this PR to pass in strings instead of arrays.
LGTM |
👍 |
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.
LGTM
What is the purpose of this pull request? (put an "X" next to item)
[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
See #7472
What changes did you make? (Give an overview)
This updates
no-useless-escape
to allow digit characters to be escaped in regular expressions. Escaped digits can either be backreferences or octal literals, so the rule should not warn when it encounters them.Is there anything you'd like reviewers to focus on?
Nothing in particular.