From b844dbca3d40ef4f41daf02d41d7be3096c7067c Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Sat, 19 Nov 2022 15:49:37 +0800 Subject: [PATCH] `prefer-string-replace-all`: Improve RegExp to string fix (#1971) --- package.json | 1 + rules/prefer-string-replace-all.js | 55 ++-- test/prefer-string-replace-all.mjs | 15 ++ .../prefer-string-replace-all.mjs.md | 234 +++++++++++++++++- .../prefer-string-replace-all.mjs.snap | Bin 1069 -> 1628 bytes 5 files changed, 273 insertions(+), 32 deletions(-) diff --git a/package.json b/package.json index dacbfded71..aee1b65c16 100644 --- a/package.json +++ b/package.json @@ -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" diff --git a/rules/prefer-string-replace-all.js b/rules/prefer-string-replace-all.js index 37caca86d5..09a22a617a 100644 --- a/rules/prefer-string-replace-all.js +++ b/rules/prefer-string-replace-all.js @@ -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'); @@ -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)) { @@ -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) { @@ -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); }, }; }, diff --git a/test/prefer-string-replace-all.mjs b/test/prefer-string-replace-all.mjs index b0a0ffa423..508a672a16 100644 --- a/test/prefer-string-replace-all.mjs +++ b/test/prefer-string-replace-all.mjs @@ -68,6 +68,7 @@ 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)', @@ -75,5 +76,19 @@ test.snapshot({ // 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, _)', ], }); diff --git a/test/snapshots/prefer-string-replace-all.mjs.md b/test/snapshots/prefer-string-replace-all.mjs.md index 32a94728f2..a10963392c 100644 --- a/test/snapshots/prefer-string-replace-all.mjs.md +++ b/test/snapshots/prefer-string-replace-all.mjs.md @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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()\`.␊ + ` diff --git a/test/snapshots/prefer-string-replace-all.mjs.snap b/test/snapshots/prefer-string-replace-all.mjs.snap index 903934852e68d07e9979881270c3e406406b5975..2acc7296e3e0e290bff534022038dbfa90de658d 100644 GIT binary patch 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~YqW^=6u6Ad`RsSx;~$F%00000000A9 z$Hc%O!oYZ-WsRM?Tbsg7)~19tFU*00Kwt>P%Ym59t+%ggn}hIgD|Zf2*S%|*z@pBK z5UdQ;!1?ss-TKPjV3x{jb-C`7rJ2B@flz!5hz08Bp5J7?egWU<_m7Wf2BkBBMZZCD zHZuc*g2C)k#>jq2$B>V|%mfyl4aGt%K$}tvispEow>x-pf%eKug-J|cQ3ojA z0>rABAFkdx#5{M){-`Onw{NsCfkpXPA@~{)8|hD%PWm+Gp`BRZlr`%8K$n6<+1Mbs z8i-vywdF*LPqWGXn$^o$e36R@EV>ek<=7b*ydMVcxL2OHK(9nut+v3rkO?g62gQeh z*heCF3De{+-*1~}*lO-AyTk+*{RqWr91IMdj~1vb**5d%!g-d@*@|;Zn82dbpqQJJ zfnlL(!n^+VlIn!{Ho{X}rJb3;qLxs+4u}`YoYv@BUCFa7t8tms)$>tIVA1(eu{T673(Eunu2E_l41Z5b=Ivu)^^J2IC+OADZVA1bToXf|+ zz|6o7PMM6X41$cxTnY+?3N;F8`T2T9sRcQS$*GPxIU4FpG3x3%3hF>olM6*d2A3VW z1`T~Bb^UahY6T#u0TBvu5TFoHl$w@Wq>vC?Qk0pOu8d@jW`aU~8Kx9aTo1)ord|ew zJN2~`lJj$OQ}arIB3k-D2_v8cCzA7#R2TtOXaJQMgOqS0Nf<-4nt;XC6V(;;_2F_R zKnYW@gbt8p24*EC76IAjAU0}BLQ?OOYOD$IjN=x<^SzK`C8H z(;D3;Pz_jo!?cqD>K!tSMl%=Tb7BD<@`Iih3Hbp@FyhgEt)0OKIsu9IZ9|#F`}R0g1J>hOC+<&Q!w{-KEuL zhPBvTnvFHeQ8myvO!Z4MNeELU)iiOYF?MHSub8koy8?@|@#?0DH`ECvjZ$n*&%u{A n1|-8pC&rSH;gD6+ISr?mVy&ofq+x_=YB>`CPCW