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

Disallow use of the RegExp constructor in favor of regular expression literals (prefer-regex-literals) #1413

Closed
feross opened this issue Sep 14, 2019 · 8 comments

Comments

@feross
Copy link
Member

feross commented Sep 14, 2019

https://eslint.org/docs/rules/prefer-regex-literals

Desired config:

prefer-regex-literals: ["error", {"disallowRedundantWrapping": true}]

There are two ways to create a regular expression:

  • Regular expression literals, e.g., /abc/u.
  • The RegExp constructor function, e.g., new RegExp("abc", "u") or RegExp("abc", "u").

The constructor function is particularly useful when you want to dynamically generate the pattern,
because it takes string arguments.

When using the constructor function with string literals, don't forget that the string escaping rules still apply.
If you want to put a backslash in the pattern, you need to escape it in the string literal.
Thus, the following are equivalent:

new RegExp("^\\d\\.$");

/^\d\.$/;

// matches "0.", "1.", "2." ... "9."

In the above example, the regular expression literal is easier to read and reason about.
Also, it's a common mistake to omit the extra \ in the string literal, which would produce a completely different regular expression:

new RegExp("^\d\.$");

// equivalent to /^d.$/, matches "d1", "d2", "da", "db" ...

When a regular expression is known in advance, it is considered a best practice to avoid the string literal notation on top
of the regular expression notation, and use regular expression literals instead of the constructor function.

Rule Details

This rule disallows the use of the RegExp constructor function with string literals as its arguments.

This rule also disallows the use of the RegExp constructor function with template literals without expressions
and String.raw tagged template literals without expressions.

The rule does not disallow all use of the RegExp constructor. It should be still used for
dynamically generated regular expressions.

Examples of incorrect code for this rule:

/*eslint prefer-regex-literals: "error"*/

new RegExp("abc");

new RegExp("abc", "u");

RegExp("abc");

RegExp("abc", "u");

new RegExp("\\d\\d\\.\\d\\d\\.\\d\\d\\d\\d");

RegExp(`^\\d\\.$`);

new RegExp(String.raw`^\d\.$`);

Examples of correct code for this rule:

/*eslint prefer-regex-literals: "error"*/

/abc/;

/abc/u;

/\d\d\.\d\d\.\d\d\d\d/;

/^\d\.$/;

// RegExp constructor is allowed for dynamically generated regular expressions

new RegExp(pattern);

RegExp("abc", flags);

new RegExp(prefix + "abc");

RegExp(`${prefix}abc`);

new RegExp(String.raw`^\d\. ${suffix}`);

Options

This rule has an object option:

  • disallowRedundantWrapping set to true additionally checks for unnecessarily wrapped regex literals (Default false).

disallowRedundantWrapping

By default, this rule doesn’t check when a regex literal is unnecessarily wrapped in a RegExp constructor call. When the option disallowRedundantWrapping is set to true, the rule will also disallow such unnecessary patterns.

Examples of incorrect code for { "disallowRedundantWrapping": true }

/*eslint prefer-regex-literals: ["error", {"disallowRedundantWrapping": true}]*/

new RegExp(/abc/);

new RegExp(/abc/, 'u');

Examples of correct code for { "disallowRedundantWrapping": true }

/*eslint prefer-regex-literals: ["error", {"disallowRedundantWrapping": true}]*/

/abc/;

/abc/u;

new RegExp(/abc/, flags);
@mightyiam
Copy link
Member

Rule description makes sense. I have very small experience with regular expressions, though. Can we confirm that we have at least 20 regular expressions in our sample user repositories collection?

@feross feross modified the milestones: standard 15, standard 16 Oct 22, 2020
@feross
Copy link
Member Author

feross commented Oct 29, 2020

1% ecosystem impact. This will ship in standard 16.

All legit cases, IMO.

@dmitryverkhovtsev
Copy link

"The rule does not disallow all use of the RegExp constructor. It should be still used for
dynamically generated regular expressions"

Sad.
I use RegExp for dynamic regexes.

1% ecosystem impact - that's a main criteria for adding new "rule" ?

@feross
Copy link
Member Author

feross commented Nov 3, 2020

The RegExp constructor is still allowed for dynamic regexes. But if there's nothing dynamic in your regex, then you should use the regex literal syntax, e.g. /abc/.

@dmitryverkhovtsev What code were you trying to write that failed with this rule?

@dmitryverkhovtsev
Copy link

dmitryverkhovtsev commented Nov 5, 2020

@feross
I use RegExp constructor with template strings for urls matching
e.g.

RegExp(`/path/to/${resource}?search=query`)

@LinusU
Copy link
Member

LinusU commented Nov 5, 2020

@dmitryverkhovtsev this rule is not supposed to error for that. See the examples here: https://eslint.org/docs/rules/prefer-regex-literals

@LinusU
Copy link
Member

LinusU commented Nov 5, 2020

side note: be careful that resource doesn't contain any regex special characters, and consider adding escaping to it

@feross
Copy link
Member Author

feross commented Nov 5, 2020

Indeed, the example you pasted is not considered an error by this rule.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
Development

No branches or pull requests

4 participants