prefer-array-some: Check Array#{findIndex,findLastIndex}() (#2370)

Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
sindresorhus · Jun 14, 2024 · 10568ab · 10568ab
1 parent ac8536e
commit 10568ab
Show file tree

Hide file tree

Showing 6 changed files with 453 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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,16 +196,16 @@ const create = context => ({
 				// The `BinaryExpression` always ends with a number or `)`, no need check for ASI
 			},
 		};
-	},
-});
+	});
+};
 
 /** @type {import('eslint').Rule.RuleModule} */
 module.exports = {
 	create: checkVueTemplate(create),
 	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
@@ -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: [