Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add prefer-array-includes rule #1062

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
58 changes: 58 additions & 0 deletions docs/rules/prefer-array-includes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Prefer `Array#includes()` over `Array#some()` when looking for a well known item

[`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, then 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 has side effects.

## Fail

```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 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;
});
```
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,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-includes': 'error',
'unicorn/prefer-array-index-of': 'error',
'unicorn/prefer-array-some': 'error',
'unicorn/prefer-date-now': 'error',
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,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-includes": "error",
"unicorn/prefer-array-index-of": "error",
"unicorn/prefer-array-some": "error",
"unicorn/prefer-date-now": "error",
Expand Down Expand Up @@ -154,6 +155,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-includes](docs/rules/prefer-array-includes.md) - Prefer `Array#includes()` over `Array#some()` when looking for a well known item. *(partly 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)*
Expand Down
162 changes: 162 additions & 0 deletions rules/prefer-array-includes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
'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 getReferences = require('./utils/get-references');

const MESSAGE_ID_SOME = 'some';
const MESSAGE_ID_REPLACE = 'replaceSome';
const messages = {
[MESSAGE_ID_SOME]: 'Use `.includes()` instead of `.some()` when looking for a well known item.',
[MESSAGE_ID_REPLACE]: 'Replace `.some()` with `.includes()`.'
};

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: 'some',
length: 1
}),
`:matches(${
[
// Matches `foo.some(bar => bar === baz)`
[
'[arguments.0.type="ArrowFunctionExpression"]',
getFunctionSelector('arguments.0'),
getBinaryExpressionSelector('arguments.0.body')
].join(''),
// Matches `foo.some(bar => {return bar === baz})`
// Matches `foo.some(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;
}

// `call` is done
if (getReferences(scope).some(reference => reference.identifier.name === 'call' && !reference.resolved)) {
Fabrice-TIERCELIN marked this conversation as resolved.
Show resolved Hide resolved
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 (getReferences(scope).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_SOME,
suggest: []
};

function * fix(fixer) {
let text = sourceCode.getText(searchValueNode);
if (isParenthesized(searchValueNode, sourceCode) && !isParenthesized(callback, sourceCode)) {
text = `(${text})`;
}

yield fixer.replaceText(method, 'includes');
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
}
};
135 changes: 135 additions & 0 deletions test/prefer-array-includes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
import {outdent} from 'outdent';
import {test} from './utils/test.js';

const MESSAGE_ID_REPLACE = 'replaceSome';

function suggestionCase({code, output}) {
return {
code,
output: code,
errors: [
{
suggestions: [
{
messageId: MESSAGE_ID_REPLACE,
output
}
]
}
]
};
}

test({
valid: [
'const some = foo.some',

// Test `foo.some`
// More/less argument(s)
'foo.some()',
'foo.some(function (x) {return x === 1;}, bar)',
'foo.some(...[function (x) {return x === 1;}])',
// Not `CallExpression`
'new foo.some(x => x === 1)',
// Not `MemberExpression`
'some(x => x === 1)',
// `callee.property` is not a `Identifier`
'foo["some"](x => x === 1)',
// Computed
'foo[some](x => x === 1)',
// Not `some`
'foo.notListedMethod(x => x === 1)',

// Test `callback` part
// Not function
'foo.some(myFunction)',
// Not one parameter
'foo.some((x, i) => x === i)',
'foo.some((x, i) => {return x === i})',
'foo.some(function(x, i) {return x === i})',
// Parameter is not `Identifier`
'foo.some(({x}) => x === 1)',
'foo.some(({x}) => {return x === 1})',
'foo.some(function({x}) {return x === 1})',
// `generator`
'foo.some(function * (x) {return x === 1})',
// `async`
'foo.some(async (x) => x === 1)',
'foo.some(async (x) => {return x === 1})',
'foo.some(async function(x) {return x === 1})',

// Test `callback` body
// Not only `return`
'foo.some(({x}) => {noop();return x === 1})',
// Not `return`
'foo.some(({x}) => {bar(x === 1)})',

// Test `BinaryExpression`
// Not `BinaryExpression`
'foo.some(x => x - 1)',
'foo.some(x => {return x - 1})',
'foo.some(function (x){return x - 1})',
// Not `===`
'foo.some(x => x !== 1)',
'foo.some(x => {return x !== 1})',
'foo.some(function (x){return x !== 1})',
// Neither `left` nor `right` is Identifier
'foo.some(x => 1 === 1.0)',
'foo.some(x => {return 1 === 1.0})',
'foo.some(function (x){return 1 === 1.0})',
// Both `left` and `right` are same as `parameter`
'foo.some(x => x === x)',

// Not the same identifier
'foo.some(x => y === 1)',

// Dynamical value
'foo.some(x => x + "foo" === "foo" + x)',

// Parameter is used
'foo.some(x => x === "foo" + x)',
// Parameter is used in a deeper scope
'foo.some(x => x === (function (){return x === "1"})())',
// FunctionName is used
'foo.some(function fn(x) {return x === fn(y)})',
// `arguments` is used
'foo.some(function(x) {return x === arguments.length})',
// `this` is used
'foo.some(function(x) {return x === this.length})',
// `call` is done
'foo.some(function(x) {return x === call()})',

// Already valid case
'foo.includes(0)'
],

invalid: [
suggestionCase({
code: 'values.some(x => x === foo())',
output: 'values.includes(foo())'
}),
suggestionCase({
code: 'values.some(x => foo() === x)',
output: 'values.includes(foo())'
})
]
});

test.typescript({
valid: [],
invalid: [
{
code: outdent`
function foo() {
return (bar as string).some(x => x === "foo");
}
`,
output: outdent`
function foo() {
return (bar as string).includes("foo");
}
`,
errors: 1
}
]
});