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-find rule #735

Merged
merged 24 commits into from May 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
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
33 changes: 33 additions & 0 deletions docs/rules/prefer-array-find.md
@@ -0,0 +1,33 @@
# Prefer `.find(…)` over the first element from `.filter(…)`

[`Array#find()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) breaks the loop as soon as it finds a match and doesn't create a new array.

This rule is fixable unless default values are used in declaration or assignment.

## Fail

```js
const item = array.filter(x => x === '🦄')[0];
```

```js
const item = array.filter(x => x === '🦄').shift();
sindresorhus marked this conversation as resolved.
Show resolved Hide resolved
```

```js
const [item] = array.filter(x => x === '🦄');
```

```js
[item] = array.filter(x => x === '🦄');
```

## Pass

```js
const item = array.find(x => x === '🦄');
```

```js
item = array.find(x => x === '🦄');
```
1 change: 1 addition & 0 deletions index.js
Expand Up @@ -48,6 +48,7 @@ module.exports = {
'unicorn/no-zero-fractions': 'error',
'unicorn/number-literal-case': 'error',
'unicorn/prefer-add-event-listener': 'error',
'unicorn/prefer-array-find': 'error',
'unicorn/prefer-dataset': 'error',
'unicorn/prefer-event-key': 'error',
'unicorn/prefer-flat-map': 'error',
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Expand Up @@ -64,6 +64,7 @@ Configure it in `package.json`.
"unicorn/no-zero-fractions": "error",
"unicorn/number-literal-case": "error",
"unicorn/prefer-add-event-listener": "error",
"unicorn/prefer-array-find": "error",
"unicorn/prefer-dataset": "error",
"unicorn/prefer-event-key": "error",
"unicorn/prefer-flat-map": "error",
Expand Down Expand Up @@ -124,6 +125,7 @@ Configure it in `package.json`.
- [no-zero-fractions](docs/rules/no-zero-fractions.md) - Disallow number literals with zero fractions or dangling dots. *(fixable)*
- [number-literal-case](docs/rules/number-literal-case.md) - Enforce proper case for numeric literals. *(fixable)*
- [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-dataset](docs/rules/prefer-dataset.md) - Prefer using `.dataset` on DOM elements over `.setAttribute(…)`. *(fixable)*
- [prefer-event-key](docs/rules/prefer-event-key.md) - Prefer `KeyboardEvent#key` over `KeyboardEvent#keyCode`. *(partly fixable)*
- [prefer-flat-map](docs/rules/prefer-flat-map.md) - Prefer `.flatMap(…)` over `.map(…).flat()`. *(fixable)*
Expand Down
322 changes: 322 additions & 0 deletions rules/prefer-array-find.js
@@ -0,0 +1,322 @@
'use strict';
const {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 ERROR_ZERO_INDEX = 'error-zero-index';
const ERROR_SHIFT = 'error-shift';
const ERROR_DESTRUCTURING_DECLARATION = 'error-destructuring-declaration';
const ERROR_DESTRUCTURING_ASSIGNMENT = 'error-destructuring-assignment';
const ERROR_DECLARATION = 'error-variable';

const SUGGESTION_NULLISH_COALESCING_OPERATOR = 'suggest-nullish-coalescing-operator';
const SUGGESTION_LOGICAL_OR_OPERATOR = 'suggest-logical-or-operator';

const filterMethodSelectorOptions = {
name: 'filter',
min: 1,
max: 2
};

const filterVariableSelector = [
'VariableDeclaration',
// Exclude `export const foo = [];`
`:not(${
[
'ExportNamedDeclaration',
'>',
'VariableDeclaration.declaration'
].join('')
})`,
'>',
'VariableDeclarator.declarations',
'[id.type="Identifier"]',
methodSelector({
...filterMethodSelectorOptions,
property: 'init'
})
].join('');

const zeroIndexSelector = [
'MemberExpression',
'[computed=true]',
'[property.type="Literal"]',
'[property.raw="0"]',
methodSelector({
...filterMethodSelectorOptions,
property: 'object'
})
].join('');

const shiftSelector = [
methodSelector({
name: 'shift',
length: 0
}),
methodSelector({
...filterMethodSelectorOptions,
property: 'callee.object'
})
].join('');

const destructuringDeclaratorSelector = [
'VariableDeclarator',
'[id.type="ArrayPattern"]',
'[id.elements.length=1]',
'[id.elements.0.type!="RestElement"]',
methodSelector({
...filterMethodSelectorOptions,
property: 'init'
})
].join('');

const destructuringAssignmentSelector = [
'AssignmentExpression',
'[left.type="ArrayPattern"]',
'[left.elements.length=1]',
'[left.elements.0.type!="RestElement"]',
methodSelector({
...filterMethodSelectorOptions,
property: 'right'
})
].join('');

// Need add `()` to the `AssignmentExpression`
// - `ObjectExpression`: `[{foo}] = array.filter(bar)` fix to `{foo} = array.find(bar)`
// - `ObjectPattern`: `[{foo = baz}] = array.filter(bar)`
const assignmentNeedParenthesize = (node, source) => {
const isAssign = node.type === 'AssignmentExpression';

if (!isAssign || isParenthesized(node, source)) {
return false;
}

const {left} = getDestructuringLeftAndRight(node);
const [element] = left.elements;
const {type} = element.type === 'AssignmentPattern' ? element.left : element;
return type === 'ObjectExpression' || type === 'ObjectPattern';
};

// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence#Table
const hasLowerPrecedence = (node, operator) => (
(node.type === 'LogicalExpression' && (
node.operator === operator ||
// https://tc39.es/proposal-nullish-coalescing/ says
// `??` has lower precedence than `||`
// But MDN says
// `??` has higher precedence than `||`
(operator === '||' && node.operator === '??') ||
(operator === '??' && (node.operator === '||' || node.operator === '&&'))
)) ||
node.type === 'ConditionalExpression' ||
// Lower than `assignment`, should already parenthesized
/* istanbul ignore next */
node.type === 'AssignmentExpression' ||
node.type === 'YieldExpression' ||
node.type === 'SequenceExpression'
);

const getDestructuringLeftAndRight = node => {
/* istanbul ignore next */
if (!node) {
return {};
}

if (node.type === 'AssignmentExpression') {
return node;
}

if (node.type === 'VariableDeclarator') {
return {left: node.id, right: node.init};
}

return {};
};

const fixDestructuring = (node, source, fixer) => {
const {left} = getDestructuringLeftAndRight(node);
const [element] = left.elements;

const leftText = source.getText(element.type === 'AssignmentPattern' ? element.left : element);
const fixes = [fixer.replaceText(left, leftText)];

// `AssignmentExpression` always starts with `[` or `(`, so we don't need check ASI
if (assignmentNeedParenthesize(node, source)) {
fixes.push(fixer.insertTextBefore(node, '('));
fixes.push(fixer.insertTextAfter(node, ')'));
}

return fixes;
};

const hasDefaultValue = node => getDestructuringLeftAndRight(node).left.elements[0].type === 'AssignmentPattern';

const fixDestructuringDefaultValue = (node, source, fixer, operator) => {
const {left, right} = getDestructuringLeftAndRight(node);
const [element] = left.elements;
const defaultValue = element.right;
let defaultValueText = source.getText(defaultValue);

if (isParenthesized(defaultValue, source) || hasLowerPrecedence(defaultValue, operator)) {
defaultValueText = `(${defaultValueText})`;
}

return fixer.insertTextAfter(right, ` ${operator} ${defaultValueText}`);
};

const fixDestructuringAndReplaceFilter = (source, node) => {
const {property} = getDestructuringLeftAndRight(node).right.callee;

let suggest;
let fix;

if (hasDefaultValue(node)) {
suggest = [
{operator: '??', messageId: SUGGESTION_NULLISH_COALESCING_OPERATOR},
{operator: '||', messageId: SUGGESTION_LOGICAL_OR_OPERATOR}
].map(({messageId, operator}) => ({
messageId,
fix: fixer => [
fixer.replaceText(property, 'find'),
fixDestructuringDefaultValue(node, source, fixer, operator),
...fixDestructuring(node, source, fixer)
]
}));
} else {
fix = fixer => [
fixer.replaceText(property, 'find'),
...fixDestructuring(node, source, fixer)
];
}

return {fix, suggest};
};

const isAccessingZeroIndex = node =>
node.parent &&
node.parent.type === 'MemberExpression' &&
node.parent.computed === true &&
node.parent.object === node &&
node.parent.property &&
node.parent.property.type === 'Literal' &&
node.parent.property.raw === '0';

const isDestructuringFirstElement = node => {
const {left, right} = getDestructuringLeftAndRight(node.parent);
return left &&
right &&
right === node &&
left.type === 'ArrayPattern' &&
left.elements &&
left.elements.length === 1 &&
left.elements[0].type !== 'RestElement';
};

const create = context => {
const source = context.getSourceCode();

return {
[zeroIndexSelector](node) {
context.report({
node: node.object.callee.property,
messageId: ERROR_ZERO_INDEX,
fix: fixer => [
fixer.replaceText(node.object.callee.property, 'find'),
fixer.removeRange([node.object.range[1], node.range[1]])
]
});
},
[shiftSelector](node) {
context.report({
node: node.callee.object.callee.property,
messageId: ERROR_SHIFT,
fix: fixer => [
fixer.replaceText(node.callee.object.callee.property, 'find'),
fixer.removeRange([node.callee.object.range[1], node.range[1]])
]
});
},
[destructuringDeclaratorSelector](node) {
context.report({
node: node.init.callee.property,
messageId: ERROR_DESTRUCTURING_DECLARATION,
...fixDestructuringAndReplaceFilter(source, node)
});
},
[destructuringAssignmentSelector](node) {
context.report({
node: node.right.callee.property,
messageId: ERROR_DESTRUCTURING_ASSIGNMENT,
...fixDestructuringAndReplaceFilter(source, node)
});
},
[filterVariableSelector](node) {
const variable = findVariable(context.getScope(), node.id);
const identifiers = getVariableIdentifiers(variable).filter(identifier => identifier !== node.id);

if (identifiers.length === 0) {
return;
}

const zeroIndexNodes = [];
const destructuringNodes = [];
for (const identifier of identifiers) {
if (isAccessingZeroIndex(identifier)) {
zeroIndexNodes.push(identifier.parent);
} else if (isDestructuringFirstElement(identifier)) {
destructuringNodes.push(identifier.parent);
} else {
return;
}
}

const problem = {
node: node.init.callee.property,
messageId: ERROR_DECLARATION
};

// `const [foo = bar] = baz` is not fixable
if (!destructuringNodes.some(node => hasDefaultValue(node))) {
problem.fix = fixer => {
const fixes = [
fixer.replaceText(node.init.callee.property, 'find')
];

for (const node of zeroIndexNodes) {
fixes.push(fixer.removeRange([node.object.range[1], node.range[1]]));
}

for (const node of destructuringNodes) {
fixes.push(...fixDestructuring(node, source, fixer));
}

return fixes;
};
}

context.report(problem);
}
};
};

module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
url: getDocumentationUrl(__filename)
},
fixable: 'code',
messages: {
[ERROR_DECLARATION]: 'Prefer `.find(…)` over `.filter(…)`.',
[ERROR_ZERO_INDEX]: 'Prefer `.find(…)` over `.filter(…)[0]`.',
[ERROR_SHIFT]: 'Prefer `.find(…)` over `.filter(…).shift()`.',
[ERROR_DESTRUCTURING_DECLARATION]: 'Prefer `.find(…)` over destructuring `.filter(…)`.',
// Same message as `ERROR_DESTRUCTURING_DECLARATION`, but different case
[ERROR_DESTRUCTURING_ASSIGNMENT]: 'Prefer `.find(…)` over destructuring `.filter(…)`.',
[SUGGESTION_NULLISH_COALESCING_OPERATOR]: 'Replace `.filter(…)` with `.find(…) ?? …`.',
[SUGGESTION_LOGICAL_OR_OPERATOR]: 'Replace `.filter(…)` with `.find(…) || …`.'
}
}
};
2 changes: 1 addition & 1 deletion rules/utils/method-selector.js
Expand Up @@ -25,7 +25,7 @@ module.exports = options => {
];

if (name) {
selector.push(`[callee.property.name="${name}"]`);
selector.push(`[${prefix}callee.property.name="${name}"]`);
}

if (Array.isArray(names) && names.length !== 0) {
Expand Down