Update: improve error message in no-control-regex (#6839)

* Update: improve error message in `no-control-regex` (fixes #6293) * fixup: this will be squashed before the PR is merged * fixup: this will be squashed before the PR is merged * fixup: Add more test cases; fix a bug. * fixup: try to improve variable names.
eslint · Aug 10, 2016 · 1ecd2a3 · 1ecd2a3
1 parent d610d6c
commit 1ecd2a3
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 15 deletions.
diff --git a/lib/rules/no-control-regex.js b/lib/rules/no-control-regex.js
@@ -51,29 +51,51 @@ module.exports = {
             return null;
         }
 
+
+        const controlChar = /[\x00-\x1f]/g; // eslint-disable-line no-control-regex
+        const consecutiveSlashes = /\\+/g;
+        const consecutiveSlashesAtEnd = /\\+$/g;
+        const stringControlChar = /\\x[01][0-9a-f]/ig;
+        const stringControlCharWithoutSlash = /x[01][0-9a-f]/ig;
+
         /**
-         * Check if given regex string has control characters in it
+         * Return a list of the control characters in the given regex string
          * @param {string} regexStr regex as string to check
-         * @returns {boolean} returns true if finds control characters on given string
+         * @returns {array} returns a list of found control characters on given string
          * @private
          */
-        function hasControlCharacters(regexStr) {
+        function getControlCharacters(regexStr) {
 
             // check control characters, if RegExp object used
-            let hasControlChars = /[\x00-\x1f]/.test(regexStr); // eslint-disable-line no-control-regex
+            const controlChars = regexStr.match(controlChar) || [];
+
+            let stringControlChars = [];
 
             // check substr, if regex literal used
-            const subStrIndex = regexStr.search(/\\x[01][0-9a-f]/i);
+            const subStrIndex = regexStr.search(stringControlChar);
 
-            if (!hasControlChars && subStrIndex > -1) {
+            if (subStrIndex > -1) {
 
                 // is it escaped, check backslash count
-                const possibleEscapeCharacters = regexStr.substr(0, subStrIndex).match(/\\+$/gi);
+                const possibleEscapeCharacters = regexStr.slice(0, subStrIndex).match(consecutiveSlashesAtEnd);
+
+                const hasControlChars = possibleEscapeCharacters === null || !(possibleEscapeCharacters[0].length % 2);
 
-                hasControlChars = possibleEscapeCharacters === null || !(possibleEscapeCharacters[0].length % 2);
+                if (hasControlChars) {
+                    stringControlChars = regexStr.slice(subStrIndex, -1)
+                        .split(consecutiveSlashes)
+                        .filter(Boolean)
+                        .map(function(x) {
+                            const match = x.match(stringControlCharWithoutSlash) || [x];
+
+                            return "\\" + match[0];
+                        });
+                }
             }
 
-            return hasControlChars;
+            return controlChars.map(function(x) {
+                return "\\x" + ("0" + x.charCodeAt(0).toString(16)).slice(-2);
+            }).concat(stringControlChars);
         }
 
         return {
@@ -83,8 +105,16 @@ module.exports = {
                 if (regex) {
                     const computedValue = regex.toString();
 
-                    if (hasControlCharacters(computedValue)) {
-                        context.report(node, "Unexpected control character in regular expression.");
+                    const controlCharacters = getControlCharacters(computedValue);
+
+                    if (controlCharacters.length > 0) {
+                        context.report({
+                            node,
+                            message: "Unexpected control character(s) in regular expression: {{controlChars}}.",
+                            data: {
+                                controlChars: controlCharacters.join(", ")
+                            }
+                        });
                     }
                 }
             }

diff --git a/tests/lib/rules/no-control-regex.js b/tests/lib/rules/no-control-regex.js
@@ -29,9 +29,13 @@ ruleTester.run("no-control-regex", rule, {
         "new (function foo(){})('\\x1f')"
     ],
     invalid: [
-        { code: "var regex = " + /\x1f/, errors: [{ message: "Unexpected control character in regular expression.", type: "Literal"}] }, // eslint-disable-line no-control-regex
-        { code: "var regex = " + /\\\x1f/, errors: [{ message: "Unexpected control character in regular expression.", type: "Literal"}] }, // eslint-disable-line no-control-regex
-        { code: "var regex = new RegExp('\\x1f')", errors: [{ message: "Unexpected control character in regular expression.", type: "Literal"}] },
-        { code: "var regex = RegExp('\\x1f')", errors: [{ message: "Unexpected control character in regular expression.", type: "Literal"}] }
+        { code: "var regex = " + /\x1f/, errors: [{ message: "Unexpected control character(s) in regular expression: \\x1f.", type: "Literal"}] }, // eslint-disable-line no-control-regex
+        { code: "var regex = " + /\\\x1f\\x1e/, errors: [{ message: "Unexpected control character(s) in regular expression: \\x1f, \\x1e.", type: "Literal"}] }, // eslint-disable-line no-control-regex
+        { code: "var regex = " + /\\\x1fFOO\\x00/, errors: [{ message: "Unexpected control character(s) in regular expression: \\x1f, \\x00.", type: "Literal"}] }, // eslint-disable-line no-control-regex
+        { code: "var regex = " + /FOO\\\x1fFOO\\x1f/, errors: [{ message: "Unexpected control character(s) in regular expression: \\x1f, \\x1f.", type: "Literal"}] }, // eslint-disable-line no-control-regex
+        { code: "var regex = new RegExp('\\x1f\\x1e')", errors: [{ message: "Unexpected control character(s) in regular expression: \\x1f, \\x1e.", type: "Literal"}] },
+        { code: "var regex = new RegExp('\\x1fFOO\\x00')", errors: [{ message: "Unexpected control character(s) in regular expression: \\x1f, \\x00.", type: "Literal"}] },
+        { code: "var regex = new RegExp('FOO\\x1fFOO\\x1f')", errors: [{ message: "Unexpected control character(s) in regular expression: \\x1f, \\x1f.", type: "Literal"}] },
+        { code: "var regex = RegExp('\\x1f')", errors: [{ message: "Unexpected control character(s) in regular expression: \\x1f.", type: "Literal"}] }
     ]
 });