Skip to content

Commit

Permalink
Add prefer-array-index-of rule (#920)
Browse files Browse the repository at this point in the history
Co-authored-by: fisker <lionkay@gmail.com>
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
  • Loading branch information
3 people committed Jan 11, 2021
1 parent 7b74b40 commit 517a782
Show file tree
Hide file tree
Showing 7 changed files with 600 additions and 0 deletions.
58 changes: 58 additions & 0 deletions docs/rules/prefer-array-index-of.md
@@ -0,0 +1,58 @@
# Prefer `Array#indexOf()` over `Array#findIndex()` when looking for the index of an item

[`Array#findIndex()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findIndex) is intended for more complex needs. If you are just looking for the index where the given item is present, then the code can be simplified to use [`Array#indexOf()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/indexOf). 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 has side effects.

## Fail

```js
const index = foo.findIndex(x => x === 'foo');
```

```js
const index = foo.findIndex(x => 'foo' === x);
```

```js
const index = foo.findIndex(x => {
return x === 'foo';
});
```

## Pass

```js
const index = foo.indexOf('foo');
```

```js
const index = foo.findIndex(x => x == undefined);
```

```js
const index = foo.findIndex(x => x !== 'foo');
```

```js
const index = foo.findIndex((x, index) => x === index);
```

```js
const index = foo.findIndex(x => (x === 'foo') && isValid());
```

```js
const index = foo.findIndex(x => y === 'foo');
```

```js
const index = foo.findIndex(x => y.x === 'foo');
```

```js
const index = foo.findIndex(x => {
const bar = getBar();
return x === bar;
});
```
1 change: 1 addition & 0 deletions index.js
Expand Up @@ -81,6 +81,7 @@ module.exports = {
'unicorn/prefer-array-find': 'error',
// TODO: Enable this by default when targeting Node.js 12.
'unicorn/prefer-array-flat-map': 'off',
'unicorn/prefer-array-index-of': 'error',
'unicorn/prefer-array-some': 'error',
'unicorn/prefer-date-now': 'error',
'unicorn/prefer-default-parameters': 'error',
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Expand Up @@ -73,6 +73,7 @@ Configure it in `package.json`.
"unicorn/prefer-add-event-listener": "error",
"unicorn/prefer-array-find": "error",
"unicorn/prefer-array-flat-map": "error",
"unicorn/prefer-array-index-of": "error",
"unicorn/prefer-array-some": "error",
"unicorn/prefer-date-now": "error",
"unicorn/prefer-default-parameters": "error",
Expand Down Expand Up @@ -147,6 +148,7 @@ Configure it in `package.json`.
- [prefer-add-event-listener](docs/rules/prefer-add-event-listener.md) - Prefer `.addEventListener()` and `.removeEventListener()` over `on`-functions. *(partly fixable)*
- [prefer-array-find](docs/rules/prefer-array-find.md) - Prefer `.find(…)` over the first element from `.filter(…)`. *(partly fixable)*
- [prefer-array-flat-map](docs/rules/prefer-array-flat-map.md) - Prefer `.flatMap(…)` over `.map(…).flat()`. *(fixable)*
- [prefer-array-index-of](docs/rules/prefer-array-index-of.md) - Prefer `Array#indexOf()` over `Array#findIndex()` when looking for the index of an item. *(partly fixable)*
- [prefer-array-some](docs/rules/prefer-array-some.md) - Prefer `.some(…)` over `.find(…)`.
- [prefer-date-now](docs/rules/prefer-date-now.md) - Prefer `Date.now()` to get the number of milliseconds since the Unix Epoch. *(fixable)*
- [prefer-default-parameters](docs/rules/prefer-default-parameters.md) - Prefer default parameters over reassignment. *(fixable)*
Expand Down
156 changes: 156 additions & 0 deletions rules/prefer-array-index-of.js
@@ -0,0 +1,156 @@
'use strict';
const {hasSideEffect, isParenthesized, findVariable} = require('eslint-utils');
const getDocumentationUrl = require('./utils/get-documentation-url');
const methodSelector = require('./utils/method-selector');
const getVariableIdentifiers = require('./utils/get-variable-identifiers');

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;

function isVariablesInCallbackUsed(scopeManager, callback, parameterInBinaryExpression) {
const scope = scopeManager.acquire(callback);

// `parameter` is used on somewhere else
const [parameter] = callback.params;
if (
getVariableIdentifiers(findVariable(scope, parameter))
.some(identifier => identifier !== parameter && identifier !== parameterInBinaryExpression)
) {
return true;
}

if (callback.type === 'FunctionExpression') {
// `this` is used
if (scope.thisFound) {
return true;
}

// The function name is used
if (
callback.id &&
getVariableIdentifiers(findVariable(scope, callback.id))
.some(identifier => identifier !== callback.id)
) {
return true;
}

// `arguments` is used
if (scope.references.some(({identifier: {name}}) => name === 'arguments')) {
return true;
}
}
}

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;
}

if (isVariablesInCallbackUsed(scopeManager, callback, parameterInBinaryExpression)) {
return;
}

const method = node.callee.property;
const problem = {
node: method,
messageId: MESSAGE_ID_FIND_INDEX,
suggest: []
};

function * fix(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);
}
};
};

module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
url: getDocumentationUrl(__filename)
},
fixable: 'code',
messages
}
};

0 comments on commit 517a782

Please sign in to comment.