diff --git a/lib/rules/no-regex-spaces.js b/lib/rules/no-regex-spaces.js index 55e79517cef..fa846ddd3f3 100644 --- a/lib/rules/no-regex-spaces.js +++ b/lib/rules/no-regex-spaces.js @@ -5,7 +5,28 @@ "use strict"; +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + const astUtils = require("./utils/ast-utils"); +const regexpp = require("regexpp"); + +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +const regExpParser = new regexpp.RegExpParser(); + +/** + * 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,41 +48,86 @@ 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 + if (!/ {2}/u.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}}` - ); - } - }); + let ast; + + try { + ast = regExpParser.parsePattern(pattern, 0, pattern.length, flags.includes("u")); + } catch (e) { - /* - * TODO: (platinumazure) Fix message to use rule message - * substitution when api.report is fixed in lib/eslint.js. - */ + // Ignore regular expressions with syntax errors + return; } + + const spaces = []; + + regexpp.visitRegExpAST(ast, { + onCharacterEnter(character) { + if ( + character.parent.type === "Alternative" && + (character.raw === " " || character.raw === "\\ ") + ) { + spaces.push(character); + } + } + }); + + for (let i = 0; i < spaces.length - 1; i++) { + let j = i; + + while ( + j < spaces.length - 1 && + spaces[j].end === spaces[j + 1].start && + spaces[j + 1].raw === " " + ) { + j++; + } + + if (j > i) { + const count = j - i + 1; + + context.report({ + node: nodeToReport, + message: "Spaces are hard to count. Use {{{count}}}.", + data: { count }, + fix(fixer) { + if (pattern !== rawPattern) { + return null; + } + return fixer.replaceTextRange( + [rawPatternStartRange + spaces[i + 1].start, rawPatternStartRange + spaces[j].end], + `{${count}}` + ); + } + }); + + // Report only the first occurence of consecutive spaces + return; + } + } + + /* + * TODO: (platinumazure) Fix message to use rule message + * substitution when api.report is fixed in lib/eslint.js. + */ } /** @@ -71,25 +137,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 +163,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 +187,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..116fc176b3e 100644 --- a/tests/lib/rules/no-regex-spaces.js +++ b/tests/lib/rules/no-regex-spaces.js @@ -16,18 +16,56 @@ 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 = /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 = / +/;", + + // 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 = new 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/;", @@ -90,6 +128,110 @@ 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 = new RegExp('[ ] ');", + output: "var foo = new RegExp('[ ] {2}');", + errors: [ + { + message: "Spaces are hard to count. Use {2}.", + type: "NewExpression" + } + ] + }, + { + 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" + } + ] } ] });