prefer-includes: Check .lastIndexOf() (#2368)

Author: fisker

docs/rules/prefer-includes.md

@@ -1,4 +1,4 @@
-# Prefer `.includes()` over `.indexOf()` and `Array#some()` when checking for existence or non-existence
+# Prefer `.includes()` over `.indexOf()`, `.lastIndexOf()`, and `Array#some()` when checking for existence or non-existence
 
 💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs-eslintconfigjs).
 
@@ -7,7 +7,7 @@
 <!-- end auto-generated rule header -->
 <!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->
 
-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()` and `.lastIndexOf()`. Prefer `.includes()` over comparing the value of `.indexOf()` and `.lastIndexOf()`.
 
 [`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.
 
@@ -16,99 +16,103 @@ This rule is fixable, unless the search expression in `Array#some()` has side ef
 ## Fail
 
 ```js
-[].indexOf('foo') !== -1;
+array.indexOf('foo') !== -1;
 ```
 
 ```js
-x.indexOf('foo') != -1;
+array.indexOf('foo') !== -1;
 ```
 
 ```js
-str.indexOf('foo') > -1;
+string.lastIndexOf('foo') !== -1;
 ```
 
 ```js
-'foobar'.indexOf('foo') >= 0;
+array.lastIndexOf('foo') !== -1;
 ```
 
 ```js
-x.indexOf('foo') === -1
+foo.indexOf('foo') != -1;
 ```
 
 ```js
-const isFound = foo.some(x => x === 'foo');
+foo.indexOf('foo') >= 0;
 ```
 
 ```js
-const isFound = foo.some(x => 'foo' === x);
+foo.indexOf('foo') > -1;
 ```
 
 ```js
-const isFound = foo.some(x => {
-	return x === 'foo';
-});
+foo.indexOf('foo') === -1
 ```
 
-## Pass
+```js
+foo.some(x => x === 'foo');
+```
 
 ```js
-const str = 'foobar';
+foo.some(x => 'foo' === x);
 ```
 
 ```js
-str.indexOf('foo') !== -n;
+foo.some(x => {
+	return x === 'foo';
+});
 ```
 
+## Pass
+
 ```js
-str.indexOf('foo') !== 1;
+foo.indexOf('foo') !== -n;
 ```
 
 ```js
-!str.indexOf('foo') === 1;
+foo.indexOf('foo') !== 1;
 ```
 
 ```js
-!str.indexOf('foo') === -n;
+foo.indexOf('foo') === 1;
 ```
 
 ```js
-str.includes('foo');
+foo.includes('foo');
 ```
 
 ```js
-[1,2,3].includes(4);
+foo.includes(4);
 ```
 
 ```js
-const isFound = foo.includes('foo');
+foo.includes('foo');
 ```
 
 ```js
-const isFound = foo.some(x => x == undefined);
+foo.some(x => x == undefined);
 ```
 
 ```js
-const isFound = foo.some(x => x !== 'foo');
+foo.some(x => x !== 'foo');
 ```
 
 ```js
-const isFound = foo.some((x, index) => x === index);
+foo.some((x, index) => x === index);
 ```
 
 ```js
-const isFound = foo.some(x => (x === 'foo') && isValid());
+foo.some(x => (x === 'foo') && isValid());
 ```
 
 ```js
-const isFound = foo.some(x => y === 'foo');
+foo.some(x => y === 'foo');
 ```
 
 ```js
-const isFound = foo.some(x => y.x === 'foo');
+foo.some(x => y.x === 'foo');
 ```
 
 ```js
-const isFound = foo.some(x => {
+foo.some(x => {
 	const bar = getBar();
 	return x === bar;
 });

readme.md

@@ -188,7 +188,7 @@ If you don't use the preset, ensure you use the same `env` and `parserOptions` c
 | [prefer-dom-node-text-content](docs/rules/prefer-dom-node-text-content.md)                       | Prefer `.textContent` over `.innerText`.                                                                                                                                                                          | ✅  |    | 💡 |
 | [prefer-event-target](docs/rules/prefer-event-target.md)                                         | Prefer `EventTarget` over `EventEmitter`.                                                                                                                                                                         | ✅  |    |    |
 | [prefer-export-from](docs/rules/prefer-export-from.md)                                           | Prefer `export…from` when re-exporting.                                                                                                                                                                           | ✅  | 🔧 | 💡 |
-| [prefer-includes](docs/rules/prefer-includes.md)                                                 | Prefer `.includes()` over `.indexOf()` and `Array#some()` when checking for existence or non-existence.                                                                                                           | ✅  | 🔧 | 💡 |
+| [prefer-includes](docs/rules/prefer-includes.md)                                                 | Prefer `.includes()` over `.indexOf()`, `.lastIndexOf()`, and `Array#some()` when checking for existence or non-existence.                                                                                        | ✅  | 🔧 | 💡 |
 | [prefer-json-parse-buffer](docs/rules/prefer-json-parse-buffer.md)                               | Prefer reading a JSON file as a buffer.                                                                                                                                                                           |    | 🔧 |    |
 | [prefer-keyboard-event-key](docs/rules/prefer-keyboard-event-key.md)                             | Prefer `KeyboardEvent#key` over `KeyboardEvent#keyCode`.                                                                                                                                                          | ✅  | 🔧 |    |
 | [prefer-logical-operator-over-ternary](docs/rules/prefer-logical-operator-over-ternary.md)       | Prefer using a logical operator over a ternary.                                                                                                                                                                   | ✅  |    | 💡 |

rules/prefer-includes.js

@@ -5,9 +5,9 @@ const {isLiteral} = require('./ast/index.js');
 
 const MESSAGE_ID = 'prefer-includes';
 const messages = {
-	[MESSAGE_ID]: 'Use `.includes()`, rather than `.indexOf()`, when checking for existence.',
+	[MESSAGE_ID]: 'Use `.includes()`, rather than `.{{method}}()`, when checking for existence.',
 };
-// Ignore {_,lodash,underscore}.indexOf
+// Ignore `{_,lodash,underscore}.{indexOf,lastIndexOf}`
 const ignoredVariables = new Set(['_', 'lodash', 'underscore']);
 const isIgnoredTarget = node => node.type === 'Identifier' && ignoredVariables.has(node.name);
 const isNegativeOne = node => node.type === 'UnaryExpression' && node.operator === '-' && node.argument && node.argument.type === 'Literal' && node.argument.value === 1;
@@ -30,6 +30,9 @@ const getProblem = (context, node, target, argumentsNodes) => {
 	return {
 		node: memberExpressionNode.property,
 		messageId: MESSAGE_ID,
+		data: {
+			method: node.left.callee.property.name,
+		},
 		fix(fixer) {
 			const replacement = `${isNegativeResult(node) ? '!' : ''}${targetSource}.includes(${argumentsSource.join(', ')})`;
 			return fixer.replaceText(node, replacement);
@@ -49,7 +52,7 @@ const create = context => {
 	context.on('BinaryExpression', node => {
 		const {left, right, operator} = node;
 
-		if (!isMethodNamed(left, 'indexOf')) {
+		if (!isMethodNamed(left, 'indexOf') && !isMethodNamed(left, 'lastIndexOf')) {
 			return;
 		}
 
@@ -86,7 +89,7 @@ module.exports = {
 	meta: {
 		type: 'suggestion',
 		docs: {
-			description: 'Prefer `.includes()` over `.indexOf()` and `Array#some()` when checking for existence or non-existence.',
+			description: 'Prefer `.includes()` over `.indexOf()`, `.lastIndexOf()`, and `Array#some()` when checking for existence or non-existence.',
 			recommended: true,
 		},
 		fixable: 'code',

test/prefer-includes.mjs

@@ -5,20 +5,22 @@ const {test} = getTester(import.meta);
 
 test.snapshot({
 	valid: [
-		'str.indexOf(\'foo\') !== -n',
-		'str.indexOf(\'foo\') !== 1',
-		'str.indexOf(\'foo\') === -2',
-		'!str.indexOf(\'foo\') === 1',
-		'!str.indexOf(\'foo\') === -n',
+		...[
+			'str.indexOf(\'foo\') !== -n',
+			'str.indexOf(\'foo\') !== 1',
+			'str.indexOf(\'foo\') === -2',
+			'!str.indexOf(\'foo\') === 1',
+			'!str.indexOf(\'foo\') === -n',
+			'null.indexOf(\'foo\') !== 1',
+			'something.indexOf(foo, 0, another) !== -1',
+			'_.indexOf(foo, bar) !== -1',
+			'lodash.indexOf(foo, bar) !== -1',
+			'underscore.indexOf(foo, bar) !== -1',
+		].flatMap(code => [code, code.replace('.indexOf', '.lastIndexOf')]),
 		'str.includes(\'foo\')',
 		'\'foobar\'.includes(\'foo\')',
 		'[1,2,3].includes(4)',
-		'null.indexOf(\'foo\') !== 1',
 		'f(0) < 0',
-		'something.indexOf(foo, 0, another) !== -1',
-		'_.indexOf(foo, bar) !== -1',
-		'lodash.indexOf(foo, bar) !== -1',
-		'underscore.indexOf(foo, bar) !== -1',
 	],
 	invalid: [
 		'\'foobar\'.indexOf(\'foo\') !== -1',
@@ -32,7 +34,7 @@ test.snapshot({
 		'(a || b).indexOf(\'foo\') === -1',
 		'foo.indexOf(bar, 0) !== -1',
 		'foo.indexOf(bar, 1) !== -1',
-	],
+	].flatMap(code => [code, code.replace('.indexOf', '.lastIndexOf')]),
 });
 
 const {snapshot, typescript} = tests({