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: improve no-misleading-character-class so that it reports on the sub-node range rather than the entire node #16682

Closed
1 task
bradzacher opened this issue Dec 20, 2022 · 1 comment · Fixed by #17515
Assignees
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules
Projects

Comments

@bradzacher
Copy link
Contributor

What rule do you want to change?

no-misleading-character-class

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

/[\u{2ea}-\u{2eb}\u{1100}-\u{11ff}\u{2e80}-\u{2e99}\u{2e9b}-\u{2ef3}\u{2f00}-\u{2fd5}\u{3005}\u{3007}\u{3021}-\u{3029}\u{302e}-\u{302f}\u{3038}-\u{303a}\u{303b}\u{3041}-\u{3096}\u{309d}-\u{309e}\u{309f}\u{30a1}-\u{30fa}\u{30fd}-\u{30fe}\u{30ff}\u{3105}-\u{312f}\u{3131}-\u{318e}\u{31a0}-\u{31bf}\u{31f0}-\u{31ff}\u{3200}-\u{321e}\u{3260}-\u{327e}\u{32d0}-\u{32fe}\u{3300}-\u{3357}\u{3400}-\u{4dbf}\u{4e00}-\u{9ffc}\u{a960}-\u{a97c}\u{ac00}-\u{d7a3}\u{d7b0}-\u{d7c6}\u{d7cb}-\u{d7fb}\u{f900}-\u{fa6d}\u{fa70}-\u{fad9}\u{ff66}-\u{ff6f}\u{ff71}-\u{ff9d}\u{ffa0}-\u{ffbe}\u{ffc2}-\u{ffc7}\u{ffca}-\u{ffcf}\u{ffd2}-\u{ffd7}\u{ffda}-\u{ffdc}\u{16ff0}-\u{16ff1}\u{1b000}\u{1b001}-\u{1b11e}\u{1b150}-\u{1b152}\u{1b164}-\u{1b167}\u{1f200}\u{20000}-\u{2a6dd}\u{2a700}-\u{2b734}\u{2b740}-\u{2b81d}\u{2b820}-\u{2cea1}\u{2ceb0}-\u{2ebe0}\u{2f800}-\u{2fa1d}\u{30000}-\u{3134a}]/u;

What does the rule currently do for this code?

The rule currently errors on the entire RegExp literal.
eslint playground example

What will the rule do after it's changed?

The rule will report on the sub-node range that violates the rule instead of the entire literal node. For long regexes it can be pretty-well impossible to understand what the actual problem is if the rule reports on the entire node.

Example with reporting on the relevant violating sub-node ranges:

/[\u{2ea}-\u{2eb}\u{1100}-\u{11ff}\u{2e80}-\u{2e99}\u{2e9b}-\u{2ef3}\u{2f00}-\u{2fd5}\u{3005}\u{3007}\u{3021}-\u{3029}\u{302e}-\u{302f}\u{3038}-\u{303a}\u{303b}\u{3041}-\u{3096}\u{309d}-\u{309e}\u{309f}\u{30a1}-\u{30fa}\u{30fd}-\u{30fe}\u{30ff}\u{3105}-\u{312f}\u{3131}-\u{318e}\u{31a0}-\u{31bf}\u{31f0}-\u{31ff}\u{3200}-\u{321e}\u{3260}-\u{327e}\u{32d0}-\u{32fe}\u{3300}-\u{3357}\u{3400}-\u{4dbf}\u{4e00}-\u{9ffc}\u{a960}-\u{a97c}\u{ac00}-\u{d7a3}\u{d7b0}-\u{d7c6}\u{d7cb}-\u{d7fb}\u{f900}-\u{fa6d}\u{fa70}-\u{fad9}\u{ff66}-\u{ff6f}\u{ff71}-\u{ff9d}\u{ffa0}-\u{ffbe}\u{ffc2}-\u{ffc7}\u{ffca}-\u{ffcf}\u{ffd2}-\u{ffd7}\u{ffda}-\u{ffdc}\u{16ff0}-\u{16ff1}\u{1b000}\u{1b001}-\u{1b11e}\u{1b150}-\u{1b152}\u{1b164}-\u{1b167}\u{1f200}\u{20000}-\u{2a6dd}\u{2a700}-\u{2b734}\u{2b740}-\u{2b81d}\u{2b820}-\u{2cea1}\u{2ceb0}-\u{2ebe0}\u{2f800}-\u{2fa1d}\u{30000}-\u{3134a}]/u;
//                                                                                                            ^^^^^^^^^^^^^^^^^^^^^^^^^                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                            ^^^^^^^^^^^^^^^^^^

This will result in more lint rule reports because it will report more than once per node.
But I think the increased understandability of the reports is worth it.

Participation

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

Additional comments

No response

@bradzacher bradzacher added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules labels Dec 20, 2022
@eslint-github-bot eslint-github-bot bot added this to Needs Triage in Triage Dec 20, 2022
@mdjermanovic mdjermanovic moved this from Needs Triage to Triaging in Triage Dec 20, 2022
@mdjermanovic
Copy link
Member

Make sense to me because the current reporting from this rule really isn't informative enough. I'll work on this.

@mdjermanovic mdjermanovic moved this from Triaging to Ready to Implement in Triage Dec 20, 2022
@mdjermanovic mdjermanovic added the accepted There is consensus among the team that this change meets the criteria for inclusion label Dec 20, 2022
@mdjermanovic mdjermanovic self-assigned this Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules
Projects
Archived in project
Triage
Ready to Implement
Development

Successfully merging a pull request may close this issue.

2 participants