Skip to content

Commit 8d28b6e

Browse files
Clement398fisker
andauthoredJun 22, 2024··
no-single-promise-in-promise-methods: Remove broken autofix for Promise.all() (#2386)
Co-authored-by: fisker <lionkay@gmail.com>
1 parent a45b24a commit 8d28b6e

4 files changed

+510
-269
lines changed
 

‎rules/no-single-promise-in-promise-methods.js

+22-9
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22
const {
33
isCommaToken,
44
} = require('@eslint-community/eslint-utils');
5-
const {isMethodCall} = require('./ast/index.js');
5+
const {
6+
isMethodCall,
7+
isExpressionStatement,
8+
} = require('./ast/index.js');
69
const {
710
getParenthesizedText,
811
isParenthesized,
@@ -77,8 +80,8 @@ const unwrapNonAwaitedCallExpression = (callExpression, sourceCode) => fixer =>
7780
const switchToPromiseResolve = (callExpression, sourceCode) => function * (fixer) {
7881
/*
7982
```
80-
Promise.all([promise,])
81-
// ^^^ methodNameNode
83+
Promise.race([promise,])
84+
// ^^^^ methodNameNode
8285
```
8386
*/
8487
const methodNameNode = callExpression.callee.property;
@@ -87,16 +90,16 @@ const switchToPromiseResolve = (callExpression, sourceCode) => function * (fixer
8790
const [arrayExpression] = callExpression.arguments;
8891
/*
8992
```
90-
Promise.all([promise,])
91-
// ^ openingBracketToken
93+
Promise.race([promise,])
94+
// ^ openingBracketToken
9295
```
9396
*/
9497
const openingBracketToken = sourceCode.getFirstToken(arrayExpression);
9598
/*
9699
```
97-
Promise.all([promise,])
98-
// ^ penultimateToken
99-
// ^ closingBracketToken
100+
Promise.race([promise,])
101+
// ^ penultimateToken
102+
// ^ closingBracketToken
100103
```
101104
*/
102105
const [
@@ -119,11 +122,13 @@ const create = context => ({
119122
return;
120123
}
121124

125+
const methodName = callExpression.callee.property.name;
126+
122127
const problem = {
123128
node: callExpression.arguments[0],
124129
messageId: MESSAGE_ID_ERROR,
125130
data: {
126-
method: callExpression.callee.property.name,
131+
method: methodName,
127132
},
128133
};
129134

@@ -132,11 +137,19 @@ const create = context => ({
132137
if (
133138
callExpression.parent.type === 'AwaitExpression'
134139
&& callExpression.parent.argument === callExpression
140+
&& (
141+
methodName !== 'all'
142+
|| isExpressionStatement(callExpression.parent.parent)
143+
)
135144
) {
136145
problem.fix = unwrapAwaitedCallExpression(callExpression, sourceCode);
137146
return problem;
138147
}
139148

149+
if (methodName === 'all') {
150+
return problem;
151+
}
152+
140153
problem.suggest = [
141154
{
142155
messageId: MESSAGE_ID_SUGGESTION_UNWRAP,

‎test/no-single-promise-in-promise-methods.mjs

+89-64
Original file line numberDiff line numberDiff line change
@@ -8,41 +8,41 @@ test.snapshot({
88
valid: [
99
],
1010
invalid: [
11-
'await Promise.all([(0, promise)])',
12-
'async function * foo() {await Promise.all([yield promise])}',
13-
'async function * foo() {await Promise.all([yield* promise])}',
14-
'await Promise.all([() => promise,],)',
15-
'await Promise.all([a ? b : c,],)',
16-
'await Promise.all([x ??= y,],)',
17-
'await Promise.all([x ||= y,],)',
18-
'await Promise.all([x &&= y,],)',
19-
'await Promise.all([x |= y,],)',
20-
'await Promise.all([x ^= y,],)',
21-
'await Promise.all([x ??= y,],)',
22-
'await Promise.all([x ||= y,],)',
23-
'await Promise.all([x &&= y,],)',
24-
'await Promise.all([x | y,],)',
25-
'await Promise.all([x ^ y,],)',
26-
'await Promise.all([x & y,],)',
27-
'await Promise.all([x !== y,],)',
28-
'await Promise.all([x == y,],)',
29-
'await Promise.all([x in y,],)',
30-
'await Promise.all([x >>> y,],)',
31-
'await Promise.all([x + y,],)',
32-
'await Promise.all([x / y,],)',
33-
'await Promise.all([x ** y,],)',
34-
'await Promise.all([promise,],)',
35-
'await Promise.all([getPromise(),],)',
36-
'await Promise.all([promises[0],],)',
37-
'await Promise.all([await promise])',
11+
'await Promise.race([(0, promise)])',
12+
'async function * foo() {await Promise.race([yield promise])}',
13+
'async function * foo() {await Promise.race([yield* promise])}',
14+
'await Promise.race([() => promise,],)',
15+
'await Promise.race([a ? b : c,],)',
16+
'await Promise.race([x ??= y,],)',
17+
'await Promise.race([x ||= y,],)',
18+
'await Promise.race([x &&= y,],)',
19+
'await Promise.race([x |= y,],)',
20+
'await Promise.race([x ^= y,],)',
21+
'await Promise.race([x ??= y,],)',
22+
'await Promise.race([x ||= y,],)',
23+
'await Promise.race([x &&= y,],)',
24+
'await Promise.race([x | y,],)',
25+
'await Promise.race([x ^ y,],)',
26+
'await Promise.race([x & y,],)',
27+
'await Promise.race([x !== y,],)',
28+
'await Promise.race([x == y,],)',
29+
'await Promise.race([x in y,],)',
30+
'await Promise.race([x >>> y,],)',
31+
'await Promise.race([x + y,],)',
32+
'await Promise.race([x / y,],)',
33+
'await Promise.race([x ** y,],)',
34+
'await Promise.race([promise,],)',
35+
'await Promise.race([getPromise(),],)',
36+
'await Promise.race([promises[0],],)',
37+
'await Promise.race([await promise])',
3838
'await Promise.any([promise])',
3939
'await Promise.race([promise])',
40-
'await Promise.all([new Promise(() => {})])',
41-
'+await Promise.all([+1])',
40+
'await Promise.race([new Promise(() => {})])',
41+
'+await Promise.race([+1])',
4242

43-
// ASI, `Promise.all()` is not really `await`ed
43+
// ASI, `Promise.race()` is not really `await`ed
4444
outdent`
45-
await Promise.all([(x,y)])
45+
await Promise.race([(x,y)])
4646
[0].toString()
4747
`,
4848
],
@@ -51,54 +51,79 @@ test.snapshot({
5151
// Not `await`ed
5252
test.snapshot({
5353
valid: [
54-
'Promise.all([promise, anotherPromise])',
55-
'Promise.all(notArrayLiteral)',
56-
'Promise.all([...promises])',
54+
'Promise.race([promise, anotherPromise])',
55+
'Promise.race(notArrayLiteral)',
56+
'Promise.race([...promises])',
5757
'Promise.any([promise, anotherPromise])',
5858
'Promise.race([promise, anotherPromise])',
5959
'Promise.notListedMethod([promise])',
60-
'Promise[all]([promise])',
61-
'Promise.all([,])',
62-
'NotPromise.all([promise])',
63-
'Promise?.all([promise])',
64-
'Promise.all?.([promise])',
65-
'Promise.all(...[promise])',
66-
'Promise.all([promise], extraArguments)',
67-
'Promise.all()',
68-
'new Promise.all([promise])',
60+
'Promise[race]([promise])',
61+
'Promise.race([,])',
62+
'NotPromise.race([promise])',
63+
'Promise?.race([promise])',
64+
'Promise.race?.([promise])',
65+
'Promise.race(...[promise])',
66+
'Promise.race([promise], extraArguments)',
67+
'Promise.race()',
68+
'new Promise.race([promise])',
6969

7070
// We are not checking these cases
71-
'globalThis.Promise.all([promise])',
72-
'Promise["all"]([promise])',
71+
'globalThis.Promise.race([promise])',
72+
'Promise["race"]([promise])',
7373

7474
// This can't be checked
7575
'Promise.allSettled([promise])',
7676
],
7777
invalid: [
78-
'Promise.all([promise,],)',
78+
'Promise.race([promise,],)',
7979
outdent`
8080
foo
81-
Promise.all([(0, promise),],)
81+
Promise.race([(0, promise),],)
8282
`,
8383
outdent`
8484
foo
85-
Promise.all([[array][0],],)
85+
Promise.race([[array][0],],)
8686
`,
87-
'Promise.all([promise]).then()',
88-
'Promise.all([1]).then()',
89-
'Promise.all([1.]).then()',
90-
'Promise.all([.1]).then()',
91-
'Promise.all([(0, promise)]).then()',
92-
'const _ = () => Promise.all([ a ?? b ,],)',
93-
'Promise.all([ {a} = 1 ,],)',
94-
'Promise.all([ function () {} ,],)',
95-
'Promise.all([ class {} ,],)',
96-
'Promise.all([ new Foo ,],).then()',
97-
'Promise.all([ new Foo ,],).toString',
98-
'foo(Promise.all([promise]))',
99-
'Promise.all([promise]).foo = 1',
100-
'Promise.all([promise])[0] ||= 1',
101-
'Promise.all([undefined]).then()',
102-
'Promise.all([null]).then()',
87+
'Promise.race([promise]).then()',
88+
'Promise.race([1]).then()',
89+
'Promise.race([1.]).then()',
90+
'Promise.race([.1]).then()',
91+
'Promise.race([(0, promise)]).then()',
92+
'const _ = () => Promise.race([ a ?? b ,],)',
93+
'Promise.race([ {a} = 1 ,],)',
94+
'Promise.race([ function () {} ,],)',
95+
'Promise.race([ class {} ,],)',
96+
'Promise.race([ new Foo ,],).then()',
97+
'Promise.race([ new Foo ,],).toString',
98+
'foo(Promise.race([promise]))',
99+
'Promise.race([promise]).foo = 1',
100+
'Promise.race([promise])[0] ||= 1',
101+
'Promise.race([undefined]).then()',
102+
'Promise.race([null]).then()',
103+
],
104+
});
105+
106+
// `Promise.all`
107+
test.snapshot({
108+
valid: [],
109+
invalid: [
110+
// Only fixable if it's in `ExpressionStatement`
111+
'Promise.all([promise])',
112+
'await Promise.all([promise])',
113+
114+
'const foo = () => Promise.all([promise])',
115+
'const foo = await Promise.all([promise])',
116+
'foo = await Promise.all([promise])',
117+
118+
// `Promise.{all, race}()` should not care if the result is used
119+
'const foo = await Promise.race([promise])',
120+
'const foo = () => Promise.race([promise])',
121+
'foo = await Promise.race([promise])',
122+
'const results = await Promise.any([promise])',
123+
'const results = await Promise.race([promise])',
124+
125+
// Fixable, but not provide at this point
126+
// https://github.com/sindresorhus/eslint-plugin-unicorn/issues/2388
127+
'const [foo] = await Promise.all([promise])',
103128
],
104129
});

‎test/snapshots/no-single-promise-in-promise-methods.mjs.md

+399-196
Large diffs are not rendered by default.
Binary file not shown.

0 commit comments

Comments
 (0)
Please sign in to comment.