prefer-includes: Add Array#some() check (#1097)

Co-authored-by: Fabrice TIERCELIN <fabrice.tiercelin@gmail.com>

docs/rules/prefer-includes.md

@@ -1,29 +1,108 @@
-# Prefer `.includes()` over `.indexOf()` when checking for existence or non-existence
+# Prefer `.includes()` over `.indexOf()` and `Array#some()` when checking for existence or non-existence
 
-All built-ins have `.includes()` in addition to `.indexOf()`. Prefer `.includes()` over comparing the value of `.indexOf()`
+All built-ins have `.includes()` in addition to `.indexOf()`. Prefer `.includes()` over comparing the value of `.indexOf()`.
 
-This rule is fixable.
+[`Array#some()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some) is intended for more complex needs. If you are just looking for the index where the given item is present, the code can be simplified to use [`Array#includes()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/includes). This applies to any search with a literal, a variable, or any expression that doesn't have any explicit side effects. However, if the expression you are looking for relies on an item related to the function (its arguments, the function self, etc.), the case is still valid.
 
+This rule is fixable, unless the search expression in `Array#some()` has side effects.
 
 ## Fail
 
 ```js
 [].indexOf('foo') !== -1;
+```
+
+```js
 x.indexOf('foo') != -1;
+```
+
+```js
 str.indexOf('foo') > -1;
+```
+
+```js
 'foobar'.indexOf('foo') >= 0;
+```
+
+```js
 x.indexOf('foo') === -1
 ```
 
+```js
+const isFound = foo.some(x => x === 'foo');
+```
+
+```js
+const isFound = foo.some(x => 'foo' === x);
+```
+
+```js
+const isFound = foo.some(x => {
+	return x === 'foo';
+});
+```
 
 ## Pass
 
 ```js
 const str = 'foobar';
+```
+
+```js
 str.indexOf('foo') !== -n;
+```
+
+```js
 str.indexOf('foo') !== 1;
+```
+
+```js
 !str.indexOf('foo') === 1;
+```
+
+```js
 !str.indexOf('foo') === -n;
+```
+
+```js
 str.includes('foo');
+```
+
+```js
 [1,2,3].includes(4);
 ```
+
+```js
+const isFound = foo.includes('foo');
+```
+
+```js
+const isFound = foo.some(x => x == undefined);
+```
+
+```js
+const isFound = foo.some(x => x !== 'foo');
+```
+
+```js
+const isFound = foo.some((x, index) => x === index);
+```
+
+```js
+const isFound = foo.some(x => (x === 'foo') && isValid());
+```
+
+```js
+const isFound = foo.some(x => y === 'foo');
+```
+
+```js
+const isFound = foo.some(x => y.x === 'foo');
+```
+
+```js
+const isFound = foo.some(x => {
+	const bar = getBar();
+	return x === bar;
+});
+```

readme.md

@@ -162,7 +162,7 @@ Configure it in `package.json`.
 - [prefer-dom-node-dataset](docs/rules/prefer-dom-node-dataset.md) - Prefer using `.dataset` on DOM elements over `.setAttribute(…)`. *(fixable)*
 - [prefer-dom-node-remove](docs/rules/prefer-dom-node-remove.md) - Prefer `childNode.remove()` over `parentNode.removeChild(childNode)`. *(fixable)*
 - [prefer-dom-node-text-content](docs/rules/prefer-dom-node-text-content.md) - Prefer `.textContent` over `.innerText`. *(fixable)*
-- [prefer-includes](docs/rules/prefer-includes.md) - Prefer `.includes()` over `.indexOf()` when checking for existence or non-existence. *(fixable)*
+- [prefer-includes](docs/rules/prefer-includes.md) - Prefer `.includes()` over `.indexOf()` and `Array#some()` when checking for existence or non-existence. *(partly fixable)*
 - [prefer-keyboard-event-key](docs/rules/prefer-keyboard-event-key.md) - Prefer `KeyboardEvent#key` over `KeyboardEvent#keyCode`. *(partly fixable)*
 - [prefer-math-trunc](docs/rules/prefer-math-trunc.md) - Enforce the use of `Math.trunc` instead of bitwise operators. *(partly fixable)*
 - [prefer-modern-dom-apis](docs/rules/prefer-modern-dom-apis.md) - Prefer `.before()` over `.insertBefore()`, `.replaceWith()` over `.replaceChild()`, prefer one of `.before()`, `.after()`, `.append()` or `.prepend()` over `insertAdjacentText()` and `insertAdjacentElement()`. *(fixable)*

rules/prefer-array-index-of.js

@@ -1,118 +1,13 @@
 'use strict';
-const {hasSideEffect, isParenthesized, findVariable} = require('eslint-utils');
 const getDocumentationUrl = require('./utils/get-documentation-url');
-const methodSelector = require('./utils/method-selector');
-const isFunctionSelfUsedInside = require('./utils/is-function-self-used-inside');
+const simpleArraySearchRule = require('./shared/simple-array-search-rule');
 
-const MESSAGE_ID_FIND_INDEX = 'findIndex';
-const MESSAGE_ID_REPLACE = 'replaceFindIndex';
-const messages = {
-	[MESSAGE_ID_FIND_INDEX]: 'Use `.indexOf()` instead of `.findIndex()` when looking for the index of an item.',
-	[MESSAGE_ID_REPLACE]: 'Replace `.findIndex()` with `.indexOf()`.'
-};
-
-const getBinaryExpressionSelector = path => [
-	`[${path}.type="BinaryExpression"]`,
-	`[${path}.operator="==="]`,
-	`:matches([${path}.left.type="Identifier"], [${path}.right.type="Identifier"])`
-].join('');
-const getFunctionSelector = path => [
-	`[${path}.generator=false]`,
-	`[${path}.async=false]`,
-	`[${path}.params.length=1]`,
-	`[${path}.params.0.type="Identifier"]`
-].join('');
-const selector = [
-	methodSelector({
-		name: 'findIndex',
-		length: 1
-	}),
-	`:matches(${
-		[
-			// Matches `foo.findIndex(bar => bar === baz)`
-			[
-				'[arguments.0.type="ArrowFunctionExpression"]',
-				getFunctionSelector('arguments.0'),
-				getBinaryExpressionSelector('arguments.0.body')
-			].join(''),
-			// Matches `foo.findIndex(bar => {return bar === baz})`
-			// Matches `foo.findIndex(function (bar) {return bar === baz})`
-			[
-				':matches([arguments.0.type="ArrowFunctionExpression"], [arguments.0.type="FunctionExpression"])',
-				getFunctionSelector('arguments.0'),
-				'[arguments.0.body.type="BlockStatement"]',
-				'[arguments.0.body.body.length=1]',
-				'[arguments.0.body.body.0.type="ReturnStatement"]',
-				getBinaryExpressionSelector('arguments.0.body.body.0.argument')
-			].join('')
-		].join(', ')
-	})`
-].join('');
-
-const isIdentifierNamed = ({type, name}, expectName) => type === 'Identifier' && name === expectName;
-
-const create = context => {
-	const sourceCode = context.getSourceCode();
-	const {scopeManager} = sourceCode;
-
-	return {
-		[selector](node) {
-			const [callback] = node.arguments;
-			const binaryExpression = callback.body.type === 'BinaryExpression' ?
-				callback.body :
-				callback.body.body[0].argument;
-			const [parameter] = callback.params;
-			const {left, right} = binaryExpression;
-			const {name} = parameter;
-
-			let searchValueNode;
-			let parameterInBinaryExpression;
-			if (isIdentifierNamed(left, name)) {
-				searchValueNode = right;
-				parameterInBinaryExpression = left;
-			} else if (isIdentifierNamed(right, name)) {
-				searchValueNode = left;
-				parameterInBinaryExpression = right;
-			} else {
-				return;
-			}
+const {messages, createListeners} = simpleArraySearchRule({
+	method: 'findIndex',
+	replacement: 'indexOf'
+});
 
-			const callbackScope = scopeManager.acquire(callback);
-			if (
-				// `parameter` is used somewhere else
-				findVariable(callbackScope, parameter).references.some(({identifier}) => identifier !== parameterInBinaryExpression) ||
-				isFunctionSelfUsedInside(callback, callbackScope)
-			) {
-				return;
-			}
-
-			const method = node.callee.property;
-			const problem = {
-				node: method,
-				messageId: MESSAGE_ID_FIND_INDEX,
-				suggest: []
-			};
-
-			const fix = function * (fixer) {
-				let text = sourceCode.getText(searchValueNode);
-				if (isParenthesized(searchValueNode, sourceCode) && !isParenthesized(callback, sourceCode)) {
-					text = `(${text})`;
-				}
-
-				yield fixer.replaceText(method, 'indexOf');
-				yield fixer.replaceText(callback, text);
-			};
-
-			if (hasSideEffect(searchValueNode, sourceCode)) {
-				problem.suggest.push({messageId: MESSAGE_ID_REPLACE, fix});
-			} else {
-				problem.fix = fix;
-			}
-
-			context.report(problem);
-		}
-	};
-};
+const create = context => createListeners(context);
 
 module.exports = {
 	create,

rules/prefer-includes.js

@@ -2,6 +2,7 @@
 const getDocumentationUrl = require('./utils/get-documentation-url');
 const isMethodNamed = require('./utils/is-method-named');
 const isLiteralValue = require('./utils/is-literal-value');
+const simpleArraySearchRule = require('./shared/simple-array-search-rule');
 
 const MESSAGE_ID = 'prefer-includes';
 const messages = {
@@ -37,6 +38,11 @@ const report = (context, node, target, argumentsNodes) => {
 	});
 };
 
+const includesOverSomeRule = simpleArraySearchRule({
+	method: 'some',
+	replacement: 'includes'
+});
+
 const create = context => ({
 	BinaryExpression: node => {
 		const {left, right, operator} = node;
@@ -69,7 +75,8 @@ const create = context => ({
 				argumentsNodes
 			);
 		}
-	}
+	},
+	...includesOverSomeRule.createListeners(context)
 });
 
 module.exports = {
@@ -80,6 +87,9 @@ module.exports = {
 			url: getDocumentationUrl(__filename)
 		},
 		fixable: 'code',
-		messages
+		messages: {
+			...messages,
+			...includesOverSomeRule.messages
+		}
 	}
 };