Skip to content

Commit

Permalink
no-single-promise-in-promise-methods: Remove broken autofix for `Pr…
Browse files Browse the repository at this point in the history 
…omise.all()` (#2386)

Co-authored-by: fisker <[email protected]>
  • Loading branch information
@Clement398 @fisker
Clement398 and fisker authored Jun 22, 2024
1 parent a45b24a commit 8d28b6e
Show file tree
Hide file tree
Showing 4 changed files with 510 additions and 269 deletions.
31 changes: 22 additions & 9 deletions rules/no-single-promise-in-promise-methods.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
const {
isCommaToken,
} = require('@eslint-community/eslint-utils');
const {isMethodCall} = require('./ast/index.js');
const {
isMethodCall,
isExpressionStatement,
} = require('./ast/index.js');
const {
getParenthesizedText,
isParenthesized,
Expand Down Expand Up @@ -77,8 +80,8 @@ const unwrapNonAwaitedCallExpression = (callExpression, sourceCode) => fixer =>
const switchToPromiseResolve = (callExpression, sourceCode) => function * (fixer) {
/*
```
Promise.all([promise,])
// ^^^ methodNameNode
Promise.race([promise,])
// ^^^^ methodNameNode
```
*/
const methodNameNode = callExpression.callee.property;
Expand All @@ -87,16 +90,16 @@ const switchToPromiseResolve = (callExpression, sourceCode) => function * (fixer
const [arrayExpression] = callExpression.arguments;
/*
```
Promise.all([promise,])
// ^ openingBracketToken
Promise.race([promise,])
// ^ openingBracketToken
```
*/
const openingBracketToken = sourceCode.getFirstToken(arrayExpression);
/*
```
Promise.all([promise,])
// ^ penultimateToken
// ^ closingBracketToken
Promise.race([promise,])
// ^ penultimateToken
// ^ closingBracketToken
```
*/
const [
Expand All @@ -119,11 +122,13 @@ const create = context => ({
return;
}

const methodName = callExpression.callee.property.name;

const problem = {
node: callExpression.arguments[0],
messageId: MESSAGE_ID_ERROR,
data: {
method: callExpression.callee.property.name,
method: methodName,
},
};

Expand All @@ -132,11 +137,19 @@ const create = context => ({
if (
callExpression.parent.type === 'AwaitExpression'
&& callExpression.parent.argument === callExpression
&& (
methodName !== 'all'
|| isExpressionStatement(callExpression.parent.parent)
)
) {
problem.fix = unwrapAwaitedCallExpression(callExpression, sourceCode);
return problem;
}

if (methodName === 'all') {
return problem;
}

problem.suggest = [
{
messageId: MESSAGE_ID_SUGGESTION_UNWRAP,
Expand Down
153 changes: 89 additions & 64 deletions test/no-single-promise-in-promise-methods.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,41 +8,41 @@ test.snapshot({
valid: [
],
invalid: [
'await Promise.all([(0, promise)])',
'async function * foo() {await Promise.all([yield promise])}',
'async function * foo() {await Promise.all([yield* promise])}',
'await Promise.all([() => promise,],)',
'await Promise.all([a ? b : c,],)',
'await Promise.all([x ??= y,],)',
'await Promise.all([x ||= y,],)',
'await Promise.all([x &&= y,],)',
'await Promise.all([x |= y,],)',
'await Promise.all([x ^= y,],)',
'await Promise.all([x ??= y,],)',
'await Promise.all([x ||= y,],)',
'await Promise.all([x &&= y,],)',
'await Promise.all([x | y,],)',
'await Promise.all([x ^ y,],)',
'await Promise.all([x & y,],)',
'await Promise.all([x !== y,],)',
'await Promise.all([x == y,],)',
'await Promise.all([x in y,],)',
'await Promise.all([x >>> y,],)',
'await Promise.all([x + y,],)',
'await Promise.all([x / y,],)',
'await Promise.all([x ** y,],)',
'await Promise.all([promise,],)',
'await Promise.all([getPromise(),],)',
'await Promise.all([promises[0],],)',
'await Promise.all([await promise])',
'await Promise.race([(0, promise)])',
'async function * foo() {await Promise.race([yield promise])}',
'async function * foo() {await Promise.race([yield* promise])}',
'await Promise.race([() => promise,],)',
'await Promise.race([a ? b : c,],)',
'await Promise.race([x ??= y,],)',
'await Promise.race([x ||= y,],)',
'await Promise.race([x &&= y,],)',
'await Promise.race([x |= y,],)',
'await Promise.race([x ^= y,],)',
'await Promise.race([x ??= y,],)',
'await Promise.race([x ||= y,],)',
'await Promise.race([x &&= y,],)',
'await Promise.race([x | y,],)',
'await Promise.race([x ^ y,],)',
'await Promise.race([x & y,],)',
'await Promise.race([x !== y,],)',
'await Promise.race([x == y,],)',
'await Promise.race([x in y,],)',
'await Promise.race([x >>> y,],)',
'await Promise.race([x + y,],)',
'await Promise.race([x / y,],)',
'await Promise.race([x ** y,],)',
'await Promise.race([promise,],)',
'await Promise.race([getPromise(),],)',
'await Promise.race([promises[0],],)',
'await Promise.race([await promise])',
'await Promise.any([promise])',
'await Promise.race([promise])',
'await Promise.all([new Promise(() => {})])',
'+await Promise.all([+1])',
'await Promise.race([new Promise(() => {})])',
'+await Promise.race([+1])',

// ASI, `Promise.all()` is not really `await`ed
// ASI, `Promise.race()` is not really `await`ed
outdent`
await Promise.all([(x,y)])
await Promise.race([(x,y)])
[0].toString()
`,
],
Expand All @@ -51,54 +51,79 @@ test.snapshot({
// Not `await`ed
test.snapshot({
valid: [
'Promise.all([promise, anotherPromise])',
'Promise.all(notArrayLiteral)',
'Promise.all([...promises])',
'Promise.race([promise, anotherPromise])',
'Promise.race(notArrayLiteral)',
'Promise.race([...promises])',
'Promise.any([promise, anotherPromise])',
'Promise.race([promise, anotherPromise])',
'Promise.notListedMethod([promise])',
'Promise[all]([promise])',
'Promise.all([,])',
'NotPromise.all([promise])',
'Promise?.all([promise])',
'Promise.all?.([promise])',
'Promise.all(...[promise])',
'Promise.all([promise], extraArguments)',
'Promise.all()',
'new Promise.all([promise])',
'Promise[race]([promise])',
'Promise.race([,])',
'NotPromise.race([promise])',
'Promise?.race([promise])',
'Promise.race?.([promise])',
'Promise.race(...[promise])',
'Promise.race([promise], extraArguments)',
'Promise.race()',
'new Promise.race([promise])',

// We are not checking these cases
'globalThis.Promise.all([promise])',
'Promise["all"]([promise])',
'globalThis.Promise.race([promise])',
'Promise["race"]([promise])',

// This can't be checked
'Promise.allSettled([promise])',
],
invalid: [
'Promise.all([promise,],)',
'Promise.race([promise,],)',
outdent`
foo
Promise.all([(0, promise),],)
Promise.race([(0, promise),],)
`,
outdent`
foo
Promise.all([[array][0],],)
Promise.race([[array][0],],)
`,
'Promise.all([promise]).then()',
'Promise.all([1]).then()',
'Promise.all([1.]).then()',
'Promise.all([.1]).then()',
'Promise.all([(0, promise)]).then()',
'const _ = () => Promise.all([ a ?? b ,],)',
'Promise.all([ {a} = 1 ,],)',
'Promise.all([ function () {} ,],)',
'Promise.all([ class {} ,],)',
'Promise.all([ new Foo ,],).then()',
'Promise.all([ new Foo ,],).toString',
'foo(Promise.all([promise]))',
'Promise.all([promise]).foo = 1',
'Promise.all([promise])[0] ||= 1',
'Promise.all([undefined]).then()',
'Promise.all([null]).then()',
'Promise.race([promise]).then()',
'Promise.race([1]).then()',
'Promise.race([1.]).then()',
'Promise.race([.1]).then()',
'Promise.race([(0, promise)]).then()',
'const _ = () => Promise.race([ a ?? b ,],)',
'Promise.race([ {a} = 1 ,],)',
'Promise.race([ function () {} ,],)',
'Promise.race([ class {} ,],)',
'Promise.race([ new Foo ,],).then()',
'Promise.race([ new Foo ,],).toString',
'foo(Promise.race([promise]))',
'Promise.race([promise]).foo = 1',
'Promise.race([promise])[0] ||= 1',
'Promise.race([undefined]).then()',
'Promise.race([null]).then()',
],
});

// `Promise.all`
test.snapshot({
valid: [],
invalid: [
// Only fixable if it's in `ExpressionStatement`
'Promise.all([promise])',
'await Promise.all([promise])',

'const foo = () => Promise.all([promise])',
'const foo = await Promise.all([promise])',
'foo = await Promise.all([promise])',

// `Promise.{all, race}()` should not care if the result is used
'const foo = await Promise.race([promise])',
'const foo = () => Promise.race([promise])',
'foo = await Promise.race([promise])',
'const results = await Promise.any([promise])',
'const results = await Promise.race([promise])',

// Fixable, but not provide at this point
// https://github.com/sindresorhus/eslint-plugin-unicorn/issues/2388
'const [foo] = await Promise.all([promise])',
],
});
Loading

0 comments on commit 8d28b6e

Please sign in to comment.