From 5720a80cc64fd78d7193c01810f8cb4bd559df12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Sun, 14 Mar 2021 13:18:35 +0100 Subject: [PATCH 1/4] refactor(no-wait-for-empty-callback): use new rule creator and helpers --- lib/detect-testing-library-utils.ts | 17 ++++++--- lib/rules/no-wait-for-empty-callback.ts | 50 +++++++++++++++++-------- lib/utils.ts | 7 ++++ 3 files changed, 54 insertions(+), 20 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 9413c881..2f6e1281 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -22,6 +22,7 @@ import { ASYNC_UTILS, PRESENCE_MATCHERS, ALL_QUERIES_COMBINATIONS, + VALID_ASYNC_UTILS, } from './utils'; export type TestingLibrarySettings = { @@ -63,7 +64,10 @@ type IsSyncQueryFn = (node: TSESTree.Identifier) => boolean; type IsAsyncQueryFn = (node: TSESTree.Identifier) => boolean; type IsQueryFn = (node: TSESTree.Identifier) => boolean; type IsCustomQueryFn = (node: TSESTree.Identifier) => boolean; -type IsAsyncUtilFn = (node: TSESTree.Identifier) => boolean; +type IsAsyncUtilFn = ( + node: TSESTree.Identifier, + validNames?: VALID_ASYNC_UTILS[] +) => boolean; type IsFireEventMethodFn = (node: TSESTree.Identifier) => boolean; type IsRenderUtilFn = (node: TSESTree.Identifier) => boolean; type IsPresenceAssertFn = (node: TSESTree.MemberExpression) => boolean; @@ -298,10 +302,13 @@ export function detectTestingLibraryUtils< * Otherwise, it means `custom-module` has been set up, so only those nodes * coming from Testing Library will be considered as valid. */ - const isAsyncUtil: IsAsyncUtilFn = (node) => { - return isTestingLibraryUtil(node, (identifierNodeName) => - ASYNC_UTILS.includes(identifierNodeName) - ); + const isAsyncUtil: IsAsyncUtilFn = (node, validNames) => { + return isTestingLibraryUtil(node, (identifierNodeName) => { + if (validNames && validNames.length > 0) { + return (validNames as string[]).includes(identifierNodeName); + } + return ASYNC_UTILS.includes(identifierNodeName); + }); }; /** diff --git a/lib/rules/no-wait-for-empty-callback.ts b/lib/rules/no-wait-for-empty-callback.ts index 7ff27dd9..1b88fc98 100644 --- a/lib/rules/no-wait-for-empty-callback.ts +++ b/lib/rules/no-wait-for-empty-callback.ts @@ -1,19 +1,16 @@ +import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; import { - ESLintUtils, - TSESTree, - ASTUtils, -} from '@typescript-eslint/experimental-utils'; -import { getDocsUrl } from '../utils'; -import { isBlockStatement, isCallExpression } from '../node-utils'; + getPropertyIdentifierNode, + isBlockStatement, + isCallExpression, +} from '../node-utils'; +import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'no-wait-for-empty-callback'; export type MessageIds = 'noWaitForEmptyCallback'; type Options = []; -const WAIT_EXPRESSION_QUERY = - 'CallExpression[callee.name=/^(waitFor|waitForElementToBeRemoved)$/]'; - -export default ESLintUtils.RuleCreator(getDocsUrl)({ +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'suggestion', @@ -33,11 +30,24 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ defaultOptions: [], // trimmed down implementation of https://github.com/eslint/eslint/blob/master/lib/rules/no-empty-function.js - // TODO: var referencing any of previously mentioned? - create: function (context) { + create(context, _, helpers) { + function isValidWaitFor(node: TSESTree.Node): boolean { + const parentCallExpression = node.parent as TSESTree.CallExpression; + const parentIdentifier = getPropertyIdentifierNode(parentCallExpression); + + return helpers.isAsyncUtil(parentIdentifier, [ + 'waitFor', + 'waitForElementToBeRemoved', + ]); + } + function reportIfEmpty( node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression ) { + if (!isValidWaitFor(node)) { + return; + } + if ( isBlockStatement(node.body) && node.body.body.length === 0 && @@ -56,17 +66,27 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ } function reportNoop(node: TSESTree.Identifier) { + if (!isValidWaitFor(node)) { + return; + } + context.report({ node, loc: node.loc.start, messageId: 'noWaitForEmptyCallback', + data: { + methodName: + isCallExpression(node.parent) && + ASTUtils.isIdentifier(node.parent.callee) && + node.parent.callee.name, + }, }); } return { - [`${WAIT_EXPRESSION_QUERY} > ArrowFunctionExpression`]: reportIfEmpty, - [`${WAIT_EXPRESSION_QUERY} > FunctionExpression`]: reportIfEmpty, - [`${WAIT_EXPRESSION_QUERY} > Identifier[name="noop"]`]: reportNoop, + 'CallExpression > ArrowFunctionExpression': reportIfEmpty, + 'CallExpression > FunctionExpression': reportIfEmpty, + 'CallExpression > Identifier[name="noop"]': reportNoop, }; }, }); diff --git a/lib/utils.ts b/lib/utils.ts index b7c6725d..71bf7ff1 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -114,6 +114,13 @@ const ALL_RETURNING_NODES = [ const PRESENCE_MATCHERS = ['toBeInTheDocument', 'toBeTruthy', 'toBeDefined']; const ABSENCE_MATCHERS = ['toBeNull', 'toBeFalsy']; +export type VALID_ASYNC_UTILS = + | 'waitFor' + | 'waitForElementToBeRemoved' + | 'wait' + | 'waitForElement' + | 'waitForDomChange'; + export { combineQueries, getDocsUrl, From 908fa2d65822c2656679913f5606f242a99277e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Sun, 14 Mar 2021 13:24:19 +0100 Subject: [PATCH 2/4] test(no-wait-for-empty-callback): improve invalid asserts --- .../rules/no-wait-for-empty-callback.test.ts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/lib/rules/no-wait-for-empty-callback.test.ts b/tests/lib/rules/no-wait-for-empty-callback.test.ts index 17d8de07..b0275ecf 100644 --- a/tests/lib/rules/no-wait-for-empty-callback.test.ts +++ b/tests/lib/rules/no-wait-for-empty-callback.test.ts @@ -36,7 +36,12 @@ ruleTester.run(RULE_NAME, rule, { code: `${m}(() => {})`, errors: [ { + line: 1, + column: 8 + m.length, messageId: 'noWaitForEmptyCallback', + data: { + methodName: m, + }, }, ], })), @@ -44,7 +49,12 @@ ruleTester.run(RULE_NAME, rule, { code: `${m}((a, b) => {})`, errors: [ { + line: 1, + column: 12 + m.length, messageId: 'noWaitForEmptyCallback', + data: { + methodName: m, + }, }, ], })), @@ -52,7 +62,12 @@ ruleTester.run(RULE_NAME, rule, { code: `${m}(() => { /* I'm empty anyway */ })`, errors: [ { + line: 1, + column: 8 + m.length, messageId: 'noWaitForEmptyCallback', + data: { + methodName: m, + }, }, ], })), @@ -63,7 +78,12 @@ ruleTester.run(RULE_NAME, rule, { })`, errors: [ { + line: 1, + column: 13 + m.length, messageId: 'noWaitForEmptyCallback', + data: { + methodName: m, + }, }, ], })), @@ -73,7 +93,12 @@ ruleTester.run(RULE_NAME, rule, { })`, errors: [ { + line: 1, + column: 14 + m.length, messageId: 'noWaitForEmptyCallback', + data: { + methodName: m, + }, }, ], })), @@ -83,7 +108,12 @@ ruleTester.run(RULE_NAME, rule, { })`, errors: [ { + line: 1, + column: 13 + m.length, messageId: 'noWaitForEmptyCallback', + data: { + methodName: m, + }, }, ], })), @@ -92,7 +122,12 @@ ruleTester.run(RULE_NAME, rule, { code: `${m}(noop)`, errors: [ { + line: 1, + column: 2 + m.length, messageId: 'noWaitForEmptyCallback', + data: { + methodName: m, + }, }, ], })), From 7ec46c0fa9efa41fe937534644258d9cc17fef26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Sun, 14 Mar 2021 13:34:04 +0100 Subject: [PATCH 3/4] test(no-wait-for-empty-callback): increase rule coverage --- lib/detect-testing-library-utils.ts | 19 +++++-- .../rules/no-wait-for-empty-callback.test.ts | 52 +++++++++++++++++++ 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 2f6e1281..601091af 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -303,12 +303,21 @@ export function detectTestingLibraryUtils< * coming from Testing Library will be considered as valid. */ const isAsyncUtil: IsAsyncUtilFn = (node, validNames) => { - return isTestingLibraryUtil(node, (identifierNodeName) => { - if (validNames && validNames.length > 0) { - return (validNames as string[]).includes(identifierNodeName); + return isTestingLibraryUtil( + node, + (identifierNodeName, originalNodeName) => { + if (validNames && validNames.length > 0) { + return ( + (validNames as string[]).includes(identifierNodeName) || + (validNames as string[]).includes(originalNodeName) + ); + } + return ( + ASYNC_UTILS.includes(identifierNodeName) || + ASYNC_UTILS.includes(originalNodeName) + ); } - return ASYNC_UTILS.includes(identifierNodeName); - }); + ); }; /** diff --git a/tests/lib/rules/no-wait-for-empty-callback.test.ts b/tests/lib/rules/no-wait-for-empty-callback.test.ts index b0275ecf..76e57789 100644 --- a/tests/lib/rules/no-wait-for-empty-callback.test.ts +++ b/tests/lib/rules/no-wait-for-empty-callback.test.ts @@ -29,6 +29,24 @@ ruleTester.run(RULE_NAME, rule, { { code: `wait(() => {})`, }, + { + code: `wait(noop)`, + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { waitFor } from 'somewhere-else' + waitFor(() => {}) + `, + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { waitFor as renamedWaitFor } from '@testing-library/react' + import { waitFor } from 'somewhere-else' + waitFor(() => {}) + `, + }, ], invalid: [ @@ -45,6 +63,40 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), + ...ALL_WAIT_METHODS.map((m) => ({ + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { ${m} } from 'test-utils'; + ${m}(() => {}); + `, + errors: [ + { + line: 3, + column: 16 + m.length, + messageId: 'noWaitForEmptyCallback', + data: { + methodName: m, + }, + }, + ], + })), + ...ALL_WAIT_METHODS.map((m) => ({ + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { ${m} as renamedAsyncUtil } from 'test-utils'; + renamedAsyncUtil(() => {}); + `, + errors: [ + { + line: 3, + column: 32, + messageId: 'noWaitForEmptyCallback', + data: { + methodName: 'renamedAsyncUtil', + }, + }, + ], + })), ...ALL_WAIT_METHODS.map((m) => ({ code: `${m}((a, b) => {})`, errors: [ From 5831fce6a73229735372703a35b5063c56d6fa1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Tue, 16 Mar 2021 20:28:16 +0100 Subject: [PATCH 4/4] refactor: improve valid names definition (PR suggestions) --- lib/detect-testing-library-utils.ts | 15 ++++----------- lib/utils.ts | 9 +-------- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 601091af..7559ae8f 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -22,7 +22,6 @@ import { ASYNC_UTILS, PRESENCE_MATCHERS, ALL_QUERIES_COMBINATIONS, - VALID_ASYNC_UTILS, } from './utils'; export type TestingLibrarySettings = { @@ -66,7 +65,7 @@ type IsQueryFn = (node: TSESTree.Identifier) => boolean; type IsCustomQueryFn = (node: TSESTree.Identifier) => boolean; type IsAsyncUtilFn = ( node: TSESTree.Identifier, - validNames?: VALID_ASYNC_UTILS[] + validNames?: readonly typeof ASYNC_UTILS[number][] ) => boolean; type IsFireEventMethodFn = (node: TSESTree.Identifier) => boolean; type IsRenderUtilFn = (node: TSESTree.Identifier) => boolean; @@ -302,19 +301,13 @@ export function detectTestingLibraryUtils< * Otherwise, it means `custom-module` has been set up, so only those nodes * coming from Testing Library will be considered as valid. */ - const isAsyncUtil: IsAsyncUtilFn = (node, validNames) => { + const isAsyncUtil: IsAsyncUtilFn = (node, validNames = ASYNC_UTILS) => { return isTestingLibraryUtil( node, (identifierNodeName, originalNodeName) => { - if (validNames && validNames.length > 0) { - return ( - (validNames as string[]).includes(identifierNodeName) || - (validNames as string[]).includes(originalNodeName) - ); - } return ( - ASYNC_UTILS.includes(identifierNodeName) || - ASYNC_UTILS.includes(originalNodeName) + (validNames as string[]).includes(identifierNodeName) || + (validNames as string[]).includes(originalNodeName) ); } ); diff --git a/lib/utils.ts b/lib/utils.ts index 71bf7ff1..a464a517 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -70,7 +70,7 @@ const ASYNC_UTILS = [ 'wait', 'waitForElement', 'waitForDomChange', -]; +] as const; const SYNC_EVENTS = ['fireEvent', 'userEvent']; @@ -114,13 +114,6 @@ const ALL_RETURNING_NODES = [ const PRESENCE_MATCHERS = ['toBeInTheDocument', 'toBeTruthy', 'toBeDefined']; const ABSENCE_MATCHERS = ['toBeNull', 'toBeFalsy']; -export type VALID_ASYNC_UTILS = - | 'waitFor' - | 'waitForElementToBeRemoved' - | 'wait' - | 'waitForElement' - | 'waitForDomChange'; - export { combineQueries, getDocsUrl,