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.

Sign up for GitHub

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

Jump to bottom

Interpret user-provided regexes in rule options with the u flag #11423

Closed
not-an-aardvark opened this issue Feb 21, 2019 · 1 comment · Fixed by #11516
Closed

Interpret user-provided regexes in rule options with the u flag #11423

not-an-aardvark opened this issue Feb 21, 2019 · 1 comment · Fixed by #11516
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 breaking This change is backwards-incompatible enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules

Comments

@not-an-aardvark
Copy link
Member

not-an-aardvark commented Feb 21, 2019 •
edited
Loading

What rule do you want to change?

camelcase, capitalized-comments, default-case, dot-notation, handle-callback-err, id-match, line-comment-position, lines-around-comment, max-len, new-cap, no-fallthrough, no-unused-vars, spaced-comment and valid-jsdoc. (These are all of the rules that have options which are interpreted as regular expressions.)

Does this change cause the rule to produce more or fewer warnings?

In the vast majority of cases, the behavior would be the same. In cases where the source text contains astral symbols or other non-BMP characters, the rule may produce more or fewer warnings.

How will the change be implemented? (New option, new default behavior, etc.)?

This will be a new default behavior. I think this is the behavior that most users expect anyway, although they might not have noticed that the current behavior is different from their expectation.

This page contains a good summary of unicode regexes.

Please provide some example code that this change will affect:

/* eslint max-len: [
  error,
  25,
  {ignorePattern: "𝌆{2}"}
] */

var longNameLongNameLongName = '𝌆𝌆';

What does the rule currently do for this code?

The rule reports an error on the line with a variable declaration, even though it seems like that line should match the ignorePattern.

What will the rule do after it's changed?

The rule will no longer report an error, because it will interpret the ignore pattern as a unicode regex.

Are you willing to submit a pull request to implement this change?

Yes
@not-an-aardvark not-an-aardvark added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules breaking This change is backwards-incompatible evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Feb 21, 2019
@not-an-aardvark not-an-aardvark mentioned this issue Feb 21, 2019
Merged
@nzakas nzakas added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Feb 28, 2019
@not-an-aardvark
Copy link
Member Author

This issue was accepted in today's TSC meeting.

not-an-aardvark added a commit that referenced this issue Mar 4, 2019
@not-an-aardvark
Fix: avoid creating invalid regexes in no-warning-comments (fixes #11471
083069a 
)

5018378 changed the codebase to use unicode regexes almost everywhere, with the exception of places where regexes are constructed from user input. However, two issues occurred to cause a bug:

* Due to an oversight, the regular expressions constructed in the `no-warning-comments` rule were changed to be unicode regexes even though those regexes were constructed from user input.
* The `no-warning-comments` rule dynamically creates regexes with unnecessary escape characters, and unnecessary escape characters are invalid in unicode regexes.

This commit fixes the first issue. The second issue isn't a problem on its own, but it will need to be fixed in order to implement #11423.
not-an-aardvark added a commit that referenced this issue Mar 4, 2019
@not-an-aardvark
Fix: avoid creating invalid regex in no-warning-comments (fixes #11471)
ce7cd8d 
5018378 changed the codebase to use unicode regexes almost everywhere, with the exception of places where regexes are constructed from user input. However, two issues occurred to cause a bug:

* Due to an oversight, the regular expressions constructed in the `no-warning-comments` rule were changed to be unicode regexes even though those regexes were constructed from user input.
* The `no-warning-comments` rule dynamically creates regexes with unnecessary escape characters, and unnecessary escape characters are invalid in unicode regexes.

This commit fixes the first issue. The second issue isn't a problem on its own, but it will need to be fixed in order to implement #11423.
@not-an-aardvark not-an-aardvark mentioned this issue Mar 4, 2019
Merged
not-an-aardvark added a commit that referenced this issue Mar 5, 2019
@not-an-aardvark
Fix: avoid creating invalid regex in no-warning-comments (fixes #11471)…
458053b 
… (#11472)

5018378 changed the codebase to use unicode regexes almost everywhere, with the exception of places where regexes are constructed from user input. However, two issues occurred to cause a bug:

* Due to an oversight, the regular expressions constructed in the `no-warning-comments` rule were changed to be unicode regexes even though those regexes were constructed from user input.
* The `no-warning-comments` rule dynamically creates regexes with unnecessary escape characters, and unnecessary escape characters are invalid in unicode regexes.

This commit fixes the first issue. The second issue isn't a problem on its own, but it will need to be fixed in order to implement #11423.
@not-an-aardvark not-an-aardvark mentioned this issue Mar 16, 2019
Merged
not-an-aardvark added a commit that referenced this issue Mar 16, 2019
@not-an-aardvark
Breaking: interpret rule options as unicode regexes (fixes #11423)
72801f9 
This updates the core rules that interpret user-provided options as regular expressions. Those rules now interpret the options as unicode regexes, which avoids some cases of unexpected behavior with astral symbols.
@not-an-aardvark not-an-aardvark mentioned this issue Mar 16, 2019
Merged
not-an-aardvark added a commit that referenced this issue Apr 1, 2019
@not-an-aardvark
Breaking: interpret rule options as unicode regexes (fixes #11423) (#…
0fb5fd4 
…11516)

This updates the core rules that interpret user-provided options as regular expressions. Those rules now interpret the options as unicode regexes, which avoids some cases of unexpected behavior with astral symbols.
@azu azu mentioned this issue Jun 23, 2019
Closed
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Sep 29, 2019
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Sep 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
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 breaking This change is backwards-incompatible enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Breaking: interpret rule options as unicode regexes (fixes #11423) eslint/eslint
2 participants
@nzakas @not-an-aardvark