diff --git a/lib/rules/no-regex-spaces.js b/lib/rules/no-regex-spaces.js index 55e79517cef..41f5e149df0 100644 --- a/lib/rules/no-regex-spaces.js +++ b/lib/rules/no-regex-spaces.js @@ -5,7 +5,29 @@ "use strict"; +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + const astUtils = require("./utils/ast-utils"); +const regexpp = require("regexpp"); + +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +const regExpParser = new regexpp.RegExpParser(); +const DOUBLE_SPACE = / {2}/u; + +/** + * 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"; +} //------------------------------------------------------------------------------ // Rule Definition @@ -27,40 +49,70 @@ module.exports = { }, create(context) { - const sourceCode = context.getSourceCode(); /** - * Validate regular expressions - * @param {ASTNode} node node to validate - * @param {string} value regular expression to validate - * @param {number} valueStart The start location of the regex/string literal. It will always be the case that - * `sourceCode.getText().slice(valueStart, valueStart + value.length) === value` + * Validate regular expression + * + * @param {ASTNode} nodeToReport Node to report. + * @param {string} pattern Regular expression pattern to validate. + * @param {string} rawPattern Raw representation of the pattern in the source code. + * @param {number} rawPatternStartRange Start range of the pattern in the source code. + * @param {string} flags Regular expression flags. * @returns {void} * @private */ - function checkRegex(node, value, valueStart) { - const multipleSpacesRegex = /( {2,})( [+*{?]|[^+*{?]|$)/u, - regexResults = multipleSpacesRegex.exec(value); + function checkRegex(nodeToReport, pattern, rawPattern, rawPatternStartRange, flags) { - if (regexResults !== null) { - const count = regexResults[1].length; + // Skip if there are no consecutive spaces in the source code, to avoid reporting e.g., RegExp(' \ '). + if (!DOUBLE_SPACE.test(rawPattern)) { + return; + } - context.report({ - node, - message: "Spaces are hard to count. Use {{{count}}}.", - data: { count }, - fix(fixer) { - return fixer.replaceTextRange( - [valueStart + regexResults.index, valueStart + regexResults.index + count], - ` {${count}}` - ); - } - }); - - /* - * TODO: (platinumazure) Fix message to use rule message - * substitution when api.report is fixed in lib/eslint.js. - */ + const characterClassNodes = []; + let regExpAST; + + try { + regExpAST = regExpParser.parsePattern(pattern, 0, pattern.length, flags.includes("u")); + } catch (e) { + + // Ignore regular expressions with syntax errors + return; + } + + regexpp.visitRegExpAST(regExpAST, { + onCharacterClassEnter(ccNode) { + characterClassNodes.push(ccNode); + } + }); + + const spacesPattern = /( {2,})(?: [+*{?]|[^+*{?]|$)/gu; + let match; + + while ((match = spacesPattern.exec(pattern))) { + const { 1: { length }, index } = match; + + // Report only consecutive spaces that are not in character classes. + if ( + characterClassNodes.every(({ start, end }) => index < start || end <= index) + ) { + context.report({ + node: nodeToReport, + message: "Spaces are hard to count. Use {{{length}}}.", + data: { length }, + fix(fixer) { + if (pattern !== rawPattern) { + return null; + } + return fixer.replaceTextRange( + [rawPatternStartRange + index, rawPatternStartRange + index + length], + ` {${length}}` + ); + } + }); + + // Report only the first occurence of consecutive spaces + return; + } } } @@ -71,25 +123,22 @@ module.exports = { * @private */ function checkLiteral(node) { - const token = sourceCode.getFirstToken(node), - nodeType = token.type, - nodeValue = token.value; + if (node.regex) { + const pattern = node.regex.pattern; + const rawPattern = node.raw.slice(1, node.raw.lastIndexOf("/")); + const rawPatternStartRange = node.range[0] + 1; + const flags = node.regex.flags; - if (nodeType === "RegularExpression") { - checkRegex(node, nodeValue, token.range[0]); + checkRegex( + node, + pattern, + rawPattern, + rawPatternStartRange, + flags + ); } } - /** - * 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 @@ -100,9 +149,22 @@ module.exports = { const scope = context.getScope(); const regExpVar = astUtils.getVariableByName(scope, "RegExp"); const shadowed = regExpVar && regExpVar.defs.length > 0; + const patternNode = node.arguments[0]; + const flagsNode = node.arguments[1]; - if (node.callee.type === "Identifier" && node.callee.name === "RegExp" && isString(node.arguments[0]) && !shadowed) { - checkRegex(node, node.arguments[0].value, node.arguments[0].range[0] + 1); + if (node.callee.type === "Identifier" && node.callee.name === "RegExp" && isString(patternNode) && !shadowed) { + const pattern = patternNode.value; + const rawPattern = patternNode.raw.slice(1, -1); + const rawPatternStartRange = patternNode.range[0] + 1; + const flags = isString(flagsNode) ? flagsNode.value : ""; + + checkRegex( + node, + pattern, + rawPattern, + rawPatternStartRange, + flags + ); } } @@ -111,6 +173,5 @@ module.exports = { CallExpression: checkFunction, NewExpression: checkFunction }; - } }; diff --git a/tests/lib/rules/no-regex-spaces.js b/tests/lib/rules/no-regex-spaces.js index febd5e0dbd6..60f9dccdfcf 100644 --- a/tests/lib/rules/no-regex-spaces.js +++ b/tests/lib/rules/no-regex-spaces.js @@ -16,18 +16,64 @@ const ruleTester = new RuleTester(); ruleTester.run("no-regex-spaces", rule, { valid: [ - "var foo = /bar {3}baz/;", - "var foo = RegExp('bar {3}baz')", + "var foo = /foo/;", + "var foo = RegExp('foo')", + "var foo = / /;", + "var foo = RegExp(' ')", + "var foo = / a b c d /;", + "var foo = /bar {3}baz/g;", + "var foo = RegExp('bar {3}baz', 'g')", "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');", "var RegExp = function() {}; var foo = new RegExp('bar baz');", "var RegExp = function() {}; var foo = RegExp('bar baz');", - "var foo = / +/;" + "var foo = / +/;", + "var foo = / ?/;", + "var foo = / */;", + "var foo = / {2}/;", + + // don't report if there are no consecutive spaces in the source code + "var foo = /bar \\ baz/;", + "var foo = /bar\\ \\ baz/;", + "var foo = /bar \\u0020 baz/;", + "var foo = /bar\\u0020\\u0020baz/;", + "var foo = new RegExp('bar \\ baz')", + "var foo = new RegExp('bar\\ \\ baz')", + "var foo = new RegExp('bar \\\\ baz')", + "var foo = new RegExp('bar \\u0020 baz')", + "var foo = new RegExp('bar\\u0020\\u0020baz')", + "var foo = new RegExp('bar \\\\u0020 baz')", + + // don't report spaces in character classes + "var foo = /[ ]/;", + "var foo = /[ ]/;", + "var foo = / [ ] /;", + "var foo = / [ ] [ ] /;", + "var foo = new RegExp('[ ]');", + "var foo = new RegExp('[ ]');", + "var foo = new RegExp(' [ ] ');", + "var foo = RegExp(' [ ] [ ] ');", + "var foo = new RegExp(' \\[ ');", + "var foo = new RegExp(' \\[ \\] ');", + + // don't report invalid regex + "var foo = new RegExp('[ ');", + "var foo = new RegExp('{ ', 'u');" ], invalid: [ + { + code: "var foo = /bar baz/;", + output: "var foo = /bar {2}baz/;", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "Literal" + } + ] + }, { code: "var foo = /bar baz/;", output: "var foo = /bar {4}baz/;", @@ -38,6 +84,26 @@ ruleTester.run("no-regex-spaces", rule, { } ] }, + { + code: "var foo = / a b c d /;", + output: "var foo = / a b {2}c d /;", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "Literal" + } + ] + }, + { + code: "var foo = RegExp(' a b c d ');", + output: "var foo = RegExp(' a b c d {2}');", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "CallExpression" + } + ] + }, { code: "var foo = RegExp('bar baz');", output: "var foo = RegExp('bar {4}baz');", @@ -71,6 +137,16 @@ ruleTester.run("no-regex-spaces", rule, { } ] }, + { + code: "var foo = /bar {3}baz/;", + output: "var foo = /bar {2} {3}baz/;", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "Literal" + } + ] + }, { code: "var foo = /bar ?baz/;", output: "var foo = /bar {3} ?baz/;", @@ -81,6 +157,26 @@ ruleTester.run("no-regex-spaces", rule, { } ] }, + { + code: "var foo = new RegExp('bar *baz')", + output: "var foo = new RegExp('bar {2} *baz')", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "NewExpression" + } + ] + }, + { + code: "var foo = RegExp('bar +baz')", + output: "var foo = RegExp('bar {2} +baz')", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "CallExpression" + } + ] + }, { code: "var foo = new RegExp('bar ');", output: "var foo = new RegExp('bar {4}');", @@ -90,6 +186,160 @@ ruleTester.run("no-regex-spaces", rule, { type: "NewExpression" } ] + }, + { + code: "var foo = /bar\\ baz/;", + output: "var foo = /bar\\ {2}baz/;", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "Literal" + } + ] + }, + { + code: "var foo = /[ ] /;", + output: "var foo = /[ ] {2}/;", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "Literal" + } + ] + }, + { + code: "var foo = / [ ] /;", + output: "var foo = / {2}[ ] /;", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "Literal" + } + ] + }, + { + code: "var foo = new RegExp('[ ] ');", + output: "var foo = new RegExp('[ ] {2}');", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "NewExpression" + } + ] + }, + { + code: "var foo = RegExp(' [ ]');", + output: "var foo = RegExp(' {2}[ ]');", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "CallExpression" + } + ] + }, + { + code: "var foo = /\\[ /;", + output: "var foo = /\\[ {2}/;", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "Literal" + } + ] + }, + { + code: "var foo = /\\[ \\]/;", + output: "var foo = /\\[ {2}\\]/;", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "Literal" + } + ] + }, + { + code: "var foo = /(?: )/;", + output: "var foo = /(?: {2})/;", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "Literal" + } + ] + }, + { + code: "var foo = RegExp('^foo(?= )');", + output: "var foo = RegExp('^foo(?= {3})');", + errors: [ + { + message: "Spaces are hard to count. Use {3}.", + type: "CallExpression" + } + ] + }, + { + code: "var foo = /\\ /", + output: "var foo = /\\ {2}/", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "Literal" + } + ] + }, + { + code: "var foo = / \\ /", + output: "var foo = / \\ {2}/", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "Literal" + } + ] + }, + + // report only the first occurence of consecutive spaces + { + code: "var foo = / foo /;", + output: "var foo = / {2}foo /;", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "Literal" + } + ] + }, + + // don't fix strings with escape sequences + { + code: "var foo = new RegExp('\\\\d ')", + output: null, + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "NewExpression" + } + ] + }, + { + code: "var foo = RegExp('\\u0041 ')", + output: null, + errors: [ + { + message: "Spaces are hard to count. Use {3}.", + type: "CallExpression" + } + ] + }, + { + code: "var foo = new RegExp('\\\\[ \\\\]');", + output: null, + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "NewExpression" + } + ] } ] });