Update: fix no-invalid-regexp false negatives with no flags specified

eslint · Jan 20, 2021 · ea9126d · ea9126d
1 parent a4fdb70
commit ea9126d
Show file tree

Hide file tree

Showing 2 changed files with 91 additions and 8 deletions.
diff --git a/lib/rules/no-invalid-regexp.js b/lib/rules/no-invalid-regexp.js
@@ -69,6 +69,28 @@ module.exports = {
             return node && node.type === "Literal" && typeof node.value === "string";
         }
 
+        /**
+         * Gets flags of a regular expression created by the given `RegExp()` or `new RegExp()` call
+         * Examples:
+         *     new RegExp(".")         // => ""
+         *     new RegExp(".", "gu")   // => "gu"
+         *     new RegExp(".", flags)  // => null
+         * @param {ASTNode} node `CallExpression` or `NewExpression` node
+         * @returns {string|null} flags if they can be determined, `null` otherwise
+         * @private
+         */
+        function getFlags(node) {
+            if (node.arguments.length < 2) {
+                return "";
+            }
+
+            if (isString(node.arguments[1])) {
+                return node.arguments[1].value;
+            }
+
+            return null;
+        }
+
         /**
          * Check syntax error in a given pattern.
          * @param {string} pattern The RegExp pattern to validate.
@@ -104,18 +126,23 @@ module.exports = {
                     return;
                 }
                 const pattern = node.arguments[0].value;
-                let flags = isString(node.arguments[1]) ? node.arguments[1].value : "";
+                let flags = getFlags(node);
 
-                if (allowedFlags) {
+                if (flags && allowedFlags) {
                     flags = flags.replace(allowedFlags, "");
                 }
 
-                // If flags are unknown, check both are errored or not.
-                const message = validateRegExpFlags(flags) || (
-                    flags
-                        ? validateRegExpPattern(pattern, flags.indexOf("u") !== -1)
-                        : validateRegExpPattern(pattern, true) && validateRegExpPattern(pattern, false)
-                );
+                const message =
+                    (
+                        flags && validateRegExpFlags(flags)
+                    ) ||
+                    (
+
+                        // If flags are unknown, report the regex only if its pattern is invalid both with and without the "u" flag
+                        flags === null
+                            ? validateRegExpPattern(pattern, true) && validateRegExpPattern(pattern, false)
+                            : validateRegExpPattern(pattern, flags.includes("u"))
+                    );
 
                 if (message) {
                     context.report({

diff --git a/tests/lib/rules/no-invalid-regexp.js b/tests/lib/rules/no-invalid-regexp.js
@@ -39,6 +39,20 @@ ruleTester.run("no-invalid-regexp", rule, {
         "new RegExp('(?<a>b)\\k<a>', 'u')",
         "new RegExp('\\\\p{Letter}', 'u')",
 
+        // unknown flags
+        "RegExp('{', flags)", // valid without the "u" flag
+        "new RegExp('{', flags)", // valid without the "u" flag
+        "RegExp('\\\\u{0}*', flags)", // valid with the "u" flag
+        "new RegExp('\\\\u{0}*', flags)", // valid with the "u" flag
+        {
+            code: "RegExp('{', flags)", // valid without the "u" flag
+            options: [{ allowConstructorFlags: ["u"] }]
+        },
+        {
+            code: "RegExp('\\\\u{0}*', flags)", // valid with the "u" flag
+            options: [{ allowConstructorFlags: ["a"] }]
+        },
+
         // ES2020
         "new RegExp('(?<\\\\ud835\\\\udc9c>.)', 'g')",
         "new RegExp('(?<\\\\u{1d49c}>.)', 'g')",
@@ -165,6 +179,48 @@ ruleTester.run("no-invalid-regexp", rule, {
                 type: "NewExpression"
             }]
         },
+        {
+            code: String.raw`RegExp('\\u{0}*');`,
+            errors: [{
+                messageId: "regexMessage",
+                data: { message: "Invalid regular expression: /\\u{0}*/: Nothing to repeat" },
+                type: "CallExpression"
+            }]
+        },
+        {
+            code: String.raw`new RegExp('\\u{0}*');`,
+            errors: [{
+                messageId: "regexMessage",
+                data: { message: "Invalid regular expression: /\\u{0}*/: Nothing to repeat" },
+                type: "NewExpression"
+            }]
+        },
+        {
+            code: String.raw`new RegExp('\\u{0}*', '');`,
+            errors: [{
+                messageId: "regexMessage",
+                data: { message: "Invalid regular expression: /\\u{0}*/: Nothing to repeat" },
+                type: "NewExpression"
+            }]
+        },
+        {
+            code: String.raw`new RegExp('\\u{0}*', 'a');`,
+            options: [{ allowConstructorFlags: ["a"] }],
+            errors: [{
+                messageId: "regexMessage",
+                data: { message: "Invalid regular expression: /\\u{0}*/: Nothing to repeat" },
+                type: "NewExpression"
+            }]
+        },
+        {
+            code: String.raw`RegExp('\\u{0}*');`,
+            options: [{ allowConstructorFlags: ["a"] }],
+            errors: [{
+                messageId: "regexMessage",
+                data: { message: "Invalid regular expression: /\\u{0}*/: Nothing to repeat" },
+                type: "CallExpression"
+            }]
+        },
 
         // https://github.com/eslint/eslint/issues/10861
         {