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

Rule Change: no-control-regex should check Unicode code point escapes #15809

Closed
1 task done
mdjermanovic opened this issue Apr 26, 2022 · 3 comments · Fixed by #15862
Closed
1 task done

Rule Change: no-control-regex should check Unicode code point escapes #15809

mdjermanovic opened this issue Apr 26, 2022 · 3 comments · Fixed by #15862
Assignees
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules
Projects

Comments

@mdjermanovic
Copy link
Member

What rule do you want to change?

no-control-regex

What change to do you want to make?

Generate more warnings

How do you think the change should be implemented?

A new default behavior

Example code

/*eslint no-control-regex: "error"*/

/\u001F/;

/\u{1F}/u;

What does the rule currently do for this code?

Reports only /\u001F/.

What will the rule do after it's changed?

Report both /\u001F/ and /\u{1F}/u.

Participation

  • I am willing to submit a pull request to implement this change.

Additional comments

I think not reporting /\u{1F}/u is actually a bug caused by forgetting to pass the u flag to regexpp, so \u{1F} is not parsed as an escape sequence. The bug is observable in (rare) patterns that are valid with the u flag but invalid without it. For example, the rule fails to report \x1F in /\u{1111}*\x1F/u because regexpp stops parsing the pattern after \u{1111}* since it is invalid without the u flag.

Here's a demo with examples: Online Demo

@mdjermanovic mdjermanovic added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules labels Apr 26, 2022
@eslint-github-bot eslint-github-bot bot added this to Needs Triage in Triage Apr 26, 2022
@mdjermanovic mdjermanovic self-assigned this Apr 26, 2022
@nzakas
Copy link
Member

nzakas commented Apr 29, 2022

Makes sense to me.

@nzakas nzakas moved this from Needs Triage to Feedback Needed in Triage Apr 29, 2022
@nzakas
Copy link
Member

nzakas commented May 5, 2022

@btmills ?

@btmills
Copy link
Member

btmills commented May 7, 2022

Makes sense to me as well as a semver-minor bug fix 👍

@btmills btmills added the accepted There is consensus among the team that this change meets the criteria for inclusion label May 7, 2022
@btmills btmills moved this from Feedback Needed to Ready to Implement in Triage May 7, 2022
@mdjermanovic mdjermanovic moved this from Ready to Implement to Pull Request Opened in Triage May 10, 2022
Triage automation moved this from Pull Request Opened to Complete May 20, 2022
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Nov 17, 2022
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Nov 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules
Projects
Archived in project
Triage
Complete
Development

Successfully merging a pull request may close this issue.

3 participants