Skip to content

Commit

Permalink
prefer-string-replace-all: Improve RegExp to string fix (#1971)
Browse files Browse the repository at this point in the history
  • Loading branch information
@fisker
fisker committed Nov 19, 2022
1 parent 48efc7a commit b844dbc
Show file tree
Hide file tree
Showing 5 changed files with 273 additions and 32 deletions.
1 change: 1 addition & 0 deletions package.json
View file
Expand Up @@ -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"
Expand Down
55 changes: 28 additions & 27 deletions rules/prefer-string-replace-all.js
View file
@@ -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');
Expand All @@ -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)) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
},
};
},
Expand Down
15 changes: 15 additions & 0 deletions test/prefer-string-replace-all.mjs
View file
Expand Up @@ -68,12 +68,27 @@ 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)',
'foo.replace(/a/uig, bar)',
// 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, _)',
],
});
234 changes: 229 additions & 5 deletions test/snapshots/prefer-string-replace-all.mjs.md
View file
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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()\`.␊
`
Binary file modified test/snapshots/prefer-string-replace-all.mjs.snap
View file
Binary file not shown.

0 comments on commit b844dbc

Please sign in to comment.