diff --git a/README.md b/README.md index 9535d10b..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 async queries to have proper `await` | ![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 493b7d03..fc635c48 100644 --- a/docs/rules/await-async-query.md +++ b/docs/rules/await-async-query.md @@ -1,75 +1,86 @@ -# Enforce async queries to have proper `await` (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: - `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 the `await` operator +- chaining the `then` method +- chaining `resolves` or `rejects` from jest +- 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: ```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/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 17c6ab1b..ba57a7cd 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; @@ -135,14 +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 /^find(All)?By.+$/.test(node.name); }; /** @@ -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/lib/node-utils.ts b/lib/node-utils.ts index 26825717..80a650cb 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -2,10 +2,40 @@ import { AST_NODE_TYPES, ASTUtils, TSESLint, + TSESLintScope, TSESTree, } 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 +108,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 belong to innermost scope of given 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; } @@ -157,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(...) @@ -169,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 the `await` expression + * - it's chained with the `then` method + * - it's returned from a function + * - 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 (hasChainedThen(node)) { + return true; + } + } + + return false; +} + export function getVariableReferences( context: RuleContext, node: TSESTree.Node @@ -180,6 +266,88 @@ export function getVariableReferences( ); } +interface InnermostFunctionScope extends TSESLintScope.FunctionScope { + block: + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression + | TSESTree.ArrowFunctionExpression; +} + +export function getInnermostFunctionScope( + context: RuleContext, + asyncQueryNode: TSESTree.Identifier +): InnermostFunctionScope | null { + const innermostScope = ASTUtils.getInnermostScope( + context.getScope(), + asyncQueryNode + ); + + if ( + innermostScope?.type === 'function' && + ASTUtils.isFunction(innermostScope.block) + ) { + return (innermostScope as unknown) as InnermostFunctionScope; + } + + 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 | undefined; + + if (!returnStatementNode) { + return null; + } + return returnStatementNode.argument; + } else if (functionNode.expression) { + // arrow function with implicit return + return functionNode.body; + } + + return null; +} + +export function getIdentifierNode( + node: TSESTree.Node +): TSESTree.Identifier | null { + if (ASTUtils.isIdentifier(node)) { + return node; + } + + if (isMemberExpression(node) && ASTUtils.isIdentifier(node.property)) { + return node.property; + } + + if (isCallExpression(node)) { + return getIdentifierNode(node.callee); + } + + 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, renderFunctions: string[] @@ -197,6 +365,7 @@ export function isRenderFunction( }); } +// TODO: should be removed after v4 is finished export function isRenderVariableDeclarator( node: TSESTree.VariableDeclarator, renderFunctions: string[] @@ -281,3 +450,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 730188c2..a436efe1 100644 --- a/lib/rules/await-async-query.ts +++ b/lib/rules/await-async-query.ts @@ -1,142 +1,121 @@ +import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; import { - ESLintUtils, - TSESTree, - ASTUtils, -} from '@typescript-eslint/experimental-utils'; -import { getDocsUrl, LIBRARY_MODULES } from '../utils'; -import { - isCallExpression, - isMemberExpression, - isAwaited, - isPromiseResolved, + findClosestCallExpressionNode, + getFunctionName, + getFunctionReturnStatementNode, + getIdentifierNode, + getInnermostFunctionScope, getVariableReferences, + isPromiseHandled, } 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'; +export type MessageIds = 'awaitAsyncQuery' | 'asyncQueryWrapper'; 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 ESLintUtils.RuleCreator(getDocsUrl)({ +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'problem', docs: { - description: 'Enforce async queries to have proper `await`', + description: 'Enforce promises from async queries to be handled', category: 'Best Practices', recommended: 'warn', }, messages: { - awaitAsyncQuery: '`{{ name }}` must have `await` operator', + awaitAsyncQuery: 'promise returned from {{ name }} query must be handled', + asyncQueryWrapper: + 'promise returned from {{ name }} wrapper over async query must be handled', }, fixable: null, schema: [], }, defaultOptions: [], - create(context) { - const testingLibraryQueryUsage: { - node: TSESTree.Identifier | TSESTree.MemberExpression; - queryName: string; - }[] = []; + create(context, _, helpers) { + const functionWrappersNames: string[] = []; - const isQueryUsage = ( - node: TSESTree.Identifier | TSESTree.MemberExpression - ) => - !isAwaited(node.parent.parent) && - !isPromiseResolved(node) && - !hasClosestExpectResolvesRejects(node); + function detectAsyncQueryWrapper(node: TSESTree.Identifier) { + const functionScope = getInnermostFunctionScope(context, node); + + if (functionScope) { + // save function wrapper calls rather than async calls to be reported later + const returnStatementNode = getFunctionReturnStatementNode( + functionScope.block + ); + + if (!returnStatementNode) { + return; + } - let hasImportedFromTestingLibraryModule = false; + const returnStatementIdentifier = getIdentifierNode( + returnStatementNode + ); - function report(params: ReportDescriptor<'awaitAsyncQuery'>) { - if (hasImportedFromTestingLibraryModule) { - context.report(params); + if (returnStatementIdentifier?.name === node.name) { + functionWrappersNames.push(getFunctionName(functionScope.block)); + } } } return { - 'ImportDeclaration > ImportSpecifier,ImportNamespaceSpecifier'( - node: TSESTree.Node - ) { - const importDeclaration = node.parent as TSESTree.ImportDeclaration; - const module = importDeclaration.source.value.toString(); + 'CallExpression Identifier'(node: TSESTree.Identifier) { + if (helpers.isAsyncQuery(node)) { + // detect async query used within wrapper function for later analysis + detectAsyncQueryWrapper(node); - 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 }); - } - }, - 'Program:exit'() { - testingLibraryQueryUsage.forEach(({ node, queryName }) => { - const references = getVariableReferences(context, node.parent.parent); + const closestCallExpressionNode = findClosestCallExpressionNode( + node, + true + ); + + if (!closestCallExpressionNode) { + return; + } + const references = getVariableReferences( + context, + closestCallExpressionNode.parent + ); + + // check direct usage of async query: + // const element = await findByRole('button') 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, - }, - }); + if (!isPromiseHandled(node)) { + return context.report({ + node, + messageId: 'awaitAsyncQuery', + data: { name: node.name }, + }); + } + } - break; - } + // check references usages of async query: + // const promise = findByRole('button') + // const element = await promise + for (const reference of references) { + if ( + ASTUtils.isIdentifier(reference.identifier) && + !isPromiseHandled(reference.identifier) + ) { + return context.report({ + node, + messageId: 'awaitAsyncQuery', + data: { name: node.name }, + }); } } - }); + } else if (functionWrappersNames.includes(node.name)) { + // check async queries used within a wrapper previously detected + if (!isPromiseHandled(node)) { + return context.report({ + node, + messageId: 'asyncQueryWrapper', + data: { name: node.name }, + }); + } + } }, }; }, 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, diff --git a/lib/rules/no-await-sync-query.ts b/lib/rules/no-await-sync-query.ts index 7e4efec1..ced9d104 100644 --- a/lib/rules/no-await-sync-query.ts +++ b/lib/rules/no-await-sync-query.ts @@ -1,13 +1,11 @@ -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'; 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', @@ -17,25 +15,27 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ 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: [], }, 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, + }, + }); + } + }, }; }, }); 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/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) => { diff --git a/tests/lib/rules/await-async-query.test.ts b/tests/lib/rules/await-async-query.test.ts index c4c4c07f..75e74e8e 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'; @@ -25,14 +27,17 @@ function createTestCode({ code, isAsync = true }: TestCode) { interface TestCaseParams { isAsync?: boolean; combinations?: string[]; - errors?: TestCaseError<'awaitAsyncQuery'>[]; + errors?: TestCaseError<'awaitAsyncQuery' | 'asyncQueryWrapper'>[]; } 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); @@ -46,6 +51,17 @@ function createTestCase( }); } +const CUSTOM_ASYNC_QUERIES_COMBINATIONS = combineQueries( + ASYNC_QUERIES_VARIANTS, + ['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 @@ -81,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) => ` @@ -114,11 +122,13 @@ 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') - return promise + async function queryWrapper() { + const promise = ${query}('foo') + return promise + } ` ), @@ -147,16 +157,9 @@ ruleTester.run(RULE_NAME, rule, { ` ), - // 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) => ({ + // 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" @@ -165,6 +168,44 @@ ruleTester.run(RULE_NAME, rule, { }) `, })), + + // non-matching query is valid + ` + 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: [ @@ -174,13 +215,13 @@ ruleTester.run(RULE_NAME, rule, { doSomething() const foo = ${query}('foo') `, - errors: [{ messageId: 'awaitAsyncQuery' }], + errors: [{ messageId: 'awaitAsyncQuery', line: 6, column: 21 }], })), // 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 }], })), ...createTestCase((query) => ({ @@ -192,6 +233,7 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { line: 5, + column: 21, messageId: 'awaitAsyncQuery', data: { name: query, @@ -199,5 +241,71 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), + + // unresolved async queries are not valid (aggressive reporting) + ...ALL_ASYNC_COMBINATIONS_TO_TEST.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 }], + })), + + // unhandled promise from async query function wrapper is 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 }], + })), + // 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 }], + })), ], }); diff --git a/tests/lib/rules/no-await-sync-query.test.ts b/tests/lib/rules/no-await-sync-query.test.ts index 138cf5c9..40b46ba5 100644 --- a/tests/lib/rules/no-await-sync-query.test.ts +++ b/tests/lib/rules/no-await-sync-query.test.ts @@ -12,7 +12,31 @@ ruleTester.run(RULE_NAME, rule, { // sync queries without await are valid ...SYNC_QUERIES_COMBINATIONS.map((query) => ({ code: `() => { - ${query}('foo') + const element = ${query}('foo') + } + `, + })), + // 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: `() => { + expect(${query}('foo')).toBeEnabled() } `, })), @@ -20,7 +44,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') } `, })), @@ -32,18 +56,87 @@ 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') + } + `, + }, ], invalid: [ // 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, + }, + ], + })), + // 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 () => { + expect(await ${query}('foo')).toBeEnabled() } `, errors: [ { messageId: 'noAwaitSyncQuery', + line: 2, + column: 22, }, ], })), @@ -51,14 +144,55 @@ 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: 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, }, ], })), + + // 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 }], + }, ], });