From d98c277b8da31810a206dc72505619a02c3d4fc4 Mon Sep 17 00:00:00 2001 From: jmoore914 <30698083+jmoore914@users.noreply.github.com> Date: Sat, 1 Feb 2020 05:43:22 -0500 Subject: [PATCH] Add `prefer-replace-all` rule (#488) Co-authored-by: fisker Cheung Co-authored-by: Sindre Sorhus --- docs/rules/prefer-replace-all.md | 21 ++++++++ index.js | 1 + readme.md | 2 + rules/prefer-replace-all.js | 67 ++++++++++++++++++++++++ test/prefer-replace-all.js | 89 ++++++++++++++++++++++++++++++++ 5 files changed, 180 insertions(+) create mode 100644 docs/rules/prefer-replace-all.md create mode 100644 rules/prefer-replace-all.js create mode 100644 test/prefer-replace-all.js diff --git a/docs/rules/prefer-replace-all.md b/docs/rules/prefer-replace-all.md new file mode 100644 index 0000000000..e31c6ad5d9 --- /dev/null +++ b/docs/rules/prefer-replace-all.md @@ -0,0 +1,21 @@ +# Prefer `String#replaceAll()` over regex searches with the global flag + +The [`String#replaceAll()`](https://github.com/tc39/proposal-string-replaceall) method is both faster and safer as you don't have to escape the regex if the string is not a literal. + +This rule is fixable. + +## Fail + +```js +string.replace(/This has no special regex symbols/g, ''); +string.replace(/\(It also checks for escaped regex symbols\)/g, ''); +``` + +## Pass + +```js +string.replace(/Non-literal characters .*/g, ''); +string.replace(/Extra flags/gi, ''); +string.replace('Not a regex expression', '') +string.replaceAll('Literal characters only', ''); +``` diff --git a/index.js b/index.js index c83f2ea4cf..6a509a7320 100644 --- a/index.js +++ b/index.js @@ -54,6 +54,7 @@ module.exports = { 'unicorn/prefer-node-remove': 'error', 'unicorn/prefer-query-selector': 'error', 'unicorn/prefer-reflect-apply': 'error', + 'unicorn/prefer-replace-all': 'off', 'unicorn/prefer-spread': 'error', 'unicorn/prefer-starts-ends-with': 'error', 'unicorn/prefer-string-slice': 'error', diff --git a/readme.md b/readme.md index 3f4eb2b39c..42f17d6ff6 100644 --- a/readme.md +++ b/readme.md @@ -72,6 +72,7 @@ Configure it in `package.json`. "unicorn/prefer-node-remove": "error", "unicorn/prefer-query-selector": "error", "unicorn/prefer-reflect-apply": "error", + "unicorn/prefer-replace-all": "off", "unicorn/prefer-spread": "error", "unicorn/prefer-starts-ends-with": "error", "unicorn/prefer-string-slice": "error", @@ -125,6 +126,7 @@ Configure it in `package.json`. - [prefer-node-remove](docs/rules/prefer-node-remove.md) - Prefer `node.remove()` over `parentNode.removeChild(node)` and `parentElement.removeChild(node)`. *(fixable)* - [prefer-query-selector](docs/rules/prefer-query-selector.md) - Prefer `.querySelector()` over `.getElementById()`, `.querySelectorAll()` over `.getElementsByClassName()` and `.getElementsByTagName()`. *(partly fixable)* - [prefer-reflect-apply](docs/rules/prefer-reflect-apply.md) - Prefer `Reflect.apply()` over `Function#apply()`. *(fixable)* +- [prefer-replace-all](docs/rules/prefer-replace-all.md) - Prefer `String#replaceAll()` over regex searches with the global flag. *(fixable)* - [prefer-spread](docs/rules/prefer-spread.md) - Prefer the spread operator over `Array.from()`. *(fixable)* - [prefer-starts-ends-with](docs/rules/prefer-starts-ends-with.md) - Prefer `String#startsWith()` & `String#endsWith()` over more complex alternatives. - [prefer-string-slice](docs/rules/prefer-string-slice.md) - Prefer `String#slice()` over `String#substr()` and `String#substring()`. *(partly fixable)* diff --git a/rules/prefer-replace-all.js b/rules/prefer-replace-all.js new file mode 100644 index 0000000000..bb45e72e05 --- /dev/null +++ b/rules/prefer-replace-all.js @@ -0,0 +1,67 @@ +'use strict'; +const getDocumentationUrl = require('./utils/get-documentation-url'); +const quoteString = require('./utils/quote-string'); + +function isRegexWithGlobalFlag(node) { + const {type, regex} = node; + return type === 'Literal' && regex && regex.flags === 'g'; +} + +function isLiteralCharactersOnly(node) { + const searchPattern = node.regex.pattern; + return !/[$()*+.?[\\\]^{}]/.test(searchPattern.replace(/\\[$()*+.?[\\\]^{}]/g, '')); +} + +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; +} + +const create = context => { + return { + 'CallExpression[callee.property.name="replace"]': node => { + const {arguments: arguments_} = node; + + if (arguments_.length !== 2) { + return; + } + + const [search] = arguments_; + + if (!isRegexWithGlobalFlag(search) || !isLiteralCharactersOnly(search)) { + return; + } + + context.report({ + node, + message: 'Prefer `String#replaceAll()` over `String#replace()`.', + fix: fixer => + [ + fixer.insertTextAfter(node.callee, 'All'), + fixer.replaceText(search, quoteString(removeEscapeCharacters(search.regex.pattern))) + ] + }); + } + }; +}; + +module.exports = { + create, + meta: { + type: 'suggestion', + docs: { + url: getDocumentationUrl(__filename) + }, + fixable: 'code' + } +}; diff --git a/test/prefer-replace-all.js b/test/prefer-replace-all.js new file mode 100644 index 0000000000..fb470e5c29 --- /dev/null +++ b/test/prefer-replace-all.js @@ -0,0 +1,89 @@ +import test from 'ava'; +import avaRuleTester from 'eslint-ava-rule-tester'; +const rule = require('../rules/prefer-replace-all'); + +const ruleTester = avaRuleTester(test, { + env: { + es6: true + } +}); + +const error = { + ruleId: 'prefer-replace-all', + message: 'Prefer `String#replaceAll()` over `String#replace()`.' +}; + +ruleTester.run('prefer-replace-all', rule, { + valid: [ + // No global flag + 'foo.replace(/a/, bar)', + // Special characters + 'foo.replace(/[a]/g, bar)', + 'foo.replace(/a?/g, bar)', + 'foo.replace(/.*/g, bar)', + 'foo.replace(/\\W/g, bar)', + // Extra flag + 'foo.replace(/a/gi, bar)', + // Not regex literal + 'foo.replace(\'string\', bar)', + // Not 2 arguments + 'foo.replace(/a/g)', + 'foo.replace(/\\\\./g)', + // New + 'new foo.replace(/a/g, bar)', + // Function call + 'replace(/a/g, bar)', + // Not call + 'foo.replace', + // Not replace + 'foo.methodNotReplace(/a/g, bar);', + // `replace` is not Identifier + 'foo[\'replace\'](/a/g, bar)' + ], + invalid: [ + { + code: 'foo.replace(/a/g, bar)', + output: 'foo.replaceAll(\'a\', bar)', + errors: [error] + }, + // Comments + { + code: ` + foo/* comment 1 */ + .replace/* comment 2 */( + /* comment 3 */ + /a/g // comment 4 + , + bar + ) + `, + output: ` + foo/* comment 1 */ + .replaceAll/* comment 2 */( + /* comment 3 */ + 'a' // comment 4 + , + bar + ) + `, + errors: [error] + }, + // Quotes + { + code: 'foo.replace(/"\'/g, \'\\\'\')', + output: 'foo.replaceAll(\'"\\\'\', \'\\\'\')', + errors: [error] + }, + // Escaped symbols + { + code: 'foo.replace(/\\./g, bar)', + output: 'foo.replaceAll(\'.\', bar)', + errors: [error] + }, + { + code: 'foo.replace(/\\\\\\./g, bar)', + output: 'foo.replaceAll(\'\\.\', bar)', + errors: [error] + } + ] +});