8000 feat(eslint-plugin): add rule prefer-find (#8216) · danvk/typescript-eslint@0498cfa · GitHub
[go: up one dir, main page]
Skip to content

Commit 0498cfa

Browse files
kirkwaiblingerauvred
authored and
danvk
committed
feat(eslint-plugin): add rule prefer-find (typescript-eslint#8216)
* Add rule prefer-find * address lots of stuff * remove console statement * tweaks * extract fix to function * improve behavior around nulls * add comments around array indexing checks * messages were backwards * filter syntax * formatting * add extra comma operator test * pr feedback round 2 * Fix the the Co-authored-by: auvred <61150013+auvred@users.noreply.github.com> * fix up imports * address intersections of arrays --------- Co-authored-by: auvred <61150013+auvred@users.noreply.github.com>
1 parent be5dcea commit 0498cfa

File tree

8 files changed

+952
-1
lines changed

8 files changed

+952
-1
lines changed

‎packages/eslint-plugin/docs/rules/prefer-find.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
description: 'Enforce the use of Array.prototype.find() over Array.prototype.filter() followed by [0] when looking for a single result.'
3+
---
4+
5+
> 🛑 This file is source code, not the primary documentation location! 🛑
6+
>
7+
> See **https://typescript-eslint.io/rules/prefer-find** for documentation.
8+
9+
When searching for the first item in an array matching a condition, it may be tempting to use code like `arr.filter(x => x > 0)[0]`.
10+
However, it is simpler to use [Array.prototype.find()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) instead, `arr.find(x => x > 0)`, which also returns the first entry matching a condition.
11+
Because the `.find()` only needs to execute the callback until it finds a match, it's also more efficient.
12+
13+
:::info
14+
15+
Beware the difference in short-circuiting behavior between the approaches.
16+
`.find()` will only execute the callback on array elements until it finds a match, whereas `.filter()` executes the callback for all array elements.
17+
Therefore, when fixing errors from this rule, be sure that your `.filter()` callbacks do not have side effects.
18+
19+
:::
20+
21+
<!--tabs-->
22+
23+
### ❌ Incorrect
24+
25+
```ts
26+
[1, 2, 3].filter(x => x > 1)[0];
27+
28+
[1, 2, 3].filter(x => x > 1).at(0);
29+
```
30+
31+
### ✅ Correct
32+
33+
```ts
34+
[1, 2, 3].find(x => x > 1);
35+
```
36+
37+
## When Not To Use It
38+
39+
If you intentionally use patterns like `.filter(callback)[0]` to execute side effects in `callback` on all array elements, you will want to avoid this rule.

‎packages/eslint-plugin/src/configs/all.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ export = {
122122
'prefer-destructuring': 'off',
123123
'@typescript-eslint/prefer-destructuring': 'error',
124124
'@typescript-eslint/prefer-enum-initializers': 'error',
125+
'@typescript-eslint/prefer-find': 'error',
125126
'@typescript-eslint/prefer-for-of': 'error',
126127
'@typescript-eslint/prefer-function-type': 'error',
127128
'@typescript-eslint/prefer-includes': 'error',

‎packages/eslint-plugin/src/configs/disable-type-checked.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ export = {
4141
'@typescript-eslint/no-useless-template-literals': 'off',
4242
'@typescript-eslint/non-nullable-type-assertion-style': 'off',
4343
'@typescript-eslint/prefer-destructuring': 'off',
44+
'@typescript-eslint/prefer-find': 'off',
4445
'@typescript-eslint/prefer-includes': 'off',
4546
'@typescript-eslint/prefer-nullish-coalescing': 'off',
4647
'@typescript-eslint/prefer-optional-chain': 'off',

‎packages/eslint-plugin/src/rules/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ import parameterProperties from './parameter-properties';
106106
import preferAsConst from './prefer-as-const';
107107
import preferDestructuring from './prefer-destructuring';
108108
import preferEnumInitializers from './prefer-enum-initializers';
109+
import preferFind from './prefer-find';
109110
import preferForOf from './prefer-for-of';
110111
import preferFunctionType from './prefer-function-type';
111112
import preferIncludes from './prefer-includes';
@@ -248,6 +249,7 @@ export default {
248249
'prefer-as-const': preferAsConst,
249250
'prefer-destructuring': preferDestructuring,
250251
'prefer-enum-initializers': preferEnumInitializers,
252+
'prefer-find': preferFind,
251253
'prefer-for-of': preferForOf,
252254
'prefer-function-type': preferFunctionType,
253255
'prefer-includes': preferIncludes,

‎packages/eslint-plugin/src/rules/prefer-find.ts

Lines changed: 320 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,320 @@
1+
import type { TSESLint, TSESTree } from '@typescript-eslint/utils';
2+
import { AST_NODE_TYPES } from '@typescript-eslint/utils';
3+
import { getScope, getSourceCode } from '@typescript-eslint/utils/eslint-utils';
4+
import type {
5+
RuleFix,
6+
Scope,
7+
SourceCode,
8+
} from '@typescript-eslint/utils/ts-eslint';
9+
import * as tsutils from 'ts-api-utils';
10+
import type { Type } from 'typescript';
11+
12+
import {
13+
createRule,
14+
getConstrainedTypeAtLocation,
15+
getParserServices,
16+
getStaticValue,
17+
nullThrows,
18+
} from '../util';
19+
20+
export default createRule({
21+
name: 'prefer-find',
22+
meta: {
23+
docs: {
24+
description:
25+
'Enforce the use of Array.prototype.find() over Array.prototype.filter() followed by [0] when looking for a single result',
26+
requiresTypeChecking: true,
27+
},
28+
messages: {
29+
preferFind: 'Prefer .find(...) instead of .filter(...)[0].',
30+
preferFindSuggestion: 'Use .find(...) instead of .filter(...)[0].',
31+
},
32+
schema: [],
33+
type: 'suggestion',
34+
hasSuggestions: true,
35+
},
36+
37+
defaultOptions: [],
38+
39+
create(context) {
40+
const globalScope = getScope(context);
41+
const services = getParserServices(context);
42+
const checker = services.program.getTypeChecker();
43+
44+
interface FilterExpressionData {
45+
isBracketSyntaxForFilter: boolean;
46+
filterNode: TSESTree.Node;
47+
}
48+
49+
function parseIfArrayFilterExpression(
50+
expression: TSESTree.Expression,
51+
): FilterExpressionData | undefined {
52+
if (expression.type === AST_NODE_TYPES.SequenceExpression) {
53+
// Only the last expression in (a, b, [1, 2, 3].filter(condition))[0] matters
54+
const lastExpression = nullThrows(
55+
expression.expressions.at(-1),
56+
'Expected to have more than zero expressions in a sequence expression',
57+
);
58+
return parseIfArrayFilterExpression(lastExpression);
59+
}
60+
61+
if (expression.type === AST_NODE_TYPES.ChainExpression) {
62+
return parseIfArrayFilterExpression(expression.expression);
63+
}
64+
65+
// Check if it looks like <<stuff>>(...), but not <<stuff>>?.(...)
66+
if (
67+
expression.type === AST_NODE_TYPES.CallExpression &&
68+
!expression.optional
69+
) {
70+
const callee = expression.callee;
71+
// Check if it looks like <<stuff>>.filter(...) or <<stuff>>['filter'](...),
72+
// or the optional chaining variants.
73+
if (callee.type === AST_NODE_TYPES.MemberExpression) {
74+
const isBracketSyntaxForFilter = callee.computed;
75+
if (isStaticMemberAccessOfValue(callee, 'filter', globalScope)) {
76+
const filterNode = callee.property;
77+
78+
const filteredObjectType = getConstrainedTypeAtLocation(
79+
services,
80+
callee.object,
81+
);
82+
83+
// As long as the object is a (possibly nullable) array,
84+
// this is an Array.prototype.filter expression.
85+
if (isArrayish(filteredObjectType)) {
86+
return {
87+
isBracketSyntaxForFilter,
88+
filterNode,
89+
};
90+
}
91+
}
92+
}
93+
}
94+
95+
return undefined;
96+
}
97+
98+
/**
99+
* Tells whether the type is a possibly nullable array/tuple or union thereof.
100+
*/
101+
function isArrayish(type: Type): boolean {
102+
let isAtLeastOneArrayishComponent = false;
103+
for (const unionPart of tsutils.unionTypeParts(type)) {
104+
if (
105+
tsutils.isIntrinsicNullType(unionPart) ||
106+
tsutils.isIntrinsicUndefinedType(unionPart)
107+
) {
108+
continue;
109+
}
110+
111+
// apparently checker.isArrayType(T[] & S[]) => false.
112+
// so we need to check the intersection parts individually.
113+
const isArrayOrIntersectionThereof = tsutils
114+
.intersectionTypeParts(unionPart)
115+
.every(
116+
intersectionPart =>
117+
checker.isArrayType(intersectionPart) ||
118+
checker.isTupleType(intersectionPart),
119+
);
120+
121+
if (!isArrayOrIntersectionThereof) {
122+
// There is a non-array, non-nullish type component,
123+
// so it's not an array.
124+
return false;
125+
}
126+
127+
isAtLeastOneArrayishComponent = true;
128+
}
129+
130+
return isAtLeastOneArrayishComponent;
131+
}
132+
133+
function getObjectIfArrayAtExpression(
134+
node: TSESTree.CallExpression,
135+
): TSESTree.Expression | undefined {
136+
// .at() should take exactly one argument.
137+
if (node.arguments.length !== 1) {
138+
return undefined;
139+
}
140+
141+
const atArgument = getStaticValue(node.arguments[0], globalScope);
142+
if (atArgument != null && isTreatedAsZeroByArrayAt(atArgument.value)) {
143+
const callee = node.callee;
144+
if (
145+
callee.type === AST_NODE_TYPES.MemberExpression &&
146+
!callee.optional &&
147+
isStaticMemberAccessOfValue(callee, 'at', globalScope)
148+
) {
149+
return callee.object;
150+
}
151+
}
152+
153+
return undefined;
154+
}
155+
156+
/**
157+
* Implements the algorithm for array indexing by `.at()` method.
158+
* @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/at#parameters
159+
*/
160+
function isTreatedAsZeroByArrayAt(value: unknown): boolean {
161+
const asNumber = Number(value);
162+
163+
if (isNaN(asNumber)) {
164+
return true;
165+
}
166+
167+
return Math.trunc(asNumber) === 0;
168+
}
169+
170+
function isMemberAccessOfZero(
171+
node: TSESTree.MemberExpressionComputedName,
172+
): boolean {
173+
const property = getStaticValue(node.property, globalScope);
174+
// Check if it looks like <<stuff>>[0] or <<stuff>>['0'], but not <<stuff>>?.[0]
175+
return (
176+
!node.optional &&
177+
property != null &&
178+
isTreatedAsZeroByMemberAccess(property.value)
179+
);
180+
}
181+
182+
/**
183+
* Implements the algorithm for array indexing by member operator.
184+
* @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array#array_indices
185+
*/
186+
function isTreatedAsZeroByMemberAccess(value: unknown): boolean {
187+
return String(value) === '0';
188+
}
189+
190+
function generateFixToRemoveArrayElementAccess(
191+
fixer: TSESLint.RuleFixer,
192+
arrayNode: TSESTree.Expression,
193+
wholeExpressionBeingFlagged: TSESTree.Expression,
194+
sourceCode: SourceCode,
195+
): RuleFix {
196+
const tokenToStartDeletingFrom = nullThrows(
197+
// The next `.` or `[` is what we're looking for.
198+
// think of (...).at(0) or (...)[0] or even (...)["at"](0).
199+
sourceCode.getTokenAfter(
200+
arrayNode,
201+
token => token.value === '.' || token.value === '[',
202+
),
203+
'Expected to find a member access token!',
204+
);
205+
return fixer.removeRange([
206+
tokenToStartDeletingFrom.range[0],
207+
wholeExpressionBeingFlagged.range[1],
208+
]);
209+
}
210+
211+
function generateFixToReplaceFilterWithFind(
212+
fixer: TSESLint.RuleFixer,
213+
filterExpression: FilterExpressionData,
214+
): TSESLint.RuleFix {
215+
return fixer.replaceText(
216+
filterExpression.filterNode,
217+
filterExpression.isBracketSyntaxForFilter ? '"find"' : 'find',
218+
);
219+
}
220+
221+
return {
222+
// This query will be used to find things like `filteredResults.at(0)`.
223+
CallExpression(node): void {
224+
const object = getObjectIfArrayAtExpression(node);
225+
if (object) {
226+
const filterExpression = parseIfArrayFilterExpression(object);
227+
if (filterExpression) {
228+
context.report({
229+
node,
230+
messageId: 'preferFind',
231+
suggest: [
232+
{
233+
messageId: 'preferFindSuggestion',
234+
fix: (fixer): TSESLint.RuleFix[] => {
235+
const sourceCode = getSourceCode(context);
236+
return [
237+
generateFixToReplaceFilterWithFind(
238+
fixer,
239+
filterExpression,
240+
),
241+
// Get rid of the .at(0) or ['at'](0).
242+
generateFixToRemoveArrayElementAccess(
243+
fixer,
244+
object,
245+
node,
246+
sourceCode,
247+
),
248+
];
249+
},
250+
},
251+
],
252+
});
253+
}
254+
}
255+
},
256+
257+
// This query will be used to find things like `filteredResults[0]`.
258+
//
259+
// Note: we're always looking for array member access to be "computed",
260+
// i.e. `filteredResults[0]`, since `filteredResults.0` isn't a thing.
261+
['MemberExpression[computed=true]'](
262+
node: TSESTree.MemberExpressionComputedName,
263+
): void {
264+
if (isMemberAccessOfZero(node)) {
265+
const object = node.object;
266+
const filterExpression = parseIfArrayFilterExpression(object);
267+
if (filterExpression) {
268+
context.report({
269+
node,
270+
messageId: 'preferFind',
271+
suggest: [
272+
{
273+
messageId: 'preferFindSuggestion',
274+
fix: (fixer): TSESLint.RuleFix[] => {
275+
const sourceCode = context.sourceCode;
276+
return [
277+
generateFixToReplaceFilterWithFind(
278+
fixer,
279+
filterExpression,
280+
),
281+
// Get rid of the [0].
282+
generateFixToRemoveArrayElementAccess(
283+
fixer,
284+
object,
285+
node,
286+
sourceCode,
287+
),
288+
];
289+
},
290+
},
291+
],
292+
});
293+
}
294+
}
295+
},
296+
};
297+
},
298+
});
299+
300+
/**
301+
* Answers whether the member expression looks like
302+
* `x.memberName`, `x['memberName']`,
303+
* or even `const mn = 'memberName'; x[mn]` (or optional variants thereof).
304+
*/
305+
function isStaticMemberAccessOfValue(
306+
memberExpression:
307+
| TSESTree.MemberExpressionComputedName
308+
| TSESTree.MemberExpressionNonComputedName,
309+
value: string,
310+
scope?: Scope.Scope | undefined,
311+
): boolean {
312+
if (!memberExpression.computed) {
313+
// x.memberName case.
314+
return memberExpression.property.name === value;
315+
}
316+
317+
// x['memberName'] cases.
318+
const staticValueResult = getStaticValue(memberExpression.property, scope);
319+
return staticValueResult != null && value === staticValueResult.value;
320+
}

0 commit comments

Comments
 (0)
0