Skip to content

Commit

Permalink
suggest instead of fix when result is stored in array
Browse files Browse the repository at this point in the history
  • Loading branch information
Clement398 committed Jun 21, 2024
1 parent 69d91a8 commit 6be6248
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 167 deletions.
21 changes: 12 additions & 9 deletions rules/no-single-promise-in-promise-methods.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,8 @@ const isPromiseMethodCallWithSingleElementArray = node =>
&& node.arguments[0].elements[0]
&& node.arguments[0].elements[0].type !== 'SpreadElement';

const isVariableAssignment = node => ['VariableDeclarator', 'AssignmentExpression'].includes(node.parent.type);

const isStoredInVariable = node => isVariableAssignment(node)
|| (node.parent.type === 'AwaitExpression' && isVariableAssignment(node.parent));

const isStoredInArray = node => isMethodCall(node, {methods: ['all']}) && isStoredInVariable(node);
const isStoredInArray = node => isMethodCall(node, {methods: ['all']})
&& ['VariableDeclarator', 'AssignmentExpression'].includes(node.parent.parent.type);

const unwrapAwaitedCallExpression = (callExpression, sourceCode) => fixer => {
const [promiseNode] = callExpression.arguments[0].elements;
Expand Down Expand Up @@ -122,8 +118,7 @@ const switchToPromiseResolve = (callExpression, sourceCode) => function * (fixer
/** @param {import('eslint').Rule.RuleContext} context */
const create = context => ({
CallExpression(callExpression) {
if (!isPromiseMethodCallWithSingleElementArray(callExpression)
|| isStoredInArray(callExpression)) {
if (!isPromiseMethodCallWithSingleElementArray(callExpression)) {
return;
}

Expand All @@ -141,7 +136,15 @@ const create = context => ({
callExpression.parent.type === 'AwaitExpression'
&& callExpression.parent.argument === callExpression
) {
problem.fix = unwrapAwaitedCallExpression(callExpression, sourceCode);
if (isStoredInArray(callExpression)) {
problem.suggest = [{
messageId: MESSAGE_ID_SUGGESTION_UNWRAP,
fix: unwrapAwaitedCallExpression(callExpression, sourceCode),
}];
} else {
problem.fix = unwrapAwaitedCallExpression(callExpression, sourceCode);
}

return problem;
}

Expand Down
12 changes: 2 additions & 10 deletions test/no-single-promise-in-promise-methods.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ const {test} = getTester(import.meta);
// `await`ed
test.snapshot({
valid: [
'const results = await Promise.all([promise])',
'results = await Promise.all([promise])',
],
invalid: [
'await Promise.all([(0, promise)])',
Expand Down Expand Up @@ -41,10 +39,10 @@ test.snapshot({
'await Promise.race([promise])',
'await Promise.all([new Promise(() => {})])',
'+await Promise.all([+1])',
'const results = await Promise.all([promise])',
'results = await Promise.all([promise])',
'const results = await Promise.any([promise])',
'results = await Promise.any([promise])',
'const results = await Promise.race([promise])',
'results = await Promise.race([promise])',

// ASI, `Promise.all()` is not really `await`ed
outdent`
Expand Down Expand Up @@ -72,8 +70,6 @@ test.snapshot({
'Promise.all([promise], extraArguments)',
'Promise.all()',
'new Promise.all([promise])',
'const results = Promise.all([promise])',
'results = Promise.all([promise])',

// We are not checking these cases
'globalThis.Promise.all([promise])',
Expand Down Expand Up @@ -108,9 +104,5 @@ test.snapshot({
'Promise.all([promise])[0] ||= 1',
'Promise.all([undefined]).then()',
'Promise.all([null]).then()',
'const results = Promise.any([promise])',
'results = Promise.any([promise])',
'const results = Promise.race([promise])',
'results = Promise.race([promise])',
],
});
173 changes: 25 additions & 148 deletions test/snapshots/no-single-promise-in-promise-methods.mjs.md
Original file line number Diff line number Diff line change
Expand Up @@ -655,54 +655,50 @@ Generated by [AVA](https://avajs.dev).
| ^^^^ Wrapping single-element array with \`Promise.all()\` is unnecessary.␊
`

## invalid(32): const results = await Promise.any([promise])
## invalid(32): const results = await Promise.all([promise])

> Input
`␊
1 | const results = await Promise.any([promise])␊
`

> Output
`␊
1 | const results = await promise␊
1 | const results = await Promise.all([promise])␊
`

> Error 1/1
`␊
> 1 | const results = await Promise.any([promise])␊
| ^^^^^^^^^ Wrapping single-element array with \`Promise.any()\` is unnecessary.␊
> 1 | const results = await Promise.all([promise])␊
| ^^^^^^^^^ Wrapping single-element array with \`Promise.all()\` is unnecessary.␊
--------------------------------------------------------------------------------␊
Suggestion 1/1: Use the value directly.␊
1 | const results = await promise␊
`

## invalid(33): results = await Promise.any([promise])
## invalid(33): results = await Promise.all([promise])

> Input
`␊
1 | results = await Promise.any([promise])␊
`

> Output
`␊
1 | results = await promise␊
1 | results = await Promise.all([promise])␊
`

> Error 1/1
`␊
> 1 | results = await Promise.any([promise])␊
| ^^^^^^^^^ Wrapping single-element array with \`Promise.any()\` is unnecessary.␊
> 1 | results = await Promise.all([promise])␊
| ^^^^^^^^^ Wrapping single-element array with \`Promise.all()\` is unnecessary.␊
--------------------------------------------------------------------------------␊
Suggestion 1/1: Use the value directly.␊
1 | results = await promise␊
`

## invalid(34): const results = await Promise.race([promise])
## invalid(34): const results = await Promise.any([promise])

> Input
`␊
1 | const results = await Promise.race([promise])␊
1 | const results = await Promise.any([promise])␊
`

> Output
Expand All @@ -714,29 +710,29 @@ Generated by [AVA](https://avajs.dev).
> Error 1/1
`␊
> 1 | const results = await Promise.race([promise])␊
| ^^^^^^^^^ Wrapping single-element array with \`Promise.race()\` is unnecessary.␊
> 1 | const results = await Promise.any([promise])␊
| ^^^^^^^^^ Wrapping single-element array with \`Promise.any()\` is unnecessary.␊
`

## invalid(35): results = await Promise.race([promise])
## invalid(35): const results = await Promise.race([promise])

> Input
`␊
1 | results = await Promise.race([promise])␊
1 | const results = await Promise.race([promise])␊
`

> Output
`␊
1 | results = await promise␊
1 | const results = await promise␊
`

> Error 1/1
`␊
> 1 | results = await Promise.race([promise])␊
| ^^^^^^^^^ Wrapping single-element array with \`Promise.race()\` is unnecessary.␊
> 1 | const results = await Promise.race([promise])␊
| ^^^^^^^^^ Wrapping single-element array with \`Promise.race()\` is unnecessary.␊
`

## invalid(36): await Promise.all([(x,y)]) [0].toString()
Expand Down Expand Up @@ -1210,122 +1206,3 @@ Generated by [AVA](https://avajs.dev).
Suggestion 2/2: Switch to \`Promise.resolve(…)\`.␊
1 | Promise.resolve(null).then()␊
`

## invalid(20): const results = Promise.any([promise])

> Input
`␊
1 | const results = Promise.any([promise])␊
`

> Error 1/1
`␊
> 1 | const results = Promise.any([promise])␊
| ^^^^^^^^^ Wrapping single-element array with \`Promise.any()\` is unnecessary.␊
--------------------------------------------------------------------------------␊
Suggestion 1/2: Use the value directly.␊
1 | const results = promise␊
--------------------------------------------------------------------------------␊
Suggestion 2/2: Switch to \`Promise.resolve(…)\`.␊
1 | const results = Promise.resolve(promise)␊
`

## invalid(21): results = Promise.any([promise])

> Input
`␊
1 | results = Promise.any([promise])␊
`

> Error 1/1
`␊
> 1 | results = Promise.any([promise])␊
| ^^^^^^^^^ Wrapping single-element array with \`Promise.any()\` is unnecessary.␊
--------------------------------------------------------------------------------␊
Suggestion 1/2: Use the value directly.␊
1 | results = promise␊
--------------------------------------------------------------------------------␊
Suggestion 2/2: Switch to \`Promise.resolve(…)\`.␊
1 | results = Promise.resolve(promise)␊
`

## invalid(22): const results = Promise.race([promise])

> Input
`␊
1 | const results = Promise.race([promise])␊
`

> Error 1/1
`␊
> 1 | const results = Promise.race([promise])␊
| ^^^^^^^^^ Wrapping single-element array with \`Promise.race()\` is unnecessary.␊
--------------------------------------------------------------------------------␊
Suggestion 1/2: Use the value directly.␊
1 | const results = promise␊
--------------------------------------------------------------------------------␊
Suggestion 2/2: Switch to \`Promise.resolve(…)\`.␊
1 | const results = Promise.resolve(promise)␊
`

## invalid(23): results = Promise.race([promise])

> Input
`␊
1 | results = Promise.race([promise])␊
`

> Error 1/1
`␊
> 1 | results = Promise.race([promise])␊
| ^^^^^^^^^ Wrapping single-element array with \`Promise.race()\` is unnecessary.␊
--------------------------------------------------------------------------------␊
Suggestion 1/2: Use the value directly.␊
1 | results = promise␊
--------------------------------------------------------------------------------␊
Suggestion 2/2: Switch to \`Promise.resolve(…)\`.␊
1 | results = Promise.resolve(promise)␊
`

## invalid(32): await Promise.all([(x,y)]) [0].toString()

> Input
`␊
1 | await Promise.all([(x,y)])␊
2 | [0].toString()␊
`

> Error 1/1
`␊
> 1 | await Promise.all([(x,y)])␊
| ^^^^^^^ Wrapping single-element array with \`Promise.all()\` is unnecessary.␊
2 | [0].toString()␊
--------------------------------------------------------------------------------␊
Suggestion 1/2: Use the value directly.␊
1 | await (x,y)␊
2 | [0].toString()␊
--------------------------------------------------------------------------------␊
Suggestion 2/2: Switch to \`Promise.resolve(…)\`.␊
1 | await Promise.resolve((x,y))␊
2 | [0].toString()␊
`
Binary file modified test/snapshots/no-single-promise-in-promise-methods.mjs.snap
Binary file not shown.

0 comments on commit 6be6248

Please sign in to comment.