Skip to content

Commit

Permalink
prefer-includes: Check .lastIndexOf() (#2368)
Browse files Browse the repository at this point in the history
  • Loading branch information
@fisker
fisker authored May 25, 2024
1 parent a449af9 commit d812ad1
Show file tree
Hide file tree
Showing 6 changed files with 294 additions and 54 deletions.
60 changes: 32 additions & 28 deletions docs/rules/prefer-includes.md
View file
Original file line number Diff line number Diff line change
@@ -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).

Expand All @@ -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.

Expand All @@ -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;
});
Expand Down
2 changes: 1 addition & 1 deletion readme.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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. | ✅ | | 💡 |
Expand Down
11 changes: 7 additions & 4 deletions rules/prefer-includes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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;
}

Expand Down Expand Up @@ -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',
Expand Down
24 changes: 13 additions & 11 deletions test/prefer-includes.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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({
Expand Down
Loading

0 comments on commit d812ad1

Please sign in to comment.