From 56bbeae16c35ef30ffa4577a04022ffcdfdedcbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Wed, 24 Mar 2021 23:35:38 +0100 Subject: [PATCH 01/15] test: improve errors location asserts --- .../rules/no-side-effects-wait-for.test.ts | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/lib/rules/no-side-effects-wait-for.test.ts b/tests/lib/rules/no-side-effects-wait-for.test.ts index 6b3fadf2..891a35c1 100644 --- a/tests/lib/rules/no-side-effects-wait-for.test.ts +++ b/tests/lib/rules/no-side-effects-wait-for.test.ts @@ -111,7 +111,7 @@ ruleTester.run(RULE_NAME, rule, { fireEvent.keyDown(input, {key: 'ArrowDown'}) }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 29, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -121,7 +121,7 @@ ruleTester.run(RULE_NAME, rule, { fireEvent.keyDown(input, {key: 'ArrowDown'}) }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 29, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -131,7 +131,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 29, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -140,7 +140,7 @@ ruleTester.run(RULE_NAME, rule, { fireEvent.keyDown(input, {key: 'ArrowDown'}) }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 34, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -150,7 +150,7 @@ ruleTester.run(RULE_NAME, rule, { fireEvent.keyDown(input, {key: 'ArrowDown'}) }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 34, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -160,7 +160,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 34, messageId: 'noSideEffectsWaitFor' }], }, // userEvent { @@ -170,7 +170,7 @@ ruleTester.run(RULE_NAME, rule, { userEvent.click(button) }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 29, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -180,7 +180,7 @@ ruleTester.run(RULE_NAME, rule, { userEvent.click(button) }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 29, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -190,7 +190,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 29, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -199,7 +199,7 @@ ruleTester.run(RULE_NAME, rule, { userEvent.click(button) }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 34, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -209,7 +209,7 @@ ruleTester.run(RULE_NAME, rule, { userEvent.click(button) }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 34, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -219,7 +219,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 34, messageId: 'noSideEffectsWaitFor' }], }, ], }); From 98d586869140e22deeb198f8055519dbb3a2c229 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Wed, 24 Mar 2021 23:41:04 +0100 Subject: [PATCH 02/15] refactor: use new rule creator --- lib/rules/no-side-effects-wait-for.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/rules/no-side-effects-wait-for.ts b/lib/rules/no-side-effects-wait-for.ts index d3b55504..54a3eeff 100644 --- a/lib/rules/no-side-effects-wait-for.ts +++ b/lib/rules/no-side-effects-wait-for.ts @@ -1,14 +1,11 @@ -import { - ESLintUtils, - TSESTree, - ASTUtils, -} from '@typescript-eslint/experimental-utils'; -import { getDocsUrl, hasTestingLibraryImportModule } from '../utils'; +import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; +import { hasTestingLibraryImportModule } from '../utils'; import { isBlockStatement, - isMemberExpression, isCallExpression, + isMemberExpression, } from '../node-utils'; +import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'no-side-effects-wait-for'; export type MessageIds = 'noSideEffectsWaitFor'; @@ -18,7 +15,7 @@ const WAIT_EXPRESSION_QUERY = 'CallExpression[callee.name=/^(waitFor)$/]'; const SIDE_EFFECTS: Array = ['fireEvent', 'userEvent']; -export default ESLintUtils.RuleCreator(getDocsUrl)({ +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'suggestion', From 56d5dd48aa23f2d57196a34e9aa2d7845bd7c205 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Wed, 24 Mar 2021 23:50:32 +0100 Subject: [PATCH 03/15] refactor: improve error reported location --- lib/rules/no-side-effects-wait-for.ts | 34 ++++++++++--------- .../rules/no-side-effects-wait-for.test.ts | 24 ++++++------- 2 files changed, 30 insertions(+), 28 deletions(-) diff --git a/lib/rules/no-side-effects-wait-for.ts b/lib/rules/no-side-effects-wait-for.ts index 54a3eeff..e39b5850 100644 --- a/lib/rules/no-side-effects-wait-for.ts +++ b/lib/rules/no-side-effects-wait-for.ts @@ -35,21 +35,24 @@ export default createTestingLibraryRule({ create: function (context) { let isImportingTestingLibrary = false; + function hasSideEffects(body: Array): boolean { + return body.some((node: TSESTree.ExpressionStatement) => { + if ( + isCallExpression(node.expression) && + isMemberExpression(node.expression.callee) && + ASTUtils.isIdentifier(node.expression.callee.object) + ) { + const object: TSESTree.Identifier = node.expression.callee.object; + const identifierName: string = object.name; + return SIDE_EFFECTS.includes(identifierName); + } else { + return false; + } + }); + } + function reportSideEffects(node: TSESTree.BlockStatement) { - const hasSideEffects = (body: Array): boolean => - body.some((node: TSESTree.ExpressionStatement) => { - if ( - isCallExpression(node.expression) && - isMemberExpression(node.expression.callee) && - ASTUtils.isIdentifier(node.expression.callee.object) - ) { - const object: TSESTree.Identifier = node.expression.callee.object; - const identifierName: string = object.name; - return SIDE_EFFECTS.includes(identifierName); - } else { - return false; - } - }); + const callExpressionNode = node.parent.parent as TSESTree.CallExpression; if ( isImportingTestingLibrary && @@ -57,8 +60,7 @@ export default createTestingLibraryRule({ hasSideEffects(node.body) ) { context.report({ - node, - loc: node.loc.start, + node: callExpressionNode, messageId: 'noSideEffectsWaitFor', }); } diff --git a/tests/lib/rules/no-side-effects-wait-for.test.ts b/tests/lib/rules/no-side-effects-wait-for.test.ts index 891a35c1..05f75196 100644 --- a/tests/lib/rules/no-side-effects-wait-for.test.ts +++ b/tests/lib/rules/no-side-effects-wait-for.test.ts @@ -111,7 +111,7 @@ ruleTester.run(RULE_NAME, rule, { fireEvent.keyDown(input, {key: 'ArrowDown'}) }) `, - errors: [{ line: 3, column: 29, messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -121,7 +121,7 @@ ruleTester.run(RULE_NAME, rule, { fireEvent.keyDown(input, {key: 'ArrowDown'}) }) `, - errors: [{ line: 3, column: 29, messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -131,7 +131,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ line: 3, column: 29, messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -140,7 +140,7 @@ ruleTester.run(RULE_NAME, rule, { fireEvent.keyDown(input, {key: 'ArrowDown'}) }) `, - errors: [{ line: 3, column: 34, messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -150,7 +150,7 @@ ruleTester.run(RULE_NAME, rule, { fireEvent.keyDown(input, {key: 'ArrowDown'}) }) `, - errors: [{ line: 3, column: 34, messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -160,7 +160,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ line: 3, column: 34, messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, // userEvent { @@ -170,7 +170,7 @@ ruleTester.run(RULE_NAME, rule, { userEvent.click(button) }) `, - errors: [{ line: 3, column: 29, messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -180,7 +180,7 @@ ruleTester.run(RULE_NAME, rule, { userEvent.click(button) }) `, - errors: [{ line: 3, column: 29, messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -190,7 +190,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ line: 3, column: 29, messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -199,7 +199,7 @@ ruleTester.run(RULE_NAME, rule, { userEvent.click(button) }) `, - errors: [{ line: 3, column: 34, messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -209,7 +209,7 @@ ruleTester.run(RULE_NAME, rule, { userEvent.click(button) }) `, - errors: [{ line: 3, column: 34, messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, { code: ` @@ -219,7 +219,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ line: 3, column: 34, messageId: 'noSideEffectsWaitFor' }], + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, ], }); From dd333e5305df0ac2b5017388ee074ced401771be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Thu, 25 Mar 2021 00:04:17 +0100 Subject: [PATCH 04/15] refactor: use new helpers for detection --- lib/rules/no-side-effects-wait-for.ts | 39 +++++++++---------- lib/utils.ts | 10 ----- .../rules/no-side-effects-wait-for.test.ts | 13 ++++++- 3 files changed, 30 insertions(+), 32 deletions(-) diff --git a/lib/rules/no-side-effects-wait-for.ts b/lib/rules/no-side-effects-wait-for.ts index e39b5850..418943b1 100644 --- a/lib/rules/no-side-effects-wait-for.ts +++ b/lib/rules/no-side-effects-wait-for.ts @@ -1,7 +1,6 @@ import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; -import { hasTestingLibraryImportModule } from '../utils'; import { - isBlockStatement, + getDeepestIdentifierNode, isCallExpression, isMemberExpression, } from '../node-utils'; @@ -11,8 +10,6 @@ export const RULE_NAME = 'no-side-effects-wait-for'; export type MessageIds = 'noSideEffectsWaitFor'; type Options = []; -const WAIT_EXPRESSION_QUERY = 'CallExpression[callee.name=/^(waitFor)$/]'; - const SIDE_EFFECTS: Array = ['fireEvent', 'userEvent']; export default createTestingLibraryRule({ @@ -32,9 +29,7 @@ export default createTestingLibraryRule({ schema: [], }, defaultOptions: [], - create: function (context) { - let isImportingTestingLibrary = false; - + create: function (context, _, helpers) { function hasSideEffects(body: Array): boolean { return body.some((node: TSESTree.ExpressionStatement) => { if ( @@ -53,25 +48,27 @@ export default createTestingLibraryRule({ function reportSideEffects(node: TSESTree.BlockStatement) { const callExpressionNode = node.parent.parent as TSESTree.CallExpression; + const callExpressionIdentifier = getDeepestIdentifierNode( + callExpressionNode + ); + + if (!helpers.isAsyncUtil(callExpressionIdentifier, ['waitFor'])) { + return; + } - if ( - isImportingTestingLibrary && - isBlockStatement(node) && - hasSideEffects(node.body) - ) { - context.report({ - node: callExpressionNode, - messageId: 'noSideEffectsWaitFor', - }); + if (!hasSideEffects(node.body)) { + return; } + + context.report({ + node: callExpressionNode, + messageId: 'noSideEffectsWaitFor', + }); } return { - [`${WAIT_EXPRESSION_QUERY} > ArrowFunctionExpression > BlockStatement`]: reportSideEffects, - [`${WAIT_EXPRESSION_QUERY} > FunctionExpression > BlockStatement`]: reportSideEffects, - ImportDeclaration(node: TSESTree.ImportDeclaration) { - isImportingTestingLibrary = hasTestingLibraryImportModule(node); - }, + 'CallExpression > ArrowFunctionExpression > BlockStatement': reportSideEffects, + 'CallExpression > FunctionExpression > BlockStatement': reportSideEffects, }; }, }); diff --git a/lib/utils.ts b/lib/utils.ts index a464a517..1e4f0064 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -1,5 +1,3 @@ -import { TSESTree } from '@typescript-eslint/experimental-utils'; - const combineQueries = (variants: string[], methods: string[]): string[] => { const combinedQueries: string[] = []; variants.forEach((variant) => { @@ -24,13 +22,6 @@ const LIBRARY_MODULES = [ '@testing-library/svelte', ]; -// TODO: should be deleted after all rules are migrated to v4 -const hasTestingLibraryImportModule = ( - node: TSESTree.ImportDeclaration -): boolean => { - return LIBRARY_MODULES.includes(node.source.value.toString()); -}; - const SYNC_QUERIES_VARIANTS = ['getBy', 'getAllBy', 'queryBy', 'queryAllBy']; const ASYNC_QUERIES_VARIANTS = ['findBy', 'findAllBy']; const ALL_QUERIES_VARIANTS = [ @@ -117,7 +108,6 @@ const ABSENCE_MATCHERS = ['toBeNull', 'toBeFalsy']; export { combineQueries, getDocsUrl, - hasTestingLibraryImportModule, SYNC_QUERIES_VARIANTS, ASYNC_QUERIES_VARIANTS, ALL_QUERIES_VARIANTS, diff --git a/tests/lib/rules/no-side-effects-wait-for.test.ts b/tests/lib/rules/no-side-effects-wait-for.test.ts index 05f75196..f49bb9ae 100644 --- a/tests/lib/rules/no-side-effects-wait-for.test.ts +++ b/tests/lib/rules/no-side-effects-wait-for.test.ts @@ -93,14 +93,25 @@ ruleTester.run(RULE_NAME, rule, { `, }, { + settings: { 'testing-library/utils-module': 'test-utils' }, code: ` - import { waitFor } from 'react'; + import { waitFor } from 'somewhere-else'; await waitFor(function() { fireEvent.keyDown(input, {key: 'ArrowDown'}) expect(b).toEqual('b') }) `, }, + { + code: ` + import { waitFor } from '@testing-library/react'; + test('side effects in functions other than waitFor are valid', () => { + fireEvent.keyDown(input, {key: 'ArrowDown'}) + userEvent.click(button) + expect(b).toEqual('b') + }); + `, + }, ], invalid: [ // fireEvent From 4cbd073306122ee1b352004f896b67b85ec9646b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Thu, 25 Mar 2021 19:52:54 +0100 Subject: [PATCH 05/15] test: add more cases --- lib/rules/no-side-effects-wait-for.ts | 1 + .../rules/no-side-effects-wait-for.test.ts | 39 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/lib/rules/no-side-effects-wait-for.ts b/lib/rules/no-side-effects-wait-for.ts index 418943b1..510cd4b6 100644 --- a/lib/rules/no-side-effects-wait-for.ts +++ b/lib/rules/no-side-effects-wait-for.ts @@ -38,6 +38,7 @@ export default createTestingLibraryRule({ ASTUtils.isIdentifier(node.expression.callee.object) ) { const object: TSESTree.Identifier = node.expression.callee.object; + // TODO: check here if `object` is fireEvent or userEvent using helpers const identifierName: string = object.name; return SIDE_EFFECTS.includes(identifierName); } else { diff --git a/tests/lib/rules/no-side-effects-wait-for.test.ts b/tests/lib/rules/no-side-effects-wait-for.test.ts index f49bb9ae..9f648856 100644 --- a/tests/lib/rules/no-side-effects-wait-for.test.ts +++ b/tests/lib/rules/no-side-effects-wait-for.test.ts @@ -105,6 +105,12 @@ ruleTester.run(RULE_NAME, rule, { { code: ` import { waitFor } from '@testing-library/react'; + + anotherFunction(() => { + fireEvent.keyDown(input, {key: 'ArrowDown'}); + userEvent.click(button); + }); + test('side effects in functions other than waitFor are valid', () => { fireEvent.keyDown(input, {key: 'ArrowDown'}) userEvent.click(button) @@ -112,6 +118,29 @@ ruleTester.run(RULE_NAME, rule, { }); `, }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { waitFor } from 'somewhere-else'; + await waitFor(() => { + fireEvent.keyDown(input, {key: 'ArrowDown'}) + }) + `, + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { waitFor as renamedWaitFor, fireEvent, userEvent } from 'test-utils'; + import { waitFor } from 'somewhere-else'; + + await waitFor(() => { + fireEvent.keyDown(input, {key: 'ArrowDown'}) + userEvent.click(button) + }) + `, + }, + + // TODO: duplicate prev test but renaming fireEvent and userEvent ], invalid: [ // fireEvent @@ -124,6 +153,16 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, + { + settings: { 'testing-library/utils-module': '~/test-utils' }, + code: ` + import { waitFor } from '~/test-utils'; + await waitFor(() => { + fireEvent.keyDown(input, {key: 'ArrowDown'}) + }) + `, + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], + }, { code: ` import { waitFor } from '@testing-library/react'; From d549af7bd83cd859194f79e46b01ee156185f29f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Thu, 25 Mar 2021 20:38:44 +0100 Subject: [PATCH 06/15] feat: detect properly if fireEvent and userEvent should be reported --- lib/detect-testing-library-utils.ts | 151 ++++++++++++------ lib/node-utils.ts | 10 ++ lib/rules/no-side-effects-wait-for.ts | 25 ++- .../rules/no-side-effects-wait-for.test.ts | 42 ++++- 4 files changed, 166 insertions(+), 62 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 204b4f64..31a0fcf0 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -67,6 +67,7 @@ type IsAsyncUtilFn = ( validNames?: readonly typeof ASYNC_UTILS[number][] ) => boolean; type IsFireEventMethodFn = (node: TSESTree.Identifier) => boolean; +type IsUserEventMethodFn = (node: TSESTree.Identifier) => boolean; type IsRenderUtilFn = (node: TSESTree.Identifier) => boolean; type IsRenderVariableDeclaratorFn = ( node: TSESTree.VariableDeclarator @@ -96,7 +97,10 @@ export interface DetectionHelpers { isQuery: IsQueryFn; isCustomQuery: IsCustomQueryFn; isAsyncUtil: IsAsyncUtilFn; + isFireEventUtil: (node: TSESTree.Identifier) => boolean; + isUserEventUtil: (node: TSESTree.Identifier) => boolean; isFireEventMethod: IsFireEventMethodFn; + isUserEventMethod: IsUserEventMethodFn; isRenderUtil: IsRenderUtilFn; isRenderVariableDeclarator: IsRenderVariableDeclaratorFn; isDebugUtil: IsDebugUtilFn; @@ -107,7 +111,6 @@ export interface DetectionHelpers { isNodeComingFromTestingLibrary: IsNodeComingFromTestingLibraryFn; } -const FIRE_EVENT_NAME = 'fireEvent'; const RENDER_NAME = 'render'; /** @@ -173,6 +176,69 @@ export function detectTestingLibraryUtils< return isNodeComingFromTestingLibrary(referenceNodeIdentifier); } + /** + * Determines whether a given node is a simulate event util related to + * Testing Library or not. + * + * In order to determine this, the node must match: + * - indicated simulate event name: fireEvent or userEvent + * - imported from valid Testing Library module (depends on Aggressive + * Reporting) + * + */ + function isTestingLibrarySimulateEventUtil( + node: TSESTree.Identifier, + utilName: 'fireEvent' | 'userEvent' + ): boolean { + const simulateEventUtil = findImportedUtilSpecifier(utilName); + let simulateEventUtilName: string | undefined; + + if (simulateEventUtil) { + simulateEventUtilName = ASTUtils.isIdentifier(simulateEventUtil) + ? simulateEventUtil.name + : simulateEventUtil.local.name; + } else if (isAggressiveModuleReportingEnabled()) { + simulateEventUtilName = utilName; + } + + if (!simulateEventUtilName) { + return false; + } + + const parentMemberExpression: + | TSESTree.MemberExpression + | undefined = isMemberExpression(node.parent) ? node.parent : undefined; + + if (!parentMemberExpression) { + return false; + } + + // make sure that given node it's not fireEvent/userEvent object itself + if ( + [simulateEventUtilName, utilName].includes(node.name) || + (ASTUtils.isIdentifier(parentMemberExpression.object) && + parentMemberExpression.object.name === node.name) + ) { + return false; + } + + // check fireEvent.click()/userEvent.click() usage + const regularCall = + ASTUtils.isIdentifier(parentMemberExpression.object) && + parentMemberExpression.object.name === simulateEventUtilName; + + // check testingLibraryUtils.fireEvent.click() or + // testingLibraryUtils.userEvent.click() usage + const wildcardCall = + isMemberExpression(parentMemberExpression.object) && + ASTUtils.isIdentifier(parentMemberExpression.object.object) && + parentMemberExpression.object.object.name === simulateEventUtilName && + ASTUtils.isIdentifier(parentMemberExpression.object.property) && + parentMemberExpression.object.property.name === utilName; + + return regularCall || wildcardCall; + } + /** * Determines whether aggressive module reporting is enabled or not. * @@ -308,55 +374,45 @@ export function detectTestingLibraryUtils< }; /** - * Determines whether a given node is fireEvent method or not + * Determines whether a given node is fireEvent util itself or not. + * + * Not to be confused with {@link isFireEventMethod} */ - const isFireEventMethod: IsFireEventMethodFn = (node) => { - const fireEventUtil = findImportedUtilSpecifier(FIRE_EVENT_NAME); - let fireEventUtilName: string | undefined; - - if (fireEventUtil) { - fireEventUtilName = ASTUtils.isIdentifier(fireEventUtil) - ? fireEventUtil.name - : fireEventUtil.local.name; - } else if (isAggressiveModuleReportingEnabled()) { - fireEventUtilName = FIRE_EVENT_NAME; - } - - if (!fireEventUtilName) { - return false; - } - - const parentMemberExpression: - | TSESTree.MemberExpression - | undefined = isMemberExpression(node.parent) ? node.parent : undefined; - - if (!parentMemberExpression) { - return false; - } - - // make sure that given node it's not fireEvent object itself - if ( - [fireEventUtilName, FIRE_EVENT_NAME].includes(node.name) || - (ASTUtils.isIdentifier(parentMemberExpression.object) && - parentMemberExpression.object.name === node.name) - ) { - return false; - } + const isFireEventUtil = (node: TSESTree.Identifier): boolean => { + return isTestingLibraryUtil( + node, + (identifierNodeName, originalNodeName) => { + return [identifierNodeName, originalNodeName].includes('fireEvent'); + } + ); + }; - // check fireEvent.click() usage - const regularCall = - ASTUtils.isIdentifier(parentMemberExpression.object) && - parentMemberExpression.object.name === fireEventUtilName; + /** + * Determines whether a given node is userEvent util itself or not. + * + * Not to be confused with {@link isUserEventMethod} + */ + const isUserEventUtil = (node: TSESTree.Identifier): boolean => { + return isTestingLibraryUtil( + node, + (identifierNodeName, originalNodeName) => { + return [identifierNodeName, originalNodeName].includes('userEvent'); + } + ); + }; - // check testingLibraryUtils.fireEvent.click() usage - const wildcardCall = - isMemberExpression(parentMemberExpression.object) && - ASTUtils.isIdentifier(parentMemberExpression.object.object) && - parentMemberExpression.object.object.name === fireEventUtilName && - ASTUtils.isIdentifier(parentMemberExpression.object.property) && - parentMemberExpression.object.property.name === FIRE_EVENT_NAME; + /** + * Determines whether a given node is fireEvent method or not + */ + const isFireEventMethod: IsFireEventMethodFn = (node) => { + return isTestingLibrarySimulateEventUtil(node, 'fireEvent'); + }; - return regularCall || wildcardCall; + /** + * Determines whether a given node is userEvent method or not + */ + const isUserEventMethod: IsUserEventMethodFn = (node) => { + return isTestingLibrarySimulateEventUtil(node, 'userEvent'); }; /** @@ -557,7 +613,10 @@ export function detectTestingLibraryUtils< isQuery, isCustomQuery, isAsyncUtil, + isFireEventUtil, + isUserEventUtil, isFireEventMethod, + isUserEventMethod, isRenderUtil, isRenderVariableDeclarator, isDebugUtil, diff --git a/lib/node-utils.ts b/lib/node-utils.ts index dc0dde00..07a9d3ff 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -101,6 +101,12 @@ export function isJSXAttribute( return node?.type === AST_NODE_TYPES.JSXAttribute; } +export function isExpressionStatement( + node: TSESTree.Node +): node is TSESTree.ExpressionStatement { + return node?.type === AST_NODE_TYPES.ExpressionStatement; +} + /** * Finds the closest CallExpression node for a given node. * @param node @@ -388,6 +394,10 @@ export function getPropertyIdentifierNode( return getPropertyIdentifierNode(node.callee); } + if (isExpressionStatement(node)) { + return getPropertyIdentifierNode(node.expression); + } + return null; } diff --git a/lib/rules/no-side-effects-wait-for.ts b/lib/rules/no-side-effects-wait-for.ts index 510cd4b6..df58d2d4 100644 --- a/lib/rules/no-side-effects-wait-for.ts +++ b/lib/rules/no-side-effects-wait-for.ts @@ -1,8 +1,7 @@ -import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; +import { TSESTree } from '@typescript-eslint/experimental-utils'; import { getDeepestIdentifierNode, - isCallExpression, - isMemberExpression, + getPropertyIdentifierNode, } from '../node-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; @@ -10,8 +9,6 @@ export const RULE_NAME = 'no-side-effects-wait-for'; export type MessageIds = 'noSideEffectsWaitFor'; type Options = []; -const SIDE_EFFECTS: Array = ['fireEvent', 'userEvent']; - export default createTestingLibraryRule({ name: RULE_NAME, meta: { @@ -32,18 +29,16 @@ export default createTestingLibraryRule({ create: function (context, _, helpers) { function hasSideEffects(body: Array): boolean { return body.some((node: TSESTree.ExpressionStatement) => { - if ( - isCallExpression(node.expression) && - isMemberExpression(node.expression.callee) && - ASTUtils.isIdentifier(node.expression.callee.object) - ) { - const object: TSESTree.Identifier = node.expression.callee.object; - // TODO: check here if `object` is fireEvent or userEvent using helpers - const identifierName: string = object.name; - return SIDE_EFFECTS.includes(identifierName); - } else { + const expressionIdentifier = getPropertyIdentifierNode(node); + + if (!expressionIdentifier) { return false; } + + return ( + helpers.isFireEventUtil(expressionIdentifier) || + helpers.isUserEventUtil(expressionIdentifier) + ); }); } diff --git a/tests/lib/rules/no-side-effects-wait-for.test.ts b/tests/lib/rules/no-side-effects-wait-for.test.ts index 9f648856..7b8a7f1c 100644 --- a/tests/lib/rules/no-side-effects-wait-for.test.ts +++ b/tests/lib/rules/no-side-effects-wait-for.test.ts @@ -139,6 +139,18 @@ ruleTester.run(RULE_NAME, rule, { }) `, }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { waitFor, fireEvent as renamedFireEvent, userEvent as renamedUserEvent } from 'test-utils'; + import { fireEvent, userEvent } from 'somewhere-else'; + + await waitFor(() => { + fireEvent.keyDown(input, {key: 'ArrowDown'}) + userEvent.click(button) + }) + `, + }, // TODO: duplicate prev test but renaming fireEvent and userEvent ], @@ -153,10 +165,19 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, + { + code: ` + import { waitFor, fireEvent as renamedFireEvent } from '@testing-library/react'; + await waitFor(() => { + renamedFireEvent.keyDown(input, {key: 'ArrowDown'}) + }) + `, + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], + }, { settings: { 'testing-library/utils-module': '~/test-utils' }, code: ` - import { waitFor } from '~/test-utils'; + import { waitFor, fireEvent } from '~/test-utils'; await waitFor(() => { fireEvent.keyDown(input, {key: 'ArrowDown'}) }) @@ -222,6 +243,25 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], }, + { + code: ` + import { waitFor, userEvent as renamedUserEvent } from '@testing-library/react'; + await waitFor(() => { + renamedUserEvent.click(button) + }) + `, + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], + }, + { + settings: { 'testing-library/utils-module': '~/test-utils' }, + code: ` + import { waitFor, userEvent } from '~/test-utils'; + await waitFor(() => { + userEvent.click(); + }) + `, + errors: [{ line: 3, column: 15, messageId: 'noSideEffectsWaitFor' }], + }, { code: ` import { waitFor } from '@testing-library/react'; From ed9fe56855a1d56b974d072d8cccb4a8fff84d2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Thu, 25 Mar 2021 20:49:35 +0100 Subject: [PATCH 07/15] test: add cases for increasing coverage up to 100% --- lib/detect-testing-library-utils.ts | 10 ---------- tests/lib/rules/no-side-effects-wait-for.test.ts | 9 +++++++-- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 31a0fcf0..cc5686fa 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -67,7 +67,6 @@ type IsAsyncUtilFn = ( validNames?: readonly typeof ASYNC_UTILS[number][] ) => boolean; type IsFireEventMethodFn = (node: TSESTree.Identifier) => boolean; -type IsUserEventMethodFn = (node: TSESTree.Identifier) => boolean; type IsRenderUtilFn = (node: TSESTree.Identifier) => boolean; type IsRenderVariableDeclaratorFn = ( node: TSESTree.VariableDeclarator @@ -100,7 +99,6 @@ export interface DetectionHelpers { isFireEventUtil: (node: TSESTree.Identifier) => boolean; isUserEventUtil: (node: TSESTree.Identifier) => boolean; isFireEventMethod: IsFireEventMethodFn; - isUserEventMethod: IsUserEventMethodFn; isRenderUtil: IsRenderUtilFn; isRenderVariableDeclarator: IsRenderVariableDeclaratorFn; isDebugUtil: IsDebugUtilFn; @@ -408,13 +406,6 @@ export function detectTestingLibraryUtils< return isTestingLibrarySimulateEventUtil(node, 'fireEvent'); }; - /** - * Determines whether a given node is userEvent method or not - */ - const isUserEventMethod: IsUserEventMethodFn = (node) => { - return isTestingLibrarySimulateEventUtil(node, 'userEvent'); - }; - /** * Determines whether a given node is a valid render util or not. * @@ -616,7 +607,6 @@ export function detectTestingLibraryUtils< isFireEventUtil, isUserEventUtil, isFireEventMethod, - isUserEventMethod, isRenderUtil, isRenderVariableDeclarator, isDebugUtil, diff --git a/tests/lib/rules/no-side-effects-wait-for.test.ts b/tests/lib/rules/no-side-effects-wait-for.test.ts index 7b8a7f1c..b4f85fa5 100644 --- a/tests/lib/rules/no-side-effects-wait-for.test.ts +++ b/tests/lib/rules/no-side-effects-wait-for.test.ts @@ -151,8 +151,13 @@ ruleTester.run(RULE_NAME, rule, { }) `, }, - - // TODO: duplicate prev test but renaming fireEvent and userEvent + { + code: `// weird case to cover 100% coverage + await waitFor(() => { + const click = firEvent['click'] + }) + `, + }, ], invalid: [ // fireEvent From a2e7d2dad3f702b460b00f3df70a348c10e4d5d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Thu, 25 Mar 2021 20:56:45 +0100 Subject: [PATCH 08/15] refactor: rename rule for consistency --- README.md | 4 +++- ...o-side-effects-wait-for.md => no-wait-for-side-effects.md} | 2 +- lib/index.ts | 4 ++-- ...o-side-effects-wait-for.ts => no-wait-for-side-effects.ts} | 2 +- ...ects-wait-for.test.ts => no-wait-for-side-effects.test.ts} | 2 +- 5 files changed, 8 insertions(+), 6 deletions(-) rename docs/rules/{no-side-effects-wait-for.md => no-wait-for-side-effects.md} (95%) rename lib/rules/{no-side-effects-wait-for.ts => no-wait-for-side-effects.ts} (97%) rename tests/lib/rules/{no-side-effects-wait-for.test.ts => no-wait-for-side-effects.test.ts} (99%) diff --git a/README.md b/README.md index 203570bd..76be7857 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,9 @@ [![Tweet][tweet-badge]][tweet-url] + [![All Contributors](https://img.shields.io/badge/all_contributors-36-orange.svg?style=flat-square)](#contributors-) + ## Installation @@ -139,8 +141,8 @@ To enable this configuration use the `extends` property in your | [no-node-access](docs/rules/no-node-access.md) | Disallow direct Node access | ![angular-badge][] ![react-badge][] ![vue-badge][] | | | [no-promise-in-fire-event](docs/rules/no-promise-in-fire-event.md) | Disallow the use of promises passed to a `fireEvent` method | | | | [no-render-in-setup](docs/rules/no-render-in-setup.md) | Disallow the use of `render` in setup functions | | | -| [no-side-effects-wait-for](docs/rules/no-side-effects-wait-for.md) | Disallow the use of side effects inside `waitFor` | | | | [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | +| [no-wait-for-side-effects](docs/rules/no-wait-for-side-effects.md) | Disallow the use of side effects inside `waitFor` | | | | [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | | | | [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | | | | [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | | diff --git a/docs/rules/no-side-effects-wait-for.md b/docs/rules/no-wait-for-side-effects.md similarity index 95% rename from docs/rules/no-side-effects-wait-for.md rename to docs/rules/no-wait-for-side-effects.md index 05cecb33..ccb3dd95 100644 --- a/docs/rules/no-side-effects-wait-for.md +++ b/docs/rules/no-wait-for-side-effects.md @@ -1,4 +1,4 @@ -# Side effects inside `waitFor` are not preferred (no-side-effects-wait-for) +# Side effects inside `waitFor` are not preferred (no-wait-for-side-effects) ## Rule Details diff --git a/lib/index.ts b/lib/index.ts index f722c9eb..6ca532a2 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -20,7 +20,7 @@ import preferUserEvent from './rules/prefer-user-event'; import preferWaitFor from './rules/prefer-wait-for'; import noMultipleAssertionsWaitFor from './rules/no-multiple-assertions-wait-for'; import preferFindBy from './rules/prefer-find-by'; -import noSideEffectsWaitFor from './rules/no-side-effects-wait-for'; +import noWaitForSideEffects from './rules/no-wait-for-side-effects'; import renderResultNamingConvention from './rules/render-result-naming-convention'; const rules = { @@ -38,8 +38,8 @@ const rules = { 'no-node-access': noNodeAccess, 'no-promise-in-fire-event': noPromiseInFireEvent, 'no-render-in-setup': noRenderInSetup, - 'no-side-effects-wait-for': noSideEffectsWaitFor, 'no-wait-for-empty-callback': noWaitForEmptyCallback, + 'no-wait-for-side-effects': noWaitForSideEffects, 'no-wait-for-snapshot': noWaitForSnapshot, 'prefer-explicit-assert': preferExplicitAssert, 'prefer-find-by': preferFindBy, diff --git a/lib/rules/no-side-effects-wait-for.ts b/lib/rules/no-wait-for-side-effects.ts similarity index 97% rename from lib/rules/no-side-effects-wait-for.ts rename to lib/rules/no-wait-for-side-effects.ts index df58d2d4..e828b7d0 100644 --- a/lib/rules/no-side-effects-wait-for.ts +++ b/lib/rules/no-wait-for-side-effects.ts @@ -5,7 +5,7 @@ import { } from '../node-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; -export const RULE_NAME = 'no-side-effects-wait-for'; +export const RULE_NAME = 'no-wait-for-side-effects'; export type MessageIds = 'noSideEffectsWaitFor'; type Options = []; diff --git a/tests/lib/rules/no-side-effects-wait-for.test.ts b/tests/lib/rules/no-wait-for-side-effects.test.ts similarity index 99% rename from tests/lib/rules/no-side-effects-wait-for.test.ts rename to tests/lib/rules/no-wait-for-side-effects.test.ts index b4f85fa5..a72a30b9 100644 --- a/tests/lib/rules/no-side-effects-wait-for.test.ts +++ b/tests/lib/rules/no-wait-for-side-effects.test.ts @@ -1,5 +1,5 @@ import { createRuleTester } from '../test-utils'; -import rule, { RULE_NAME } from '../../../lib/rules/no-side-effects-wait-for'; +import rule, { RULE_NAME } from '../../../lib/rules/no-wait-for-side-effects'; const ruleTester = createRuleTester(); From d740d030336b2cb8b38fbfb0c001af64c83d1312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Thu, 25 Mar 2021 20:57:21 +0100 Subject: [PATCH 09/15] docs: remove duplicated no-wait-for-snapshot row --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 76be7857..e72bb9b1 100644 --- a/README.md +++ b/README.md @@ -144,7 +144,6 @@ To enable this configuration use the `extends` property in your | [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | | [no-wait-for-side-effects](docs/rules/no-wait-for-side-effects.md) | Disallow the use of side effects inside `waitFor` | | | | [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | | | -| [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | | | | [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | | | [prefer-find-by](docs/rules/prefer-find-by.md) | Suggest using `findBy*` methods instead of the `waitFor` + `getBy` queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] | | [prefer-presence-queries](docs/rules/prefer-presence-queries.md) | Enforce specific queries when checking element is present or not | | | From 6fad6768abeb7d087dc402562639d0948cd7fbe1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Fri, 26 Mar 2021 19:14:57 +0100 Subject: [PATCH 10/15] refactor: rename rule --- README.md | 2 +- ...-wait-for.md => no-wait-for-multiple-assertions.md} | 2 +- lib/index.ts | 4 ++-- ...-wait-for.ts => no-wait-for-multiple-assertions.ts} | 8 ++++---- ...test.ts => no-wait-for-multiple-assertions.test.ts} | 10 +++++----- 5 files changed, 13 insertions(+), 13 deletions(-) rename docs/rules/{no-multiple-assertions-wait-for.md => no-wait-for-multiple-assertions.md} (93%) rename lib/rules/{no-multiple-assertions-wait-for.ts => no-wait-for-multiple-assertions.ts} (90%) rename tests/lib/rules/{no-multiple-assertions-wait-for.test.ts => no-wait-for-multiple-assertions.test.ts} (86%) diff --git a/README.md b/README.md index e72bb9b1..e89fc02a 100644 --- a/README.md +++ b/README.md @@ -137,11 +137,11 @@ To enable this configuration use the `extends` property in your | [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | | | [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] | | [no-manual-cleanup](docs/rules/no-manual-cleanup.md) | Disallow the use of `cleanup` | | | -| [no-multiple-assertions-wait-for](docs/rules/no-multiple-assertions-wait-for.md) | Disallow the use of multiple expect inside `waitFor` | | | | [no-node-access](docs/rules/no-node-access.md) | Disallow direct Node access | ![angular-badge][] ![react-badge][] ![vue-badge][] | | | [no-promise-in-fire-event](docs/rules/no-promise-in-fire-event.md) | Disallow the use of promises passed to a `fireEvent` method | | | | [no-render-in-setup](docs/rules/no-render-in-setup.md) | Disallow the use of `render` in setup functions | | | | [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | +| [no-wait-for-multiple-assertions](docs/rules/no-wait-for-multiple-assertions.md) | Disallow the use of multiple expect inside `waitFor` | | | | [no-wait-for-side-effects](docs/rules/no-wait-for-side-effects.md) | Disallow the use of side effects inside `waitFor` | | | | [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | | | | [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | | diff --git a/docs/rules/no-multiple-assertions-wait-for.md b/docs/rules/no-wait-for-multiple-assertions.md similarity index 93% rename from docs/rules/no-multiple-assertions-wait-for.md rename to docs/rules/no-wait-for-multiple-assertions.md index 78c5cf92..5b836dbf 100644 --- a/docs/rules/no-multiple-assertions-wait-for.md +++ b/docs/rules/no-wait-for-multiple-assertions.md @@ -1,4 +1,4 @@ -# Multiple assertions inside `waitFor` are not preferred (no-multiple-assertions-wait-for) +# Multiple assertions inside `waitFor` are not preferred (no-wait-for-multiple-assertions) ## Rule Details diff --git a/lib/index.ts b/lib/index.ts index 6ca532a2..f00fd7b1 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -18,7 +18,7 @@ import preferPresenceQueries from './rules/prefer-presence-queries'; import preferScreenQueries from './rules/prefer-screen-queries'; import preferUserEvent from './rules/prefer-user-event'; import preferWaitFor from './rules/prefer-wait-for'; -import noMultipleAssertionsWaitFor from './rules/no-multiple-assertions-wait-for'; +import noWaitForMultipleAssertions from './rules/no-wait-for-multiple-assertions'; import preferFindBy from './rules/prefer-find-by'; import noWaitForSideEffects from './rules/no-wait-for-side-effects'; import renderResultNamingConvention from './rules/render-result-naming-convention'; @@ -34,11 +34,11 @@ const rules = { 'no-debug': noDebug, 'no-dom-import': noDomImport, 'no-manual-cleanup': noManualCleanup, - 'no-multiple-assertions-wait-for': noMultipleAssertionsWaitFor, 'no-node-access': noNodeAccess, 'no-promise-in-fire-event': noPromiseInFireEvent, 'no-render-in-setup': noRenderInSetup, 'no-wait-for-empty-callback': noWaitForEmptyCallback, + 'no-wait-for-multiple-assertions': noWaitForMultipleAssertions, 'no-wait-for-side-effects': noWaitForSideEffects, 'no-wait-for-snapshot': noWaitForSnapshot, 'prefer-explicit-assert': preferExplicitAssert, diff --git a/lib/rules/no-multiple-assertions-wait-for.ts b/lib/rules/no-wait-for-multiple-assertions.ts similarity index 90% rename from lib/rules/no-multiple-assertions-wait-for.ts rename to lib/rules/no-wait-for-multiple-assertions.ts index 99fb11b5..8f1e9f77 100644 --- a/lib/rules/no-multiple-assertions-wait-for.ts +++ b/lib/rules/no-wait-for-multiple-assertions.ts @@ -10,8 +10,8 @@ import { isCallExpression, } from '../node-utils'; -export const RULE_NAME = 'no-multiple-assertions-wait-for'; -export type MessageIds = 'noMultipleAssertionWaitFor'; +export const RULE_NAME = 'no-wait-for-multiple-assertions'; +export type MessageIds = 'noWaitForMultipleAssertion'; type Options = []; const WAIT_EXPRESSION_QUERY = 'CallExpression[callee.name=/^(waitFor)$/]'; @@ -26,7 +26,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ recommended: false, }, messages: { - noMultipleAssertionWaitFor: + noWaitForMultipleAssertion: 'Avoid using multiple assertions within `waitFor` callback', }, fixable: null, @@ -56,7 +56,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ context.report({ node, loc: node.loc.start, - messageId: 'noMultipleAssertionWaitFor', + messageId: 'noWaitForMultipleAssertion', }); } } diff --git a/tests/lib/rules/no-multiple-assertions-wait-for.test.ts b/tests/lib/rules/no-wait-for-multiple-assertions.test.ts similarity index 86% rename from tests/lib/rules/no-multiple-assertions-wait-for.test.ts rename to tests/lib/rules/no-wait-for-multiple-assertions.test.ts index 4398662a..6ec420e8 100644 --- a/tests/lib/rules/no-multiple-assertions-wait-for.test.ts +++ b/tests/lib/rules/no-wait-for-multiple-assertions.test.ts @@ -1,7 +1,7 @@ import { createRuleTester } from '../test-utils'; import rule, { RULE_NAME, -} from '../../../lib/rules/no-multiple-assertions-wait-for'; +} from '../../../lib/rules/no-wait-for-multiple-assertions'; const ruleTester = createRuleTester(); @@ -78,7 +78,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ messageId: 'noMultipleAssertionWaitFor' }], + errors: [{ messageId: 'noWaitForMultipleAssertion' }], }, { code: ` @@ -88,7 +88,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ messageId: 'noMultipleAssertionWaitFor' }], + errors: [{ messageId: 'noWaitForMultipleAssertion' }], }, { code: ` @@ -97,7 +97,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ messageId: 'noMultipleAssertionWaitFor' }], + errors: [{ messageId: 'noWaitForMultipleAssertion' }], }, { code: ` @@ -107,7 +107,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ messageId: 'noMultipleAssertionWaitFor' }], + errors: [{ messageId: 'noWaitForMultipleAssertion' }], }, ], }); From af5375ef99d1589d7a902d407d8df83c8d911b1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Fri, 26 Mar 2021 19:19:44 +0100 Subject: [PATCH 11/15] test: improve errors location asserts --- .../no-wait-for-multiple-assertions.test.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/lib/rules/no-wait-for-multiple-assertions.test.ts b/tests/lib/rules/no-wait-for-multiple-assertions.test.ts index 6ec420e8..560891b7 100644 --- a/tests/lib/rules/no-wait-for-multiple-assertions.test.ts +++ b/tests/lib/rules/no-wait-for-multiple-assertions.test.ts @@ -78,7 +78,9 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ messageId: 'noWaitForMultipleAssertion' }], + errors: [ + { line: 2, column: 29, messageId: 'noWaitForMultipleAssertion' }, + ], }, { code: ` @@ -88,7 +90,9 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ messageId: 'noWaitForMultipleAssertion' }], + errors: [ + { line: 2, column: 29, messageId: 'noWaitForMultipleAssertion' }, + ], }, { code: ` @@ -97,7 +101,9 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ messageId: 'noWaitForMultipleAssertion' }], + errors: [ + { line: 2, column: 34, messageId: 'noWaitForMultipleAssertion' }, + ], }, { code: ` @@ -107,7 +113,9 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }) `, - errors: [{ messageId: 'noWaitForMultipleAssertion' }], + errors: [ + { line: 2, column: 34, messageId: 'noWaitForMultipleAssertion' }, + ], }, ], }); From f7f348da3dc8893202024b9cd22dabf705ec7efa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Fri, 26 Mar 2021 19:35:48 +0100 Subject: [PATCH 12/15] refactor: use new rule creator --- lib/rules/no-wait-for-multiple-assertions.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/rules/no-wait-for-multiple-assertions.ts b/lib/rules/no-wait-for-multiple-assertions.ts index 8f1e9f77..071260dc 100644 --- a/lib/rules/no-wait-for-multiple-assertions.ts +++ b/lib/rules/no-wait-for-multiple-assertions.ts @@ -1,14 +1,10 @@ -import { - ESLintUtils, - TSESTree, - ASTUtils, -} from '@typescript-eslint/experimental-utils'; -import { getDocsUrl } from '../utils'; +import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; import { isBlockStatement, - isMemberExpression, isCallExpression, + isMemberExpression, } from '../node-utils'; +import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'no-wait-for-multiple-assertions'; export type MessageIds = 'noWaitForMultipleAssertion'; @@ -16,7 +12,7 @@ type Options = []; const WAIT_EXPRESSION_QUERY = 'CallExpression[callee.name=/^(waitFor)$/]'; -export default ESLintUtils.RuleCreator(getDocsUrl)({ +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'suggestion', From 61b131ca4efcd15a0e3ff2d2d8e957f4688499e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Fri, 26 Mar 2021 19:46:50 +0100 Subject: [PATCH 13/15] refactor: use new helpers for detection --- lib/rules/no-wait-for-multiple-assertions.ts | 66 ++++++++++---------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/lib/rules/no-wait-for-multiple-assertions.ts b/lib/rules/no-wait-for-multiple-assertions.ts index 071260dc..99ca3282 100644 --- a/lib/rules/no-wait-for-multiple-assertions.ts +++ b/lib/rules/no-wait-for-multiple-assertions.ts @@ -1,17 +1,11 @@ -import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; -import { - isBlockStatement, - isCallExpression, - isMemberExpression, -} from '../node-utils'; +import { TSESTree } from '@typescript-eslint/experimental-utils'; +import { getPropertyIdentifierNode } from '../node-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'no-wait-for-multiple-assertions'; export type MessageIds = 'noWaitForMultipleAssertion'; type Options = []; -const WAIT_EXPRESSION_QUERY = 'CallExpression[callee.name=/^(waitFor)$/]'; - export default createTestingLibraryRule({ name: RULE_NAME, meta: { @@ -29,37 +23,43 @@ export default createTestingLibraryRule({ schema: [], }, defaultOptions: [], - create: function (context) { + create: function (context, _, helpers) { + function totalExpect(body: Array): Array { + return body.filter((node: TSESTree.ExpressionStatement) => { + const expressionIdentifier = getPropertyIdentifierNode(node); + + if (!expressionIdentifier) { + return false; + } + + return expressionIdentifier.name === 'expect'; + }); + } + function reportMultipleAssertion(node: TSESTree.BlockStatement) { - const totalExpect = (body: Array): Array => - body.filter((node: TSESTree.ExpressionStatement) => { - if ( - isCallExpression(node.expression) && - isMemberExpression(node.expression.callee) && - isCallExpression(node.expression.callee.object) - ) { - const object: TSESTree.CallExpression = - node.expression.callee.object; - const expressionName: string = - ASTUtils.isIdentifier(object.callee) && object.callee.name; - return expressionName === 'expect'; - } else { - return false; - } - }); + const callExpressionNode = node.parent.parent as TSESTree.CallExpression; + const callExpressionIdentifier = getPropertyIdentifierNode( + callExpressionNode + ); + + if (!helpers.isAsyncUtil(callExpressionIdentifier, ['waitFor'])) { + return; + } - if (isBlockStatement(node) && totalExpect(node.body).length > 1) { - context.report({ - node, - loc: node.loc.start, - messageId: 'noWaitForMultipleAssertion', - }); + if (totalExpect(node.body).length <= 1) { + return; } + + context.report({ + node, + loc: node.loc.start, + messageId: 'noWaitForMultipleAssertion', + }); } return { - [`${WAIT_EXPRESSION_QUERY} > ArrowFunctionExpression > BlockStatement`]: reportMultipleAssertion, - [`${WAIT_EXPRESSION_QUERY} > FunctionExpression > BlockStatement`]: reportMultipleAssertion, + 'CallExpression > ArrowFunctionExpression > BlockStatement': reportMultipleAssertion, + 'CallExpression > FunctionExpression > BlockStatement': reportMultipleAssertion, }; }, }); From ccee1a6882a0a7cf3ea54a6fe66fd7c38e6ada65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Fri, 26 Mar 2021 19:51:09 +0100 Subject: [PATCH 14/15] refactor: improve error reported location --- lib/rules/no-wait-for-multiple-assertions.ts | 3 +-- tests/lib/rules/no-wait-for-multiple-assertions.test.ts | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/rules/no-wait-for-multiple-assertions.ts b/lib/rules/no-wait-for-multiple-assertions.ts index 99ca3282..d92aea3d 100644 --- a/lib/rules/no-wait-for-multiple-assertions.ts +++ b/lib/rules/no-wait-for-multiple-assertions.ts @@ -51,8 +51,7 @@ export default createTestingLibraryRule({ } context.report({ - node, - loc: node.loc.start, + node: callExpressionNode, messageId: 'noWaitForMultipleAssertion', }); } diff --git a/tests/lib/rules/no-wait-for-multiple-assertions.test.ts b/tests/lib/rules/no-wait-for-multiple-assertions.test.ts index 560891b7..23efb858 100644 --- a/tests/lib/rules/no-wait-for-multiple-assertions.test.ts +++ b/tests/lib/rules/no-wait-for-multiple-assertions.test.ts @@ -79,7 +79,7 @@ ruleTester.run(RULE_NAME, rule, { }) `, errors: [ - { line: 2, column: 29, messageId: 'noWaitForMultipleAssertion' }, + { line: 2, column: 15, messageId: 'noWaitForMultipleAssertion' }, ], }, { @@ -91,7 +91,7 @@ ruleTester.run(RULE_NAME, rule, { }) `, errors: [ - { line: 2, column: 29, messageId: 'noWaitForMultipleAssertion' }, + { line: 2, column: 15, messageId: 'noWaitForMultipleAssertion' }, ], }, { @@ -102,7 +102,7 @@ ruleTester.run(RULE_NAME, rule, { }) `, errors: [ - { line: 2, column: 34, messageId: 'noWaitForMultipleAssertion' }, + { line: 2, column: 15, messageId: 'noWaitForMultipleAssertion' }, ], }, { @@ -114,7 +114,7 @@ ruleTester.run(RULE_NAME, rule, { }) `, errors: [ - { line: 2, column: 34, messageId: 'noWaitForMultipleAssertion' }, + { line: 2, column: 15, messageId: 'noWaitForMultipleAssertion' }, ], }, ], From c8784badb670ad0cb46135d6ca5071956c87a0e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Fri, 26 Mar 2021 20:02:07 +0100 Subject: [PATCH 15/15] test: add more cases --- .../no-wait-for-multiple-assertions.test.ts | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/tests/lib/rules/no-wait-for-multiple-assertions.test.ts b/tests/lib/rules/no-wait-for-multiple-assertions.test.ts index 23efb858..1be3055a 100644 --- a/tests/lib/rules/no-wait-for-multiple-assertions.test.ts +++ b/tests/lib/rules/no-wait-for-multiple-assertions.test.ts @@ -19,6 +19,27 @@ ruleTester.run(RULE_NAME, rule, { }) `, }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: `// Aggressive Reporting disabled - module imported not matching + import { waitFor } from 'somewhere-else' + await waitFor(() => { + expect(a).toEqual('a') + expect(b).toEqual('b') + }) + `, + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: `// Aggressive Reporting disabled - waitFor renamed + import { waitFor as renamedWaitFor } from '@testing-library/react' + import { waitFor } from 'somewhere-else' + await waitFor(() => { + expect(a).toEqual('a') + expect(b).toEqual('b') + }) + `, + }, // this needs to be check by other rule { code: ` @@ -82,6 +103,32 @@ ruleTester.run(RULE_NAME, rule, { { line: 2, column: 15, messageId: 'noWaitForMultipleAssertion' }, ], }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: `// Aggressive Reporting disabled + import { waitFor } from '@testing-library/react' + await waitFor(() => { + expect(a).toEqual('a') + expect(b).toEqual('b') + }) + `, + errors: [ + { line: 3, column: 15, messageId: 'noWaitForMultipleAssertion' }, + ], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: `// Aggressive Reporting disabled + import { waitFor as renamedWaitFor } from 'test-utils' + await renamedWaitFor(() => { + expect(a).toEqual('a') + expect(b).toEqual('b') + }) + `, + errors: [ + { line: 3, column: 15, messageId: 'noWaitForMultipleAssertion' }, + ], + }, { code: ` await waitFor(() => { @@ -94,6 +141,32 @@ ruleTester.run(RULE_NAME, rule, { { line: 2, column: 15, messageId: 'noWaitForMultipleAssertion' }, ], }, + { + code: ` + test('should whatever', async () => { + await waitFor(() => { + expect(a).toEqual('a') + console.log('testing-library') + expect(b).toEqual('b') + }) + }) + `, + errors: [ + { line: 3, column: 17, messageId: 'noWaitForMultipleAssertion' }, + ], + }, + { + code: ` + await waitFor(async () => { + expect(a).toEqual('a') + await somethingAsync() + expect(b).toEqual('b') + }) + `, + errors: [ + { line: 2, column: 15, messageId: 'noWaitForMultipleAssertion' }, + ], + }, { code: ` await waitFor(function() { @@ -117,5 +190,17 @@ ruleTester.run(RULE_NAME, rule, { { line: 2, column: 15, messageId: 'noWaitForMultipleAssertion' }, ], }, + { + code: ` + await waitFor(async function() { + expect(a).toEqual('a') + await somethingAsync() + expect(b).toEqual('b') + }) + `, + errors: [ + { line: 2, column: 15, messageId: 'noWaitForMultipleAssertion' }, + ], + }, ], });