Skip to content

Commit 767f1be

Browse files
ChamionChamion
authored
fix(await-async-events): false positive reports on awaited expressions evaluating to promise (#890)
1 parent 6b39e60 commit 767f1be

3 files changed

Lines changed: 173 additions & 24 deletions

File tree

‎lib/node-utils/index.ts‎

Lines changed: 47 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -222,40 +222,63 @@ export function isPromiseHandled(nodeIdentifier: TSESTree.Identifier): boolean {
222222
nodeIdentifier,
223223
true
224224
);
225+
const callRootExpression =
226+
closestCallExpressionNode == null
227+
? null
228+
: getRootExpression(closestCallExpressionNode);
225229

226-
const suspiciousNodes = [nodeIdentifier, closestCallExpressionNode].filter(
227-
Boolean
230+
const suspiciousNodes = [nodeIdentifier, callRootExpression].filter(
231+
(node): node is NonNullable<typeof node> => node != null
228232
);
229233

230-
for (const node of suspiciousNodes) {
231-
if (!node?.parent) {
232-
continue;
233-
}
234-
if (ASTUtils.isAwaitExpression(node.parent)) {
235-
return true;
236-
}
237-
234+
return suspiciousNodes.some((node) => {
235+
if (!node.parent) return false;
236+
if (ASTUtils.isAwaitExpression(node.parent)) return true;
238237
if (
239238
isArrowFunctionExpression(node.parent) ||
240239
isReturnStatement(node.parent)
241-
) {
242-
return true;
243-
}
244-
245-
if (hasClosestExpectResolvesRejects(node.parent)) {
246-
return true;
247-
}
248-
249-
if (hasChainedThen(node)) {
240+
)
250241
return true;
251-
}
242+
if (hasClosestExpectResolvesRejects(node.parent)) return true;
243+
if (hasChainedThen(node)) return true;
244+
if (isPromisesArrayResolved(node)) return true;
245+
});
246+
}
252247

253-
if (isPromisesArrayResolved(node)) {
254-
return true;
248+
/**
249+
* For an expression in a parent that evaluates to the expression or another child returns the parent node recursively.
250+
*/
251+
function getRootExpression(
252+
expression: TSESTree.Expression
253+
): TSESTree.Expression {
254+
const { parent } = expression;
255+
if (parent == null) return expression;
256+
switch (parent.type) {
257+
case AST_NODE_TYPES.ConditionalExpression:
258+
return getRootExpression(parent);
259+
case AST_NODE_TYPES.LogicalExpression: {
260+
let rootExpression;
261+
switch (parent.operator) {
262+
case '??':
263+
case '||':
264+
rootExpression = getRootExpression(parent);
265+
break;
266+
case '&&':
267+
rootExpression =
268+
parent.right === expression
269+
? getRootExpression(parent)
270+
: expression;
271+
break;
272+
}
273+
return rootExpression ?? expression;
255274
}
275+
case AST_NODE_TYPES.SequenceExpression:
276+
return parent.expressions[parent.expressions.length - 1] === expression
277+
? getRootExpression(parent)
278+
: expression;
279+
default:
280+
return expression;
256281
}
257-
258-
return false;
259282
}
260283

261284
export function getVariableReferences(

‎tests/lib/rules/await-async-events.test.ts‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,19 @@ ruleTester.run(RULE_NAME, rule, {
311311
312312
await triggerEvent()
313313
})
314+
`,
315+
options: [{ eventModule: 'userEvent' }] as const,
316+
})),
317+
...USER_EVENT_ASYNC_FUNCTIONS.map((eventMethod) => ({
318+
code: `
319+
import userEvent from '${testingFramework}'
320+
test('await expression that evaluates to promise is valid', async () => {
321+
await (null, userEvent.${eventMethod}(getByLabelText('username')));
322+
await (condition ? null : userEvent.${eventMethod}(getByLabelText('username')));
323+
await (condition && userEvent.${eventMethod}(getByLabelText('username')));
324+
await (userEvent.${eventMethod}(getByLabelText('username')) || userEvent.${eventMethod}(getByLabelText('username')));
325+
await (userEvent.${eventMethod}(getByLabelText('username')) ?? userEvent.${eventMethod}(getByLabelText('username')));
326+
})
314327
`,
315328
options: [{ eventModule: 'userEvent' }] as const,
316329
})),
@@ -960,6 +973,92 @@ ruleTester.run(RULE_NAME, rule, {
960973
}
961974
962975
triggerEvent()
976+
`,
977+
} as const)
978+
),
979+
...USER_EVENT_ASYNC_FUNCTIONS.map(
980+
(eventMethod) =>
981+
({
982+
code: `
983+
import userEvent from '${testingFramework}'
984+
test('unhandled expression that evaluates to promise is invalid', () => {
985+
condition ? null : (null, true && userEvent.${eventMethod}(getByLabelText('username')));
986+
});
987+
`,
988+
errors: [
989+
{
990+
line: 4,
991+
column: 38,
992+
messageId: 'awaitAsyncEvent',
993+
data: { name: eventMethod },
994+
},
995+
],
996+
options: [{ eventModule: 'userEvent' }],
997+
output: `
998+
import userEvent from '${testingFramework}'
999+
test('unhandled expression that evaluates to promise is invalid', async () => {
1000+
condition ? null : (null, true && await userEvent.${eventMethod}(getByLabelText('username')));
1001+
});
1002+
`,
1003+
} as const)
1004+
),
1005+
...USER_EVENT_ASYNC_FUNCTIONS.map(
1006+
(eventMethod) =>
1007+
({
1008+
code: `
1009+
import userEvent from '${testingFramework}'
1010+
test('handled AND expression with left promise is invalid', async () => {
1011+
await (userEvent.${eventMethod}(getByLabelText('username')) && userEvent.${eventMethod}(getByLabelText('username')));
1012+
});
1013+
`,
1014+
errors: [
1015+
{
1016+
line: 4,
1017+
column: 11,
1018+
messageId: 'awaitAsyncEvent',
1019+
data: { name: eventMethod },
1020+
},
1021+
],
1022+
options: [{ eventModule: 'userEvent' }],
1023+
output: `
1024+
import userEvent from '${testingFramework}'
1025+
test('handled AND expression with left promise is invalid', async () => {
1026+
await (await userEvent.${eventMethod}(getByLabelText('username')) && userEvent.${eventMethod}(getByLabelText('username')));
1027+
});
1028+
`,
1029+
} as const)
1030+
),
1031+
...USER_EVENT_ASYNC_FUNCTIONS.map(
1032+
(eventMethod) =>
1033+
({
1034+
code: `
1035+
import userEvent from '${testingFramework}'
1036+
test('voided promise is invalid', async () => {
1037+
await void userEvent.${eventMethod}(getByLabelText('username'));
1038+
await (userEvent.${eventMethod}(getByLabelText('username')), null);
1039+
});
1040+
`,
1041+
errors: [
1042+
{
1043+
line: 4,
1044+
column: 15,
1045+
messageId: 'awaitAsyncEvent',
1046+
data: { name: eventMethod },
1047+
},
1048+
{
1049+
line: 5,
1050+
column: 11,
1051+
messageId: 'awaitAsyncEvent',
1052+
data: { name: eventMethod },
1053+
},
1054+
],
1055+
options: [{ eventModule: 'userEvent' }],
1056+
output: `
1057+
import userEvent from '${testingFramework}'
1058+
test('voided promise is invalid', async () => {
1059+
await void await userEvent.${eventMethod}(getByLabelText('username'));
1060+
await (await userEvent.${eventMethod}(getByLabelText('username')), null);
1061+
});
9631062
`,
9641063
} as const)
9651064
),

‎tests/lib/rules/await-async-utils.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,33 @@ ruleTester.run(RULE_NAME, rule, {
418418
doSomethingElse(aPromise);
419419
${asyncUtil}(() => getByLabelText('email'));
420420
});
421+
`,
422+
errors: [
423+
{
424+
line: 4,
425+
column: 28,
426+
messageId: 'awaitAsyncUtil',
427+
data: { name: asyncUtil },
428+
},
429+
{
430+
line: 6,
431+
column: 11,
432+
messageId: 'awaitAsyncUtil',
433+
data: { name: asyncUtil },
434+
},
435+
],
436+
} as const)
437+
),
438+
...ASYNC_UTILS.map(
439+
(asyncUtil) =>
440+
({
441+
code: `
442+
import { ${asyncUtil} } from '${testingFramework}';
443+
test('unhandled expression that evaluates to promise is invalid', () => {
444+
const aPromise = ${asyncUtil}(() => getByLabelText('username'));
445+
doSomethingElse(aPromise);
446+
${asyncUtil}(() => getByLabelText('email'));
447+
});
421448
`,
422449
errors: [
423450
{

0 commit comments

Comments
 (0)