diff --git a/docs/rules/prefer-regex-literals.md b/docs/rules/prefer-regex-literals.md new file mode 100644 index 00000000000..01d4149cf20 --- /dev/null +++ b/docs/rules/prefer-regex-literals.md @@ -0,0 +1,90 @@ +# Disallow use of the `RegExp` constructor in favor of regular expression literals (prefer-regex-literals) + +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: + +```js +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: + +```js +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: + +```js +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: + +```js +/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\. ${sufix}`); +``` + +## Further Reading + +* [MDN: Regular Expressions](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions) +* [MDN: RegExp Constructor](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp) diff --git a/lib/rules/index.js b/lib/rules/index.js index 51d224d219f..dcfe2e1c3c6 100644 --- a/lib/rules/index.js +++ b/lib/rules/index.js @@ -242,6 +242,7 @@ module.exports = new LazyLoadingRuleMap(Object.entries({ "prefer-object-spread": () => require("./prefer-object-spread"), "prefer-promise-reject-errors": () => require("./prefer-promise-reject-errors"), "prefer-reflect": () => require("./prefer-reflect"), + "prefer-regex-literals": () => require("./prefer-regex-literals"), "prefer-rest-params": () => require("./prefer-rest-params"), "prefer-spread": () => require("./prefer-spread"), "prefer-template": () => require("./prefer-template"), diff --git a/lib/rules/prefer-regex-literals.js b/lib/rules/prefer-regex-literals.js new file mode 100644 index 00000000000..47b2b090f82 --- /dev/null +++ b/lib/rules/prefer-regex-literals.js @@ -0,0 +1,125 @@ +/** + * @fileoverview Rule to disallow use of the `RegExp` constructor in favor of regular expression literals + * @author Milos Djermanovic + */ + +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const astUtils = require("./utils/ast-utils"); +const { CALL, CONSTRUCT, ReferenceTracker, findVariable } = require("eslint-utils"); + +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +/** + * Determines whether the given node is a string literal. + * @param {ASTNode} node Node to check. + * @returns {boolean} True if the node is a string literal. + */ +function isStringLiteral(node) { + return node.type === "Literal" && typeof node.value === "string"; +} + +/** + * Determines whether the given node is a template literal without expressions. + * @param {ASTNode} node Node to check. + * @returns {boolean} True if the node is a template literal without expressions. + */ +function isStaticTemplateLiteral(node) { + return node.type === "TemplateLiteral" && node.expressions.length === 0; +} + + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +module.exports = { + meta: { + type: "suggestion", + + docs: { + description: "disallow use of the `RegExp` constructor in favor of regular expression literals", + category: "Best Practices", + recommended: false, + url: "https://eslint.org/docs/rules/prefer-regex-literals" + }, + + schema: [], + + messages: { + unexpectedRegExp: "Use a regular expression literal instead of the 'RegExp' constructor." + } + }, + + create(context) { + + /** + * Determines whether the given identifier node is a reference to a global variable. + * @param {ASTNode} node `Identifier` node to check. + * @returns {boolean} True if the identifier is a reference to a global variable. + */ + function isGlobalReference(node) { + const scope = context.getScope(); + const variable = findVariable(scope, node); + + return variable !== null && variable.scope.type === "global" && variable.defs.length === 0; + } + + /** + * Determines whether the given node is a String.raw`` tagged template expression + * with a static template literal. + * @param {ASTNode} node Node to check. + * @returns {boolean} True if the node is String.raw`` with a static template. + */ + function isStringRawTaggedStaticTemplateLiteral(node) { + return node.type === "TaggedTemplateExpression" && + node.tag.type === "MemberExpression" && + node.tag.object.type === "Identifier" && + node.tag.object.name === "String" && + isGlobalReference(node.tag.object) && + astUtils.getStaticPropertyName(node.tag) === "raw" && + isStaticTemplateLiteral(node.quasi); + } + + /** + * Determines whether the given node is considered to be a static string by the logic of this rule. + * @param {ASTNode} node Node to check. + * @returns {boolean} True if the node is a static string. + */ + function isStaticString(node) { + return isStringLiteral(node) || + isStaticTemplateLiteral(node) || + isStringRawTaggedStaticTemplateLiteral(node); + } + + return { + Program() { + const scope = context.getScope(); + const tracker = new ReferenceTracker(scope); + const traceMap = { + RegExp: { + [CALL]: true, + [CONSTRUCT]: true + } + }; + + for (const { node } of tracker.iterateGlobalReferences(traceMap)) { + const args = node.arguments; + + if ( + (args.length === 1 || args.length === 2) && + args.every(isStaticString) + ) { + context.report({ node, messageId: "unexpectedRegExp" }); + } + } + } + }; + } +}; diff --git a/tests/lib/rules/prefer-regex-literals.js b/tests/lib/rules/prefer-regex-literals.js new file mode 100644 index 00000000000..e263ef03dc7 --- /dev/null +++ b/tests/lib/rules/prefer-regex-literals.js @@ -0,0 +1,182 @@ +/** + * @fileoverview Tests for the prefer-regex-literals rule + * @author Milos Djermanovic + */ + +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const rule = require("../../../lib/rules/prefer-regex-literals"); +const { RuleTester } = require("../../../lib/rule-tester"); + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2015 } }); + +ruleTester.run("prefer-regex-literals", rule, { + valid: [ + "/abc/", + "/abc/g", + + // considered as dynamic + "new RegExp(pattern)", + "RegExp(pattern, 'g')", + "new RegExp(f('a'))", + "RegExp(prefix + 'a')", + "new RegExp('a' + sufix)", + "RegExp(`a` + sufix);", + "new RegExp(String.raw`a` + sufix);", + "RegExp('a', flags)", + "RegExp('a', 'g' + flags)", + "new RegExp(String.raw`a`, flags);", + "RegExp(`${prefix}abc`)", + "new RegExp(`a${b}c`);", + "new RegExp(`a${''}c`);", + "new RegExp(String.raw`a${b}c`);", + "new RegExp(String.raw`a${''}c`);", + "new RegExp('a' + 'b')", + "RegExp(1)", + + // invalid number of arguments + "new RegExp;", + "new RegExp();", + "RegExp();", + "new RegExp('a', 'g', 'b');", + "RegExp('a', 'g', 'b');", + "new RegExp(`a`, `g`, `b`);", + "RegExp(`a`, `g`, `b`);", + "new RegExp(String.raw`a`, String.raw`g`, String.raw`b`);", + "RegExp(String.raw`a`, String.raw`g`, String.raw`b`);", + + // not String.raw`` + "new RegExp(String`a`);", + "RegExp(raw`a`);", + "new RegExp(f(String.raw)`a`);", + "RegExp(string.raw`a`);", + "new RegExp(String.Raw`a`);", + "new RegExp(String[raw]`a`);", + "RegExp(String.raw.foo`a`);", + "new RegExp(String.foo.raw`a`);", + "RegExp(foo.String.raw`a`);", + "new RegExp(String.raw);", + + // not the global String in String.raw`` + "let String; new RegExp(String.raw`a`);", + "function foo() { var String; new RegExp(String.raw`a`); }", + "function foo(String) { RegExp(String.raw`a`); }", + "if (foo) { const String = bar; RegExp(String.raw`a`); }", + "/* globals String:off */ new RegExp(String.raw`a`);", + { + code: "RegExp('a', String.raw`g`);", + globals: { String: "off" } + }, + + // not RegExp + "new Regexp('abc');", + "Regexp(`a`);", + "new Regexp(String.raw`a`);", + + // not the global RegExp + "let RegExp; new RegExp('a');", + "function foo() { var RegExp; RegExp('a', 'g'); }", + "function foo(RegExp) { new RegExp(String.raw`a`); }", + "if (foo) { const RegExp = bar; RegExp('a'); }", + "/* globals RegExp:off */ new RegExp('a');", + { + code: "RegExp('a');", + globals: { RegExp: "off" } + } + ], + + invalid: [ + { + code: "new RegExp('abc');", + errors: [{ messageId: "unexpectedRegExp", type: "NewExpression" }] + }, + { + code: "RegExp('abc');", + errors: [{ messageId: "unexpectedRegExp", type: "CallExpression" }] + }, + { + code: "new RegExp('abc', 'g');", + errors: [{ messageId: "unexpectedRegExp", type: "NewExpression" }] + }, + { + code: "RegExp('abc', 'g');", + errors: [{ messageId: "unexpectedRegExp", type: "CallExpression" }] + }, + { + code: "new RegExp(`abc`);", + errors: [{ messageId: "unexpectedRegExp", type: "NewExpression" }] + }, + { + code: "RegExp(`abc`);", + errors: [{ messageId: "unexpectedRegExp", type: "CallExpression" }] + }, + { + code: "new RegExp(`abc`, `g`);", + errors: [{ messageId: "unexpectedRegExp", type: "NewExpression" }] + }, + { + code: "RegExp(`abc`, `g`);", + errors: [{ messageId: "unexpectedRegExp", type: "CallExpression" }] + }, + { + code: "new RegExp(String.raw`abc`);", + errors: [{ messageId: "unexpectedRegExp", type: "NewExpression" }] + }, + { + code: "RegExp(String.raw`abc`);", + errors: [{ messageId: "unexpectedRegExp", type: "CallExpression" }] + }, + { + code: "new RegExp(String.raw`abc`, String.raw`g`);", + errors: [{ messageId: "unexpectedRegExp", type: "NewExpression" }] + }, + { + code: "RegExp(String.raw`abc`, String.raw`g`);", + errors: [{ messageId: "unexpectedRegExp", type: "CallExpression" }] + }, + { + code: "new RegExp(String['raw']`a`);", + errors: [{ messageId: "unexpectedRegExp", type: "NewExpression" }] + }, + { + code: "new RegExp('');", + errors: [{ messageId: "unexpectedRegExp", type: "NewExpression" }] + }, + { + code: "RegExp('', '');", + errors: [{ messageId: "unexpectedRegExp", type: "CallExpression" }] + }, + { + code: "new RegExp(String.raw``);", + errors: [{ messageId: "unexpectedRegExp", type: "NewExpression" }] + }, + { + code: "new RegExp('a', `g`);", + errors: [{ messageId: "unexpectedRegExp", type: "NewExpression" }] + }, + { + code: "RegExp(`a`, 'g');", + errors: [{ messageId: "unexpectedRegExp", type: "CallExpression" }] + }, + { + code: "RegExp(String.raw`a`, 'g');", + errors: [{ messageId: "unexpectedRegExp", type: "CallExpression" }] + }, + { + code: "new RegExp(String.raw`\\d`, `g`);", + errors: [{ messageId: "unexpectedRegExp", type: "NewExpression" }] + }, + { + code: "RegExp('a', String.raw`g`);", + errors: [{ messageId: "unexpectedRegExp", type: "CallExpression" }] + } + ] +}); diff --git a/tools/rule-types.json b/tools/rule-types.json index 74cae9391cb..ea15a2cfd4c 100644 --- a/tools/rule-types.json +++ b/tools/rule-types.json @@ -229,6 +229,7 @@ "prefer-object-spread": "suggestion", "prefer-promise-reject-errors": "suggestion", "prefer-reflect": "suggestion", + "prefer-regex-literals": "suggestion", "prefer-rest-params": "suggestion", "prefer-spread": "suggestion", "prefer-template": "suggestion",