Update: Check RegExp strings for no-regex-spaces (fixes #3586) (#6379)

eslint · Jun 28, 2016 · e2b2030 · e2b2030
1 parent 397e51b
commit e2b2030
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 27 deletions.
diff --git a/docs/rules/no-regex-spaces.md b/docs/rules/no-regex-spaces.md
@@ -24,6 +24,7 @@ Examples of **incorrect** code for this rule:
 /*eslint no-regex-spaces: "error"*/
 
 var re = /foo   bar/;
+var re = new RegExp("foo   bar");
 ```
 
 Examples of **correct** code for this rule:
@@ -32,18 +33,7 @@ Examples of **correct** code for this rule:
 /*eslint no-regex-spaces: "error"*/
 
 var re = /foo {3}bar/;
-```
-
-## Known Limitations
-
-This rule does not report multiple spaces in the string argument of calls to the `RegExp` constructor.
-
-Example of a *false negative* when this rule reports correct code:
-
-```js
-/*eslint no-regex-spaces: "error"*/
-
-var re = new RegExp("foo   bar");
+var re = new RegExp("foo {3}bar");
 ```
 
 ## When Not To Use It

diff --git a/lib/rules/no-regex-spaces.js b/lib/rules/no-regex-spaces.js
@@ -12,7 +12,7 @@
 module.exports = {
     meta: {
         docs: {
-            description: "disallow multiple spaces in regular expression literals",
+            description: "disallow multiple spaces in regular expressions",
             category: "Possible Errors",
             recommended: true
         },
@@ -23,23 +23,64 @@ module.exports = {
     create: function(context) {
         var sourceCode = context.getSourceCode();
 
-        return {
+        /**
+         * Validate regular expressions
+         * @param {ASTNode} node node to validate
+         * @param {string} value regular expression to validate
+         * @returns {void}
+         * @private
+         */
+        function checkRegex(node, value) {
+            var multipleSpacesRegex = /( {2,})+?/,
+                regexResults = multipleSpacesRegex.exec(value);
 
-            Literal: function(node) {
-                var token = sourceCode.getFirstToken(node),
-                    nodeType = token.type,
-                    nodeValue = token.value,
-                    multipleSpacesRegex = /( {2,})+?/,
-                    regexResults;
+            if (regexResults !== null) {
+                context.report(node, "Spaces are hard to count. Use {" + regexResults[0].length + "}.");
+            }
+        }
 
-                if (nodeType === "RegularExpression") {
-                    regexResults = multipleSpacesRegex.exec(nodeValue);
+        /**
+         * Validate regular expression literals
+         * @param {ASTNode} node node to validate
+         * @returns {void}
+         * @private
+         */
+        function checkLiteral(node) {
+            var token = sourceCode.getFirstToken(node),
+                nodeType = token.type,
+                nodeValue = token.value;
 
-                    if (regexResults !== null) {
-                        context.report(node, "Spaces are hard to count. Use {" + regexResults[0].length + "}.");
-                    }
-                }
+            if (nodeType === "RegularExpression") {
+                checkRegex(node, nodeValue);
             }
+        }
+
+        /**
+         * Check if node is a string
+         * @param {ASTNode} node node to evaluate
+         * @returns {boolean} True if its a string
+         * @private
+         */
+        function isString(node) {
+            return node && node.type === "Literal" && typeof node.value === "string";
+        }
+
+        /**
+         * Validate strings passed to the RegExp constructor
+         * @param {ASTNode} node node to validate
+         * @returns {void}
+         * @private
+         */
+        function checkFunction(node) {
+            if (node.callee.type === "Identifier" && node.callee.name === "RegExp" && isString(node.arguments[0])) {
+                checkRegex(node, node.arguments[0].value);
+            }
+        }
+
+        return {
+            Literal: checkLiteral,
+            CallExpression: checkFunction,
+            NewExpression: checkFunction
         };
 
     }

diff --git a/tests/lib/rules/no-regex-spaces.js b/tests/lib/rules/no-regex-spaces.js
@@ -17,7 +17,11 @@ var ruleTester = new RuleTester();
 ruleTester.run("no-regex-spaces", rule, {
     valid: [
         "var foo = /bar {3}baz/;",
-        "var foo = /bar\t\t\tbaz/;"
+        "var foo = RegExp('bar {3}baz')",
+        "var foo = new RegExp('bar {3}baz')",
+        "var foo = /bar\t\t\tbaz/;",
+        "var foo = RegExp('bar\t\t\tbaz');",
+        "var foo = new RegExp('bar\t\t\tbaz');"
     ],
 
     invalid: [
@@ -29,6 +33,24 @@ ruleTester.run("no-regex-spaces", rule, {
                     type: "Literal"
                 }
             ]
+        },
+        {
+            code: "var foo = RegExp('bar    baz');",
+            errors: [
+                {
+                    message: "Spaces are hard to count. Use {4}.",
+                    type: "CallExpression"
+                }
+            ]
+        },
+        {
+            code: "var foo = new RegExp('bar    baz');",
+            errors: [
+                {
+                    message: "Spaces are hard to count. Use {4}.",
+                    type: "NewExpression"
+                }
+            ]
         }
     ]
 });