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

prefer-array-some: Check .findLast() #1897

Merged
merged 2 commits into from Sep 17, 2022
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
36 changes: 32 additions & 4 deletions docs/rules/prefer-array-some.md
@@ -1,4 +1,4 @@
# Prefer `.some(…)` over `.filter(…).length` check and `.find(…)`
# Prefer `.some(…)` over `.filter(…).length` check and `.{find,findLast}(…)`

<!-- Do not manually modify RULE_NOTICE part. Run: `npm run generate-rule-notices` -->
<!-- RULE_NOTICE -->
Expand All @@ -13,11 +13,11 @@ Prefer using [`Array#some`](https://developer.mozilla.org/en-US/docs/Web/JavaScr

We only check `.filter().length > 0` and `.filter().length !== 0`. These two non-zero length check styles are allowed in [`unicorn/explicit-length-check`](./explicit-length-check.md#options) rule. It is recommended to use them together.

- Using [`Array#find()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) to ensure at least one element in the array passes a given check.
- Using [`Array#find()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) or [`Array#findLast()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findLast) to ensure at least one element in the array passes a given check.

- Comparing the result of [`Array#find()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) with `undefined`.
- Comparing the result of [`Array#find()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) or [`Array#findLast()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findLast) with `undefined`.

This rule is fixable for `.filter(…).length` check and has a suggestion for `.find(…)`.
This rule is fixable for `.filter(…).length` check and has a suggestion for `.{find,findLast}(…)`.

## Fail

Expand Down Expand Up @@ -51,12 +51,36 @@ const hasUnicorn = array.find(element => isUnicorn(element) !== undefined;
const hasUnicorn = array.find(element => isUnicorn(element) != null;
```

```js
if (array.find(element => isUnicorn(element))) {
// …
}
```

```js
const foo = array.findLast(element => isUnicorn(element)) ? bar : baz;
```

```js
const hasUnicorn = array.findLast(element => isUnicorn(element) !== undefined;
```

```js
const hasUnicorn = array.findLast(element => isUnicorn(element) != null;
```

```vue
<template>
<div v-if="array.find(element => isUnicorn(element))">Vue</div>
</template>
```

```vue
<template>
<div v-if="array.findLast(element => isUnicorn(element))">Vue</div>
</template>
```

```vue
<template>
<div v-if="array.filter(element => isUnicorn(element)).length > 0">Vue</div>
Expand All @@ -79,6 +103,10 @@ if (array.some(element => isUnicorn(element))) {
const foo = array.find(element => isUnicorn(element)) || bar;
```

```js
const foo = array.findLast(element => isUnicorn(element)) || bar;
```

```vue
<template>
<div v-if="array.some(element => isUnicorn(element))">Vue</div>
Expand Down
2 changes: 1 addition & 1 deletion readme.md
Expand Up @@ -109,7 +109,7 @@ Each rule has emojis denoting:
| [prefer-array-flat](docs/rules/prefer-array-flat.md) | Prefer `Array#flat()` over legacy techniques to flatten arrays. | ✅ | 🔧 | |
| [prefer-array-flat-map](docs/rules/prefer-array-flat-map.md) | Prefer `.flatMap(…)` over `.map(…).flat()`. | ✅ | 🔧 | |
| [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. | ✅ | 🔧 | 💡 |
| [prefer-array-some](docs/rules/prefer-array-some.md) | Prefer `.some(…)` over `.filter(…).length` check and `.find(…)`. | ✅ | 🔧 | 💡 |
| [prefer-array-some](docs/rules/prefer-array-some.md) | Prefer `.some(…)` over `.filter(…).length` check and `.{find,findLast}(…)`. | ✅ | 🔧 | 💡 |
| [prefer-at](docs/rules/prefer-at.md) | Prefer `.at()` method for index access and `String#charAt()`. | | 🔧 | 💡 |
| [prefer-code-point](docs/rules/prefer-code-point.md) | Prefer `String#codePointAt(…)` over `String#charCodeAt(…)` and `String.fromCodePoint(…)` over `String.fromCharCode(…)`. | ✅ | | 💡 |
| [prefer-date-now](docs/rules/prefer-date-now.md) | Prefer `Date.now()` to get the number of milliseconds since the Unix Epoch. | ✅ | 🔧 | |
Expand Down
30 changes: 16 additions & 14 deletions rules/prefer-array-some.js
Expand Up @@ -10,13 +10,13 @@ const ERROR_ID_ARRAY_SOME = 'some';
const SUGGESTION_ID_ARRAY_SOME = 'some-suggestion';
const ERROR_ID_ARRAY_FILTER = 'filter';
const messages = {
[ERROR_ID_ARRAY_SOME]: 'Prefer `.some(…)` over `.find(…)`.',
[SUGGESTION_ID_ARRAY_SOME]: 'Replace `.find(…)` with `.some(…)`.',
[ERROR_ID_ARRAY_SOME]: 'Prefer `.some(…)` over `.{{method}}(…)`.',
[SUGGESTION_ID_ARRAY_SOME]: 'Replace `.{{method}}(…)` with `.some(…)`.',
[ERROR_ID_ARRAY_FILTER]: 'Prefer `.some(…)` over non-zero length check from `.filter(…)`.',
};

const arrayFindCallSelector = methodCallSelector({
method: 'find',
const arrayFindOrFindLastCallSelector = methodCallSelector({
methods: ['find', 'findLast'],
minimumArguments: 1,
maximumArguments: 2,
});
Expand Down Expand Up @@ -59,30 +59,32 @@ const arrayFilterCallSelector = [

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => ({
[arrayFindCallSelector](findCall) {
const isCompare = isCheckingUndefined(findCall);
if (!isCompare && !isBooleanNode(findCall)) {
[arrayFindOrFindLastCallSelector](callExpression) {
const isCompare = isCheckingUndefined(callExpression);
if (!isCompare && !isBooleanNode(callExpression)) {
return;
}

const findProperty = findCall.callee.property;
const methodNode = callExpression.callee.property;
return {
node: findProperty,
node: methodNode,
messageId: ERROR_ID_ARRAY_SOME,
data: {method: methodNode.name},
suggest: [
{
messageId: SUGGESTION_ID_ARRAY_SOME,
data: {method: methodNode.name},
* fix(fixer) {
yield fixer.replaceText(findProperty, 'some');
yield fixer.replaceText(methodNode, 'some');

if (!isCompare) {
return;
}

const parenthesizedRange = getParenthesizedRange(findCall, context.getSourceCode());
yield fixer.replaceTextRange([parenthesizedRange[1], findCall.parent.range[1]], '');
const parenthesizedRange = getParenthesizedRange(callExpression, context.getSourceCode());
yield fixer.replaceTextRange([parenthesizedRange[1], callExpression.parent.range[1]], '');

if (findCall.parent.operator === '!=' || findCall.parent.operator === '!==') {
if (callExpression.parent.operator === '!=' || callExpression.parent.operator === '!==') {
return;
}

Expand Down Expand Up @@ -133,7 +135,7 @@ module.exports = {
meta: {
type: 'suggestion',
docs: {
description: 'Prefer `.some(…)` over `.filter(…).length` check and `.find(…)`.',
description: 'Prefer `.some(…)` over `.filter(…).length` check and `.{find,findLast}(…)`.',
},
fixable: 'code',
messages,
Expand Down
45 changes: 38 additions & 7 deletions test/prefer-array-some.mjs
Expand Up @@ -5,14 +5,16 @@ const {test} = getTester(import.meta);

const ERROR_ID_ARRAY_SOME = 'some';
const SUGGESTION_ID_ARRAY_SOME = 'some-suggestion';
const invalidCase = ({code, suggestionOutput}) => ({
const invalidCase = ({code, suggestionOutput, method}) => ({
code,
errors: [
{
messageId: ERROR_ID_ARRAY_SOME,
data: {method},
suggestions: [
{
messageId: SUGGESTION_ID_ARRAY_SOME,
data: {method},
output: suggestionOutput,
},
],
Expand Down Expand Up @@ -45,7 +47,10 @@ test({
'foo.find()',
'foo.find(fn, thisArgument, extraArgument)',
'foo.find(...argumentsArray)',
].map(code => `if (${code}) {}`),
].flatMap(code => [
`if (${code}) {}`,
`if (${code.replace('find', 'findLast')}) {}`,
]),
],
invalid: [
...[
Expand All @@ -56,20 +61,30 @@ test({
'while (foo.find(fn)) foo.shift();',
'do {foo.shift();} while (foo.find(fn));',
'for (; foo.find(fn); ) foo.shift();',
].map(code => invalidCase({
code,
suggestionOutput: code.replace('find', 'some'),
})),
].flatMap(code => [
invalidCase({
code,
suggestionOutput: code.replace('find', 'some'),
method: 'find',
}),
invalidCase({
code: code.replace('find', 'findLast'),
suggestionOutput: code.replace('find', 'some'),
method: 'findLast',
}),
]),
// Comments
invalidCase({
code: 'console.log(foo /* comment 1 */ . /* comment 2 */ find /* comment 3 */ (fn) ? a : b)',
suggestionOutput: 'console.log(foo /* comment 1 */ . /* comment 2 */ some /* comment 3 */ (fn) ? a : b)',
method: 'find',
}),
// This should not be reported, but `jQuery.find()` is always `truly`,
// It should not use as a boolean
invalidCase({
code: 'if (jQuery.find(".outer > div")) {}',
suggestionOutput: 'if (jQuery.some(".outer > div")) {}',
method: 'find',
}),
// Actual messages
{
Expand All @@ -86,6 +101,20 @@ test({
},
],
},
{
code: 'if (foo.findLast(fn)) {}',
errors: [
{
message: 'Prefer `.some(…)` over `.findLast(…)`.',
suggestions: [
{
desc: 'Replace `.findLast(…)` with `.some(…)`.',
output: 'if (foo.some(fn)) {}',
},
],
},
],
},
],
});

Expand Down Expand Up @@ -184,10 +213,12 @@ test.vue({
invalidCase({
code: '<template><div v-if="foo.find(fn)"></div></template>',
suggestionOutput: '<template><div v-if="foo.some(fn)"></div></template>',
method: 'find',
}),
invalidCase({
code: '<script>if (foo.find(fn));</script>',
code: '<script>if (foo.findLast(fn));</script>',
suggestionOutput: '<script>if (foo.some(fn));</script>',
method: 'findLast',
}),
{
code: '<template><div v-if="foo.filter(fn).length > 0"></div></template>',
Expand Down