prefer-string-replace-all: Report all String#replace() when the pattern has g flag

docs/rules/prefer-string-replace-all.md

@@ -8,11 +8,16 @@
 <!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->
 
 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.
+Even if regexp has to be used, `String#replaceAll()` has more clear intention.
 
 ## Fail
 
 ```js
-string.replace(/This has no special regex symbols/g, '');
+string.replace(/RegExp with global flag/igu, '');
+```
+
+```js
+string.replace(/RegExp without special symbols/g, '');
 ```
 
 ```js
@@ -26,17 +31,17 @@ string.replace(/Works for u flag too/gu, '');
 ## Pass
 
 ```js
-string.replace(/Non-literal characters .*/g, '');
+string.replace(/Non-global regexp/iu, '');
 ```
 
 ```js
-string.replace(/Extra flags/gi, '');
+string.replace('Not a regex expression', '')
 ```
 
 ```js
-string.replace('Not a regex expression', '')
+string.replaceAll('string', '');
 ```
 
 ```js
-string.replaceAll('Literal characters only', '');
+string.replaceAll(/\s/, '');
 ```

rules/ast/index.js

@@ -21,4 +21,5 @@ module.exports = {
 	isEmptyNode: require('./is-empty-node.js'),
 	isStaticRequire: require('./is-static-require.js'),
 	isUndefined: require('./is-undefined.js'),
+	isNewExpression: require('./is-new-expression.js'),
 };

rules/ast/is-new-expression.js

@@ -0,0 +1,22 @@
+'use strict';
+
+function isNewExpression(node, options) {
+	if (node?.type !== 'NewExpression') {
+		return false;
+	}
+
+	const {
+		name,
+	} = {
+		...options,
+	};
+
+	if (name) {
+		return node.callee.type === 'Identifier' && node.callee.name === name;
+	}
+
+	/* c8 ignore next */
+	return true;
+}
+
+module.exports = isNewExpression;

rules/no-unsafe-regex.js

@@ -1,6 +1,7 @@
 'use strict';
 const safeRegex = require('safe-regex');
 const {newExpressionSelector} = require('./selectors/index.js');
+const {isNewExpression} = require('./ast/index.js');
 
 const MESSAGE_ID = 'no-unsafe-regex';
 const messages = {
@@ -16,10 +17,7 @@ const newRegExpSelector = [
 const create = () => ({
 	'Literal[regex]'(node) {
 		// Handle regex literal inside RegExp constructor in the other handler
-		if (
-			node.parent.type === 'NewExpression'
-				&& node.parent.callee.name === 'RegExp'
-		) {
+		if (isNewExpression(node.parent, {name: 'RegExp'})) {
 			return;
 		}
 

rules/prefer-regexp-test.js

@@ -2,7 +2,7 @@
 const {isParenthesized, getStaticValue} = require('eslint-utils');
 const {checkVueTemplate} = require('./utils/rule.js');
 const {methodCallSelector} = require('./selectors/index.js');
-const {isRegexLiteral} = require('./ast/index.js');
+const {isRegexLiteral, isNewExpression} = require('./ast/index.js');
 const {isBooleanNode} = require('./utils/boolean.js');
 const shouldAddParenthesesToMemberExpressionObject = require('./utils/should-add-parentheses-to-member-expression-object.js');
 
@@ -69,15 +69,9 @@ const cases = [
 	},
 ];
 
-const isRegExpNode = node =>
-	isRegexLiteral(node)
-	|| (
-		node.type === 'NewExpression'
-		&& node.callee.type === 'Identifier'
-		&& node.callee.name === 'RegExp'
-	);
+const isRegExpNode = node => isRegexLiteral(node) || isNewExpression(node, {name: 'RegExp'});
 
-const isRegExpWithoutGFlag = (node, scope) => {
+const isRegExpWithoutGlobalFlag = (node, scope) => {
 	const staticResult = getStaticValue(node, scope);
 
 	// Don't know if there is `g` flag
@@ -88,7 +82,7 @@ const isRegExpWithoutGFlag = (node, scope) => {
 	const {value} = staticResult;
 	return (
 		Object.prototype.toString.call(value) === '[object RegExp]'
-		&& !value.flags.includes('g')
+		&& !value.global
 	);
 };
 
@@ -118,7 +112,7 @@ const create = context => Object.fromEntries(
 
 			if (
 				isRegExpNode(regexpNode)
-				|| isRegExpWithoutGFlag(regexpNode, context.getScope())
+				|| isRegExpWithoutGlobalFlag(regexpNode, context.getScope())
 			) {
 				problem.fix = fixFunction;
 			} else {

rules/prefer-set-size.js

@@ -2,6 +2,7 @@
 const {findVariable} = require('eslint-utils');
 const {memberExpressionSelector} = require('./selectors/index.js');
 const {fixSpaceAroundKeyword} = require('./fix/index.js');
+const {isNewExpression} = require('./ast/index.js');
 
 const MESSAGE_ID = 'prefer-set-size';
 const messages = {
@@ -15,13 +16,10 @@ const lengthAccessSelector = [
 	'[object.elements.0.type="SpreadElement"]',
 ].join('');
 
-const isNewSet = node =>
-	node?.type === 'NewExpression'
-	&& node.callee.type === 'Identifier'
-	&& node.callee.name === 'Set';
+const isNewSet = node => isNewExpression(node, {name: 'Set'});
 
 function isSet(node, scope) {
-	if (isNewSet(node)) {
+	if (isNewExpression(node, {name: 'Set'})) {
 		return true;
 	}
 

rules/prefer-string-replace-all.js

@@ -1,7 +1,8 @@
 'use strict';
+const {getStaticValue} = require('eslint-utils');
 const quoteString = require('./utils/quote-string.js');
 const {methodCallSelector} = require('./selectors/index.js');
-const {isRegexLiteral} = require('./ast/index.js');
+const {isRegexLiteral, isNewExpression} = require('./ast/index.js');
 
 const MESSAGE_ID = 'prefer-string-replace-all';
 const messages = {
@@ -13,14 +14,38 @@ const selector = methodCallSelector({
 	argumentsLength: 2,
 });
 
-const isRegexWithGlobalFlag = node =>
+const canReplacePatternWithString = node =>
 	isRegexLiteral(node)
-	&& node.regex.flags.replace('u', '') === 'g';
+	&& node.regex.flags.replace('u', '') === 'g'
+	&& !/[$()*+.?[\\\]^{|}]/.test(node.regex.pattern.replace(/\\[$()*+.?[\\\]^{|}]/g, ''));
 
-function isLiteralCharactersOnly(node) {
-	const searchPattern = node.regex.pattern;
-	return !/[$()*+.?[\\\]^{|}]/.test(searchPattern.replace(/\\[$()*+.?[\\\]^{|}]/g, ''));
-}
+const isRegExpWithGlobalFlag = (node, scope) => {
+	if (isRegexLiteral(node)) {
+		return node.regex.flags.includes('g');
+	}
+
+	if (
+		isNewExpression(node, {name: 'RegExp'})
+		&& node.arguments[0]?.type !== 'SpreadElement'
+		&& node.arguments[1]?.type === 'Literal'
+		&& typeof node.arguments[1].value === 'string'
+	) {
+		return node.arguments[1].value.includes('g');
+	}
+
+	const staticResult = getStaticValue(node, scope);
+
+	// Don't know if there is `g` flag
+	if (!staticResult) {
+		return false;
+	}
+
+	const {value} = staticResult;
+	return (
+		Object.prototype.toString.call(value) === '[object RegExp]'
+		&& value.global
+	);
+};
 
 function removeEscapeCharacters(regexString) {
 	let fixedString = regexString;
@@ -38,22 +63,32 @@ function removeEscapeCharacters(regexString) {
 }
 
 /** @param {import('eslint').Rule.RuleContext} context */
-const create = () => ({
+const create = context => ({
 	[selector](node) {
-		const {arguments: arguments_, callee} = node;
-		const [search] = arguments_;
+		const {
+			arguments: [pattern],
+			callee: {property},
+		} = node;
 
-		if (!isRegexWithGlobalFlag(search) || !isLiteralCharactersOnly(search)) {
+		if (!isRegExpWithGlobalFlag(pattern, context.getScope())) {
 			return;
 		}
 
 		return {
-			node,
+			node: property,
 			messageId: MESSAGE_ID,
-			fix: fixer => [
-				fixer.insertTextAfter(callee, 'All'),
-				fixer.replaceText(search, quoteString(removeEscapeCharacters(search.regex.pattern))),
-			],
+			/** @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, quoteString(string));
+			},
 		};
 	},
 });

test/prefer-string-replace-all.mjs

@@ -1,29 +1,14 @@
+import outdent from 'outdent';
 import {getTester} from './utils/test.mjs';
 
 const {test} = getTester(import.meta);
 
-const error = {
-	message: 'Prefer `String#replaceAll()` over `String#replace()`.',
-};
-
-test({
+test.snapshot({
 	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(/a|b/g, bar)',
-		'foo.replace(/\\W/g, bar)',
-		'foo.replace(/\\u{61}/g, bar)',
-		'foo.replace(/\\u{61}/gu, bar)',
-		// Extra flag
-		'foo.replace(/a/gi, bar)',
-		'foo.replace(/a/gui, bar)',
-		'foo.replace(/a/uig, bar)',
 		// Not regex literal
-		'foo.replace(\'string\', bar)',
+		'foo.replace("string", bar)',
 		// Not 2 arguments
 		'foo.replace(/a/g)',
 		'foo.replace(/\\\\./g)',
@@ -41,64 +26,54 @@ test({
 		'foo.replace(/a/g, bar, extra);',
 		'foo.replace();',
 		'foo.replace(...argumentsArray, ...argumentsArray2)',
+		// Unknown/non-regexp/non-global value
+		'foo.replace(unknown, bar)',
+		'const pattern = new RegExp("foo", unknown); foo.replace(pattern, bar)',
+		'const pattern = new RegExp("foo"); foo.replace(pattern, bar)',
+		'const pattern = new RegExp(); foo.replace(pattern, bar)',
+		'const pattern = "string"; foo.replace(pattern, bar)',
+		'const pattern = new RegExp("foo", "g"); foo.replace(...[pattern], bar)',
+		'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: [
-		{
-			code: 'foo.replace(/a/g, bar)',
-			output: 'foo.replaceAll(\'a\', bar)',
-			errors: [error],
-		},
+		'foo.replace(/a/g, bar)',
 		// 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],
-		},
+		outdent`
+			foo/* comment 1 */
+				.replace/* comment 2 */(
+					/* comment 3 */
+					/a/g // comment 4
+					,
+					bar
+				)
+		`,
 		// Quotes
-		{
-			code: 'foo.replace(/"\'/g, \'\\\'\')',
-			output: 'foo.replaceAll(\'"\\\'\', \'\\\'\')',
-			errors: [error],
-		},
+		'foo.replace(/"\'/g, \'\\\'\')',
 		// Escaped symbols
-		{
-			code: 'foo.replace(/\\./g, bar)',
-			output: 'foo.replaceAll(\'.\', bar)',
-			errors: [error],
-		},
-		{
-			code: 'foo.replace(/\\\\\\./g, bar)',
-			output: 'foo.replaceAll(\'\\\\.\', bar)',
-			errors: [error],
-		},
-		{
-			code: 'foo.replace(/\\|/g, bar)',
-			output: 'foo.replaceAll(\'|\', bar)',
-			errors: [error],
-		},
-	],
-});
-
-test.snapshot({
-	valid: [],
-	invalid: [
+		'foo.replace(/\\./g, bar)',
+		'foo.replace(/\\\\\\./g, bar)',
+		'foo.replace(/\\|/g, bar)',
+		// `u` flag
 		'foo.replace(/a/gu, bar)',
 		'foo.replace(/a/ug, bar)',
+		// Special characters
+		'foo.replace(/[a]/g, bar)',
+		'foo.replace(/a?/g, bar)',
+		'foo.replace(/.*/g, bar)',
+		'foo.replace(/a|b/g, bar)',
+		'foo.replace(/\\W/g, bar)',
+		'foo.replace(/\\u{61}/g, bar)',
+		'foo.replace(/\\u{61}/gu, bar)',
+		// Extra flag
+		'foo.replace(/a/gi, bar)',
+		'foo.replace(/a/gui, bar)',
+		'foo.replace(/a/uig, bar)',
+		// Variables
+		'const pattern = new RegExp("foo", "g"); foo.replace(pattern, bar)',
+		'foo.replace(new RegExp("foo", "g"), bar)',
 	],
 });