diff --git a/package.json b/package.json index dacbfded71..aee1b65c16 100644 --- a/package.json +++ b/package.json @@ -58,6 +58,7 @@ "pluralize": "^8.0.0", "read-pkg-up": "^7.0.1", "regexp-tree": "^0.1.24", + "regjsparser": "0.9.1", "safe-regex": "^2.1.1", "semver": "^7.3.7", "strip-indent": "^3.0.0" diff --git a/rules/prefer-string-replace-all.js b/rules/prefer-string-replace-all.js index 37caca86d5..09a22a617a 100644 --- a/rules/prefer-string-replace-all.js +++ b/rules/prefer-string-replace-all.js @@ -1,5 +1,6 @@ 'use strict'; const {getStaticValue} = require('eslint-utils'); +const {parse: parseRegExp} = require('regjsparser'); const escapeString = require('./utils/escape-string.js'); const {methodCallSelector} = require('./selectors/index.js'); const {isRegexLiteral, isNewExpression} = require('./ast/index.js'); @@ -14,10 +15,32 @@ const selector = methodCallSelector({ argumentsLength: 2, }); -const canReplacePatternWithString = node => - isRegexLiteral(node) - && node.regex.flags.replace('u', '') === 'g' - && !/[$()*+.?[\\\]^{|}]/.test(node.regex.pattern.replace(/\\[$()*+.?[\\\]^{|}]/g, '')); +function * convertRegExpToString(node, fixer) { + if (!isRegexLiteral(node)) { + return; + } + + const {pattern, flags} = node.regex; + if (flags.replace('u', '') !== 'g') { + return; + } + + const tree = parseRegExp(pattern, flags, { + unicodePropertyEscape: true, + namedGroups: true, + lookbehind: true, + }); + + const parts = tree.type === 'alternative' ? tree.body : [tree]; + if (parts.some(part => part.type !== 'value')) { + return; + } + + // TODO: Preserve escape + const string = String.fromCodePoint(...parts.map(part => part.codePoint)); + + yield fixer.replaceText(node, escapeString(string)); +} const isRegExpWithGlobalFlag = (node, scope) => { if (isRegexLiteral(node)) { @@ -47,21 +70,6 @@ const isRegExpWithGlobalFlag = (node, scope) => { ); }; -function removeEscapeCharacters(regexString) { - let fixedString = regexString; - let index = 0; - do { - index = fixedString.indexOf('\\', index); - - if (index >= 0) { - fixedString = fixedString.slice(0, index) + fixedString.slice(index + 1); - index++; - } - } while (index >= 0); - - return fixedString; -} - /** @param {import('eslint').Rule.RuleContext} context */ const create = context => ({ [selector](node) { @@ -80,14 +88,7 @@ const create = context => ({ /** @param {import('eslint').Rule.RuleFixer} fixer */ * fix(fixer) { yield fixer.insertTextAfter(property, 'All'); - - if (!canReplacePatternWithString(pattern)) { - return; - } - - const string = removeEscapeCharacters(pattern.regex.pattern); - - yield fixer.replaceText(pattern, escapeString(string)); + yield * convertRegExpToString(pattern, fixer); }, }; }, diff --git a/test/prefer-string-replace-all.mjs b/test/prefer-string-replace-all.mjs index b0a0ffa423..508a672a16 100644 --- a/test/prefer-string-replace-all.mjs +++ b/test/prefer-string-replace-all.mjs @@ -68,6 +68,7 @@ test.snapshot({ 'foo.replace(/\\W/g, bar)', 'foo.replace(/\\u{61}/g, bar)', 'foo.replace(/\\u{61}/gu, bar)', + 'foo.replace(/]/g, "bar")', // Extra flag 'foo.replace(/a/gi, bar)', 'foo.replace(/a/gui, bar)', @@ -75,5 +76,19 @@ test.snapshot({ // Variables 'const pattern = new RegExp("foo", "g"); foo.replace(pattern, bar)', 'foo.replace(new RegExp("foo", "g"), bar)', + + 'foo.replace(/a]/g, _)', + 'foo.replace(/[a]/g, _)', + 'foo.replace(/a{1/g, _)', + 'foo.replace(/a{1}/g, _)', + 'foo.replace(/\\u0022/g, _)', + 'foo.replace(/\\u0027/g, _)', + 'foo.replace(/\\cM\\cj/g, _)', + 'foo.replace(/\\x22/g, _)', + 'foo.replace(/\\x27/g, _)', + 'foo.replace(/\\uD83D\\ude00/g, _)', + 'foo.replace(/\\u{1f600}/gu, _)', + 'foo.replace(/\\n/g, _)', + 'foo.replace(/\\u{20}/gu, _)', ], }); diff --git a/test/snapshots/prefer-string-replace-all.mjs.md b/test/snapshots/prefer-string-replace-all.mjs.md index 32a94728f2..a10963392c 100644 --- a/test/snapshots/prefer-string-replace-all.mjs.md +++ b/test/snapshots/prefer-string-replace-all.mjs.md @@ -252,7 +252,7 @@ Generated by [AVA](https://avajs.dev). > Output `␊ - 1 | foo.replaceAll(/\\u{61}/gu, bar)␊ + 1 | foo.replaceAll('a', bar)␊ ` > Error 1/1 @@ -263,6 +263,22 @@ Generated by [AVA](https://avajs.dev). ` ## Invalid #16 + 1 | foo.replace(/]/g, "bar") + +> Output + + `␊ + 1 | foo.replaceAll(']', "bar")␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replace(/]/g, "bar")␊ + | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ + ` + +## Invalid #17 1 | foo.replace(/a/gi, bar) > Output @@ -278,7 +294,7 @@ Generated by [AVA](https://avajs.dev). | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ ` -## Invalid #17 +## Invalid #18 1 | foo.replace(/a/gui, bar) > Output @@ -294,7 +310,7 @@ Generated by [AVA](https://avajs.dev). | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ ` -## Invalid #18 +## Invalid #19 1 | foo.replace(/a/uig, bar) > Output @@ -310,7 +326,7 @@ Generated by [AVA](https://avajs.dev). | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ ` -## Invalid #19 +## Invalid #20 1 | const pattern = new RegExp("foo", "g"); foo.replace(pattern, bar) > Output @@ -326,7 +342,7 @@ Generated by [AVA](https://avajs.dev). | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ ` -## Invalid #20 +## Invalid #21 1 | foo.replace(new RegExp("foo", "g"), bar) > Output @@ -341,3 +357,211 @@ Generated by [AVA](https://avajs.dev). > 1 | foo.replace(new RegExp("foo", "g"), bar)␊ | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ ` + +## Invalid #22 + 1 | foo.replace(/a]/g, _) + +> Output + + `␊ + 1 | foo.replaceAll('a]', _)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replace(/a]/g, _)␊ + | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ + ` + +## Invalid #23 + 1 | foo.replace(/[a]/g, _) + +> Output + + `␊ + 1 | foo.replaceAll(/[a]/g, _)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replace(/[a]/g, _)␊ + | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ + ` + +## Invalid #24 + 1 | foo.replace(/a{1/g, _) + +> Output + + `␊ + 1 | foo.replaceAll('a{1', _)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replace(/a{1/g, _)␊ + | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ + ` + +## Invalid #25 + 1 | foo.replace(/a{1}/g, _) + +> Output + + `␊ + 1 | foo.replaceAll(/a{1}/g, _)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replace(/a{1}/g, _)␊ + | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ + ` + +## Invalid #26 + 1 | foo.replace(/\u0022/g, _) + +> Output + + `␊ + 1 | foo.replaceAll('"', _)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replace(/\\u0022/g, _)␊ + | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ + ` + +## Invalid #27 + 1 | foo.replace(/\u0027/g, _) + +> Output + + `␊ + 1 | foo.replaceAll('\\'', _)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replace(/\\u0027/g, _)␊ + | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ + ` + +## Invalid #28 + 1 | foo.replace(/\cM\cj/g, _) + +> Output + + `␊ + 1 | foo.replaceAll('\\r\\n', _)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replace(/\\cM\\cj/g, _)␊ + | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ + ` + +## Invalid #29 + 1 | foo.replace(/\x22/g, _) + +> Output + + `␊ + 1 | foo.replaceAll('"', _)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replace(/\\x22/g, _)␊ + | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ + ` + +## Invalid #30 + 1 | foo.replace(/\x27/g, _) + +> Output + + `␊ + 1 | foo.replaceAll('\\'', _)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replace(/\\x27/g, _)␊ + | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ + ` + +## Invalid #31 + 1 | foo.replace(/\uD83D\ude00/g, _) + +> Output + + `␊ + 1 | foo.replaceAll('😀', _)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replace(/\\uD83D\\ude00/g, _)␊ + | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ + ` + +## Invalid #32 + 1 | foo.replace(/\u{1f600}/gu, _) + +> Output + + `␊ + 1 | foo.replaceAll('😀', _)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replace(/\\u{1f600}/gu, _)␊ + | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ + ` + +## Invalid #33 + 1 | foo.replace(/\n/g, _) + +> Output + + `␊ + 1 | foo.replaceAll('\\n', _)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replace(/\\n/g, _)␊ + | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ + ` + +## Invalid #34 + 1 | foo.replace(/\u{20}/gu, _) + +> Output + + `␊ + 1 | foo.replaceAll(' ', _)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replace(/\\u{20}/gu, _)␊ + | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ + ` diff --git a/test/snapshots/prefer-string-replace-all.mjs.snap b/test/snapshots/prefer-string-replace-all.mjs.snap index 903934852e..2acc7296e3 100644 Binary files a/test/snapshots/prefer-string-replace-all.mjs.snap and b/test/snapshots/prefer-string-replace-all.mjs.snap differ