diff --git a/docs/rules/prefer-string-replace-all.md b/docs/rules/prefer-string-replace-all.md index 63f6580ffb..ea147892d5 100644 --- a/docs/rules/prefer-string-replace-all.md +++ b/docs/rules/prefer-string-replace-all.md @@ -27,6 +27,10 @@ string.replace(/\(It also checks for escaped regex symbols\)/g, ''); string.replace(/Works for u flag too/gu, ''); ``` +```js +string.replaceAll(/foo/g, 'bar'); +``` + ## Pass ```js @@ -44,3 +48,7 @@ string.replaceAll('string', ''); ```js string.replaceAll(/\s/, ''); ``` + +```js +string.replaceAll('foo', 'bar'); +``` diff --git a/rules/prefer-string-replace-all.js b/rules/prefer-string-replace-all.js index 09a22a617a..5e80f9bc35 100644 --- a/rules/prefer-string-replace-all.js +++ b/rules/prefer-string-replace-all.js @@ -5,17 +5,19 @@ const escapeString = require('./utils/escape-string.js'); const {methodCallSelector} = require('./selectors/index.js'); const {isRegexLiteral, isNewExpression} = require('./ast/index.js'); -const MESSAGE_ID = 'prefer-string-replace-all'; +const MESSAGE_ID_USE_REPLACE_ALL = 'method'; +const MESSAGE_ID_USE_STRING = 'pattern'; const messages = { - [MESSAGE_ID]: 'Prefer `String#replaceAll()` over `String#replace()`.', + [MESSAGE_ID_USE_REPLACE_ALL]: 'Prefer `String#replaceAll()` over `String#replace()`.', + [MESSAGE_ID_USE_STRING]: 'This pattern can be replaced with a string {{replacement}}.', }; const selector = methodCallSelector({ - method: 'replace', + methods: ['replace', 'replaceAll'], argumentsLength: 2, }); -function * convertRegExpToString(node, fixer) { +function getPatternReplacement(node) { if (!isRegexLiteral(node)) { return; } @@ -39,7 +41,7 @@ function * convertRegExpToString(node, fixer) { // TODO: Preserve escape const string = String.fromCodePoint(...parts.map(part => part.codePoint)); - yield fixer.replaceText(node, escapeString(string)); + return escapeString(string); } const isRegExpWithGlobalFlag = (node, scope) => { @@ -82,13 +84,38 @@ const create = context => ({ return; } + const methodName = property.name; + const patternReplacement = getPatternReplacement(pattern); + + if (methodName === 'replaceAll') { + if (!patternReplacement) { + return; + } + + return { + node: pattern, + messageId: MESSAGE_ID_USE_STRING, + data: { + // Show `This pattern can be replaced with a string literal.` for long strings + replacement: patternReplacement.length < 20 ? patternReplacement : 'literal', + }, + /** @param {import('eslint').Rule.RuleFixer} fixer */ + fix: fixer => fixer.replaceText(pattern, patternReplacement), + }; + } + return { node: property, - messageId: MESSAGE_ID, + messageId: MESSAGE_ID_USE_REPLACE_ALL, /** @param {import('eslint').Rule.RuleFixer} fixer */ * fix(fixer) { yield fixer.insertTextAfter(property, 'All'); - yield * convertRegExpToString(pattern, fixer); + + if (!patternReplacement) { + return; + } + + yield fixer.replaceText(pattern, patternReplacement); }, }; }, diff --git a/test/prefer-string-replace-all.mjs b/test/prefer-string-replace-all.mjs index 508a672a16..1ac58c935d 100644 --- a/test/prefer-string-replace-all.mjs +++ b/test/prefer-string-replace-all.mjs @@ -7,25 +7,36 @@ test.snapshot({ valid: [ // No global flag 'foo.replace(/a/, bar)', + 'foo.replaceAll(/a/, bar)', // Not regex literal 'foo.replace("string", bar)', + 'foo.replaceAll("string", bar)', // Not 2 arguments 'foo.replace(/a/g)', + 'foo.replaceAll(/a/g)', 'foo.replace(/\\\\./g)', + 'foo.replaceAll(/\\\\./g)', // Not `CallExpression` 'new foo.replace(/a/g, bar)', + 'new foo.replaceAll(/a/g, bar)', // Not `MemberExpression` 'replace(/a/g, bar)', + 'replaceAll(/a/g, bar)', // Computed 'foo[replace](/a/g, bar);', + 'foo[replaceAll](/a/g, bar);', // Not replace 'foo.methodNotReplace(/a/g, bar);', // `callee.property` is not a `Identifier` 'foo[\'replace\'](/a/g, bar)', + 'foo[\'replaceAll\'](/a/g, bar)', // More or less argument(s) 'foo.replace(/a/g, bar, extra);', + 'foo.replaceAll(/a/g, bar, extra);', 'foo.replace();', + 'foo.replaceAll();', 'foo.replace(...argumentsArray, ...argumentsArray2)', + 'foo.replaceAll(...argumentsArray, ...argumentsArray2)', // Unknown/non-regexp/non-global value 'foo.replace(unknown, bar)', 'const pattern = new RegExp("foo", unknown); foo.replace(pattern, bar)', @@ -36,8 +47,6 @@ test.snapshot({ 'const pattern = "not-a-regexp"; foo.replace(pattern, bar)', 'const pattern = new RegExp("foo", "i"); foo.replace(pattern, bar)', 'foo.replace(new NotRegExp("foo", "g"), bar)', - // We are not checking this - 'foo.replaceAll(/a/g, bar)', ], invalid: [ 'foo.replace(/a/g, bar)', @@ -90,5 +99,9 @@ test.snapshot({ 'foo.replace(/\\u{1f600}/gu, _)', 'foo.replace(/\\n/g, _)', 'foo.replace(/\\u{20}/gu, _)', + + 'foo.replaceAll(/a]/g, _)', + 'foo.replaceAll(/\\r\\n\\u{1f600}/gu, _)', + `foo.replaceAll(/a${' very'.repeat(30)} string/g, _)`, ], }); diff --git a/test/snapshots/prefer-string-replace-all.mjs.md b/test/snapshots/prefer-string-replace-all.mjs.md index a10963392c..1bc94d70d9 100644 --- a/test/snapshots/prefer-string-replace-all.mjs.md +++ b/test/snapshots/prefer-string-replace-all.mjs.md @@ -565,3 +565,51 @@ Generated by [AVA](https://avajs.dev). > 1 | foo.replace(/\\u{20}/gu, _)␊ | ^^^^^^^ Prefer \`String#replaceAll()\` over \`String#replace()\`.␊ ` + +## Invalid #35 + 1 | foo.replaceAll(/a]/g, _) + +> Output + + `␊ + 1 | foo.replaceAll('a]', _)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replaceAll(/a]/g, _)␊ + | ^^^^^ This pattern can be replaced with a string 'a]'.␊ + ` + +## Invalid #36 + 1 | foo.replaceAll(/\r\n\u{1f600}/gu, _) + +> Output + + `␊ + 1 | foo.replaceAll('\\r\\n😀', _)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replaceAll(/\\r\\n\\u{1f600}/gu, _)␊ + | ^^^^^^^^^^^^^^^^^ This pattern can be replaced with a string '\\r\\n😀'.␊ + ` + +## Invalid #37 + 1 | foo.replaceAll(/a very very very very very very very very very very very very very very very very very very very very very very very very very very very very very very string/g, _) + +> Output + + `␊ + 1 | foo.replaceAll('a very very very very very very very very very very very very very very very very very very very very very very very very very very very very very very string', _)␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.replaceAll(/a very very very very very very very very very very very very very very very very very very very very very very very very very very very very very very string/g, _)␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This pattern can be replaced with a string literal.␊ + ` diff --git a/test/snapshots/prefer-string-replace-all.mjs.snap b/test/snapshots/prefer-string-replace-all.mjs.snap index 2acc7296e3..1ba0cdf437 100644 Binary files a/test/snapshots/prefer-string-replace-all.mjs.snap and b/test/snapshots/prefer-string-replace-all.mjs.snap differ