Skip to content

Commit 10568ab

Browse files
fiskersindresorhus
fisker
and
sindresorhus
authored
prefer-array-some: Check Array#{findIndex,findLastIndex}() (#2370)
Co-authored-by: Sindre Sorhus <[email protected]>
1 parent ac8536e commit 10568ab

File tree

6 files changed

+453
-14
lines changed

6 files changed

+453
-14
lines changed

‎docs/rules/prefer-array-some.md

+18-6
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Prefer `.some(…)` over `.filter(…).length` check and `.{find,findLast}(…)`
1+
# Prefer `.some(…)` over `.filter(…).length` check and `.{find,findLast,findIndex,findLastIndex}(…)`
22

33
💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs-eslintconfigjs).
44

@@ -17,7 +17,11 @@ We only check `.filter().length > 0` and `.filter().length !== 0`. These two non
1717

1818
- 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`.
1919

20-
This rule is fixable for `.filter(…).length` check and has a suggestion for `.{find,findLast}(…)`.
20+
- 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.
21+
22+
This rule is fixable for `.filter(…).length` checks and `.{findIndex,findLastIndex}(…)`.
23+
24+
This rule provides a suggestion for `.{find,findLast}(…)`.
2125

2226
## Fail
2327

@@ -44,11 +48,11 @@ const foo = array.find(element => isUnicorn(element)) ? bar : baz;
4448
```
4549

4650
```js
47-
const hasUnicorn = array.find(element => isUnicorn(element) !== undefined;
51+
const hasUnicorn = array.find(element => isUnicorn(element)) !== undefined;
4852
```
4953

5054
```js
51-
const hasUnicorn = array.find(element => isUnicorn(element) != null;
55+
const hasUnicorn = array.find(element => isUnicorn(element)) != null;
5256
```
5357

5458
```js
@@ -62,11 +66,19 @@ const foo = array.findLast(element => isUnicorn(element)) ? bar : baz;
6266
```
6367

6468
```js
65-
const hasUnicorn = array.findLast(element => isUnicorn(element) !== undefined;
69+
const hasUnicorn = array.findLast(element => isUnicorn(element)) !== undefined;
70+
```
71+
72+
```js
73+
const hasUnicorn = array.findLast(element => isUnicorn(element)) != null;
74+
```
75+
76+
```js
77+
const hasUnicorn = array.findIndex(element => isUnicorn(element)) !== -1;
6678
```
6779

6880
```js
69-
const hasUnicorn = array.findLast(element => isUnicorn(element) != null;
81+
const hasUnicorn = array.findLastIndex(element => isUnicorn(element)) !== -1;
7082
```
7183

7284
```vue

‎readme.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ If you don't use the preset, ensure you use the same `env` and `parserOptions` c
176176
| [prefer-array-flat](docs/rules/prefer-array-flat.md) | Prefer `Array#flat()` over legacy techniques to flatten arrays. | ✅ | 🔧 | |
177177
| [prefer-array-flat-map](docs/rules/prefer-array-flat-map.md) | Prefer `.flatMap(…)` over `.map(…).flat()`. | ✅ | 🔧 | |
178178
| [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. | ✅ | 🔧 | 💡 |
179-
| [prefer-array-some](docs/rules/prefer-array-some.md) | Prefer `.some(…)` over `.filter(…).length` check and `.{find,findLast}(…)`. | ✅ | 🔧 | 💡 |
179+
| [prefer-array-some](docs/rules/prefer-array-some.md) | Prefer `.some(…)` over `.filter(…).length` check and `.{find,findLast,findIndex,findLastIndex}(…)`. | ✅ | 🔧 | 💡 |
180180
| [prefer-at](docs/rules/prefer-at.md) | Prefer `.at()` method for index access and `String#charAt()`. | ✅ | 🔧 | 💡 |
181181
| [prefer-blob-reading-methods](docs/rules/prefer-blob-reading-methods.md) | Prefer `Blob#arrayBuffer()` over `FileReader#readAsArrayBuffer(…)` and `Blob#text()` over `FileReader#readAsText(…)`. | ✅ | | |
182182
| [prefer-code-point](docs/rules/prefer-code-point.md) | Prefer `String#codePointAt(…)` over `String#charCodeAt(…)` and `String.fromCodePoint(…)` over `String.fromCharCode(…)`. | ✅ | | 💡 |

‎rules/prefer-array-some.js

+64-7
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,14 @@ const isCheckingUndefined = node =>
4040
&& isLiteral(node.parent.right, null)
4141
)
4242
);
43+
const isNegativeOne = node => node.type === 'UnaryExpression' && node.operator === '-' && node.argument && node.argument.type === 'Literal' && node.argument.value === 1;
44+
const isLiteralZero = node => isLiteral(node, 0);
4345

4446
/** @param {import('eslint').Rule.RuleContext} context */
45-
const create = context => ({
46-
CallExpression(callExpression) {
47+
const create = context => {
48+
// `.find(…)`
49+
// `.findLast(…)`
50+
context.on('CallExpression', callExpression => {
4751
if (!isMethodCall(callExpression, {
4852
methods: ['find', 'findLast'],
4953
minimumArguments: 1,
@@ -86,8 +90,61 @@ const create = context => ({
8690
},
8791
],
8892
};
89-
},
90-
BinaryExpression(binaryExpression) {
93+
});
94+
95+
// These operators also used in `prefer-includes`, try to reuse the code in future
96+
// `.{findIndex,findLastIndex}(…) !== -1`
97+
// `.{findIndex,findLastIndex}(…) != -1`
98+
// `.{findIndex,findLastIndex}(…) > -1`
99+
// `.{findIndex,findLastIndex}(…) === -1`
100+
// `.{findIndex,findLastIndex}(…) == -1`
101+
// `.{findIndex,findLastIndex}(…) >= 0`
102+
// `.{findIndex,findLastIndex}(…) < 0`
103+
context.on('BinaryExpression', binaryExpression => {
104+
const {left, right, operator} = binaryExpression;
105+
106+
if (!(
107+
isMethodCall(left, {
108+
methods: ['findIndex', 'findLastIndex'],
109+
argumentsLength: 1,
110+
optionalCall: false,
111+
optionalMember: false,
112+
})
113+
&& (
114+
(['!==', '!=', '>', '===', '=='].includes(operator) && isNegativeOne(right))
115+
|| (['>=', '<'].includes(operator) && isLiteralZero(right))
116+
)
117+
)) {
118+
return;
119+
}
120+
121+
const methodNode = left.callee.property;
122+
return {
123+
node: methodNode,
124+
messageId: ERROR_ID_ARRAY_SOME,
125+
data: {method: methodNode.name},
126+
* fix(fixer) {
127+
if (['===', '==', '<'].includes(operator)) {
128+
yield fixer.insertTextBefore(binaryExpression, '!');
129+
}
130+
131+
yield fixer.replaceText(methodNode, 'some');
132+
133+
const operatorToken = context.sourceCode.getTokenAfter(
134+
left,
135+
token => token.type === 'Punctuator' && token.value === operator,
136+
);
137+
const [start] = operatorToken.range;
138+
const [, end] = binaryExpression.range;
139+
140+
yield fixer.removeRange([start, end]);
141+
},
142+
};
143+
});
144+
145+
// `.filter(…).length > 0`
146+
// `.filter(…).length !== 0`
147+
context.on('BinaryExpression', binaryExpression => {
91148
if (!(
92149
// We assume the user already follows `unicorn/explicit-length-check`. These are allowed in that rule.
93150
(binaryExpression.operator === '>' || binaryExpression.operator === '!==')
@@ -139,16 +196,16 @@ const create = context => ({
139196
// The `BinaryExpression` always ends with a number or `)`, no need check for ASI
140197
},
141198
};
142-
},
143-
});
199+
});
200+
};
144201

145202
/** @type {import('eslint').Rule.RuleModule} */
146203
module.exports = {
147204
create: checkVueTemplate(create),
148205
meta: {
149206
type: 'suggestion',
150207
docs: {
151-
description: 'Prefer `.some(…)` over `.filter(…).length` check and `.{find,findLast}(…)`.',
208+
description: 'Prefer `.some(…)` over `.filter(…).length` check and `.{find,findLast,findIndex,findLastIndex}(…)`.',
152209
recommended: true,
153210
},
154211
fixable: 'code',

‎test/prefer-array-some.mjs

+34
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,40 @@ test.snapshot({
210210
],
211211
});
212212

213+
// `.{findIndex,findLastIndex}(…) !== -1`
214+
// `.{findIndex,findLastIndex}(…) != -1`
215+
// `.{findIndex,findLastIndex}(…) > -1`
216+
// `.{findIndex,findLastIndex}(…) === -1`
217+
// `.{findIndex,findLastIndex}(…) == -1`
218+
// `.{findIndex,findLastIndex}(…) >= 0`
219+
// `.{findIndex,findLastIndex}(…) < 0`
220+
test.snapshot({
221+
valid: [
222+
'foo.notMatchedMethod(bar) !== -1',
223+
'new foo.findIndex(bar) !== -1',
224+
'foo.findIndex(bar, extraArgument) !== -1',
225+
'foo.findIndex(bar) instanceof -1',
226+
'foo.findIndex(...bar) !== -1',
227+
// We are not ignoring ``{_,lodash,underscore}.{findIndex,findLastIndex}`
228+
// but it doesn't make sense to use them with one argument
229+
'_.findIndex(bar)',
230+
'_.findIndex(foo, bar)',
231+
],
232+
invalid: [
233+
...[
234+
'foo.findIndex(bar) !== -1',
235+
'foo.findIndex(bar) != -1',
236+
'foo.findIndex(bar) > - 1',
237+
'foo.findIndex(bar) === -1',
238+
'foo.findIndex(bar) == - 1',
239+
'foo.findIndex(bar) >= 0',
240+
'foo.findIndex(bar) < 0',
241+
].flatMap(code => [code, code.replace('findIndex', 'findLastIndex')]),
242+
'foo.findIndex(bar) !== (( - 1 ))',
243+
'foo.findIndex(element => element.bar === 1) !== (( - 1 ))',
244+
],
245+
});
246+
213247
test.vue({
214248
valid: [],
215249
invalid: [

0 commit comments

Comments
 (0)