From 10568ab392207010ca0da4bbb8e4720e3c23d704 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Fri, 14 Jun 2024 20:35:03 +0800 Subject: [PATCH] `prefer-array-some`: Check `Array#{findIndex,findLastIndex}()` (#2370) Co-authored-by: Sindre Sorhus --- docs/rules/prefer-array-some.md | 24 +- readme.md | 2 +- rules/prefer-array-some.js | 71 ++++- test/prefer-array-some.mjs | 34 +++ test/snapshots/prefer-array-some.mjs.md | 336 ++++++++++++++++++++++ test/snapshots/prefer-array-some.mjs.snap | Bin 1121 -> 1589 bytes 6 files changed, 453 insertions(+), 14 deletions(-) diff --git a/docs/rules/prefer-array-some.md b/docs/rules/prefer-array-some.md index cc11083488..e7b1899483 100644 --- a/docs/rules/prefer-array-some.md +++ b/docs/rules/prefer-array-some.md @@ -1,4 +1,4 @@ -# Prefer `.some(…)` over `.filter(…).length` check and `.{find,findLast}(…)` +# Prefer `.some(…)` over `.filter(…).length` check and `.{find,findLast,findIndex,findLastIndex}(…)` 💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs-eslintconfigjs). @@ -17,7 +17,11 @@ We only check `.filter().length > 0` and `.filter().length !== 0`. These two non - Comparing the result of [`Array#find()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) or [`Array#findLast()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findLast) with `undefined`. -This rule is fixable for `.filter(…).length` check and has a suggestion for `.{find,findLast}(…)`. +- Using [`Array#findIndex()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findIndex) or [`Array#findLastIndex()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findLastIndex) to ensure at least one element in the array passes a given check. + +This rule is fixable for `.filter(…).length` checks and `.{findIndex,findLastIndex}(…)`. + +This rule provides a suggestion for `.{find,findLast}(…)`. ## Fail @@ -44,11 +48,11 @@ const foo = array.find(element => isUnicorn(element)) ? bar : baz; ``` ```js -const hasUnicorn = array.find(element => isUnicorn(element) !== undefined; +const hasUnicorn = array.find(element => isUnicorn(element)) !== undefined; ``` ```js -const hasUnicorn = array.find(element => isUnicorn(element) != null; +const hasUnicorn = array.find(element => isUnicorn(element)) != null; ``` ```js @@ -62,11 +66,19 @@ const foo = array.findLast(element => isUnicorn(element)) ? bar : baz; ``` ```js -const hasUnicorn = array.findLast(element => isUnicorn(element) !== undefined; +const hasUnicorn = array.findLast(element => isUnicorn(element)) !== undefined; +``` + +```js +const hasUnicorn = array.findLast(element => isUnicorn(element)) != null; +``` + +```js +const hasUnicorn = array.findIndex(element => isUnicorn(element)) !== -1; ``` ```js -const hasUnicorn = array.findLast(element => isUnicorn(element) != null; +const hasUnicorn = array.findLastIndex(element => isUnicorn(element)) !== -1; ``` ```vue diff --git a/readme.md b/readme.md index 9d715b4905..bdfcb60e9a 100644 --- a/readme.md +++ b/readme.md @@ -176,7 +176,7 @@ If you don't use the preset, ensure you use the same `env` and `parserOptions` c | [prefer-array-flat](docs/rules/prefer-array-flat.md) | Prefer `Array#flat()` over legacy techniques to flatten arrays. | ✅ | 🔧 | | | [prefer-array-flat-map](docs/rules/prefer-array-flat-map.md) | Prefer `.flatMap(…)` over `.map(…).flat()`. | ✅ | 🔧 | | | [prefer-array-index-of](docs/rules/prefer-array-index-of.md) | Prefer `Array#{indexOf,lastIndexOf}()` over `Array#{findIndex,findLastIndex}()` when looking for the index of an item. | ✅ | 🔧 | 💡 | -| [prefer-array-some](docs/rules/prefer-array-some.md) | Prefer `.some(…)` over `.filter(…).length` check and `.{find,findLast}(…)`. | ✅ | 🔧 | 💡 | +| [prefer-array-some](docs/rules/prefer-array-some.md) | Prefer `.some(…)` over `.filter(…).length` check and `.{find,findLast,findIndex,findLastIndex}(…)`. | ✅ | 🔧 | 💡 | | [prefer-at](docs/rules/prefer-at.md) | Prefer `.at()` method for index access and `String#charAt()`. | ✅ | 🔧 | 💡 | | [prefer-blob-reading-methods](docs/rules/prefer-blob-reading-methods.md) | Prefer `Blob#arrayBuffer()` over `FileReader#readAsArrayBuffer(…)` and `Blob#text()` over `FileReader#readAsText(…)`. | ✅ | | | | [prefer-code-point](docs/rules/prefer-code-point.md) | Prefer `String#codePointAt(…)` over `String#charCodeAt(…)` and `String.fromCodePoint(…)` over `String.fromCharCode(…)`. | ✅ | | 💡 | diff --git a/rules/prefer-array-some.js b/rules/prefer-array-some.js index 563dd39b79..3da1cf5409 100644 --- a/rules/prefer-array-some.js +++ b/rules/prefer-array-some.js @@ -40,10 +40,14 @@ const isCheckingUndefined = node => && isLiteral(node.parent.right, null) ) ); +const isNegativeOne = node => node.type === 'UnaryExpression' && node.operator === '-' && node.argument && node.argument.type === 'Literal' && node.argument.value === 1; +const isLiteralZero = node => isLiteral(node, 0); /** @param {import('eslint').Rule.RuleContext} context */ -const create = context => ({ - CallExpression(callExpression) { +const create = context => { + // `.find(…)` + // `.findLast(…)` + context.on('CallExpression', callExpression => { if (!isMethodCall(callExpression, { methods: ['find', 'findLast'], minimumArguments: 1, @@ -86,8 +90,61 @@ const create = context => ({ }, ], }; - }, - BinaryExpression(binaryExpression) { + }); + + // These operators also used in `prefer-includes`, try to reuse the code in future + // `.{findIndex,findLastIndex}(…) !== -1` + // `.{findIndex,findLastIndex}(…) != -1` + // `.{findIndex,findLastIndex}(…) > -1` + // `.{findIndex,findLastIndex}(…) === -1` + // `.{findIndex,findLastIndex}(…) == -1` + // `.{findIndex,findLastIndex}(…) >= 0` + // `.{findIndex,findLastIndex}(…) < 0` + context.on('BinaryExpression', binaryExpression => { + const {left, right, operator} = binaryExpression; + + if (!( + isMethodCall(left, { + methods: ['findIndex', 'findLastIndex'], + argumentsLength: 1, + optionalCall: false, + optionalMember: false, + }) + && ( + (['!==', '!=', '>', '===', '=='].includes(operator) && isNegativeOne(right)) + || (['>=', '<'].includes(operator) && isLiteralZero(right)) + ) + )) { + return; + } + + const methodNode = left.callee.property; + return { + node: methodNode, + messageId: ERROR_ID_ARRAY_SOME, + data: {method: methodNode.name}, + * fix(fixer) { + if (['===', '==', '<'].includes(operator)) { + yield fixer.insertTextBefore(binaryExpression, '!'); + } + + yield fixer.replaceText(methodNode, 'some'); + + const operatorToken = context.sourceCode.getTokenAfter( + left, + token => token.type === 'Punctuator' && token.value === operator, + ); + const [start] = operatorToken.range; + const [, end] = binaryExpression.range; + + yield fixer.removeRange([start, end]); + }, + }; + }); + + // `.filter(…).length > 0` + // `.filter(…).length !== 0` + context.on('BinaryExpression', binaryExpression => { if (!( // We assume the user already follows `unicorn/explicit-length-check`. These are allowed in that rule. (binaryExpression.operator === '>' || binaryExpression.operator === '!==') @@ -139,8 +196,8 @@ const create = context => ({ // The `BinaryExpression` always ends with a number or `)`, no need check for ASI }, }; - }, -}); + }); +}; /** @type {import('eslint').Rule.RuleModule} */ module.exports = { @@ -148,7 +205,7 @@ module.exports = { meta: { type: 'suggestion', docs: { - description: 'Prefer `.some(…)` over `.filter(…).length` check and `.{find,findLast}(…)`.', + description: 'Prefer `.some(…)` over `.filter(…).length` check and `.{find,findLast,findIndex,findLastIndex}(…)`.', recommended: true, }, fixable: 'code', diff --git a/test/prefer-array-some.mjs b/test/prefer-array-some.mjs index 8dadeabcd5..181cbf5143 100644 --- a/test/prefer-array-some.mjs +++ b/test/prefer-array-some.mjs @@ -210,6 +210,40 @@ test.snapshot({ ], }); +// `.{findIndex,findLastIndex}(…) !== -1` +// `.{findIndex,findLastIndex}(…) != -1` +// `.{findIndex,findLastIndex}(…) > -1` +// `.{findIndex,findLastIndex}(…) === -1` +// `.{findIndex,findLastIndex}(…) == -1` +// `.{findIndex,findLastIndex}(…) >= 0` +// `.{findIndex,findLastIndex}(…) < 0` +test.snapshot({ + valid: [ + 'foo.notMatchedMethod(bar) !== -1', + 'new foo.findIndex(bar) !== -1', + 'foo.findIndex(bar, extraArgument) !== -1', + 'foo.findIndex(bar) instanceof -1', + 'foo.findIndex(...bar) !== -1', + // We are not ignoring ``{_,lodash,underscore}.{findIndex,findLastIndex}` + // but it doesn't make sense to use them with one argument + '_.findIndex(bar)', + '_.findIndex(foo, bar)', + ], + invalid: [ + ...[ + 'foo.findIndex(bar) !== -1', + 'foo.findIndex(bar) != -1', + 'foo.findIndex(bar) > - 1', + 'foo.findIndex(bar) === -1', + 'foo.findIndex(bar) == - 1', + 'foo.findIndex(bar) >= 0', + 'foo.findIndex(bar) < 0', + ].flatMap(code => [code, code.replace('findIndex', 'findLastIndex')]), + 'foo.findIndex(bar) !== (( - 1 ))', + 'foo.findIndex(element => element.bar === 1) !== (( - 1 ))', + ], +}); + test.vue({ valid: [], invalid: [ diff --git a/test/snapshots/prefer-array-some.mjs.md b/test/snapshots/prefer-array-some.mjs.md index 4bc92a86e3..d0a5d9a363 100644 --- a/test/snapshots/prefer-array-some.mjs.md +++ b/test/snapshots/prefer-array-some.mjs.md @@ -184,6 +184,342 @@ Generated by [AVA](https://avajs.dev). 15 | );␊ ` +## invalid(1): foo.findIndex(bar) !== -1 + +> Input + + `␊ + 1 | foo.findIndex(bar) !== -1␊ + ` + +> Output + + `␊ + 1 | foo.some(bar) ␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.findIndex(bar) !== -1␊ + | ^^^^^^^^^ Prefer \`.some(…)\` over \`.findIndex(…)\`.␊ + ` + +## invalid(2): foo.findLastIndex(bar) !== -1 + +> Input + + `␊ + 1 | foo.findLastIndex(bar) !== -1␊ + ` + +> Output + + `␊ + 1 | foo.some(bar) ␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.findLastIndex(bar) !== -1␊ + | ^^^^^^^^^^^^^ Prefer \`.some(…)\` over \`.findLastIndex(…)\`.␊ + ` + +## invalid(3): foo.findIndex(bar) != -1 + +> Input + + `␊ + 1 | foo.findIndex(bar) != -1␊ + ` + +> Output + + `␊ + 1 | foo.some(bar) ␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.findIndex(bar) != -1␊ + | ^^^^^^^^^ Prefer \`.some(…)\` over \`.findIndex(…)\`.␊ + ` + +## invalid(4): foo.findLastIndex(bar) != -1 + +> Input + + `␊ + 1 | foo.findLastIndex(bar) != -1␊ + ` + +> Output + + `␊ + 1 | foo.some(bar) ␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.findLastIndex(bar) != -1␊ + | ^^^^^^^^^^^^^ Prefer \`.some(…)\` over \`.findLastIndex(…)\`.␊ + ` + +## invalid(5): foo.findIndex(bar) > - 1 + +> Input + + `␊ + 1 | foo.findIndex(bar) > - 1␊ + ` + +> Output + + `␊ + 1 | foo.some(bar) ␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.findIndex(bar) > - 1␊ + | ^^^^^^^^^ Prefer \`.some(…)\` over \`.findIndex(…)\`.␊ + ` + +## invalid(6): foo.findLastIndex(bar) > - 1 + +> Input + + `␊ + 1 | foo.findLastIndex(bar) > - 1␊ + ` + +> Output + + `␊ + 1 | foo.some(bar) ␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.findLastIndex(bar) > - 1␊ + | ^^^^^^^^^^^^^ Prefer \`.some(…)\` over \`.findLastIndex(…)\`.␊ + ` + +## invalid(7): foo.findIndex(bar) === -1 + +> Input + + `␊ + 1 | foo.findIndex(bar) === -1␊ + ` + +> Output + + `␊ + 1 | !foo.some(bar) ␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.findIndex(bar) === -1␊ + | ^^^^^^^^^ Prefer \`.some(…)\` over \`.findIndex(…)\`.␊ + ` + +## invalid(8): foo.findLastIndex(bar) === -1 + +> Input + + `␊ + 1 | foo.findLastIndex(bar) === -1␊ + ` + +> Output + + `␊ + 1 | !foo.some(bar) ␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.findLastIndex(bar) === -1␊ + | ^^^^^^^^^^^^^ Prefer \`.some(…)\` over \`.findLastIndex(…)\`.␊ + ` + +## invalid(9): foo.findIndex(bar) == - 1 + +> Input + + `␊ + 1 | foo.findIndex(bar) == - 1␊ + ` + +> Output + + `␊ + 1 | !foo.some(bar) ␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.findIndex(bar) == - 1␊ + | ^^^^^^^^^ Prefer \`.some(…)\` over \`.findIndex(…)\`.␊ + ` + +## invalid(10): foo.findLastIndex(bar) == - 1 + +> Input + + `␊ + 1 | foo.findLastIndex(bar) == - 1␊ + ` + +> Output + + `␊ + 1 | !foo.some(bar) ␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.findLastIndex(bar) == - 1␊ + | ^^^^^^^^^^^^^ Prefer \`.some(…)\` over \`.findLastIndex(…)\`.␊ + ` + +## invalid(11): foo.findIndex(bar) >= 0 + +> Input + + `␊ + 1 | foo.findIndex(bar) >= 0␊ + ` + +> Output + + `␊ + 1 | foo.some(bar) ␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.findIndex(bar) >= 0␊ + | ^^^^^^^^^ Prefer \`.some(…)\` over \`.findIndex(…)\`.␊ + ` + +## invalid(12): foo.findLastIndex(bar) >= 0 + +> Input + + `␊ + 1 | foo.findLastIndex(bar) >= 0␊ + ` + +> Output + + `␊ + 1 | foo.some(bar) ␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.findLastIndex(bar) >= 0␊ + | ^^^^^^^^^^^^^ Prefer \`.some(…)\` over \`.findLastIndex(…)\`.␊ + ` + +## invalid(13): foo.findIndex(bar) < 0 + +> Input + + `␊ + 1 | foo.findIndex(bar) < 0␊ + ` + +> Output + + `␊ + 1 | !foo.some(bar) ␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.findIndex(bar) < 0␊ + | ^^^^^^^^^ Prefer \`.some(…)\` over \`.findIndex(…)\`.␊ + ` + +## invalid(14): foo.findLastIndex(bar) < 0 + +> Input + + `␊ + 1 | foo.findLastIndex(bar) < 0␊ + ` + +> Output + + `␊ + 1 | !foo.some(bar) ␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.findLastIndex(bar) < 0␊ + | ^^^^^^^^^^^^^ Prefer \`.some(…)\` over \`.findLastIndex(…)\`.␊ + ` + +## invalid(15): foo.findIndex(bar) !== (( - 1 )) + +> Input + + `␊ + 1 | foo.findIndex(bar) !== (( - 1 ))␊ + ` + +> Output + + `␊ + 1 | foo.some(bar) ␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.findIndex(bar) !== (( - 1 ))␊ + | ^^^^^^^^^ Prefer \`.some(…)\` over \`.findIndex(…)\`.␊ + ` + +## invalid(16): foo.findIndex(element => element.bar === 1) !== (( - 1 )) + +> Input + + `␊ + 1 | foo.findIndex(element => element.bar === 1) !== (( - 1 ))␊ + ` + +> Output + + `␊ + 1 | foo.some(element => element.bar === 1) ␊ + ` + +> Error 1/1 + + `␊ + > 1 | foo.findIndex(element => element.bar === 1) !== (( - 1 ))␊ + | ^^^^^^^^^ Prefer \`.some(…)\` over \`.findIndex(…)\`.␊ + ` + ## invalid(1): foo.find(fn) == null > Input diff --git a/test/snapshots/prefer-array-some.mjs.snap b/test/snapshots/prefer-array-some.mjs.snap index 67bff2893c423ea1abaa10e1fe41a7c76c8b1de3..a7086be00a030019fdea346fc42c2dfaab80dd4a 100644 GIT binary patch literal 1589 zcmV-52Fm$CRzVvTv9)bEJA2;uuRJ8a>09KeX4;2_Wj0k^TLrTPs^PlEak*vMO_f+= zpV&Us>d-r^)u8m`heuzPG!0HaH9gxn@Y;^=eetl_G<;*@ol6tXPtafC^27^;6$LZo*0O&&eYcv`S_`oGC;=)$hbM}e)`)7}| zEpQIm6{eS6DJ#ls&Oha&&h9SpeABT(pVL?2LvmmlI|SK<2d3XfV=JeiW%mY#)_mJ_ z9hX{+sBkx;!o0Q$JC5!7&~hB8!NB0c4cInZScPrl40= zOAgk~vgabOC=JU1Y~lqWO{?!BS}lYXCNKag;BPX!rpNwAH#80A<^pK%wM`Gaw$rhi zaCiM4xa7cb{ZP*l)o{c{SK*6|MVNb*RKS_+^-bI_eKsLVzSHI4s|oMoL~p zN|@&@TMOk{>m5H9{2B?Kg#AKhJ=q8v4LNab$DTbQt^=X`PMhrPLCbaa8Bdt{kQX9g zwPb%5jjKolnuc}(IXBY>TBamdaKAx<-9F$3_QwqG$EiN0CjNq+rD+gpd(bxgO>#)w z%{Fm~#%_hiH4W-irb3VC-}Q7r9-~_P8|^pAauC!5l^q7vAUI};9-RdF9Gz9evtAWp z8A~s)62Y>T-4}R3pIHf>VFG$36bRj-mvnY1xXng5Ri}s12${$90vl#1_K~C)?<(6lO7?O-!Gu)&*XAp#6Hyr3oAs=G=oTl*X&XoR&-Y&))djp*0!g(y&x zrGG@`?--sx0Dk=bAytwqhNnny{|ttF`y3nx!7^6X2hZtb!{COK3uPI(0x~M*jiQYN zw`5ElB@xvKxN9;Za>1KN9!&S+Vcr0qAVI4i4adsDMLrAbFbl)5kjeiyEDTJKoP|{} z3kBe1VqyLOW@4cb#9Ud}>&r`g7S@LC%Qaz7nL#RE3RM*IY|OPWHRfe86GZ@KYRsbm zF;iz2134Ntu87#k&+80q6eaao*f`R-|Kf#g$vlM%Tm=okB&T2Q;2j~!PvrCy z0GKQLW_v&i<|uT+uBx;agQlh7tRjb0`QlAaPp&C_hy>g6HJo?|qoU@m9-`G0qUmq8 zW%`wtzIy3>p5n42#GW@@|K&{zzxl16q>&Tp?Owg1H5HhhDmXV z>Qxk1ufI_^9&^J{yinCETk%@t#9_GNRqU4`mrzLl5QXHTB}lf~RZYib*xFUhrL7b% zVC@v!%Xsqd~DlI0#u8kW|vYisFLg|=zCC+x%J1A`RXfw>Yv~xrH nE(+;Nw1xGY7C75hpXRuSC=>%2A_mC7jcM^OIs}aY2QL5sv;^_Y literal 1121 zcmV-n1fKgrRzVud7<1RVUE?nuytNy*^p&+`b z|IQg%Palg200000000B!Sj~>pRuoQ#qH4VBrmHSKhtm31B9qL(3=g4 zsub`f*AolRb!0okFa!y;T~w92>H}0|!M1Nu^$mCh#0$UzShm&uapD`>naoTYmBEUX z>z{M(J?Hz*cYOW3yXgn*JJBzXXzay4-EDeo$ML<6RVNL2E-WVuoqJW+V;zh7bepmm zO&h~uvx&-!-yZ*1Aq4jycoB1YQ8$RA?;mwKPV8)aexmeli67;YrT5C~22jV~;EoOI zJDb#}U$S05macD>FQkj?>MfYH%vwOY6qQkRCfU5$cmYCU`SnA}FNBd!R) z;!4#p&dm5QzU^;q(J1x;hWb*yf$OyAJ8i1Xi@RRj)z((cq*mOUI1 z2^E$}1MPrCG29?P6H_M_7qRJtXkgRXyO8C^hQ^JrQ~*bxDVHogjVw+=mKn@8&Yt7I zqHJ2GU{f9lSz7&?&}u~~jGO?O#D6O0^&;_~+#m!?OER@Lx?Y5+8}$7Su3Wu=koJNw zR(^K5hh4F{TE2;ToZvokbnA&mm%mD>>N0?346rDcZ2&9$WO3T%8^Bp^FgXeXIG1v( zMPmS~MIMIMYZ&m*4B$Kl$koqQlBAcVVM8s=*&r4c4{(a7ra;f{PrMZ+xgZd0%rPB6k^4cJ9@F zJTm-AGu%`CV$$sCM$l?aCN2xunLQc?Q1-|3Mi|s<%0Pw1FV)2*d0VV-(dNTsg(6~pK`c`a`65Z)E851NS_=M{YYGH^rb~bv z*GCa8rz6s6ezUA?e-_S{|KphK>0m|j%q;6@4s^*DDbmfw4FcICT}IGsg7tkr`)2ov zCOsc0&C0XQBk4aHpf5^FxpK`%g7qeE1xy~>n3!jQIGt`rgxw|4Jnn7k^&WaQ*+0_LN18yic>L}Y3jGL?K}_Jii62F>dNXx>(AhU#^892@fp z>SY$I*WVf(&vW50kCl2IOuWwJ;BYkYGWV;H6B;Bhiy%2}1#&R$TFv5eRP8cn(pJo4 zs9jS*^H~DT8sB`PDYr%G+z`=pEsKkZ-1l5Wg#{HfP`O8=O3nx2uQk~0WIK5%@41kE np+UNyd;~t^3^^Zu-(=iX7EGCjgaMP1JEFzEqD(x_NE!eDv+g01