From 4d878d12360d7675a61540bd696c1bb624c77066 Mon Sep 17 00:00:00 2001 From: Jemi Salo Date: Thu, 21 Mar 2024 15:04:07 +0200 Subject: [PATCH 1/4] fix: silence false positive await-async-events reports --- lib/node-utils/index.ts | 40 ++++++++++++ lib/rules/await-async-events.ts | 22 ++++--- tests/lib/rules/await-async-events.test.ts | 71 ++++++++++++++++++++++ 3 files changed, 124 insertions(+), 9 deletions(-) diff --git a/lib/node-utils/index.ts b/lib/node-utils/index.ts index 0b41bd4a..909d8a14 100644 --- a/lib/node-utils/index.ts +++ b/lib/node-utils/index.ts @@ -258,6 +258,46 @@ export function isPromiseHandled(nodeIdentifier: TSESTree.Identifier): boolean { return false; } +/** + * For an expression in a parent expression that evaluates to the expression or another child returns the parent node recursively. + */ +function getRootExpression( + expression: TSESTree.Expression +): TSESTree.Expression { + const { parent } = expression; + if (parent == null) return expression; + switch (parent.type) { + case AST_NODE_TYPES.ConditionalExpression: + case AST_NODE_TYPES.LogicalExpression: + return getRootExpression(parent); + case AST_NODE_TYPES.SequenceExpression: + return parent.expressions[parent.expressions.length - 1] === expression + ? getRootExpression(parent) + : expression; + default: + return expression; + } +} + +/** + * Determines whether a given promise expression is considered unhandled. + * + * It will be considered unhandled if an ancestor voids the expression. + */ +export function isPromiseUnhandled(expression: TSESTree.Expression): boolean { + const { parent } = getRootExpression(expression); + if (parent == null) return false; + switch (parent.type) { + case AST_NODE_TYPES.ExpressionStatement: + case AST_NODE_TYPES.SequenceExpression: + case AST_NODE_TYPES.UnaryExpression: + case AST_NODE_TYPES.VariableDeclarator: + return true; + default: + return false; + } +} + export function getVariableReferences( context: TSESLint.RuleContext, node: TSESTree.Node diff --git a/lib/rules/await-async-events.ts b/lib/rules/await-async-events.ts index 6ca29473..25a56efa 100644 --- a/lib/rules/await-async-events.ts +++ b/lib/rules/await-async-events.ts @@ -8,7 +8,7 @@ import { getInnermostReturningFunction, getVariableReferences, isMemberExpression, - isPromiseHandled, + isPromiseUnhandled, } from '../node-utils'; import { EVENTS_SIMULATORS } from '../utils'; @@ -91,7 +91,7 @@ export default createTestingLibraryRule({ messageId?: MessageIds; fix?: TSESLint.ReportFixFunction; }): void { - if (!isPromiseHandled(node)) { + if (isPromiseUnhandled(closestCallExpression)) { context.report({ node: closestCallExpression.callee, messageId, @@ -176,13 +176,17 @@ export default createTestingLibraryRule({ }, }); } else { - for (const reference of references) { - if (ASTUtils.isIdentifier(reference.identifier)) { - reportUnhandledNode({ - node: reference.identifier, - closestCallExpression, - }); - } + const referenceIdentifiers = references + .map(({ identifier }) => identifier) + .filter(ASTUtils.isIdentifier); + if (referenceIdentifiers.every(isPromiseUnhandled)) { + referenceIdentifiers.forEach( + (id) => + void reportUnhandledNode({ + node: id, + closestCallExpression, + }) + ); } } } else if (functionWrappersNames.includes(node.name)) { diff --git a/tests/lib/rules/await-async-events.test.ts b/tests/lib/rules/await-async-events.test.ts index ba7110a5..d21d006c 100644 --- a/tests/lib/rules/await-async-events.test.ts +++ b/tests/lib/rules/await-async-events.test.ts @@ -311,6 +311,17 @@ ruleTester.run(RULE_NAME, rule, { await triggerEvent() }) + `, + options: [{ eventModule: 'userEvent' }] as const, + })), + ...USER_EVENT_ASYNC_FUNCTIONS.map((eventMethod) => ({ + code: ` + import userEvent from '${testingFramework}' + test('await expression that evaluates to promise is valid', async () => { + await (null, userEvent.${eventMethod}(getByLabelText('username'))); + await (condition ? null : userEvent.${eventMethod}(getByLabelText('username'))); + await (condition && userEvent.${eventMethod}(getByLabelText('username'))); + }) `, options: [{ eventModule: 'userEvent' }] as const, })), @@ -960,6 +971,66 @@ ruleTester.run(RULE_NAME, rule, { } triggerEvent() + `, + } as const) + ), + ...USER_EVENT_ASYNC_FUNCTIONS.map( + (eventMethod) => + ({ + code: ` + import userEvent from '${testingFramework}' + test('unhandled expression that evaluates to promise is invalid', () => { + condition ? null : (null, true && userEvent.${eventMethod}(getByLabelText('username'))); + }); + `, + errors: [ + { + line: 4, + column: 38, + messageId: 'awaitAsyncEvent', + data: { name: eventMethod }, + }, + ], + options: [{ eventModule: 'userEvent' }], + output: ` + import userEvent from '${testingFramework}' + test('unhandled expression that evaluates to promise is invalid', async () => { + condition ? null : (null, true && await userEvent.${eventMethod}(getByLabelText('username'))); + }); + `, + } as const) + ), + ...USER_EVENT_ASYNC_FUNCTIONS.map( + (eventMethod) => + ({ + code: ` + import userEvent from '${testingFramework}' + test('voided promise is invalid', async () => { + await void userEvent.${eventMethod}(getByLabelText('username')); + await (userEvent.${eventMethod}(getByLabelText('username')), null); + }); + `, + errors: [ + { + line: 4, + column: 15, + messageId: 'awaitAsyncEvent', + data: { name: eventMethod }, + }, + { + line: 5, + column: 11, + messageId: 'awaitAsyncEvent', + data: { name: eventMethod }, + }, + ], + options: [{ eventModule: 'userEvent' }], + output: ` + import userEvent from '${testingFramework}' + test('voided promise is invalid', async () => { + await void await userEvent.${eventMethod}(getByLabelText('username')); + await (await userEvent.${eventMethod}(getByLabelText('username')), null); + }); `, } as const) ), From 8bd9d582a974e2f68e9cfece151da7b763b54912 Mon Sep 17 00:00:00 2001 From: Jemi Salo Date: Fri, 22 Mar 2024 15:17:25 +0200 Subject: [PATCH 2/4] fix: revert await-async-events to aggressive behaviour --- lib/node-utils/index.ts | 62 ++++++----------------- lib/rules/await-async-events.ts | 22 ++++---- tests/lib/rules/await-async-utils.test.ts | 27 ++++++++++ 3 files changed, 51 insertions(+), 60 deletions(-) diff --git a/lib/node-utils/index.ts b/lib/node-utils/index.ts index 909d8a14..26d06aac 100644 --- a/lib/node-utils/index.ts +++ b/lib/node-utils/index.ts @@ -222,44 +222,31 @@ export function isPromiseHandled(nodeIdentifier: TSESTree.Identifier): boolean { nodeIdentifier, true ); + const callRootExpression = + closestCallExpressionNode == null + ? null + : getRootExpression(closestCallExpressionNode); - const suspiciousNodes = [nodeIdentifier, closestCallExpressionNode].filter( - Boolean + const suspiciousNodes = [nodeIdentifier, callRootExpression].filter( + (node): node is NonNullable => node != null ); - for (const node of suspiciousNodes) { - if (!node?.parent) { - continue; - } - if (ASTUtils.isAwaitExpression(node.parent)) { - return true; - } - + return suspiciousNodes.some((node) => { + if (!node.parent) return false; + if (ASTUtils.isAwaitExpression(node.parent)) return true; if ( isArrowFunctionExpression(node.parent) || isReturnStatement(node.parent) - ) { - return true; - } - - if (hasClosestExpectResolvesRejects(node.parent)) { - return true; - } - - if (hasChainedThen(node)) { - return true; - } - - if (isPromisesArrayResolved(node)) { + ) return true; - } - } - - return false; + if (hasClosestExpectResolvesRejects(node.parent)) return true; + if (hasChainedThen(node)) return true; + if (isPromisesArrayResolved(node)) return true; + }); } /** - * For an expression in a parent expression that evaluates to the expression or another child returns the parent node recursively. + * For an expression in a parent that evaluates to the expression or another child returns the parent node recursively. */ function getRootExpression( expression: TSESTree.Expression @@ -279,25 +266,6 @@ function getRootExpression( } } -/** - * Determines whether a given promise expression is considered unhandled. - * - * It will be considered unhandled if an ancestor voids the expression. - */ -export function isPromiseUnhandled(expression: TSESTree.Expression): boolean { - const { parent } = getRootExpression(expression); - if (parent == null) return false; - switch (parent.type) { - case AST_NODE_TYPES.ExpressionStatement: - case AST_NODE_TYPES.SequenceExpression: - case AST_NODE_TYPES.UnaryExpression: - case AST_NODE_TYPES.VariableDeclarator: - return true; - default: - return false; - } -} - export function getVariableReferences( context: TSESLint.RuleContext, node: TSESTree.Node diff --git a/lib/rules/await-async-events.ts b/lib/rules/await-async-events.ts index 25a56efa..6ca29473 100644 --- a/lib/rules/await-async-events.ts +++ b/lib/rules/await-async-events.ts @@ -8,7 +8,7 @@ import { getInnermostReturningFunction, getVariableReferences, isMemberExpression, - isPromiseUnhandled, + isPromiseHandled, } from '../node-utils'; import { EVENTS_SIMULATORS } from '../utils'; @@ -91,7 +91,7 @@ export default createTestingLibraryRule({ messageId?: MessageIds; fix?: TSESLint.ReportFixFunction; }): void { - if (isPromiseUnhandled(closestCallExpression)) { + if (!isPromiseHandled(node)) { context.report({ node: closestCallExpression.callee, messageId, @@ -176,17 +176,13 @@ export default createTestingLibraryRule({ }, }); } else { - const referenceIdentifiers = references - .map(({ identifier }) => identifier) - .filter(ASTUtils.isIdentifier); - if (referenceIdentifiers.every(isPromiseUnhandled)) { - referenceIdentifiers.forEach( - (id) => - void reportUnhandledNode({ - node: id, - closestCallExpression, - }) - ); + for (const reference of references) { + if (ASTUtils.isIdentifier(reference.identifier)) { + reportUnhandledNode({ + node: reference.identifier, + closestCallExpression, + }); + } } } } else if (functionWrappersNames.includes(node.name)) { diff --git a/tests/lib/rules/await-async-utils.test.ts b/tests/lib/rules/await-async-utils.test.ts index 7eb211bb..e91517b3 100644 --- a/tests/lib/rules/await-async-utils.test.ts +++ b/tests/lib/rules/await-async-utils.test.ts @@ -418,6 +418,33 @@ ruleTester.run(RULE_NAME, rule, { doSomethingElse(aPromise); ${asyncUtil}(() => getByLabelText('email')); }); + `, + errors: [ + { + line: 4, + column: 28, + messageId: 'awaitAsyncUtil', + data: { name: asyncUtil }, + }, + { + line: 6, + column: 11, + messageId: 'awaitAsyncUtil', + data: { name: asyncUtil }, + }, + ], + } as const) + ), + ...ASYNC_UTILS.map( + (asyncUtil) => + ({ + code: ` + import { ${asyncUtil} } from '${testingFramework}'; + test('unhandled expression that evaluates to promise is invalid', () => { + const aPromise = ${asyncUtil}(() => getByLabelText('username')); + doSomethingElse(aPromise); + ${asyncUtil}(() => getByLabelText('email')); + }); `, errors: [ { From aa75e2ef12e865aee1130134c0d23ff420a50a9b Mon Sep 17 00:00:00 2001 From: Jemi Salo Date: Fri, 22 Mar 2024 15:40:24 +0200 Subject: [PATCH 3/4] fix: exclude AND left from promise handled consideration --- lib/node-utils/index.ts | 13 +++++++++- tests/lib/rules/await-async-events.test.ts | 28 ++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/lib/node-utils/index.ts b/lib/node-utils/index.ts index 26d06aac..503fbbfc 100644 --- a/lib/node-utils/index.ts +++ b/lib/node-utils/index.ts @@ -255,8 +255,19 @@ function getRootExpression( if (parent == null) return expression; switch (parent.type) { case AST_NODE_TYPES.ConditionalExpression: - case AST_NODE_TYPES.LogicalExpression: return getRootExpression(parent); + case AST_NODE_TYPES.LogicalExpression: + switch (parent.operator) { + case '??': + case '||': + return getRootExpression(parent); + case '&&': + return parent.right === expression + ? getRootExpression(parent) + : expression; + default: + return expression; + } case AST_NODE_TYPES.SequenceExpression: return parent.expressions[parent.expressions.length - 1] === expression ? getRootExpression(parent) diff --git a/tests/lib/rules/await-async-events.test.ts b/tests/lib/rules/await-async-events.test.ts index d21d006c..894f7f92 100644 --- a/tests/lib/rules/await-async-events.test.ts +++ b/tests/lib/rules/await-async-events.test.ts @@ -321,6 +321,8 @@ ruleTester.run(RULE_NAME, rule, { await (null, userEvent.${eventMethod}(getByLabelText('username'))); await (condition ? null : userEvent.${eventMethod}(getByLabelText('username'))); await (condition && userEvent.${eventMethod}(getByLabelText('username'))); + await (userEvent.${eventMethod}(getByLabelText('username')) || userEvent.${eventMethod}(getByLabelText('username'))); + await (userEvent.${eventMethod}(getByLabelText('username')) ?? userEvent.${eventMethod}(getByLabelText('username'))); }) `, options: [{ eventModule: 'userEvent' }] as const, @@ -997,6 +999,32 @@ ruleTester.run(RULE_NAME, rule, { test('unhandled expression that evaluates to promise is invalid', async () => { condition ? null : (null, true && await userEvent.${eventMethod}(getByLabelText('username'))); }); + `, + } as const) + ), + ...USER_EVENT_ASYNC_FUNCTIONS.map( + (eventMethod) => + ({ + code: ` + import userEvent from '${testingFramework}' + test('handled AND expression with left promise is invalid', async () => { + await (userEvent.${eventMethod}(getByLabelText('username')) && userEvent.${eventMethod}(getByLabelText('username'))); + }); + `, + errors: [ + { + line: 4, + column: 11, + messageId: 'awaitAsyncEvent', + data: { name: eventMethod }, + }, + ], + options: [{ eventModule: 'userEvent' }], + output: ` + import userEvent from '${testingFramework}' + test('handled AND expression with left promise is invalid', async () => { + await (await userEvent.${eventMethod}(getByLabelText('username')) && userEvent.${eventMethod}(getByLabelText('username'))); + }); `, } as const) ), From 7d0e2851c90c0878846c62bf5a75259b8623709e Mon Sep 17 00:00:00 2001 From: Jemi Salo Date: Tue, 26 Mar 2024 09:41:10 +0200 Subject: [PATCH 4/4] chore: contort code to pass test coverage check --- lib/node-utils/index.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/node-utils/index.ts b/lib/node-utils/index.ts index 503fbbfc..0f748c5c 100644 --- a/lib/node-utils/index.ts +++ b/lib/node-utils/index.ts @@ -256,18 +256,22 @@ function getRootExpression( switch (parent.type) { case AST_NODE_TYPES.ConditionalExpression: return getRootExpression(parent); - case AST_NODE_TYPES.LogicalExpression: + case AST_NODE_TYPES.LogicalExpression: { + let rootExpression; switch (parent.operator) { case '??': case '||': - return getRootExpression(parent); + rootExpression = getRootExpression(parent); + break; case '&&': - return parent.right === expression - ? getRootExpression(parent) - : expression; - default: - return expression; + rootExpression = + parent.right === expression + ? getRootExpression(parent) + : expression; + break; } + return rootExpression ?? expression; + } case AST_NODE_TYPES.SequenceExpression: return parent.expressions[parent.expressions.length - 1] === expression ? getRootExpression(parent)