From e8c51568427f6a9473f6b82179ecad5e4796390b Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Mon, 21 Nov 2022 00:38:49 +0800 Subject: [PATCH] `prefer-string-replace-all`: Check pattern even if it's already using `.replaceAll` (#1981) --- docs/rules/prefer-string-replace-all.md | 8 +++ rules/prefer-string-replace-all.js | 41 ++++++++++++--- test/prefer-string-replace-all.mjs | 17 ++++++- .../prefer-string-replace-all.mjs.md | 48 ++++++++++++++++++ .../prefer-string-replace-all.mjs.snap | Bin 1628 -> 1837 bytes 5 files changed, 105 insertions(+), 9 deletions(-) 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 2acc7296e3e0e290bff534022038dbfa90de658d..1ba0cdf437e4f456ab08735b2ac49e18452cc802 100644 GIT binary patch literal 1837 zcmV+|2h#XKRzV;)O(|wyQ1b7d@`jDOZGH<^9H}Bhj|NZ~B`>-xiGG=K9T#T$cthJC~pufs%Zo7krdCV9daD*xZSg-rt|jA6l41WkJmDClHwe;5Nu(@8|m!CrUJbT19mW zKyP8#nFOa83wph6*rVKQ-hHQK7mG5{Tp0E_L6jK)<0lLqWF1#-YWH_ex<%YMONiMV zf*$6`BFAa7zaPozX)1iKLCpFPe2p-vzor1LdGh>X)3%4(t1!<}zJ!=;o2VvwdyHLCnSz+(Xzp zuw0y0l+<;5#G>ss7b`!8m>t;DfGmW9NS#%Wpt3ntIcG(>joD}`Fzj&x&t3rRbS+%f zXV|uVdrVELv&WVLh}i^!mk5Ou124J6!+W(G>`R8Yt=$AM>)6|Xe1yW4!;b~q>>m8X z(Mv&E4M1~Y*h+$N)&P8ys^4)jt!C%VJzuR`JD~U_#B2h=LxkgrMMF$)bsu~`(pg;l zbY2d`?4UjdWFeF$R7@y8t~ofgZkAQ*!<4lUvnL2f^hN8V(j2jP6jEK5vN%3>`Qr?T zSrx%GguB7|TCX)gma;-(7%!n#7tt|lgRclrDMz3)UrK01TcOl-T(OY3UXwXSJQHp4dMyb^y z#)(E9Zm7e;NWPJU8?q216L-eM!l=XDFw_I{BDsj7>H0no)G-`)IHM;|{1hgSMbBQ$ zb7POdb1i7|Zmf+(7~?F9|LLk%O^=q-s@I;Mz=0z~q_I-%!_6BOqBxn;#3d_1^C)4) z(99UAKG`+gQzDs9tMqtiL&O-hkfx2AIZqu&#mIF!WvrG`$hB0Mk|O^-mr79U=23Dg z4m*cx(AGGP1Qf?yQqUxj2b#=W#TM(*6eiZOF(08W5VA|8NiKmDi@*quO5(eXk%+5Dcp-sMX2`eaYTL7@5iM{=Yo z;{olPr;=S+7M=a5o$UH{K0x&9Hl!&d>kg3cS+0N@)|=0)503yYp%faQ)bV}{w^=Ng zsyhT+_h=nJHTO(jdB@k0TU+iQqtwVq9@W|#6XNOPELB9j;ROCKSgFe$Lm&VE_1TDg literal 1628 zcmV-i2BY~wRzVYUOK>`htBMB1{go0Qdrz&zd6+95} z7(6Hx(RQ?28Cz_z4vcsVIF-@XY87imi$}*&neN+cHQ+u4>x;gbFZ1TxfAhZm_dj;C z>;Mn}!M7{RgTkhK%KT)%BC5PO5CZm*6YN27n39wG@xh5MZSRCR4h*gO0%CTuzyeP+ zLHOH)i_6k-=Gvt-X6nM$yF<**A*excUbeOVK;XXZeQR%DIT073g_wOnkkA7FMz+N) z_#kg+@Vr0UBHM4@g_zw!FaT-w)Fc^;rq>63cXGQ=X&Unx#Oy?ZuMiX?0`{-_GUj-N z?hoh0K2x(HW-k-Wvqu)vjqcH{MVEsH&B-qp=OLvmyOQ7`LdZ0q5w6L#4#WR0%n>G^ zae|l?J6KSJ5K`#9P2j)NHS_0BR_W>{T!xsfBM9yZz>Fcf61e`}Urhm$@m^IaXCY<{ z1Xo#$Q$rSv|MALd!`cQ($&EE=EiCKeXhAN5D!#_meRzIEh4gB|S#T{LVz!x}QV4*$ zx#=HM8!T{Zigh}a**+a&_98)q69D>FiRy=8!w=a%aGq#a?Q{)db~!;iLek;=7k-hQ zzHmPL)_dbF6;Fkj_333nF+ytLx$6x*!&Ra=A$2m}6Lt`@^#q~4(SOZM6;@|y`!vb& z2G3d*<_0lqCb)^P=GX0SlpNgra7Xdm*Bz2|CWu-0J{GJ)$d42Zh;fWNwySYj@VTO+ zp%Al23FLhN*chnNU)LX3RQcYT(7IuW3Sw48(15UGOjJwWDpPvYws9``q3)9*W_$Fr zAQfTfuv$rWSz7Pi@ymC+HPn9yFbQFV1FrpQZ0a(wd-#|7DDOt@bi8t@Mha4&x(2R&D$E^fk zN!$I!g%Go41RkycR4=KUR(ni$$fx~%=gd191rW2d32G3UwYqPFt&x{%GFGG%t-hNJ zF?)|dKM;W5{l0dbRGIxKJU2N)Fg3b2#B4Ev%OC*mS9|WSDQ+26GuY&Ea!pwy#B2z` z5rp_LBlrxU}-nG7b@VDK|)k`mQ1 zn&8AliCCl*i+vd}`t@?^Fk!J%Q0EB}sYoo<{%6ggCxiWAA}tRy+o*}v7@4TKCS#ml z>uL2FuPDZl^7Ihm{W^RV=71+UrQS@8L8sH`O^ET9q5%aO5L(H%vQQuk2{Q3#O@vkh z{+6i$m=~+Xj8yt~EC3CR!2`bNZ7hC^RvXcKAp73YBk;^Kv;}q4MiIt3OZY!s^}OXt z)l%&XuTQ{mgh&Po5HzckwU!ni$_zF>urVLuArR3eO4Sn{fA{$!t&D9cQmj<^Q9T() z^(3<9S0>kXtEnPmsh;Flp$C^j6_U89lRynspo&$v$i~s==iQBHv@*8oHq&E~p4B`@ zB4<{o%a@Y<>q1sPF@6owg)UKQ=GBiQIV9fMw&@c}#^|VfZBCcRj+M!fFU|Zpa|Tfw zW6&p?m?X8yq%rE5@r+)xgo)5-LsOF^A|y-X%ZRihuXj7Ih)ke39v%i{RWI<@o&NX9 z%Dej5p!VV8T9*<+nW_lnGadE< z&2hB)LOq>ZTif(x!}enl`mCrcDsf9D);4V?%K6=B%T~s`h#bLnAL~YXEBFjaE`bb1 zU_3`BQSjVGUP@191@gDgoSB^wO^-%;W?Na?wm{O_4gJDmmdW^SIR12v^u|teZ7N0g zGiSf@%;)wrues0B+NBF0->05`H(bc?-Rn)RB5zI~Yq