Skip to content

Commit a7d8a6c

Browse files
sindresorhussindresorhus
authored
Add no-unused-array-method-return rule (#2940)
1 parent 4fd8dd5 commit a7d8a6c

7 files changed

Lines changed: 1307 additions & 0 deletions

‎docs/rules/no-unused-array-method-return.md‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# no-unused-array-method-return
2+
3+
📝 Disallow ignoring the return value of selected array methods.
4+
5+
💼 This rule is enabled in the following [configs](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config): ✅ `recommended`, ☑️ `unopinionated`.
6+
7+
<!-- end auto-generated rule header -->
8+
9+
Ignoring the result of methods like `.map()`, `.find()`, `.includes()`, or `.join()` is usually a bug or dead code.
10+
11+
If you intentionally want to discard the return value, use `void` to make that explicit.
12+
13+
This rule covers selected array instance methods that return a computed value:
14+
15+
- `.at()`
16+
- `.concat()`
17+
- `.entries()`
18+
- `.every()`
19+
- `.filter()`
20+
- `.find()`
21+
- `.findIndex()`
22+
- `.findLast()`
23+
- `.findLastIndex()`
24+
- `.flat()`
25+
- `.flatMap()`
26+
- `.includes()`
27+
- `.indexOf()`
28+
- `.join()`
29+
- `.keys()`
30+
- `.lastIndexOf()`
31+
- `.map()`
32+
- `.some()`
33+
- `.slice()`
34+
- `.toReversed()`
35+
- `.toSorted()`
36+
- `.toSpliced()`
37+
- `.values()`
38+
- `.with()`
39+
40+
It does not report mutating methods like `.copyWithin()`, `.fill()`, `.forEach()`, `.pop()`, `.push()`, `.reverse()`, `.shift()`, `.sort()`, `.splice()`, or `.unshift()`. Those are often called for their side effects, so reporting them would be much noisier.
41+
42+
This is a syntax-only rule with a narrow inference boundary. It skips some obvious non-arrays such as literals, direct object literals, `String(value)`, `new Foo()`, parameter defaults, and simple declaration-based destructuring, but it intentionally does not reason about object spreads, string destructuring, `for…of` bindings, or other broader value-flow patterns. Unknown values with similarly named methods may still be reported.
43+
44+
## Examples
45+
46+
```js
47+
// ❌
48+
array.map(element => transform(element));
49+
50+
// ✅
51+
const transformed = array.map(element => transform(element));
52+
```
53+
54+
```js
55+
// ❌
56+
array.find(element => element.id === targetId);
57+
58+
// ✅
59+
const match = array.find(element => element.id === targetId);
60+
```
61+
62+
```js
63+
// ✅
64+
void array.map(element => transform(element));
65+
```

‎readme.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ export default [
122122
| [no-unnecessary-slice-end](docs/rules/no-unnecessary-slice-end.md) | Disallow using `.length` or `Infinity` as the `end` argument of `{Array,String,TypedArray}#slice()`. | ✅ ☑️ | 🔧 | |
123123
| [no-unreadable-array-destructuring](docs/rules/no-unreadable-array-destructuring.md) | Disallow unreadable array destructuring. | ✅ ☑️ | 🔧 | |
124124
| [no-unreadable-iife](docs/rules/no-unreadable-iife.md) | Disallow unreadable IIFEs. | ✅ ☑️ | | |
125+
| [no-unused-array-method-return](docs/rules/no-unused-array-method-return.md) | Disallow ignoring the return value of selected array methods. | ✅ ☑️ | | |
125126
| [no-unused-properties](docs/rules/no-unused-properties.md) | Disallow unused object properties. | | | |
126127
| [no-useless-collection-argument](docs/rules/no-useless-collection-argument.md) | Disallow useless values or fallbacks in `Set`, `Map`, `WeakSet`, or `WeakMap`. | ✅ ☑️ | 🔧 | 💡 |
127128
| [no-useless-error-capture-stack-trace](docs/rules/no-useless-error-capture-stack-trace.md) | Disallow unnecessary `Error.captureStackTrace(…)`. | ✅ ☑️ | 🔧 | |

‎rules/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ export {default as 'no-unnecessary-polyfills'} from './no-unnecessary-polyfills.
6363
export {default as 'no-unnecessary-slice-end'} from './no-unnecessary-slice-end.js';
6464
export {default as 'no-unreadable-array-destructuring'} from './no-unreadable-array-destructuring.js';
6565
export {default as 'no-unreadable-iife'} from './no-unreadable-iife.js';
66+
export {default as 'no-unused-array-method-return'} from './no-unused-array-method-return.js';
6667
export {default as 'no-unused-properties'} from './no-unused-properties.js';
6768
export {default as 'no-useless-collection-argument'} from './no-useless-collection-argument.js';
6869
export {default as 'no-useless-error-capture-stack-trace'} from './no-useless-error-capture-stack-trace.js';

‎rules/no-unused-array-method-return.js‎

Lines changed: 301 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,301 @@
1+
import {findVariable, getPropertyName} from '@eslint-community/eslint-utils';
2+
import {isValueNotUsable} from './utils/index.js';
3+
4+
const MESSAGE_ID = 'no-unused-array-method-return';
5+
const messages = {
6+
[MESSAGE_ID]: 'Do not ignore the return value of `.{{method}}(…)`.',
7+
};
8+
9+
const methods = new Set([
10+
'at',
11+
'concat',
12+
'entries',
13+
'every',
14+
'filter',
15+
'find',
16+
'findIndex',
17+
'findLast',
18+
'findLastIndex',
19+
'flat',
20+
'flatMap',
21+
'includes',
22+
'indexOf',
23+
'join',
24+
'keys',
25+
'lastIndexOf',
26+
'map',
27+
// Using these as short-circuiting `forEach()` alternatives is an anti-pattern.
28+
'some',
29+
'slice',
30+
// This list is the implementation contract. We intentionally exclude
31+
// `toString()` and `toLocaleString()` because they exist on almost every
32+
// object, and tracking them in this syntax-only rule creates too many
33+
// non-array false positives.
34+
'toReversed',
35+
'toSorted',
36+
'toSpliced',
37+
'values',
38+
'with',
39+
]);
40+
41+
const pascalCaseNamePattern = /^\p{Lu}/v;
42+
const uncertainValue = Symbol('uncertainValue');
43+
const nonArrayFactoryFunctions = new Set([
44+
'BigInt',
45+
'Boolean',
46+
'Number',
47+
'RegExp',
48+
'String',
49+
]);
50+
51+
const isPascalCaseIdentifier = node =>
52+
node.type === 'Identifier'
53+
&& pascalCaseNamePattern.test(node.name);
54+
55+
const isGlobalIdentifier = (node, name, context) =>
56+
node.type === 'Identifier'
57+
&& node.name === name
58+
&& context.sourceCode.isGlobalReference(node);
59+
60+
const isUndefined = (node, context) =>
61+
isGlobalIdentifier(node, 'undefined', context);
62+
63+
// Treat `new Foo()` as non-array unless it is the global `Array`. Local `Array`
64+
// subclasses are intentionally out of scope for this best-effort inference.
65+
const isKnownNonArrayConstruction = (node, context) =>
66+
node.type === 'NewExpression'
67+
&& node.callee.type === 'Identifier'
68+
&& !isGlobalIdentifier(node.callee, 'Array', context);
69+
70+
const isKnownNonArrayFactoryCall = (node, context) =>
71+
node.type === 'CallExpression'
72+
&& node.callee.type === 'Identifier'
73+
&& nonArrayFactoryFunctions.has(node.callee.name)
74+
&& context.sourceCode.isGlobalReference(node.callee);
75+
76+
const isDefinitelyArrayExpression = (node, context) =>
77+
node.type === 'ArrayExpression'
78+
|| (
79+
(node.type === 'CallExpression' || node.type === 'NewExpression')
80+
&& isGlobalIdentifier(node.callee, 'Array', context)
81+
);
82+
83+
const isDefinitelyNonArrayExpression = (node, context) =>
84+
isUndefined(node, context)
85+
|| node.type === 'ObjectExpression'
86+
|| node.type === 'Literal'
87+
|| node.type === 'TemplateLiteral'
88+
|| node.type === 'ArrowFunctionExpression'
89+
|| node.type === 'FunctionExpression'
90+
|| node.type === 'ClassExpression'
91+
|| isKnownNonArrayConstruction(node, context)
92+
|| isKnownNonArrayFactoryCall(node, context);
93+
94+
function getVariable(node, context) {
95+
if (node.type !== 'Identifier') {
96+
return;
97+
}
98+
99+
return findVariable(context.sourceCode.getScope(node), node);
100+
}
101+
102+
function hasEarlierWrite(variable, node, context) {
103+
if (!variable) {
104+
return false;
105+
}
106+
107+
const [nodeStart] = context.sourceCode.getRange(node);
108+
109+
return variable.references.some(reference => !reference.init && reference.isWrite() && context.sourceCode.getRange(reference.identifier)[0] < nodeStart);
110+
}
111+
112+
function getVariableValue(node, context) {
113+
const variable = getVariable(node, context);
114+
if (!variable) {
115+
return;
116+
}
117+
118+
if (variable.defs.length !== 1) {
119+
return uncertainValue;
120+
}
121+
122+
// Supported variable inference boundary:
123+
// - exactly one binding definition
124+
// - a `VariableDeclarator` whose id is the same identifier we are resolving
125+
// - the original declarator initializer only
126+
//
127+
// Unsupported on purpose:
128+
// - any destructuring, including defaults
129+
// - any write before the call site
130+
// - aliasing through another identifier
131+
// - parameter defaults, `for…of`, catch bindings, and any non-declarator binding
132+
// - control-flow-sensitive value tracking
133+
//
134+
// This is intentionally extremely small. The rule only trusts the initializer
135+
// syntax of `const value = ...` or `let value = ...` when the binding has not
136+
// been written again. Everything else stays unresolved on purpose.
137+
if (hasEarlierWrite(variable, node, context)) {
138+
return uncertainValue;
139+
}
140+
141+
const [definition] = variable.defs;
142+
if (
143+
definition.type === 'Variable'
144+
&& definition.node.type === 'VariableDeclarator'
145+
&& definition.node.id.type === 'Identifier'
146+
&& definition.node.id.name === node.name
147+
&& definition.node.init
148+
&& definition.parent.type === 'VariableDeclaration'
149+
) {
150+
return definition.node.init;
151+
}
152+
153+
return uncertainValue;
154+
}
155+
156+
function getStaticPropertyName(node, context) {
157+
return getPropertyName(node, context.sourceCode.getScope(node));
158+
}
159+
160+
function resolveReceiver(node, context, visitedNodes = new Set()) {
161+
if (!node || node === uncertainValue) {
162+
return node;
163+
}
164+
165+
if (visitedNodes.has(node)) {
166+
return node;
167+
}
168+
169+
visitedNodes.add(node);
170+
171+
if (node.type === 'Identifier') {
172+
const value = getVariableValue(node, context);
173+
if (value === uncertainValue) {
174+
return value;
175+
}
176+
177+
return value === undefined ? node : resolveReceiver(value, context, visitedNodes);
178+
}
179+
180+
if (node.type === 'ChainExpression') {
181+
return resolveReceiver(node.expression, context, visitedNodes);
182+
}
183+
184+
if (node.type === 'MemberExpression') {
185+
return uncertainValue;
186+
}
187+
188+
// Supported receiver inference boundary:
189+
// - the receiver expression itself, if it is direct array syntax like `[]`
190+
// - trivial identifier aliases to that same initializer, like `const alias = values`
191+
//
192+
// Unsupported on purpose:
193+
// - any destructuring, including defaults
194+
// - any member/property receiver, including `wrapper.items`, `alias.items`, and `this.items`
195+
// - any object, class-field, or `this`-based inference
196+
// - any class field or constructor reasoning, even when `this.items = []` looks obvious
197+
// - any write before the call site
198+
// - any "latest value" reconstruction after assignments
199+
//
200+
// This comment is intentionally blunt because this boundary is the feature:
201+
// the rule is not a general value tracker anymore. If a case requires
202+
// following properties, destructuring, or writes, we leave it unresolved.
203+
return node;
204+
}
205+
206+
const isObviouslyNonArrayReceiver = (node, context) => {
207+
node = resolveReceiver(node, context);
208+
209+
return node === uncertainValue
210+
|| isDefinitelyNonArrayExpression(node, context)
211+
|| (
212+
isPascalCaseIdentifier(node)
213+
&& !isDefinitelyArrayExpression(node, context)
214+
);
215+
};
216+
217+
const getTrackedMethodName = (node, context) =>
218+
node.callee.type === 'MemberExpression'
219+
? getStaticPropertyName(node.callee, context)
220+
: undefined;
221+
222+
// Supported discarded-value boundary:
223+
// - direct unused expressions handled by `isValueNotUsable()`
224+
// - `await foo.map()` when the awaited expression is itself directly discarded
225+
// - TypeScript assertion wrappers around that same direct discard site
226+
// - direct `for` init/update expressions like `for (foo.map(); ; )` and `for (; ; foo.map())`
227+
//
228+
// Unsupported on purpose:
229+
// - comma-expression wrappers
230+
// - logical wrappers like `condition && foo.map()`
231+
// - conditional wrappers like `condition ? foo.map() : other()`
232+
// - any other parent-expression pattern not listed above
233+
//
234+
// The rule stops after this short fixed wrapper list. We intentionally do not
235+
// keep climbing through arbitrary parent expressions just to catch one more
236+
// nested discard shape.
237+
const isDiscardedExpression = node => {
238+
while (true) {
239+
if (isValueNotUsable(node)) {
240+
return true;
241+
}
242+
243+
const {parent} = node;
244+
if (
245+
parent.type === 'ForStatement'
246+
&& (parent.init === node || parent.update === node)
247+
) {
248+
return true;
249+
}
250+
251+
if (
252+
parent.type !== 'ChainExpression'
253+
&& parent.type !== 'AwaitExpression'
254+
&& parent.type !== 'TSAsExpression'
255+
&& parent.type !== 'TSTypeAssertion'
256+
&& parent.type !== 'TSNonNullExpression'
257+
&& parent.type !== 'TSSatisfiesExpression'
258+
) {
259+
return false;
260+
}
261+
262+
node = parent;
263+
}
264+
};
265+
266+
/** @param {import('eslint').Rule.RuleContext} context */
267+
const create = context => {
268+
context.on('CallExpression', node => {
269+
const method = getTrackedMethodName(node, context);
270+
if (
271+
!methods.has(method)
272+
|| !isDiscardedExpression(node)
273+
|| isObviouslyNonArrayReceiver(node.callee.object, context)
274+
) {
275+
return;
276+
}
277+
278+
return {
279+
node: node.callee.property,
280+
messageId: MESSAGE_ID,
281+
data: {
282+
method,
283+
},
284+
};
285+
});
286+
};
287+
288+
/** @type {import('eslint').Rule.RuleModule} */
289+
const config = {
290+
create,
291+
meta: {
292+
type: 'suggestion',
293+
docs: {
294+
description: 'Disallow ignoring the return value of selected array methods.',
295+
recommended: 'unopinionated',
296+
},
297+
messages,
298+
},
299+
};
300+
301+
export default config;

0 commit comments

Comments
 (0)