From cb8c50ca3c5b70769bcfbb9123eb81a743a9ceee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Wed, 11 Nov 2020 19:45:36 +0100 Subject: [PATCH 01/27] refactor(no-await-sync-query): use custom rule creator --- lib/rules/no-await-sync-query.ts | 6 +++--- tests/lib/rules/no-await-sync-query.test.ts | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/rules/no-await-sync-query.ts b/lib/rules/no-await-sync-query.ts index 7e4efec1..3819891a 100644 --- a/lib/rules/no-await-sync-query.ts +++ b/lib/rules/no-await-sync-query.ts @@ -1,5 +1,5 @@ -import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; -import { getDocsUrl } from '../utils'; +import { TSESTree } from '@typescript-eslint/experimental-utils'; +import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'no-await-sync-query'; export type MessageIds = 'noAwaitSyncQuery'; @@ -7,7 +7,7 @@ type Options = []; const SYNC_QUERIES_REGEXP = /^(get|query)(All)?By(LabelText|PlaceholderText|Text|AltText|Title|DisplayValue|Role|TestId)$/; -export default ESLintUtils.RuleCreator(getDocsUrl)({ +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'problem', diff --git a/tests/lib/rules/no-await-sync-query.test.ts b/tests/lib/rules/no-await-sync-query.test.ts index 138cf5c9..d71ae4be 100644 --- a/tests/lib/rules/no-await-sync-query.test.ts +++ b/tests/lib/rules/no-await-sync-query.test.ts @@ -8,6 +8,7 @@ import { const ruleTester = createRuleTester(); ruleTester.run(RULE_NAME, rule, { + // TODO: add variants for custom queries for each map valid: [ // sync queries without await are valid ...SYNC_QUERIES_COMBINATIONS.map((query) => ({ @@ -34,6 +35,7 @@ ruleTester.run(RULE_NAME, rule, { })), ], + // TODO: add variants for custom queries for each map invalid: [ // sync queries with await operator are not valid ...SYNC_QUERIES_COMBINATIONS.map((query) => ({ From 501f604d4fb78211d4e39d91266e5dbe44e0539c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Wed, 11 Nov 2020 19:47:43 +0100 Subject: [PATCH 02/27] refactor(no-await-sync-query): improve error message --- lib/rules/no-await-sync-query.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/rules/no-await-sync-query.ts b/lib/rules/no-await-sync-query.ts index 3819891a..c09fb5a9 100644 --- a/lib/rules/no-await-sync-query.ts +++ b/lib/rules/no-await-sync-query.ts @@ -17,7 +17,8 @@ export default createTestingLibraryRule({ recommended: 'error', }, messages: { - noAwaitSyncQuery: '`{{ name }}` does not need `await` operator', + noAwaitSyncQuery: + '`{{ name }}` query is sync so it does not need to be awaited', }, fixable: null, schema: [], From 4279c72546e5a5725d141b7bbd4030bb708e7b21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Wed, 11 Nov 2020 19:48:17 +0100 Subject: [PATCH 03/27] test(no-await-sync-query): check error location in invalid cases --- tests/lib/rules/no-await-sync-query.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/lib/rules/no-await-sync-query.test.ts b/tests/lib/rules/no-await-sync-query.test.ts index d71ae4be..588b4558 100644 --- a/tests/lib/rules/no-await-sync-query.test.ts +++ b/tests/lib/rules/no-await-sync-query.test.ts @@ -46,6 +46,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'noAwaitSyncQuery', + line: 2, + column: 15, }, ], })), @@ -59,6 +61,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'noAwaitSyncQuery', + line: 2, + column: 22, }, ], })), From 19fbcf3f055e59f583b40e919d5f8c2fcdb413f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Wed, 11 Nov 2020 20:02:30 +0100 Subject: [PATCH 04/27] refactor(no-await-sync-query): catch sync queries with detection helper --- lib/rules/no-await-sync-query.ts | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/lib/rules/no-await-sync-query.ts b/lib/rules/no-await-sync-query.ts index c09fb5a9..ced9d104 100644 --- a/lib/rules/no-await-sync-query.ts +++ b/lib/rules/no-await-sync-query.ts @@ -5,8 +5,6 @@ export const RULE_NAME = 'no-await-sync-query'; export type MessageIds = 'noAwaitSyncQuery'; type Options = []; -const SYNC_QUERIES_REGEXP = /^(get|query)(All)?By(LabelText|PlaceholderText|Text|AltText|Title|DisplayValue|Role|TestId)$/; - export default createTestingLibraryRule({ name: RULE_NAME, meta: { @@ -25,18 +23,19 @@ export default createTestingLibraryRule({ }, defaultOptions: [], - create(context) { - const reportError = (node: TSESTree.Identifier) => - context.report({ - node, - messageId: 'noAwaitSyncQuery', - data: { - name: node.name, - }, - }); + create(context, _, helpers) { return { - [`AwaitExpression > CallExpression > Identifier[name=${SYNC_QUERIES_REGEXP}]`]: reportError, - [`AwaitExpression > CallExpression > MemberExpression > Identifier[name=${SYNC_QUERIES_REGEXP}]`]: reportError, + 'AwaitExpression > CallExpression Identifier'(node: TSESTree.Identifier) { + if (helpers.isSyncQuery(node)) { + context.report({ + node, + messageId: 'noAwaitSyncQuery', + data: { + name: node.name, + }, + }); + } + }, }; }, }); From 13641820214891116daa18d347d805f1e9b6dfd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Wed, 11 Nov 2020 20:14:36 +0100 Subject: [PATCH 05/27] test(no-await-sync-query): use more realistic scenarios --- tests/lib/rules/no-await-sync-query.test.ts | 48 ++++++++++++++++++--- 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/tests/lib/rules/no-await-sync-query.test.ts b/tests/lib/rules/no-await-sync-query.test.ts index 588b4558..1e64e658 100644 --- a/tests/lib/rules/no-await-sync-query.test.ts +++ b/tests/lib/rules/no-await-sync-query.test.ts @@ -13,7 +13,14 @@ ruleTester.run(RULE_NAME, rule, { // sync queries without await are valid ...SYNC_QUERIES_COMBINATIONS.map((query) => ({ code: `() => { - ${query}('foo') + const element = ${query}('foo') + } + `, + })), + // sync queries without await inside assert are valid + ...SYNC_QUERIES_COMBINATIONS.map((query) => ({ + code: `() => { + expect(${query}('foo')).toBeEnabled() } `, })), @@ -21,7 +28,7 @@ ruleTester.run(RULE_NAME, rule, { // async queries with await operator are valid ...ASYNC_QUERIES_COMBINATIONS.map((query) => ({ code: `async () => { - await ${query}('foo') + const element = await ${query}('foo') } `, })), @@ -40,14 +47,28 @@ ruleTester.run(RULE_NAME, rule, { // sync queries with await operator are not valid ...SYNC_QUERIES_COMBINATIONS.map((query) => ({ code: `async () => { - await ${query}('foo') + const element = await ${query}('foo') + } + `, + errors: [ + { + messageId: 'noAwaitSyncQuery', + line: 2, + column: 31, + }, + ], + })), + // sync queries with await operator inside assert are not valid + ...SYNC_QUERIES_COMBINATIONS.map((query) => ({ + code: `async () => { + expect(await ${query}('foo')).toBeEnabled() } `, errors: [ { messageId: 'noAwaitSyncQuery', line: 2, - column: 15, + column: 22, }, ], })), @@ -55,14 +76,29 @@ ruleTester.run(RULE_NAME, rule, { // sync queries in screen with await operator are not valid ...SYNC_QUERIES_COMBINATIONS.map((query) => ({ code: `async () => { - await screen.${query}('foo') + const element = await screen.${query}('foo') } `, errors: [ { messageId: 'noAwaitSyncQuery', line: 2, - column: 22, + column: 38, + }, + ], + })), + + // sync queries in screen with await operator inside assert are not valid + ...SYNC_QUERIES_COMBINATIONS.map((query) => ({ + code: `async () => { + expect(await screen.${query}('foo')).toBeEnabled() + } + `, + errors: [ + { + messageId: 'noAwaitSyncQuery', + line: 2, + column: 29, }, ], })), From 79973d5d25fa155ab55a382a74507bcc00d00483 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Wed, 11 Nov 2020 20:52:50 +0100 Subject: [PATCH 06/27] test(no-await-sync-query): add more cases for custom queries and settings --- tests/lib/rules/no-await-sync-query.test.ts | 96 ++++++++++++++++++++- 1 file changed, 94 insertions(+), 2 deletions(-) diff --git a/tests/lib/rules/no-await-sync-query.test.ts b/tests/lib/rules/no-await-sync-query.test.ts index 1e64e658..40b46ba5 100644 --- a/tests/lib/rules/no-await-sync-query.test.ts +++ b/tests/lib/rules/no-await-sync-query.test.ts @@ -8,7 +8,6 @@ import { const ruleTester = createRuleTester(); ruleTester.run(RULE_NAME, rule, { - // TODO: add variants for custom queries for each map valid: [ // sync queries without await are valid ...SYNC_QUERIES_COMBINATIONS.map((query) => ({ @@ -17,6 +16,23 @@ ruleTester.run(RULE_NAME, rule, { } `, })), + // custom sync queries without await are valid + `() => { + const element = getByIcon('search') + } + `, + `() => { + const element = queryByIcon('search') + } + `, + `() => { + const element = getAllByIcon('search') + } + `, + `() => { + const element = queryAllByIcon('search') + } + `, // sync queries without await inside assert are valid ...SYNC_QUERIES_COMBINATIONS.map((query) => ({ code: `() => { @@ -40,9 +56,28 @@ ruleTester.run(RULE_NAME, rule, { } `, })), + + // sync query awaited but not related to custom module is invalid but not reported + { + settings: { 'testing-library/module': 'test-utils' }, + code: ` + import { screen } from 'somewhere-else' + () => { + const element = await screen.getByRole('button') + } + `, + }, + // sync query awaited but not matching filename pattern is invalid but not reported + { + settings: { 'testing-library/filename-pattern': '^.*\\.(nope)\\.js$' }, + code: ` + () => { + const element = await getByRole('button') + } + `, + }, ], - // TODO: add variants for custom queries for each map invalid: [ // sync queries with await operator are not valid ...SYNC_QUERIES_COMBINATIONS.map((query) => ({ @@ -58,6 +93,39 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), + // custom sync queries with await operator are not valid + { + code: ` + async () => { + const element = await getByIcon('search') + } + `, + errors: [{ messageId: 'noAwaitSyncQuery', line: 3, column: 31 }], + }, + { + code: ` + async () => { + const element = await queryByIcon('search') + } + `, + errors: [{ messageId: 'noAwaitSyncQuery', line: 3, column: 31 }], + }, + { + code: ` + async () => { + const element = await screen.getAllByIcon('search') + } + `, + errors: [{ messageId: 'noAwaitSyncQuery', line: 3, column: 38 }], + }, + { + code: ` + async () => { + const element = await screen.queryAllByIcon('search') + } + `, + errors: [{ messageId: 'noAwaitSyncQuery', line: 3, column: 38 }], + }, // sync queries with await operator inside assert are not valid ...SYNC_QUERIES_COMBINATIONS.map((query) => ({ code: `async () => { @@ -102,5 +170,29 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), + + // sync query awaited and related to testing library module + // with custom module setting is not valid + { + settings: { 'testing-library/module': 'test-utils' }, + code: ` + import { screen } from '@testing-library/react' + () => { + const element = await screen.getByRole('button') + } + `, + errors: [{ messageId: 'noAwaitSyncQuery', line: 4, column: 38 }], + }, + // sync query awaited and related to custom module is not valid + { + settings: { 'testing-library/module': 'test-utils' }, + code: ` + import { screen } from 'test-utils' + () => { + const element = await screen.getByRole('button') + } + `, + errors: [{ messageId: 'noAwaitSyncQuery', line: 4, column: 38 }], + }, ], }); From 9447804134e5eab23231c3e29ad823fbb9cb87af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sat, 14 Nov 2020 20:30:14 +0100 Subject: [PATCH 07/27] refactor(await-async-query): use custom rule creator --- lib/rules/await-async-query.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/rules/await-async-query.ts b/lib/rules/await-async-query.ts index 730188c2..53212a1f 100644 --- a/lib/rules/await-async-query.ts +++ b/lib/rules/await-async-query.ts @@ -1,17 +1,17 @@ import { - ESLintUtils, TSESTree, ASTUtils, } from '@typescript-eslint/experimental-utils'; -import { getDocsUrl, LIBRARY_MODULES } from '../utils'; +import { LIBRARY_MODULES } from '../utils'; import { + getVariableReferences, + isAwaited, isCallExpression, isMemberExpression, - isAwaited, isPromiseResolved, - getVariableReferences, } from '../node-utils'; import { ReportDescriptor } from '@typescript-eslint/experimental-utils/dist/ts-eslint'; +import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'await-async-query'; export type MessageIds = 'awaitAsyncQuery'; @@ -40,7 +40,7 @@ function hasClosestExpectResolvesRejects(node: TSESTree.Node): boolean { } } -export default ESLintUtils.RuleCreator(getDocsUrl)({ +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'problem', From 2cdafb7a7c1f3fda3a0f50660448c2b6fee6690c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sat, 14 Nov 2020 20:32:13 +0100 Subject: [PATCH 08/27] refactor(await-async-query): improve error message --- lib/rules/await-async-query.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/rules/await-async-query.ts b/lib/rules/await-async-query.ts index 53212a1f..8c3f9b3d 100644 --- a/lib/rules/await-async-query.ts +++ b/lib/rules/await-async-query.ts @@ -50,7 +50,8 @@ export default createTestingLibraryRule({ recommended: 'warn', }, messages: { - awaitAsyncQuery: '`{{ name }}` must have `await` operator', + awaitAsyncQuery: + '`{{ name }}` query is async but returned promise is unhandled', }, fixable: null, schema: [], From b4f506d6eaa8d49e1325ccb276a80ff7d0d563bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sun, 15 Nov 2020 14:17:18 +0100 Subject: [PATCH 09/27] feat: new detection helpers for findBy queries --- lib/detect-testing-library-utils.ts | 18 ++++ tests/create-testing-library-rule.test.ts | 101 +++++++++++++++++++++- tests/fake-rule.ts | 6 ++ 3 files changed, 124 insertions(+), 1 deletion(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 17c6ab1b..2102a39b 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -50,7 +50,9 @@ export type DetectionHelpers = { isValidFilename: () => boolean; isGetByQuery: (node: TSESTree.Identifier) => boolean; isQueryByQuery: (node: TSESTree.Identifier) => boolean; + isFindByQuery: (node: TSESTree.Identifier) => boolean; isSyncQuery: (node: TSESTree.Identifier) => boolean; + isAsyncQuery: (node: TSESTree.Identifier) => boolean; isPresenceAssert: (node: TSESTree.MemberExpression) => boolean; isAbsenceAssert: (node: TSESTree.MemberExpression) => boolean; canReportErrors: () => boolean; @@ -145,6 +147,13 @@ export function detectTestingLibraryUtils< return !!node.name.match(/^query(All)?By.+$/); }; + /** + * Determines whether a given node is `findBy*` or `findAllBy*` query variant or not. + */ + const isFindByQuery: DetectionHelpers['isFindByQuery'] = (node) => { + return !!node.name.match(/^find(All)?By.+$/); + }; + /** * Determines whether a given node is sync query or not. */ @@ -152,6 +161,13 @@ export function detectTestingLibraryUtils< return isGetByQuery(node) || isQueryByQuery(node); }; + /** + * Determines whether a given node is async query or not. + */ + const isAsyncQuery: DetectionHelpers['isAsyncQuery'] = (node) => { + return isFindByQuery(node); + }; + /** * Determines whether a given MemberExpression node is a presence assert * @@ -293,7 +309,9 @@ export function detectTestingLibraryUtils< isValidFilename, isGetByQuery, isQueryByQuery, + isFindByQuery, isSyncQuery, + isAsyncQuery, isPresenceAssert, isAbsenceAssert, canReportErrors, diff --git a/tests/create-testing-library-rule.test.ts b/tests/create-testing-library-rule.test.ts index 2cc4836a..aa047282 100644 --- a/tests/create-testing-library-rule.test.ts +++ b/tests/create-testing-library-rule.test.ts @@ -147,6 +147,12 @@ ruleTester.run(RULE_NAME, rule, { querySomeElement('button') `, }, + { + code: ` + // case: custom method not matching "findBy*" variant pattern + findSomeElement('button') + `, + }, { settings: { 'testing-library/module': 'test-utils', @@ -167,6 +173,16 @@ ruleTester.run(RULE_NAME, rule, { queryByRole('button') `, }, + { + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + // case: built-in "findBy*" query not reported because custom module not imported + import { render } from 'other-module' + findByRole('button') + `, + }, { settings: { 'testing-library/filename-pattern': 'testing-library\\.js', @@ -185,6 +201,15 @@ ruleTester.run(RULE_NAME, rule, { queryByRole('button') `, }, + { + settings: { + 'testing-library/filename-pattern': 'testing-library\\.js', + }, + code: ` + // case: built-in "findBy*" query not reported because custom filename doesn't match + findByRole('button') + `, + }, { settings: { 'testing-library/module': 'test-utils', @@ -430,6 +455,13 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [{ line: 3, column: 7, messageId: 'queryByError' }], }, + { + code: ` + // case: built-in "findBy*" query reported without import (aggressive reporting) + findByRole('button') + `, + errors: [{ line: 3, column: 7, messageId: 'findByError' }], + }, { filename: 'MyComponent.spec.js', code: ` @@ -445,6 +477,13 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [{ line: 3, column: 7, messageId: 'queryByError' }], }, + { + code: ` + // case: custom "findBy*" query reported without import (aggressive reporting) + findByIcon('search') + `, + errors: [{ line: 3, column: 7, messageId: 'findByError' }], + }, { settings: { 'testing-library/module': 'test-utils', @@ -458,13 +497,28 @@ ruleTester.run(RULE_NAME, rule, { }, { filename: 'MyComponent.spec.js', + settings: { + 'testing-library/module': 'test-utils', + }, code: ` // case: built-in "queryBy*" query reported with custom module + Testing Library package import - import { render } from '@testing-library/framework' + import { render } from '@testing-library/react' queryByRole('button') `, errors: [{ line: 4, column: 7, messageId: 'queryByError' }], }, + { + filename: 'MyComponent.spec.js', + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + // case: built-in "findBy*" query reported with custom module + Testing Library package import + import { render } from '@testing-library/react' + findByRole('button') + `, + errors: [{ line: 4, column: 7, messageId: 'findByError' }], + }, { settings: { 'testing-library/module': 'test-utils', @@ -478,6 +532,9 @@ ruleTester.run(RULE_NAME, rule, { }, { filename: 'MyComponent.spec.js', + settings: { + 'testing-library/module': 'test-utils', + }, code: ` // case: built-in "queryBy*" query reported with custom module + custom module import import { render } from 'test-utils' @@ -485,6 +542,18 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [{ line: 4, column: 7, messageId: 'queryByError' }], }, + { + filename: 'MyComponent.spec.js', + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + // case: built-in "queryBy*" query reported with custom module + custom module import + import { render } from 'test-utils' + findByRole('button') + `, + errors: [{ line: 4, column: 7, messageId: 'findByError' }], + }, { settings: { @@ -499,6 +568,9 @@ ruleTester.run(RULE_NAME, rule, { }, { filename: 'MyComponent.spec.js', + settings: { + 'testing-library/module': 'test-utils', + }, code: ` // case: custom "queryBy*" query reported with custom module + Testing Library package import import { render } from '@testing-library/framework' @@ -506,6 +578,18 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [{ line: 4, column: 7, messageId: 'queryByError' }], }, + { + filename: 'MyComponent.spec.js', + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + // case: custom "findBy*" query reported with custom module + Testing Library package import + import { render } from '@testing-library/framework' + findByIcon('search') + `, + errors: [{ line: 4, column: 7, messageId: 'findByError' }], + }, { settings: { 'testing-library/module': 'test-utils', @@ -519,6 +603,9 @@ ruleTester.run(RULE_NAME, rule, { }, { filename: 'MyComponent.spec.js', + settings: { + 'testing-library/module': 'test-utils', + }, code: ` // case: custom "queryBy*" query reported with custom module + custom module import import { render } from 'test-utils' @@ -526,6 +613,18 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [{ line: 4, column: 7, messageId: 'queryByError' }], }, + { + filename: 'MyComponent.spec.js', + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + // case: custom "findBy*" query reported with custom module + custom module import + import { render } from 'test-utils' + findByIcon('search') + `, + errors: [{ line: 4, column: 7, messageId: 'findByError' }], + }, { settings: { 'testing-library/module': 'test-utils', diff --git a/tests/fake-rule.ts b/tests/fake-rule.ts index f1284b84..25f2237e 100644 --- a/tests/fake-rule.ts +++ b/tests/fake-rule.ts @@ -11,6 +11,7 @@ type MessageIds = | 'fakeError' | 'getByError' | 'queryByError' + | 'findByError' | 'presenceAssertError' | 'absenceAssertError'; @@ -27,6 +28,7 @@ export default createTestingLibraryRule({ fakeError: 'fake error reported', getByError: 'some error related to getBy reported', queryByError: 'some error related to queryBy reported', + findByError: 'some error related to findBy reported', presenceAssertError: 'some error related to presence assert reported', absenceAssertError: 'some error related to absence assert reported', }, @@ -49,6 +51,10 @@ export default createTestingLibraryRule({ if (helpers.isQueryByQuery(node)) { return context.report({ node, messageId: 'queryByError' }); } + + if (helpers.isFindByQuery(node)) { + return context.report({ node, messageId: 'findByError' }); + } }; const reportMemberExpression = (node: TSESTree.MemberExpression) => { From 6ac7555999b6b12f013113cd1425ff679afada85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Mon, 23 Nov 2020 10:30:41 +0100 Subject: [PATCH 10/27] refactor(await-async-query): detection helpers + aggressive reporting --- lib/node-utils.ts | 74 +++++++++- lib/rules/await-async-query.ts | 162 ++++++++-------------- tests/lib/rules/await-async-query.test.ts | 38 +++-- 3 files changed, 147 insertions(+), 127 deletions(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 26825717..3ec7a43e 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -6,6 +6,35 @@ import { } from '@typescript-eslint/experimental-utils'; import { RuleContext } from '@typescript-eslint/experimental-utils/dist/ts-eslint'; +const ValidLeftHandSideExpressions = [ + AST_NODE_TYPES.CallExpression, + AST_NODE_TYPES.ClassExpression, + AST_NODE_TYPES.ClassDeclaration, + AST_NODE_TYPES.FunctionExpression, + AST_NODE_TYPES.Literal, + AST_NODE_TYPES.TemplateLiteral, + AST_NODE_TYPES.MemberExpression, + AST_NODE_TYPES.ArrayExpression, + AST_NODE_TYPES.ArrayPattern, + AST_NODE_TYPES.ClassExpression, + AST_NODE_TYPES.FunctionExpression, + AST_NODE_TYPES.Identifier, + AST_NODE_TYPES.JSXElement, + AST_NODE_TYPES.JSXFragment, + AST_NODE_TYPES.JSXOpeningElement, + AST_NODE_TYPES.MetaProperty, + AST_NODE_TYPES.ObjectExpression, + AST_NODE_TYPES.ObjectPattern, + AST_NODE_TYPES.Super, + AST_NODE_TYPES.TemplateLiteral, + AST_NODE_TYPES.ThisExpression, + AST_NODE_TYPES.TSNullKeyword, + AST_NODE_TYPES.TaggedTemplateExpression, + AST_NODE_TYPES.TSNonNullExpression, + AST_NODE_TYPES.TSAsExpression, + AST_NODE_TYPES.ArrowFunctionExpression, +]; + export function isCallExpression( node: TSESTree.Node | null | undefined ): node is TSESTree.CallExpression { @@ -78,14 +107,27 @@ export function isJSXAttribute( return node?.type === AST_NODE_TYPES.JSXAttribute; } +/** + * Finds the closest CallExpression node for a given node. + * @param node + * @param shouldRestrictInnerScope - If true, CallExpression must be directly related to node + */ export function findClosestCallExpressionNode( - node: TSESTree.Node -): TSESTree.CallExpression { + node: TSESTree.Node, + shouldRestrictInnerScope = false +): TSESTree.CallExpression | null { if (isCallExpression(node)) { return node; } - if (!node.parent) { + if (!node || !node.parent) { + return null; + } + + if ( + shouldRestrictInnerScope && + !ValidLeftHandSideExpressions.includes(node.parent.type) + ) { return null; } @@ -281,3 +323,29 @@ export function getAssertNodeInfo( return { matcher, isNegated }; } + +/** + * Determines whether a node belongs to an async assertion + * fulfilled by `resolves` or `rejects` properties. + * + */ +export function hasClosestExpectResolvesRejects(node: TSESTree.Node): boolean { + if ( + isCallExpression(node) && + ASTUtils.isIdentifier(node.callee) && + isMemberExpression(node.parent) && + node.callee.name === 'expect' + ) { + const expectMatcher = node.parent.property; + return ( + ASTUtils.isIdentifier(expectMatcher) && + (expectMatcher.name === 'resolves' || expectMatcher.name === 'rejects') + ); + } + + if (!node.parent) { + return false; + } + + return hasClosestExpectResolvesRejects(node.parent); +} diff --git a/lib/rules/await-async-query.ts b/lib/rules/await-async-query.ts index 8c3f9b3d..76ca02d9 100644 --- a/lib/rules/await-async-query.ts +++ b/lib/rules/await-async-query.ts @@ -1,45 +1,17 @@ +import { TSESTree } from '@typescript-eslint/experimental-utils'; import { - TSESTree, - ASTUtils, -} from '@typescript-eslint/experimental-utils'; -import { LIBRARY_MODULES } from '../utils'; -import { + findClosestCallExpressionNode, getVariableReferences, + hasClosestExpectResolvesRejects, isAwaited, - isCallExpression, - isMemberExpression, isPromiseResolved, } from '../node-utils'; -import { ReportDescriptor } from '@typescript-eslint/experimental-utils/dist/ts-eslint'; import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'await-async-query'; export type MessageIds = 'awaitAsyncQuery'; type Options = []; -const ASYNC_QUERIES_REGEXP = /^find(All)?By(LabelText|PlaceholderText|Text|AltText|Title|DisplayValue|Role|TestId)$/; - -function hasClosestExpectResolvesRejects(node: TSESTree.Node): boolean { - if (!node.parent) { - return false; - } - - if ( - isCallExpression(node) && - ASTUtils.isIdentifier(node.callee) && - isMemberExpression(node.parent) && - node.callee.name === 'expect' - ) { - const expectMatcher = node.parent.property; - return ( - ASTUtils.isIdentifier(expectMatcher) && - (expectMatcher.name === 'resolves' || expectMatcher.name === 'rejects') - ); - } else { - return hasClosestExpectResolvesRejects(node.parent); - } -} - export default createTestingLibraryRule({ name: RULE_NAME, meta: { @@ -50,94 +22,76 @@ export default createTestingLibraryRule({ recommended: 'warn', }, messages: { - awaitAsyncQuery: - '`{{ name }}` query is async but returned promise is unhandled', + awaitAsyncQuery: '`{{ name }}` must have `await` operator', }, fixable: null, schema: [], }, defaultOptions: [], - create(context) { - const testingLibraryQueryUsage: { - node: TSESTree.Identifier | TSESTree.MemberExpression; - queryName: string; - }[] = []; + create(context, _, helpers) { + return { + 'CallExpression Identifier'(node: TSESTree.Identifier) { + // stop checking if not async query + if (!helpers.isAsyncQuery(node)) { + return; + } - const isQueryUsage = ( - node: TSESTree.Identifier | TSESTree.MemberExpression - ) => - !isAwaited(node.parent.parent) && - !isPromiseResolved(node) && - !hasClosestExpectResolvesRejects(node); + const closestCallExpressionNode = findClosestCallExpressionNode( + node, + true + ); - let hasImportedFromTestingLibraryModule = false; + // stop checking if Identifier doesn't belong to CallExpression + if (!closestCallExpressionNode) { + return; + } - function report(params: ReportDescriptor<'awaitAsyncQuery'>) { - if (hasImportedFromTestingLibraryModule) { - context.report(params); - } - } + const references = getVariableReferences( + context, + closestCallExpressionNode.parent + ); - return { - 'ImportDeclaration > ImportSpecifier,ImportNamespaceSpecifier'( - node: TSESTree.Node - ) { - const importDeclaration = node.parent as TSESTree.ImportDeclaration; - const module = importDeclaration.source.value.toString(); + if (references && references.length === 0) { + // stop checking if already awaited + if (isAwaited(closestCallExpressionNode.parent)) { + return; + } - if (LIBRARY_MODULES.includes(module)) { - hasImportedFromTestingLibraryModule = true; - } - }, - [`CallExpression > Identifier[name=${ASYNC_QUERIES_REGEXP}]`]( - node: TSESTree.Identifier - ) { - if (isQueryUsage(node)) { - testingLibraryQueryUsage.push({ node, queryName: node.name }); - } - }, - [`MemberExpression > Identifier[name=${ASYNC_QUERIES_REGEXP}]`]( - node: TSESTree.Identifier - ) { - // Perform checks in parent MemberExpression instead of current identifier - const parent = node.parent as TSESTree.MemberExpression; - if (isQueryUsage(parent)) { - testingLibraryQueryUsage.push({ node: parent, queryName: node.name }); + // stop checking if promise already handled + if (isPromiseResolved(node)) { + return; + } + + // stop checking if belongs to async assert already handled + if (hasClosestExpectResolvesRejects(closestCallExpressionNode)) { + return; + } + + return context.report({ + node, + messageId: 'awaitAsyncQuery', + data: { name: node.name }, + }); } - }, - 'Program:exit'() { - testingLibraryQueryUsage.forEach(({ node, queryName }) => { - const references = getVariableReferences(context, node.parent.parent); - if (references && references.length === 0) { - report({ - node, - messageId: 'awaitAsyncQuery', - data: { - name: queryName, - }, - }); - } else { - for (const reference of references) { - const referenceNode = reference.identifier; - if ( - !isAwaited(referenceNode.parent) && - !isPromiseResolved(referenceNode) - ) { - report({ - node, - messageId: 'awaitAsyncQuery', - data: { - name: queryName, - }, - }); + for (const reference of references) { + const referenceNode = reference.identifier; + + if (isAwaited(referenceNode.parent)) { + return; + } - break; - } - } + if (isPromiseResolved(referenceNode)) { + return; } - }); + + return context.report({ + node, + messageId: 'awaitAsyncQuery', + data: { name: node.name }, + }); + } }, }; }, diff --git a/tests/lib/rules/await-async-query.test.ts b/tests/lib/rules/await-async-query.test.ts index c4c4c07f..20affbc5 100644 --- a/tests/lib/rules/await-async-query.test.ts +++ b/tests/lib/rules/await-async-query.test.ts @@ -28,6 +28,7 @@ interface TestCaseParams { errors?: TestCaseError<'awaitAsyncQuery'>[]; } +// TODO: simplify test cases generation function createTestCase( getTest: ( query: string @@ -114,7 +115,7 @@ ruleTester.run(RULE_NAME, rule, { // async queries are valid with promise returned in regular function ...createTestCase((query) => `function foo() { return ${query}('foo') }`), - // async queries are valid with promise in variable and returned in regular functio + // async queries are valid with promise in variable and returned in regular function ...createTestCase( (query) => ` const promise = ${query}('foo') @@ -146,27 +147,12 @@ ruleTester.run(RULE_NAME, rule, { expect(wrappedQuery(${query}("foo"))).rejects.toBe("bar") ` ), - - // non existing queries are valid - createTestCode({ - code: ` - doSomething() - const foo = findByNonExistingTestingLibraryQuery('foo') - `, - }), - - // unresolved async queries are valid if there are no imports from a testing library module - ...ASYNC_QUERIES_COMBINATIONS.map((query) => ({ - code: ` - import { render } from "another-library" - - test('An example test', async () => { - const example = ${query}("my example") - }) - `, - })), ], + // TODO: improve invalid cases by + // - checking line and column + // - include custom query in async queries list to check aggressive reporting + // - check references from functions invalid: [ // async queries without await operator or then method are not valid ...createTestCase((query) => ({ @@ -199,5 +185,17 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), + + // unresolved async queries are not valid (aggressive reporting) + ...ASYNC_QUERIES_COMBINATIONS.map((query) => ({ + code: ` + import { render } from "another-library" + + test('An example test', async () => { + const example = ${query}("my example") + }) + `, + errors: [{ messageId: 'awaitAsyncQuery', line: 5, column: 27 }], + })), ], }); From ee1b791068bc9d5f2029fc9d46872b954149c6bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Mon, 23 Nov 2020 12:33:48 +0100 Subject: [PATCH 11/27] test(await-async-query): add cases for custom queries --- lib/utils.ts | 3 +- tests/lib/rules/await-async-query.test.ts | 46 +++++++++++++++++++++-- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/lib/utils.ts b/lib/utils.ts index a6d0fa68..ef71be3c 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -1,6 +1,6 @@ import { TSESTree } from '@typescript-eslint/experimental-utils'; -const combineQueries = (variants: string[], methods: string[]) => { +const combineQueries = (variants: string[], methods: string[]): string[] => { const combinedQueries: string[] = []; variants.forEach((variant) => { const variantPrefix = variant.replace('By', ''); @@ -115,6 +115,7 @@ const PRESENCE_MATCHERS = ['toBeInTheDocument', 'toBeTruthy', 'toBeDefined']; const ABSENCE_MATCHERS = ['toBeNull', 'toBeFalsy']; export { + combineQueries, getDocsUrl, hasTestingLibraryImportModule, SYNC_QUERIES_VARIANTS, diff --git a/tests/lib/rules/await-async-query.test.ts b/tests/lib/rules/await-async-query.test.ts index 20affbc5..b8591665 100644 --- a/tests/lib/rules/await-async-query.test.ts +++ b/tests/lib/rules/await-async-query.test.ts @@ -3,6 +3,8 @@ import { createRuleTester } from '../test-utils'; import rule, { RULE_NAME } from '../../../lib/rules/await-async-query'; import { ASYNC_QUERIES_COMBINATIONS, + ASYNC_QUERIES_VARIANTS, + combineQueries, SYNC_QUERIES_COMBINATIONS, } from '../../../lib/utils'; @@ -47,6 +49,11 @@ function createTestCase( }); } +const CUSTOM_ASYNC_QUERIES_COMBINATIONS = combineQueries( + ASYNC_QUERIES_VARIANTS, + ['ByIcon', 'ByButton'] +); + ruleTester.run(RULE_NAME, rule, { valid: [ // async queries declaration from render functions are valid @@ -151,10 +158,9 @@ ruleTester.run(RULE_NAME, rule, { // TODO: improve invalid cases by // - checking line and column - // - include custom query in async queries list to check aggressive reporting // - check references from functions invalid: [ - // async queries without await operator or then method are not valid + // built-in async queries without await operator or then method are not valid ...createTestCase((query) => ({ code: ` doSomething() @@ -162,13 +168,33 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [{ messageId: 'awaitAsyncQuery' }], })), + // custom async queries without await operator or then method are not valid + ...createTestCase( + (query) => ({ + code: ` + doSomething() + const foo = ${query}('foo') + `, + errors: [{ messageId: 'awaitAsyncQuery' }], + }), + { combinations: CUSTOM_ASYNC_QUERIES_COMBINATIONS } + ), - // async screen queries without await operator or then method are not valid + // built-in async screen queries without await operator or then method are not valid ...createTestCase((query) => ({ code: `screen.${query}('foo')`, errors: [{ messageId: 'awaitAsyncQuery' }], })), + // custom async screen queries without await operator or then method are not valid + ...createTestCase( + (query) => ({ + code: `screen.${query}('foo')`, + errors: [{ messageId: 'awaitAsyncQuery' }], + }), + { combinations: CUSTOM_ASYNC_QUERIES_COMBINATIONS } + ), + ...createTestCase((query) => ({ code: ` const foo = ${query}('foo') @@ -186,7 +212,7 @@ ruleTester.run(RULE_NAME, rule, { ], })), - // unresolved async queries are not valid (aggressive reporting) + // unresolved built-in async queries are not valid (aggressive reporting) ...ASYNC_QUERIES_COMBINATIONS.map((query) => ({ code: ` import { render } from "another-library" @@ -197,5 +223,17 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [{ messageId: 'awaitAsyncQuery', line: 5, column: 27 }], })), + + // unresolved custom async queries are not valid (aggressive reporting) + ...CUSTOM_ASYNC_QUERIES_COMBINATIONS.map((query) => ({ + code: ` + import { render } from "another-library" + + test('An example test', async () => { + const example = ${query}("my example") + }) + `, + errors: [{ messageId: 'awaitAsyncQuery', line: 5, column: 27 }], + })), ], }); From 0260208ca84015122ba7795936b15f56b0de7adc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Mon, 23 Nov 2020 12:44:23 +0100 Subject: [PATCH 12/27] test(await-async-query): add more cases for custom queries --- tests/lib/rules/await-async-query.test.ts | 39 +++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/tests/lib/rules/await-async-query.test.ts b/tests/lib/rules/await-async-query.test.ts index b8591665..2adf56ee 100644 --- a/tests/lib/rules/await-async-query.test.ts +++ b/tests/lib/rules/await-async-query.test.ts @@ -72,7 +72,7 @@ ruleTester.run(RULE_NAME, rule, { ` ), - // async queries are valid when saved in a variable with await operator + // built-in async queries are valid when saved in a variable with await operator ...createTestCase( (query) => ` doSomething() @@ -81,6 +81,16 @@ ruleTester.run(RULE_NAME, rule, { ` ), + // custom async queries are valid when saved in a variable with await operator + ...createTestCase( + (query) => ` + doSomething() + const foo = await ${query}('foo') + expect(foo).toBeInTheDocument(); + `, + { combinations: CUSTOM_ASYNC_QUERIES_COMBINATIONS } + ), + // async queries are valid when saved in a promise variable immediately resolved ...createTestCase( (query) => ` @@ -154,6 +164,30 @@ ruleTester.run(RULE_NAME, rule, { expect(wrappedQuery(${query}("foo"))).rejects.toBe("bar") ` ), + + // unresolved built-in async queries with aggressive reporting opted-out are valid + ...ASYNC_QUERIES_COMBINATIONS.map((query) => ({ + settings: { 'testing-library/module': 'test-utils' }, + code: ` + import { render } from "another-library" + + test('An example test', async () => { + const example = ${query}("my example") + }) + `, + })), + + // unresolved custom async queries with aggressive reporting opted-out are valid + ...CUSTOM_ASYNC_QUERIES_COMBINATIONS.map((query) => ({ + settings: { 'testing-library/module': 'test-utils' }, + code: ` + import { render } from "another-library" + + test('An example test', async () => { + const example = ${query}("my example") + }) + `, + })), ], // TODO: improve invalid cases by @@ -226,8 +260,9 @@ ruleTester.run(RULE_NAME, rule, { // unresolved custom async queries are not valid (aggressive reporting) ...CUSTOM_ASYNC_QUERIES_COMBINATIONS.map((query) => ({ + settings: { 'testing-library/module': 'test-utils' }, code: ` - import { render } from "another-library" + import { render } from "test-utils" test('An example test', async () => { const example = ${query}("my example") From 12d5c6791f77f885f10cfeade130e1ff786bbd79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Mon, 23 Nov 2020 12:48:21 +0100 Subject: [PATCH 13/27] test(await-async-query): check errors locations --- tests/lib/rules/await-async-query.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/lib/rules/await-async-query.test.ts b/tests/lib/rules/await-async-query.test.ts index 2adf56ee..2d03dbef 100644 --- a/tests/lib/rules/await-async-query.test.ts +++ b/tests/lib/rules/await-async-query.test.ts @@ -191,7 +191,6 @@ ruleTester.run(RULE_NAME, rule, { ], // TODO: improve invalid cases by - // - checking line and column // - check references from functions invalid: [ // built-in async queries without await operator or then method are not valid @@ -200,7 +199,7 @@ ruleTester.run(RULE_NAME, rule, { doSomething() const foo = ${query}('foo') `, - errors: [{ messageId: 'awaitAsyncQuery' }], + errors: [{ messageId: 'awaitAsyncQuery', line: 6, column: 21 }], })), // custom async queries without await operator or then method are not valid ...createTestCase( @@ -209,7 +208,7 @@ ruleTester.run(RULE_NAME, rule, { doSomething() const foo = ${query}('foo') `, - errors: [{ messageId: 'awaitAsyncQuery' }], + errors: [{ messageId: 'awaitAsyncQuery', line: 6, column: 21 }], }), { combinations: CUSTOM_ASYNC_QUERIES_COMBINATIONS } ), @@ -217,14 +216,14 @@ ruleTester.run(RULE_NAME, rule, { // built-in async screen queries without await operator or then method are not valid ...createTestCase((query) => ({ code: `screen.${query}('foo')`, - errors: [{ messageId: 'awaitAsyncQuery' }], + errors: [{ messageId: 'awaitAsyncQuery', line: 4, column: 14 }], })), // custom async screen queries without await operator or then method are not valid ...createTestCase( (query) => ({ code: `screen.${query}('foo')`, - errors: [{ messageId: 'awaitAsyncQuery' }], + errors: [{ messageId: 'awaitAsyncQuery', line: 4, column: 14 }], }), { combinations: CUSTOM_ASYNC_QUERIES_COMBINATIONS } ), @@ -238,6 +237,7 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { line: 5, + column: 21, messageId: 'awaitAsyncQuery', data: { name: query, From 10af260596cefda4388e7a230656f49d68bd4f4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Mon, 23 Nov 2020 12:51:11 +0100 Subject: [PATCH 14/27] test(await-async-query): mix built-in and custom queries --- tests/lib/rules/await-async-query.test.ts | 81 +++++------------------ 1 file changed, 17 insertions(+), 64 deletions(-) diff --git a/tests/lib/rules/await-async-query.test.ts b/tests/lib/rules/await-async-query.test.ts index 2d03dbef..d05c59fc 100644 --- a/tests/lib/rules/await-async-query.test.ts +++ b/tests/lib/rules/await-async-query.test.ts @@ -30,12 +30,14 @@ interface TestCaseParams { errors?: TestCaseError<'awaitAsyncQuery'>[]; } -// TODO: simplify test cases generation function createTestCase( getTest: ( query: string ) => string | { code: string; errors?: TestCaseError<'awaitAsyncQuery'>[] }, - { combinations = ASYNC_QUERIES_COMBINATIONS, isAsync }: TestCaseParams = {} + { + combinations = ALL_ASYNC_COMBINATIONS_TO_TEST, + isAsync, + }: TestCaseParams = {} ) { return combinations.map((query) => { const test = getTest(query); @@ -54,6 +56,12 @@ const CUSTOM_ASYNC_QUERIES_COMBINATIONS = combineQueries( ['ByIcon', 'ByButton'] ); +// built-in queries + custom queries +const ALL_ASYNC_COMBINATIONS_TO_TEST = [ + ...ASYNC_QUERIES_COMBINATIONS, + ...CUSTOM_ASYNC_QUERIES_COMBINATIONS, +]; + ruleTester.run(RULE_NAME, rule, { valid: [ // async queries declaration from render functions are valid @@ -72,7 +80,7 @@ ruleTester.run(RULE_NAME, rule, { ` ), - // built-in async queries are valid when saved in a variable with await operator + // async queries are valid when saved in a variable with await operator ...createTestCase( (query) => ` doSomething() @@ -81,16 +89,6 @@ ruleTester.run(RULE_NAME, rule, { ` ), - // custom async queries are valid when saved in a variable with await operator - ...createTestCase( - (query) => ` - doSomething() - const foo = await ${query}('foo') - expect(foo).toBeInTheDocument(); - `, - { combinations: CUSTOM_ASYNC_QUERIES_COMBINATIONS } - ), - // async queries are valid when saved in a promise variable immediately resolved ...createTestCase( (query) => ` @@ -165,20 +163,8 @@ ruleTester.run(RULE_NAME, rule, { ` ), - // unresolved built-in async queries with aggressive reporting opted-out are valid - ...ASYNC_QUERIES_COMBINATIONS.map((query) => ({ - settings: { 'testing-library/module': 'test-utils' }, - code: ` - import { render } from "another-library" - - test('An example test', async () => { - const example = ${query}("my example") - }) - `, - })), - - // unresolved custom async queries with aggressive reporting opted-out are valid - ...CUSTOM_ASYNC_QUERIES_COMBINATIONS.map((query) => ({ + // unresolved async queries with aggressive reporting opted-out are valid + ...ALL_ASYNC_COMBINATIONS_TO_TEST.map((query) => ({ settings: { 'testing-library/module': 'test-utils' }, code: ` import { render } from "another-library" @@ -193,7 +179,7 @@ ruleTester.run(RULE_NAME, rule, { // TODO: improve invalid cases by // - check references from functions invalid: [ - // built-in async queries without await operator or then method are not valid + // async queries without await operator or then method are not valid ...createTestCase((query) => ({ code: ` doSomething() @@ -201,33 +187,13 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [{ messageId: 'awaitAsyncQuery', line: 6, column: 21 }], })), - // custom async queries without await operator or then method are not valid - ...createTestCase( - (query) => ({ - code: ` - doSomething() - const foo = ${query}('foo') - `, - errors: [{ messageId: 'awaitAsyncQuery', line: 6, column: 21 }], - }), - { combinations: CUSTOM_ASYNC_QUERIES_COMBINATIONS } - ), - // built-in async screen queries without await operator or then method are not valid + // async screen queries without await operator or then method are not valid ...createTestCase((query) => ({ code: `screen.${query}('foo')`, errors: [{ messageId: 'awaitAsyncQuery', line: 4, column: 14 }], })), - // custom async screen queries without await operator or then method are not valid - ...createTestCase( - (query) => ({ - code: `screen.${query}('foo')`, - errors: [{ messageId: 'awaitAsyncQuery', line: 4, column: 14 }], - }), - { combinations: CUSTOM_ASYNC_QUERIES_COMBINATIONS } - ), - ...createTestCase((query) => ({ code: ` const foo = ${query}('foo') @@ -246,8 +212,8 @@ ruleTester.run(RULE_NAME, rule, { ], })), - // unresolved built-in async queries are not valid (aggressive reporting) - ...ASYNC_QUERIES_COMBINATIONS.map((query) => ({ + // unresolved async queries are not valid (aggressive reporting) + ...ALL_ASYNC_COMBINATIONS_TO_TEST.map((query) => ({ code: ` import { render } from "another-library" @@ -257,18 +223,5 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [{ messageId: 'awaitAsyncQuery', line: 5, column: 27 }], })), - - // unresolved custom async queries are not valid (aggressive reporting) - ...CUSTOM_ASYNC_QUERIES_COMBINATIONS.map((query) => ({ - settings: { 'testing-library/module': 'test-utils' }, - code: ` - import { render } from "test-utils" - - test('An example test', async () => { - const example = ${query}("my example") - }) - `, - errors: [{ messageId: 'awaitAsyncQuery', line: 5, column: 27 }], - })), ], }); From c13aea10bf1a2f29017e05eb26bc60e3173a7f66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Mon, 23 Nov 2020 12:52:49 +0100 Subject: [PATCH 15/27] test(await-async-query): non-matching query case --- tests/lib/rules/await-async-query.test.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/lib/rules/await-async-query.test.ts b/tests/lib/rules/await-async-query.test.ts index d05c59fc..e6de7a04 100644 --- a/tests/lib/rules/await-async-query.test.ts +++ b/tests/lib/rules/await-async-query.test.ts @@ -174,6 +174,13 @@ ruleTester.run(RULE_NAME, rule, { }) `, })), + + // non-matching query is valid + ` + test('An example test', async () => { + const example = findText("my example") + }) + `, ], // TODO: improve invalid cases by From 743f29de0535781dc35ab99a8827bf5c61dfd91d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Mon, 23 Nov 2020 18:03:39 +0100 Subject: [PATCH 16/27] feat(await-async-query): report query wrappers --- lib/node-utils.ts | 2 + lib/rules/await-async-query.ts | 149 ++++++++++++++++------ tests/lib/rules/await-async-query.test.ts | 33 +++-- 3 files changed, 134 insertions(+), 50 deletions(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 3ec7a43e..75854d23 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -222,6 +222,7 @@ export function getVariableReferences( ); } +// TODO: should be removed after v4 is finished export function isRenderFunction( callNode: TSESTree.CallExpression, renderFunctions: string[] @@ -239,6 +240,7 @@ export function isRenderFunction( }); } +// TODO: should be removed after v4 is finished export function isRenderVariableDeclarator( node: TSESTree.VariableDeclarator, renderFunctions: string[] diff --git a/lib/rules/await-async-query.ts b/lib/rules/await-async-query.ts index 76ca02d9..e4c3970c 100644 --- a/lib/rules/await-async-query.ts +++ b/lib/rules/await-async-query.ts @@ -1,15 +1,22 @@ -import { TSESTree } from '@typescript-eslint/experimental-utils'; +import { + TSESTree, + ASTUtils, + TSESLintScope, +} from '@typescript-eslint/experimental-utils'; import { findClosestCallExpressionNode, getVariableReferences, hasClosestExpectResolvesRejects, isAwaited, + isBlockStatement, + isCallExpression, isPromiseResolved, + isReturnStatement, } from '../node-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'await-async-query'; -export type MessageIds = 'awaitAsyncQuery'; +export type MessageIds = 'awaitAsyncQuery' | 'asyncQueryWrapper'; type Options = []; export default createTestingLibraryRule({ @@ -22,7 +29,9 @@ export default createTestingLibraryRule({ recommended: 'warn', }, messages: { - awaitAsyncQuery: '`{{ name }}` must have `await` operator', + awaitAsyncQuery: 'promise returned from {{ name }} query must be handled', + asyncQueryWrapper: + 'promise returned from {{ name }} wrapper must be handled', }, fixable: null, schema: [], @@ -30,65 +39,127 @@ export default createTestingLibraryRule({ defaultOptions: [], create(context, _, helpers) { + const functionWrappersNames: string[] = []; + + function getInnermostFunctionScope( + asyncQueryNode: TSESTree.Identifier + ): TSESLintScope.FunctionScope | null { + const innermostScope = ASTUtils.getInnermostScope( + context.getScope(), + asyncQueryNode + ); + + if (innermostScope?.type === 'function') { + return (innermostScope as unknown) as TSESLintScope.FunctionScope; + } + + return null; + } + return { 'CallExpression Identifier'(node: TSESTree.Identifier) { - // stop checking if not async query - if (!helpers.isAsyncQuery(node)) { - return; - } + // check async query direct calls or related function wrapper + if (helpers.isAsyncQuery(node)) { + const functionScope = getInnermostFunctionScope(node); - const closestCallExpressionNode = findClosestCallExpressionNode( - node, - true - ); - - // stop checking if Identifier doesn't belong to CallExpression - if (!closestCallExpressionNode) { - return; - } + if ( + functionScope && + ASTUtils.isFunction(functionScope.block) && + isBlockStatement(functionScope.block.body) + ) { + // report function wrapper calls rather than async calls - const references = getVariableReferences( - context, - closestCallExpressionNode.parent - ); + const returnStatementNode = functionScope.block.body.body.find( + (statement) => isReturnStatement(statement) + ) as TSESTree.ReturnStatement | null; - if (references && references.length === 0) { - // stop checking if already awaited - if (isAwaited(closestCallExpressionNode.parent)) { - return; + if ( + returnStatementNode && + isCallExpression(returnStatementNode.argument) + ) { + // TODO: improve this check, + // assume for now the query is within the return statement + functionWrappersNames.push(functionScope.block.id.name); + } } - // stop checking if promise already handled - if (isPromiseResolved(node)) { + const closestCallExpressionNode = findClosestCallExpressionNode( + node, + true + ); + + // stop checking if Identifier doesn't belong to CallExpression + if (!closestCallExpressionNode) { return; } - // stop checking if belongs to async assert already handled - if (hasClosestExpectResolvesRejects(closestCallExpressionNode)) { - return; + const references = getVariableReferences( + context, + closestCallExpressionNode.parent + ); + + if (references && references.length === 0) { + // stop checking if already awaited + if (isAwaited(closestCallExpressionNode.parent)) { + return; + } + + // stop checking if promise already handled + if (isPromiseResolved(node)) { + return; + } + + // stop checking if belongs to async assert already handled + if (hasClosestExpectResolvesRejects(closestCallExpressionNode)) { + return; + } + + return context.report({ + node, + messageId: 'awaitAsyncQuery', + data: { name: node.name }, + }); } - return context.report({ + for (const reference of references) { + const referenceNode = reference.identifier; + + if (isAwaited(referenceNode.parent)) { + return; + } + + if (isPromiseResolved(referenceNode)) { + return; + } + + return context.report({ + node, + messageId: 'awaitAsyncQuery', + data: { name: node.name }, + }); + } + } else if (functionWrappersNames.includes(node.name)) { + const closestCallExpressionNode = findClosestCallExpressionNode( node, - messageId: 'awaitAsyncQuery', - data: { name: node.name }, - }); - } + true + ); - for (const reference of references) { - const referenceNode = reference.identifier; + // stop checking if Identifier doesn't belong to CallExpression + if (!closestCallExpressionNode) { + return; + } - if (isAwaited(referenceNode.parent)) { + if (isAwaited(closestCallExpressionNode.parent)) { return; } - if (isPromiseResolved(referenceNode)) { + if (isPromiseResolved(node)) { return; } return context.report({ node, - messageId: 'awaitAsyncQuery', + messageId: 'asyncQueryWrapper', data: { name: node.name }, }); } diff --git a/tests/lib/rules/await-async-query.test.ts b/tests/lib/rules/await-async-query.test.ts index e6de7a04..e9542c58 100644 --- a/tests/lib/rules/await-async-query.test.ts +++ b/tests/lib/rules/await-async-query.test.ts @@ -27,7 +27,7 @@ function createTestCode({ code, isAsync = true }: TestCode) { interface TestCaseParams { isAsync?: boolean; combinations?: string[]; - errors?: TestCaseError<'awaitAsyncQuery'>[]; + errors?: TestCaseError<'awaitAsyncQuery' | 'asyncQueryWrapper'>[]; } function createTestCase( @@ -97,14 +97,6 @@ ruleTester.run(RULE_NAME, rule, { ` ), - // async queries are valid when saved in a promise variable resolved by an await operator - ...createTestCase( - (query) => ` - const promise = ${query}('foo') - await promise - ` - ), - // async queries are valid when used with then method ...createTestCase( (query) => ` @@ -183,8 +175,6 @@ ruleTester.run(RULE_NAME, rule, { `, ], - // TODO: improve invalid cases by - // - check references from functions invalid: [ // async queries without await operator or then method are not valid ...createTestCase((query) => ({ @@ -230,5 +220,26 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [{ messageId: 'awaitAsyncQuery', line: 5, column: 27 }], })), + + // TODO: add cases for arrow functions (both implicit and explicit return) + // unhandled promise returned from async queries function wrapper are invalid + ...ALL_ASYNC_COMBINATIONS_TO_TEST.map((query) => ({ + code: ` + function queryWrapper() { + doSomethingElse(); + + return screen.${query}('foo') + } + + test("An invalid example test", () => { + const element = queryWrapper() + }) + + test("An valid example test", async () => { + const element = await queryWrapper() + }) + `, + errors: [{ messageId: 'asyncQueryWrapper', line: 9, column: 27 }], + })), ], }); From ae51b6a61a1426fb2f2a80a544fed7f4b43cf340 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Tue, 24 Nov 2020 11:42:20 +0100 Subject: [PATCH 17/27] refactor(await-async-query): extract ast utils for functions --- lib/node-utils.ts | 70 +++++++++++++++++++++++ lib/rules/await-async-query.ts | 53 +++++------------ tests/lib/rules/await-async-query.test.ts | 3 +- 3 files changed, 84 insertions(+), 42 deletions(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 75854d23..7f6ad37c 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -2,6 +2,7 @@ import { AST_NODE_TYPES, ASTUtils, TSESLint, + TSESLintScope, TSESTree, } from '@typescript-eslint/experimental-utils'; import { RuleContext } from '@typescript-eslint/experimental-utils/dist/ts-eslint'; @@ -222,6 +223,75 @@ export function getVariableReferences( ); } +export function getInnermostFunctionScope( + context: RuleContext, + asyncQueryNode: TSESTree.Identifier +): TSESLintScope.FunctionScope | null { + const innermostScope = ASTUtils.getInnermostScope( + context.getScope(), + asyncQueryNode + ); + + if ( + innermostScope?.type === 'function' && + ASTUtils.isFunction(innermostScope.block) + ) { + return (innermostScope as unknown) as TSESLintScope.FunctionScope; + } + + return null; +} + +export function getFunctionReturnStatementIdentifier( + functionScope: TSESLintScope.FunctionScope +): TSESTree.Identifier | null { + if (!ASTUtils.isFunction(functionScope.block)) { + return null; + } + + if (!isBlockStatement(functionScope.block.body)) { + return null; + } + + const returnStatementNode = functionScope.block.body.body.find((statement) => + isReturnStatement(statement) + ) as TSESTree.ReturnStatement | null; + + if (!returnStatementNode) { + return null; + } + + if (!isCallExpression(returnStatementNode.argument)) { + return null; + } + + const returnCallee = returnStatementNode.argument.callee; + + if (ASTUtils.isIdentifier(returnCallee)) { + return returnCallee; + } else if ( + isMemberExpression(returnCallee) && + ASTUtils.isIdentifier(returnCallee.property) + ) { + return returnCallee.property; + } + + return null; +} + +export function getFunctionName( + node: + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression + | TSESTree.ArrowFunctionExpression +): string { + return ( + ASTUtils.getFunctionNameWithKind(node) + .match(/('\w+')/g)?.[0] + .replace(/'/g, '') ?? '' + ); +} + // TODO: should be removed after v4 is finished export function isRenderFunction( callNode: TSESTree.CallExpression, diff --git a/lib/rules/await-async-query.ts b/lib/rules/await-async-query.ts index e4c3970c..decbc52e 100644 --- a/lib/rules/await-async-query.ts +++ b/lib/rules/await-async-query.ts @@ -1,17 +1,13 @@ -import { - TSESTree, - ASTUtils, - TSESLintScope, -} from '@typescript-eslint/experimental-utils'; +import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; import { findClosestCallExpressionNode, + getFunctionName, + getFunctionReturnStatementIdentifier, + getInnermostFunctionScope, getVariableReferences, hasClosestExpectResolvesRejects, isAwaited, - isBlockStatement, - isCallExpression, isPromiseResolved, - isReturnStatement, } from '../node-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; @@ -41,45 +37,22 @@ export default createTestingLibraryRule({ create(context, _, helpers) { const functionWrappersNames: string[] = []; - function getInnermostFunctionScope( - asyncQueryNode: TSESTree.Identifier - ): TSESLintScope.FunctionScope | null { - const innermostScope = ASTUtils.getInnermostScope( - context.getScope(), - asyncQueryNode - ); - - if (innermostScope?.type === 'function') { - return (innermostScope as unknown) as TSESLintScope.FunctionScope; - } - - return null; - } - return { 'CallExpression Identifier'(node: TSESTree.Identifier) { // check async query direct calls or related function wrapper if (helpers.isAsyncQuery(node)) { - const functionScope = getInnermostFunctionScope(node); - - if ( - functionScope && - ASTUtils.isFunction(functionScope.block) && - isBlockStatement(functionScope.block.body) - ) { - // report function wrapper calls rather than async calls - - const returnStatementNode = functionScope.block.body.body.find( - (statement) => isReturnStatement(statement) - ) as TSESTree.ReturnStatement | null; + const functionScope = getInnermostFunctionScope(context, node); + if (functionScope) { + // save function wrapper calls rather than async calls to be reported later + const returnStatementIdentifier = getFunctionReturnStatementIdentifier( + functionScope + ); if ( - returnStatementNode && - isCallExpression(returnStatementNode.argument) + returnStatementIdentifier?.name === node.name && + ASTUtils.isFunction(functionScope.block) ) { - // TODO: improve this check, - // assume for now the query is within the return statement - functionWrappersNames.push(functionScope.block.id.name); + functionWrappersNames.push(getFunctionName(functionScope.block)); } } diff --git a/tests/lib/rules/await-async-query.test.ts b/tests/lib/rules/await-async-query.test.ts index e9542c58..523edc63 100644 --- a/tests/lib/rules/await-async-query.test.ts +++ b/tests/lib/rules/await-async-query.test.ts @@ -221,8 +221,7 @@ ruleTester.run(RULE_NAME, rule, { errors: [{ messageId: 'awaitAsyncQuery', line: 5, column: 27 }], })), - // TODO: add cases for arrow functions (both implicit and explicit return) - // unhandled promise returned from async queries function wrapper are invalid + // unhandled promise from async query function wrapper is invalid ...ALL_ASYNC_COMBINATIONS_TO_TEST.map((query) => ({ code: ` function queryWrapper() { From 30caeeac6ea68a1dcb9e42fb12c0a13af08b4b59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Tue, 24 Nov 2020 12:15:59 +0100 Subject: [PATCH 18/27] test(await-async-query): cases for arrow functions --- lib/node-utils.ts | 64 +++++++++++++---------- lib/rules/await-async-query.ts | 45 ++++++++++------ tests/lib/rules/await-async-query.test.ts | 34 ++++++++++++ 3 files changed, 100 insertions(+), 43 deletions(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 7f6ad37c..2999b75f 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -242,38 +242,46 @@ export function getInnermostFunctionScope( return null; } -export function getFunctionReturnStatementIdentifier( - functionScope: TSESLintScope.FunctionScope -): TSESTree.Identifier | null { - if (!ASTUtils.isFunction(functionScope.block)) { - return null; - } - - if (!isBlockStatement(functionScope.block.body)) { - return null; - } - - const returnStatementNode = functionScope.block.body.body.find((statement) => - isReturnStatement(statement) - ) as TSESTree.ReturnStatement | null; - - if (!returnStatementNode) { - return null; +export function getFunctionReturnStatementNode( + functionNode: + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression + | TSESTree.ArrowFunctionExpression +): TSESTree.Node | null { + if (isBlockStatement(functionNode.body)) { + // regular function or arrow function with block + const returnStatementNode = functionNode.body.body.find((statement) => + isReturnStatement(statement) + ) as TSESTree.ReturnStatement | null; + + if (!returnStatementNode) { + return null; + } + return returnStatementNode.argument; + } else if (functionNode.expression) { + // arrow function with implicit return + return functionNode.body; } +} - if (!isCallExpression(returnStatementNode.argument)) { - return null; +export function getIdentifierNode( + node: TSESTree.Node +): TSESTree.Identifier | null { + if (ASTUtils.isIdentifier(node)) { + return node; } - const returnCallee = returnStatementNode.argument.callee; - - if (ASTUtils.isIdentifier(returnCallee)) { - return returnCallee; - } else if ( - isMemberExpression(returnCallee) && - ASTUtils.isIdentifier(returnCallee.property) - ) { - return returnCallee.property; + if (isCallExpression(node)) { + const callExpressionCallee = node.callee; + + if (ASTUtils.isIdentifier(callExpressionCallee)) { + return callExpressionCallee; + } else if ( + isMemberExpression(callExpressionCallee) && + ASTUtils.isIdentifier(callExpressionCallee.property) + ) { + return callExpressionCallee.property; + } } return null; diff --git a/lib/rules/await-async-query.ts b/lib/rules/await-async-query.ts index decbc52e..a82ce6d6 100644 --- a/lib/rules/await-async-query.ts +++ b/lib/rules/await-async-query.ts @@ -2,7 +2,8 @@ import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; import { findClosestCallExpressionNode, getFunctionName, - getFunctionReturnStatementIdentifier, + getFunctionReturnStatementNode, + getIdentifierNode, getInnermostFunctionScope, getVariableReferences, hasClosestExpectResolvesRejects, @@ -37,24 +38,38 @@ export default createTestingLibraryRule({ create(context, _, helpers) { const functionWrappersNames: string[] = []; + function detectAsyncQueryWrapper(node: TSESTree.Identifier) { + const functionScope = getInnermostFunctionScope(context, node); + + if (functionScope && ASTUtils.isFunction(functionScope.block)) { + // save function wrapper calls rather than async calls to be reported later + const returnStatementNode = getFunctionReturnStatementNode( + functionScope.block + ); + + if (!returnStatementNode) { + return; + } + + const returnStatementIdentifier = getIdentifierNode( + returnStatementNode + ); + + if (!returnStatementIdentifier) { + return; + } + + if (returnStatementIdentifier?.name === node.name) { + functionWrappersNames.push(getFunctionName(functionScope.block)); + } + } + } + return { 'CallExpression Identifier'(node: TSESTree.Identifier) { // check async query direct calls or related function wrapper if (helpers.isAsyncQuery(node)) { - const functionScope = getInnermostFunctionScope(context, node); - - if (functionScope) { - // save function wrapper calls rather than async calls to be reported later - const returnStatementIdentifier = getFunctionReturnStatementIdentifier( - functionScope - ); - if ( - returnStatementIdentifier?.name === node.name && - ASTUtils.isFunction(functionScope.block) - ) { - functionWrappersNames.push(getFunctionName(functionScope.block)); - } - } + detectAsyncQueryWrapper(node); const closestCallExpressionNode = findClosestCallExpressionNode( node, diff --git a/tests/lib/rules/await-async-query.test.ts b/tests/lib/rules/await-async-query.test.ts index 523edc63..a1b8e1fa 100644 --- a/tests/lib/rules/await-async-query.test.ts +++ b/tests/lib/rules/await-async-query.test.ts @@ -240,5 +240,39 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [{ messageId: 'asyncQueryWrapper', line: 9, column: 27 }], })), + // unhandled promise from async query arrow function wrapper is invalid + ...ALL_ASYNC_COMBINATIONS_TO_TEST.map((query) => ({ + code: ` + const queryWrapper = () => { + doSomethingElse(); + + return ${query}('foo') + } + + test("An invalid example test", () => { + const element = queryWrapper() + }) + + test("An valid example test", async () => { + const element = await queryWrapper() + }) + `, + errors: [{ messageId: 'asyncQueryWrapper', line: 9, column: 27 }], + })), + // unhandled promise implicitly returned from async query arrow function wrapper is invalid + ...ALL_ASYNC_COMBINATIONS_TO_TEST.map((query) => ({ + code: ` + const queryWrapper = () => screen.${query}('foo') + + test("An invalid example test", () => { + const element = queryWrapper() + }) + + test("An valid example test", async () => { + const element = await queryWrapper() + }) + `, + errors: [{ messageId: 'asyncQueryWrapper', line: 5, column: 27 }], + })), ], }); From 0312ec935e0a58948ce257b86544d5d2b6dbfa02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Tue, 24 Nov 2020 12:57:38 +0100 Subject: [PATCH 19/27] refactor(await-async-query): extract ast util for promise handled --- lib/node-utils.ts | 45 +++++++++++++++++- lib/rules/await-async-query.ts | 84 +++++++++------------------------- 2 files changed, 65 insertions(+), 64 deletions(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 2999b75f..863d5abb 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -111,7 +111,7 @@ export function isJSXAttribute( /** * Finds the closest CallExpression node for a given node. * @param node - * @param shouldRestrictInnerScope - If true, CallExpression must be directly related to node + * @param shouldRestrictInnerScope - If true, CallExpression must belong to innermost scope of given node */ export function findClosestCallExpressionNode( node: TSESTree.Node, @@ -212,6 +212,49 @@ export function isPromiseResolved(node: TSESTree.Node): boolean { return hasThenProperty(parent); } +/** + * Determines whether an Identifier related to a promise is considered as handled. + * + * It will be considered as handled if: + * - it belongs to await expression + * - it's returned from a function + * - it's resolved with `then` method + * - has `resolves` or `rejects` + */ +export function isPromiseHandled(nodeIdentifier: TSESTree.Identifier): boolean { + const closestCallExpressionNode = findClosestCallExpressionNode( + nodeIdentifier, + true + ); + + const suspiciousNodes = [nodeIdentifier, closestCallExpressionNode].filter( + Boolean + ); + + for (const node of suspiciousNodes) { + if (ASTUtils.isAwaitExpression(node.parent)) { + return true; + } + + if ( + isArrowFunctionExpression(node.parent) || + isReturnStatement(node.parent) + ) { + return true; + } + + if (hasClosestExpectResolvesRejects(node.parent)) { + return true; + } + + if (isPromiseResolved(node)) { + return true; + } + } + + return false; +} + export function getVariableReferences( context: RuleContext, node: TSESTree.Node diff --git a/lib/rules/await-async-query.ts b/lib/rules/await-async-query.ts index a82ce6d6..10aaa995 100644 --- a/lib/rules/await-async-query.ts +++ b/lib/rules/await-async-query.ts @@ -6,9 +6,7 @@ import { getIdentifierNode, getInnermostFunctionScope, getVariableReferences, - hasClosestExpectResolvesRejects, - isAwaited, - isPromiseResolved, + isPromiseHandled, } from '../node-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; @@ -28,7 +26,7 @@ export default createTestingLibraryRule({ messages: { awaitAsyncQuery: 'promise returned from {{ name }} query must be handled', asyncQueryWrapper: - 'promise returned from {{ name }} wrapper must be handled', + 'promise returned from {{ name }} wrapper over async query must be handled', }, fixable: null, schema: [], @@ -55,10 +53,6 @@ export default createTestingLibraryRule({ returnStatementNode ); - if (!returnStatementIdentifier) { - return; - } - if (returnStatementIdentifier?.name === node.name) { functionWrappersNames.push(getFunctionName(functionScope.block)); } @@ -67,7 +61,6 @@ export default createTestingLibraryRule({ return { 'CallExpression Identifier'(node: TSESTree.Identifier) { - // check async query direct calls or related function wrapper if (helpers.isAsyncQuery(node)) { detectAsyncQueryWrapper(node); @@ -76,7 +69,6 @@ export default createTestingLibraryRule({ true ); - // stop checking if Identifier doesn't belong to CallExpression if (!closestCallExpressionNode) { return; } @@ -87,69 +79,35 @@ export default createTestingLibraryRule({ ); if (references && references.length === 0) { - // stop checking if already awaited - if (isAwaited(closestCallExpressionNode.parent)) { - return; - } - - // stop checking if promise already handled - if (isPromiseResolved(node)) { - return; + if (!isPromiseHandled(node)) { + return context.report({ + node, + messageId: 'awaitAsyncQuery', + data: { name: node.name }, + }); } - - // stop checking if belongs to async assert already handled - if (hasClosestExpectResolvesRejects(closestCallExpressionNode)) { - return; - } - - return context.report({ - node, - messageId: 'awaitAsyncQuery', - data: { name: node.name }, - }); } for (const reference of references) { - const referenceNode = reference.identifier; - - if (isAwaited(referenceNode.parent)) { - return; + if ( + ASTUtils.isIdentifier(reference.identifier) && + !isPromiseHandled(reference.identifier) + ) { + return context.report({ + node, + messageId: 'awaitAsyncQuery', + data: { name: node.name }, + }); } - - if (isPromiseResolved(referenceNode)) { - return; - } - + } + } else if (functionWrappersNames.includes(node.name)) { + if (!isPromiseHandled(node)) { return context.report({ node, - messageId: 'awaitAsyncQuery', + messageId: 'asyncQueryWrapper', data: { name: node.name }, }); } - } else if (functionWrappersNames.includes(node.name)) { - const closestCallExpressionNode = findClosestCallExpressionNode( - node, - true - ); - - // stop checking if Identifier doesn't belong to CallExpression - if (!closestCallExpressionNode) { - return; - } - - if (isAwaited(closestCallExpressionNode.parent)) { - return; - } - - if (isPromiseResolved(node)) { - return; - } - - return context.report({ - node, - messageId: 'asyncQueryWrapper', - data: { name: node.name }, - }); } }, }; From e0a8e42ff28a921b0384547ab010dd12334bed00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Tue, 24 Nov 2020 16:50:18 +0100 Subject: [PATCH 20/27] test(await-async-query): increase coverage --- lib/node-utils.ts | 26 ++++++++------- lib/rules/await-async-query.ts | 9 +++++- tests/lib/rules/await-async-query.test.ts | 39 +++++++++++++++++++++-- 3 files changed, 58 insertions(+), 16 deletions(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 863d5abb..97302d8a 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -266,10 +266,17 @@ export function getVariableReferences( ); } +interface InnermostFunctionScope extends TSESLintScope.FunctionScope { + block: + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression + | TSESTree.ArrowFunctionExpression; +} + export function getInnermostFunctionScope( context: RuleContext, asyncQueryNode: TSESTree.Identifier -): TSESLintScope.FunctionScope | null { +): InnermostFunctionScope | null { const innermostScope = ASTUtils.getInnermostScope( context.getScope(), asyncQueryNode @@ -279,7 +286,7 @@ export function getInnermostFunctionScope( innermostScope?.type === 'function' && ASTUtils.isFunction(innermostScope.block) ) { - return (innermostScope as unknown) as TSESLintScope.FunctionScope; + return (innermostScope as unknown) as InnermostFunctionScope; } return null; @@ -314,17 +321,12 @@ export function getIdentifierNode( return node; } - if (isCallExpression(node)) { - const callExpressionCallee = node.callee; + if (isMemberExpression(node) && ASTUtils.isIdentifier(node.property)) { + return node.property; + } - if (ASTUtils.isIdentifier(callExpressionCallee)) { - return callExpressionCallee; - } else if ( - isMemberExpression(callExpressionCallee) && - ASTUtils.isIdentifier(callExpressionCallee.property) - ) { - return callExpressionCallee.property; - } + if (isCallExpression(node)) { + return getIdentifierNode(node.callee); } return null; diff --git a/lib/rules/await-async-query.ts b/lib/rules/await-async-query.ts index 10aaa995..9c60a247 100644 --- a/lib/rules/await-async-query.ts +++ b/lib/rules/await-async-query.ts @@ -39,7 +39,7 @@ export default createTestingLibraryRule({ function detectAsyncQueryWrapper(node: TSESTree.Identifier) { const functionScope = getInnermostFunctionScope(context, node); - if (functionScope && ASTUtils.isFunction(functionScope.block)) { + if (functionScope) { // save function wrapper calls rather than async calls to be reported later const returnStatementNode = getFunctionReturnStatementNode( functionScope.block @@ -62,6 +62,7 @@ export default createTestingLibraryRule({ return { 'CallExpression Identifier'(node: TSESTree.Identifier) { if (helpers.isAsyncQuery(node)) { + // detect async query used within wrapper function for later analysis detectAsyncQueryWrapper(node); const closestCallExpressionNode = findClosestCallExpressionNode( @@ -78,6 +79,8 @@ export default createTestingLibraryRule({ closestCallExpressionNode.parent ); + // check direct usage of async query: + // const element = await findByRole('button') if (references && references.length === 0) { if (!isPromiseHandled(node)) { return context.report({ @@ -88,6 +91,9 @@ export default createTestingLibraryRule({ } } + // check references usages of async query: + // const promise = findByRole('button') + // const element = await promise for (const reference of references) { if ( ASTUtils.isIdentifier(reference.identifier) && @@ -101,6 +107,7 @@ export default createTestingLibraryRule({ } } } else if (functionWrappersNames.includes(node.name)) { + // check async queries used within a wrapper previously detected if (!isPromiseHandled(node)) { return context.report({ node, diff --git a/tests/lib/rules/await-async-query.test.ts b/tests/lib/rules/await-async-query.test.ts index a1b8e1fa..75e74e8e 100644 --- a/tests/lib/rules/await-async-query.test.ts +++ b/tests/lib/rules/await-async-query.test.ts @@ -125,8 +125,10 @@ ruleTester.run(RULE_NAME, rule, { // async queries are valid with promise in variable and returned in regular function ...createTestCase( (query) => ` - const promise = ${query}('foo') - return promise + async function queryWrapper() { + const promise = ${query}('foo') + return promise + } ` ), @@ -169,10 +171,41 @@ ruleTester.run(RULE_NAME, rule, { // non-matching query is valid ` - test('An example test', async () => { + test('An valid example test', async () => { const example = findText("my example") }) `, + + // unhandled promise from non-matching query is valid + ` + async function findButton() { + const element = findByText('outer element') + return somethingElse(element) + } + + test('An valid example test', async () => { + // findButton doesn't match async query pattern + const button = findButton() + }) + `, + + // edge case for coverage + // return non-matching query and other than Identifier or CallExpression + ` + async function someSetup() { + const element = await findByText('outer element') + return element ? findSomethingElse(element) : null + } + + test('An valid example test', async () => { + someSetup() + }) + `, + + // edge case for coverage + // valid async query usage without any function defined + // so there is no innermost function scope found + `const element = await findByRole('button')`, ], invalid: [ From 11e1b1fc63928b951a5c91ec4b9d625d4502f4de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Tue, 24 Nov 2020 16:57:00 +0100 Subject: [PATCH 21/27] refactor(await-async-query): rename isPromiseResolved to hasChainedThen --- lib/node-utils.ts | 4 ++-- lib/rules/await-async-utils.ts | 6 +++--- lib/rules/await-fire-event.ts | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 97302d8a..43084598 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -200,7 +200,7 @@ export function isAwaited(node: TSESTree.Node): boolean { ); } -export function isPromiseResolved(node: TSESTree.Node): boolean { +export function hasChainedThen(node: TSESTree.Node): boolean { const parent = node.parent; // wait(...).then(...) @@ -247,7 +247,7 @@ export function isPromiseHandled(nodeIdentifier: TSESTree.Identifier): boolean { return true; } - if (isPromiseResolved(node)) { + if (hasChainedThen(node)) { return true; } } diff --git a/lib/rules/await-async-utils.ts b/lib/rules/await-async-utils.ts index 6301bd4e..2c2cda9e 100644 --- a/lib/rules/await-async-utils.ts +++ b/lib/rules/await-async-utils.ts @@ -7,7 +7,7 @@ import { import { getDocsUrl, ASYNC_UTILS, LIBRARY_MODULES } from '../utils'; import { isAwaited, - isPromiseResolved, + hasChainedThen, getVariableReferences, isMemberExpression, isImportSpecifier, @@ -121,7 +121,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ references && references.length === 0 && !isAwaited(node.parent.parent) && - !isPromiseResolved(node) && + !hasChainedThen(node) && !isInPromiseAll(node) ) { context.report({ @@ -136,7 +136,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ const referenceNode = reference.identifier; if ( !isAwaited(referenceNode.parent) && - !isPromiseResolved(referenceNode) + !hasChainedThen(referenceNode) ) { context.report({ node, diff --git a/lib/rules/await-fire-event.ts b/lib/rules/await-fire-event.ts index a347658d..84f7d877 100644 --- a/lib/rules/await-fire-event.ts +++ b/lib/rules/await-fire-event.ts @@ -4,7 +4,7 @@ import { ASTUtils, } from '@typescript-eslint/experimental-utils'; import { getDocsUrl } from '../utils'; -import { isAwaited, isPromiseResolved } from '../node-utils'; +import { isAwaited, hasChainedThen } from '../node-utils'; export const RULE_NAME = 'await-fire-event'; export type MessageIds = 'awaitFireEvent'; @@ -37,7 +37,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ if ( ASTUtils.isIdentifier(fireEventMethodNode) && !isAwaited(node.parent.parent.parent) && - !isPromiseResolved(fireEventMethodNode.parent) + !hasChainedThen(fireEventMethodNode.parent) ) { context.report({ node: fireEventMethodNode, From 03131e044e771bf07db0f3a87266744fb67cec4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Tue, 24 Nov 2020 17:18:13 +0100 Subject: [PATCH 22/27] docs(await-async-query): update rule description and examples --- README.md | 2 +- docs/rules/await-async-query.md | 87 +++++++++++++++++++-------------- lib/node-utils.ts | 4 +- lib/rules/await-async-query.ts | 2 +- 4 files changed, 53 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index 9535d10b..174b78ff 100644 --- a/README.md +++ b/README.md @@ -127,7 +127,7 @@ To enable this configuration use the `extends` property in your | Rule | Description | Configurations | Fixable | | -------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------- | ----------------------------------------------------------------- | ------------------ | -| [await-async-query](docs/rules/await-async-query.md) | Enforce async queries to have proper `await` | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | +| [await-async-query](docs/rules/await-async-query.md) | Enforce promise from async queries to be handled | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | | [await-async-utils](docs/rules/await-async-utils.md) | Enforce async utils to be awaited properly | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | | [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | | | [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensure `data-testid` values match a provided regex. | | | diff --git a/docs/rules/await-async-query.md b/docs/rules/await-async-query.md index 493b7d03..68cca17d 100644 --- a/docs/rules/await-async-query.md +++ b/docs/rules/await-async-query.md @@ -1,4 +1,4 @@ -# Enforce async queries to have proper `await` (await-async-query) +# Enforce promise from async queries to be handled (await-async-query) Ensure that promises returned by async queries are handled properly. @@ -11,65 +11,76 @@ found. Those queries variants are: - `findBy*` - `findAllBy*` -This rule aims to prevent users from forgetting to await the returned +This rule aims to prevent users from forgetting to handle the returned promise from those async queries to be fulfilled, which could lead to -errors in the tests. The promises can be handled by using either `await` -operator or `then` method. +errors in the tests. The promise will be considered as handled when: + +- using `await` operator +- chaining `then` method +- chaining `resolves` or `rejects` from jest +- is returned from a function (in this case, that particular function will be analyzed by this rule too) Examples of **incorrect** code for this rule: ```js -const foo = () => { - // ... - const rows = findAllByRole('row'); - // ... -}; - -const bar = () => { - // ... - findByText('submit'); - // ... -}; - -const baz = () => { - // ... - screen.findAllByPlaceholderText('name'); - // ... -}; +// async query without handling promise +const rows = findAllByRole('row'); + +findByIcon('search'); + +screen.findAllByPlaceholderText('name'); +``` + +```js +// promise from async query returned within wrapper function without being handled +const findMyButton = () => findByText('my button'); + +const someButton = findMyButton(); // promise unhandled here ``` Examples of **correct** code for this rule: ```js // `await` operator is correct -const foo = async () => { - // ... - const rows = await findAllByRole('row'); - // ... -}; +const rows = await findAllByRole('row'); + +await screen.findAllByPlaceholderText('name'); + +const promise = findByIcon('search'); +const element = await promise; +``` +```js // `then` method is correct -const bar = () => { - // ... - findByText('submit').then(() => { - // ... - }); -}; - -const baz = () => { - // ... - await screen.findAllByPlaceholderText('name'); - // ... -}; +findByText('submit').then(() => {}); +const promise = findByRole('button'); +promise.then(() => {}); +``` + +```js // return the promise within a function is correct too! const findMyButton = () => findByText('my button'); +``` + +```js +// promise from async query returned within wrapper function being handled +const findMyButton = () => findByText('my button'); +const someButton = await findMyButton(); +``` + +```js // using a resolves/rejects matcher is also correct expect(findByTestId('alert')).resolves.toBe('Success'); expect(findByTestId('alert')).rejects.toBe('Error'); ``` +```js +// sync queries don't need to handle any promise +const element = getByRole('role'); +``` + ## Further Reading - [Async queries variants](https://testing-library.com/docs/dom-testing-library/api-queries#findby) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 43084598..6c86404c 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -216,9 +216,9 @@ export function hasChainedThen(node: TSESTree.Node): boolean { * Determines whether an Identifier related to a promise is considered as handled. * * It will be considered as handled if: - * - it belongs to await expression + * - it belongs to `await` expression + * - it's chained with `then` method * - it's returned from a function - * - it's resolved with `then` method * - has `resolves` or `rejects` */ export function isPromiseHandled(nodeIdentifier: TSESTree.Identifier): boolean { diff --git a/lib/rules/await-async-query.ts b/lib/rules/await-async-query.ts index 9c60a247..b6c3b2ef 100644 --- a/lib/rules/await-async-query.ts +++ b/lib/rules/await-async-query.ts @@ -19,7 +19,7 @@ export default createTestingLibraryRule({ meta: { type: 'problem', docs: { - description: 'Enforce async queries to have proper `await`', + description: 'Enforce promise from async queries to be handled', category: 'Best Practices', recommended: 'warn', }, From 2cce1230bf2d77112398d6930a62e6067b60ed4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Fri, 27 Nov 2020 17:33:41 +0100 Subject: [PATCH 23/27] docs(await-async-query): minor improvements --- README.md | 2 +- docs/rules/await-async-query.md | 4 ++-- lib/rules/await-async-query.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 174b78ff..46643a26 100644 --- a/README.md +++ b/README.md @@ -127,7 +127,7 @@ To enable this configuration use the `extends` property in your | Rule | Description | Configurations | Fixable | | -------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------- | ----------------------------------------------------------------- | ------------------ | -| [await-async-query](docs/rules/await-async-query.md) | Enforce promise from async queries to be handled | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | +| [await-async-query](docs/rules/await-async-query.md) | Enforce promises from async queries to be handled | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | | [await-async-utils](docs/rules/await-async-utils.md) | Enforce async utils to be awaited properly | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | | [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | | | [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensure `data-testid` values match a provided regex. | | | diff --git a/docs/rules/await-async-query.md b/docs/rules/await-async-query.md index 68cca17d..76848fac 100644 --- a/docs/rules/await-async-query.md +++ b/docs/rules/await-async-query.md @@ -1,10 +1,10 @@ -# Enforce promise from async queries to be handled (await-async-query) +# Enforce promises from async queries to be handled (await-async-query) Ensure that promises returned by async queries are handled properly. ## Rule Details -Some of the queries variants that Testing Library provides are +Some queries variants that Testing Library provides are asynchronous as they return a promise which resolves when elements are found. Those queries variants are: diff --git a/lib/rules/await-async-query.ts b/lib/rules/await-async-query.ts index b6c3b2ef..a436efe1 100644 --- a/lib/rules/await-async-query.ts +++ b/lib/rules/await-async-query.ts @@ -19,7 +19,7 @@ export default createTestingLibraryRule({ meta: { type: 'problem', docs: { - description: 'Enforce promise from async queries to be handled', + description: 'Enforce promises from async queries to be handled', category: 'Best Practices', recommended: 'warn', }, From 2d5438a1febc5e3567b4b2348e3192d28e839e7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sat, 28 Nov 2020 13:21:18 +0100 Subject: [PATCH 24/27] refactor: minor type fix --- lib/node-utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 6c86404c..8463fd3e 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -302,7 +302,7 @@ export function getFunctionReturnStatementNode( // regular function or arrow function with block const returnStatementNode = functionNode.body.body.find((statement) => isReturnStatement(statement) - ) as TSESTree.ReturnStatement | null; + ) as TSESTree.ReturnStatement | undefined; if (!returnStatementNode) { return null; From e8b93ef5e5540df2eea7fbf27e4b2d3b7921ae1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sat, 28 Nov 2020 18:46:54 +0100 Subject: [PATCH 25/27] docs(await-async-query): more fixes --- docs/rules/await-async-query.md | 6 +++--- lib/node-utils.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/rules/await-async-query.md b/docs/rules/await-async-query.md index 76848fac..fc635c48 100644 --- a/docs/rules/await-async-query.md +++ b/docs/rules/await-async-query.md @@ -15,10 +15,10 @@ This rule aims to prevent users from forgetting to handle the returned promise from those async queries to be fulfilled, which could lead to errors in the tests. The promise will be considered as handled when: -- using `await` operator -- chaining `then` method +- using the `await` operator +- chaining the `then` method - chaining `resolves` or `rejects` from jest -- is returned from a function (in this case, that particular function will be analyzed by this rule too) +- it's returned from a function (in this case, that particular function will be analyzed by this rule too) Examples of **incorrect** code for this rule: diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 8463fd3e..f968e4c5 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -216,8 +216,8 @@ export function hasChainedThen(node: TSESTree.Node): boolean { * Determines whether an Identifier related to a promise is considered as handled. * * It will be considered as handled if: - * - it belongs to `await` expression - * - it's chained with `then` method + * - it belongs to the `await` expression + * - it's chained with the `then` method * - it's returned from a function * - has `resolves` or `rejects` */ From 7fa901ddb8664d84177f1b4c9ca420f92e6428c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sat, 28 Nov 2020 18:49:24 +0100 Subject: [PATCH 26/27] docs(await-async-query): wrong return type --- lib/node-utils.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index f968e4c5..80a650cb 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -312,6 +312,8 @@ export function getFunctionReturnStatementNode( // arrow function with implicit return return functionNode.body; } + + return null; } export function getIdentifierNode( From bce0486ceb77d38a636c77499a00674356b30a8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sat, 28 Nov 2020 18:54:09 +0100 Subject: [PATCH 27/27] refactor(await-async-query): check regex more efficiently --- lib/detect-testing-library-utils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 2102a39b..ba57a7cd 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -137,21 +137,21 @@ export function detectTestingLibraryUtils< * Determines whether a given node is `getBy*` or `getAllBy*` query variant or not. */ const isGetByQuery: DetectionHelpers['isGetByQuery'] = (node) => { - return !!node.name.match(/^get(All)?By.+$/); + return /^get(All)?By.+$/.test(node.name); }; /** * Determines whether a given node is `queryBy*` or `queryAllBy*` query variant or not. */ const isQueryByQuery: DetectionHelpers['isQueryByQuery'] = (node) => { - return !!node.name.match(/^query(All)?By.+$/); + return /^query(All)?By.+$/.test(node.name); }; /** * Determines whether a given node is `findBy*` or `findAllBy*` query variant or not. */ const isFindByQuery: DetectionHelpers['isFindByQuery'] = (node) => { - return !!node.name.match(/^find(All)?By.+$/); + return /^find(All)?By.+$/.test(node.name); }; /**