From 9d5554cb6bd44f3dbb9e370d67b684b54010c704 Mon Sep 17 00:00:00 2001 From: Spencer Miskoviak <5247455+skovy@users.noreply.github.com> Date: Fri, 14 Oct 2022 04:53:39 -0700 Subject: [PATCH] fix(await-async-events): improve fixer (#675) --- lib/node-utils/index.ts | 24 +++ lib/node-utils/is-node-of-type.ts | 3 + lib/rules/await-async-events.ts | 46 ++++- tests/lib/rules/await-async-events.test.ts | 187 +++++++++++++++++++-- 4 files changed, 240 insertions(+), 20 deletions(-) diff --git a/lib/node-utils/index.ts b/lib/node-utils/index.ts index 2e805ab0..497029d2 100644 --- a/lib/node-utils/index.ts +++ b/lib/node-utils/index.ts @@ -13,6 +13,8 @@ import { isBlockStatement, isCallExpression, isExpressionStatement, + isFunctionExpression, + isFunctionDeclaration, isImportDeclaration, isImportNamespaceSpecifier, isImportSpecifier, @@ -95,6 +97,28 @@ export function findClosestVariableDeclaratorNode( return findClosestVariableDeclaratorNode(node.parent); } +export function findClosestFunctionExpressionNode( + node: TSESTree.Node | undefined +): + | TSESTree.ArrowFunctionExpression + | TSESTree.FunctionExpression + | TSESTree.FunctionDeclaration + | null { + if (!node) { + return null; + } + + if ( + isArrowFunctionExpression(node) || + isFunctionExpression(node) || + isFunctionDeclaration(node) + ) { + return node; + } + + return findClosestFunctionExpressionNode(node.parent); +} + /** * TODO: remove this one in favor of {@link findClosestCallExpressionNode} */ diff --git a/lib/node-utils/is-node-of-type.ts b/lib/node-utils/is-node-of-type.ts index 8f74a9a5..afa2b3fc 100644 --- a/lib/node-utils/is-node-of-type.ts +++ b/lib/node-utils/is-node-of-type.ts @@ -59,3 +59,6 @@ export const isReturnStatement = ASTUtils.isNodeOfType( export const isFunctionExpression = ASTUtils.isNodeOfType( AST_NODE_TYPES.FunctionExpression ); +export const isFunctionDeclaration = ASTUtils.isNodeOfType( + AST_NODE_TYPES.FunctionDeclaration +); diff --git a/lib/rules/await-async-events.ts b/lib/rules/await-async-events.ts index 57ce0694..e1dbfcf5 100644 --- a/lib/rules/await-async-events.ts +++ b/lib/rules/await-async-events.ts @@ -3,6 +3,7 @@ import { ASTUtils, TSESLint, TSESTree } from '@typescript-eslint/utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; import { findClosestCallExpressionNode, + findClosestFunctionExpressionNode, getFunctionName, getInnermostReturningFunction, getVariableReferences, @@ -142,7 +143,28 @@ export default createTestingLibraryRule({ closestCallExpression, fix: (fixer) => { if (isMemberExpression(node.parent)) { - return fixer.insertTextBefore(node.parent, 'await '); + const functionExpression = + findClosestFunctionExpressionNode(node); + + if (functionExpression) { + const memberExpressionFixer = fixer.insertTextBefore( + node.parent, + 'await ' + ); + + if (functionExpression.async) { + return memberExpressionFixer; + } else { + // Mutate the actual node so if other nodes exist in this + // function expression body they don't also try to fix it. + functionExpression.async = true; + + return [ + memberExpressionFixer, + fixer.insertTextBefore(functionExpression, 'async '), + ]; + } + } } return null; @@ -175,7 +197,27 @@ export default createTestingLibraryRule({ closestCallExpression, messageId: 'awaitAsyncEventWrapper', fix: (fixer) => { - return fixer.insertTextBefore(node, 'await '); + const functionExpression = + findClosestFunctionExpressionNode(node); + + if (functionExpression) { + const nodeFixer = fixer.insertTextBefore(node, 'await '); + + if (functionExpression.async) { + return nodeFixer; + } else { + // Mutate the actual node so if other nodes exist in this + // function expression body they don't also try to fix it. + functionExpression.async = true; + + return [ + nodeFixer, + fixer.insertTextBefore(functionExpression, 'async '), + ]; + } + } + + return null; }, }); } diff --git a/tests/lib/rules/await-async-events.test.ts b/tests/lib/rules/await-async-events.test.ts index e0acd43c..fe133c0c 100644 --- a/tests/lib/rules/await-async-events.test.ts +++ b/tests/lib/rules/await-async-events.test.ts @@ -348,7 +348,7 @@ ruleTester.run(RULE_NAME, rule, { ({ code: ` import { fireEvent } from '${testingFramework}' - test('unhandled promise from event method is invalid', async () => { + test('unhandled promise from event method is invalid', () => { fireEvent.${eventMethod}(getByLabelText('username')) }) `, @@ -367,6 +367,64 @@ ruleTester.run(RULE_NAME, rule, { test('unhandled promise from event method is invalid', async () => { await fireEvent.${eventMethod}(getByLabelText('username')) }) + `, + } as const) + ), + ...FIRE_EVENT_ASYNC_FUNCTIONS.map( + (eventMethod) => + ({ + code: ` + import { fireEvent } from '${testingFramework}' + + fireEvent.${eventMethod}(getByLabelText('username')) + `, + errors: [ + { + line: 4, + column: 7, + endColumn: 17 + eventMethod.length, + messageId: 'awaitAsyncEvent', + data: { name: eventMethod }, + }, + ], + options: [{ eventModule: 'fireEvent' }], + output: ` + import { fireEvent } from '${testingFramework}' + + fireEvent.${eventMethod}(getByLabelText('username')) + `, + } as const) + ), + ...FIRE_EVENT_ASYNC_FUNCTIONS.map( + (eventMethod) => + ({ + code: ` + import { fireEvent } from '${testingFramework}' + + function run() { + fireEvent.${eventMethod}(getByLabelText('username')) + } + + test('should handle external function', run) + `, + errors: [ + { + line: 5, + column: 9, + endColumn: 19 + eventMethod.length, + messageId: 'awaitAsyncEvent', + data: { name: eventMethod }, + }, + ], + options: [{ eventModule: 'fireEvent' }], + output: ` + import { fireEvent } from '${testingFramework}' + + async function run() { + await fireEvent.${eventMethod}(getByLabelText('username')) + } + + test('should handle external function', run) `, } as const) ), @@ -429,7 +487,7 @@ ruleTester.run(RULE_NAME, rule, { ({ code: ` import { fireEvent } from '${testingFramework}' - test('several unhandled promises from event methods is invalid', async () => { + test('several unhandled promises from event methods is invalid', async function() { fireEvent.${eventMethod}(getByLabelText('username')) fireEvent.${eventMethod}(getByLabelText('username')) }) @@ -451,7 +509,7 @@ ruleTester.run(RULE_NAME, rule, { options: [{ eventModule: 'fireEvent' }], output: ` import { fireEvent } from '${testingFramework}' - test('several unhandled promises from event methods is invalid', async () => { + test('several unhandled promises from event methods is invalid', async function() { await fireEvent.${eventMethod}(getByLabelText('username')) await fireEvent.${eventMethod}(getByLabelText('username')) }) @@ -466,7 +524,7 @@ ruleTester.run(RULE_NAME, rule, { }, code: ` import { fireEvent } from '${testingFramework}' - test('unhandled promise from event method with aggressive reporting opted-out is invalid', async () => { + test('unhandled promise from event method with aggressive reporting opted-out is invalid', function() { fireEvent.${eventMethod}(getByLabelText('username')) }) `, @@ -481,7 +539,7 @@ ruleTester.run(RULE_NAME, rule, { options: [{ eventModule: 'fireEvent' }], output: ` import { fireEvent } from '${testingFramework}' - test('unhandled promise from event method with aggressive reporting opted-out is invalid', async () => { + test('unhandled promise from event method with aggressive reporting opted-out is invalid', async function() { await fireEvent.${eventMethod}(getByLabelText('username')) }) `, @@ -514,7 +572,7 @@ ruleTester.run(RULE_NAME, rule, { import { fireEvent } from 'test-utils' test( 'unhandled promise from event method imported from custom module with aggressive reporting opted-out is invalid', - () => { + async () => { await fireEvent.${eventMethod}(getByLabelText('username')) }) `, @@ -547,7 +605,7 @@ ruleTester.run(RULE_NAME, rule, { import { fireEvent } from '${testingFramework}' test( 'unhandled promise from event method imported from default module with aggressive reporting opted-out is invalid', - () => { + async () => { await fireEvent.${eventMethod}(getByLabelText('username')) }) `, @@ -578,7 +636,7 @@ ruleTester.run(RULE_NAME, rule, { import { fireEvent } from '${testingFramework}' test( 'unhandled promise from event method kept in a var is invalid', - () => { + async () => { const promise = await fireEvent.${eventMethod}(getByLabelText('username')) }) `, @@ -609,7 +667,7 @@ ruleTester.run(RULE_NAME, rule, { options: [{ eventModule: 'fireEvent' }], output: ` import { fireEvent } from '${testingFramework}' - test('unhandled promise returned from function wrapping event method is invalid', () => { + test('unhandled promise returned from function wrapping event method is invalid', async () => { function triggerEvent() { doSomething() return fireEvent.${eventMethod}(getByLabelText('username')) @@ -617,6 +675,40 @@ ruleTester.run(RULE_NAME, rule, { await triggerEvent() }) + `, + } as const) + ), + ...FIRE_EVENT_ASYNC_FUNCTIONS.map( + (eventMethod) => + ({ + code: ` + import { fireEvent } from '${testingFramework}' + + function triggerEvent() { + doSomething() + return fireEvent.${eventMethod}(getByLabelText('username')) + } + + triggerEvent() + `, + errors: [ + { + line: 9, + column: 7, + messageId: 'awaitAsyncEventWrapper', + data: { name: 'triggerEvent' }, + }, + ], + options: [{ eventModule: 'fireEvent' }], + output: ` + import { fireEvent } from '${testingFramework}' + + function triggerEvent() { + doSomething() + return fireEvent.${eventMethod}(getByLabelText('username')) + } + + triggerEvent() `, } as const) ), @@ -627,7 +719,7 @@ ruleTester.run(RULE_NAME, rule, { ({ code: ` import userEvent from '${testingFramework}' - test('unhandled promise from event method is invalid', async () => { + test('unhandled promise from event method is invalid', () => { userEvent.${eventMethod}(getByLabelText('username')) }) `, @@ -646,6 +738,31 @@ ruleTester.run(RULE_NAME, rule, { test('unhandled promise from event method is invalid', async () => { await userEvent.${eventMethod}(getByLabelText('username')) }) + `, + } as const) + ), + ...USER_EVENT_ASYNC_FUNCTIONS.map( + (eventMethod) => + ({ + code: ` + import userEvent from '${testingFramework}' + + userEvent.${eventMethod}(getByLabelText('username')) + `, + errors: [ + { + line: 4, + column: 7, + endColumn: 17 + eventMethod.length, + messageId: 'awaitAsyncEvent', + data: { name: eventMethod }, + }, + ], + options: [{ eventModule: 'userEvent' }], + output: ` + import userEvent from '${testingFramework}' + + userEvent.${eventMethod}(getByLabelText('username')) `, } as const) ), @@ -654,7 +771,7 @@ ruleTester.run(RULE_NAME, rule, { ({ code: ` import testingLibraryUserEvent from '${testingFramework}' - test('unhandled promise imported from alternate name event method is invalid', async () => { + test('unhandled promise imported from alternate name event method is invalid', () => { testingLibraryUserEvent.${eventMethod}(getByLabelText('username')) }) `, @@ -681,7 +798,7 @@ ruleTester.run(RULE_NAME, rule, { ({ code: ` import userEvent from '${testingFramework}' - test('several unhandled promises from event methods is invalid', async () => { + test('several unhandled promises from event methods is invalid', () => { userEvent.${eventMethod}(getByLabelText('username')) userEvent.${eventMethod}(getByLabelText('username')) }) @@ -734,7 +851,7 @@ ruleTester.run(RULE_NAME, rule, { import userEvent from '${testingFramework}' test( 'unhandled promise from event method kept in a var is invalid', - () => { + async () => { const promise = await userEvent.${eventMethod}(getByLabelText('username')) }) `, @@ -745,7 +862,7 @@ ruleTester.run(RULE_NAME, rule, { ({ code: ` import userEvent from '${testingFramework}' - test('unhandled promise returned from function wrapping event method is invalid', () => { + test('unhandled promise returned from function wrapping event method is invalid', function() { function triggerEvent() { doSomething() return userEvent.${eventMethod}(getByLabelText('username')) @@ -765,7 +882,7 @@ ruleTester.run(RULE_NAME, rule, { options: [{ eventModule: 'userEvent' }], output: ` import userEvent from '${testingFramework}' - test('unhandled promise returned from function wrapping event method is invalid', () => { + test('unhandled promise returned from function wrapping event method is invalid', async function() { function triggerEvent() { doSomething() return userEvent.${eventMethod}(getByLabelText('username')) @@ -773,6 +890,40 @@ ruleTester.run(RULE_NAME, rule, { await triggerEvent() }) + `, + } as const) + ), + ...USER_EVENT_ASYNC_FUNCTIONS.map( + (eventMethod) => + ({ + code: ` + import userEvent from '${testingFramework}' + + function triggerEvent() { + doSomething() + return userEvent.${eventMethod}(getByLabelText('username')) + } + + triggerEvent() + `, + errors: [ + { + line: 9, + column: 7, + messageId: 'awaitAsyncEventWrapper', + data: { name: 'triggerEvent' }, + }, + ], + options: [{ eventModule: 'userEvent' }], + output: ` + import userEvent from '${testingFramework}' + + function triggerEvent() { + doSomething() + return userEvent.${eventMethod}(getByLabelText('username')) + } + + triggerEvent() `, } as const) ), @@ -781,7 +932,7 @@ ruleTester.run(RULE_NAME, rule, { code: ` import userEvent from '${USER_EVENT_ASYNC_FRAMEWORKS[0]}' import { fireEvent } from '${FIRE_EVENT_ASYNC_FRAMEWORKS[0]}' - test('unhandled promises from multiple event modules', async () => { + test('unhandled promises from multiple event modules', () => { fireEvent.click(getByLabelText('username')) userEvent.click(getByLabelText('username')) }) @@ -814,7 +965,7 @@ ruleTester.run(RULE_NAME, rule, { code: ` import userEvent from '${USER_EVENT_ASYNC_FRAMEWORKS[0]}' import { fireEvent } from '${FIRE_EVENT_ASYNC_FRAMEWORKS[0]}' - test('unhandled promise from userEvent relying on default options', async () => { + test('unhandled promise from userEvent relying on default options', async function() { fireEvent.click(getByLabelText('username')) userEvent.click(getByLabelText('username')) }) @@ -830,7 +981,7 @@ ruleTester.run(RULE_NAME, rule, { output: ` import userEvent from '${USER_EVENT_ASYNC_FRAMEWORKS[0]}' import { fireEvent } from '${FIRE_EVENT_ASYNC_FRAMEWORKS[0]}' - test('unhandled promise from userEvent relying on default options', async () => { + test('unhandled promise from userEvent relying on default options', async function() { fireEvent.click(getByLabelText('username')) await userEvent.click(getByLabelText('username')) })