Add prefer-replace-all rule (#488)

Co-authored-by: fisker Cheung <[email protected]>
Co-authored-by: Sindre Sorhus <[email protected]>

Author: 3 people

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', '');
+```

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',

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)*

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'
+	}
+};

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]
+		}
+	]
+});