diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 312c3b63..9413c881 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -61,6 +61,7 @@ type IsQueryQueryVariantFn = (node: TSESTree.Identifier) => boolean; type IsFindQueryVariantFn = (node: TSESTree.Identifier) => boolean; type IsSyncQueryFn = (node: TSESTree.Identifier) => boolean; type IsAsyncQueryFn = (node: TSESTree.Identifier) => boolean; +type IsQueryFn = (node: TSESTree.Identifier) => boolean; type IsCustomQueryFn = (node: TSESTree.Identifier) => boolean; type IsAsyncUtilFn = (node: TSESTree.Identifier) => boolean; type IsFireEventMethodFn = (node: TSESTree.Identifier) => boolean; @@ -87,6 +88,7 @@ export interface DetectionHelpers { isFindQueryVariant: IsFindQueryVariantFn; isSyncQuery: IsSyncQueryFn; isAsyncQuery: IsAsyncQueryFn; + isQuery: IsQueryFn; isCustomQuery: IsCustomQueryFn; isAsyncUtil: IsAsyncUtilFn; isFireEventMethod: IsFireEventMethodFn; @@ -271,11 +273,16 @@ export function detectTestingLibraryUtils< return isFindQueryVariant(node); }; + /** + * Determines whether a given node is a valid query, + * either built-in or custom + */ + const isQuery: IsQueryFn = (node) => { + return isSyncQuery(node) || isAsyncQuery(node); + }; + const isCustomQuery: IsCustomQueryFn = (node) => { - return ( - (isSyncQuery(node) || isAsyncQuery(node)) && - !ALL_QUERIES_COMBINATIONS.includes(node.name) - ); + return isQuery(node) && !ALL_QUERIES_COMBINATIONS.includes(node.name); }; /** @@ -528,6 +535,7 @@ export function detectTestingLibraryUtils< isFindQueryVariant, isSyncQuery, isAsyncQuery, + isQuery, isCustomQuery, isAsyncUtil, isFireEventMethod, diff --git a/lib/rules/prefer-screen-queries.ts b/lib/rules/prefer-screen-queries.ts index 32f0f486..86f85da8 100644 --- a/lib/rules/prefer-screen-queries.ts +++ b/lib/rules/prefer-screen-queries.ts @@ -1,16 +1,12 @@ +import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; import { - ESLintUtils, - TSESTree, - ASTUtils, -} from '@typescript-eslint/experimental-utils'; -import { getDocsUrl, ALL_QUERIES_COMBINATIONS } from '../utils'; -import { + isCallExpression, isMemberExpression, + isObjectExpression, isObjectPattern, - isCallExpression, isProperty, - isObjectExpression, } from '../node-utils'; +import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'prefer-screen-queries'; export type MessageIds = 'preferScreenQueries'; @@ -20,7 +16,6 @@ const ALLOWED_RENDER_PROPERTIES_FOR_DESTRUCTURING = [ 'container', 'baseElement', ]; -const ALL_QUERIES_COMBINATIONS_REGEXP = ALL_QUERIES_COMBINATIONS.join('|'); function usesContainerOrBaseElement(node: TSESTree.CallExpression) { const secondArgument = node.arguments[1]; @@ -35,7 +30,7 @@ function usesContainerOrBaseElement(node: TSESTree.CallExpression) { ); } -export default ESLintUtils.RuleCreator(getDocsUrl)({ +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'suggestion', @@ -53,7 +48,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ }, defaultOptions: [], - create(context) { + create(context, _, helpers) { function reportInvalidUsage(node: TSESTree.Identifier) { context.report({ node, @@ -64,8 +59,26 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ }); } - const queriesRegex = new RegExp(ALL_QUERIES_COMBINATIONS_REGEXP); - const queriesDestructuredInWithinDeclaration: string[] = []; + function saveSafeDestructuredQueries(node: TSESTree.VariableDeclarator) { + if (isObjectPattern(node.id)) { + const identifiers = node.id.properties + .filter( + (property) => + isProperty(property) && + ASTUtils.isIdentifier(property.key) && + helpers.isQuery(property.key) + ) + .map( + (property: TSESTree.Property) => + (property.key as TSESTree.Identifier).name + ); + safeDestructuredQueries.push(...identifiers); + } + } + + // keep here those queries which are safe and shouldn't be reported + // (from within, from render + container/base element, not related to TL, etc) + const safeDestructuredQueries: string[] = []; // use an array as within might be used more than once in a test const withinDeclaredVariables: string[] = []; @@ -77,31 +90,27 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ ) { return; } + + const isComingFromValidRender = helpers.isRenderUtil(node.init.callee); + + if (!isComingFromValidRender) { + // save the destructured query methods as safe since they are coming + // from render not related to TL + saveSafeDestructuredQueries(node); + } + const isWithinFunction = node.init.callee.name === 'within'; - // TODO add the custom render option #198 const usesRenderOptions = - node.init.callee.name === 'render' && - usesContainerOrBaseElement(node.init); + isComingFromValidRender && usesContainerOrBaseElement(node.init); if (!isWithinFunction && !usesRenderOptions) { return; } if (isObjectPattern(node.id)) { - // save the destructured query methods - const identifiers = node.id.properties - .filter( - (property) => - isProperty(property) && - ASTUtils.isIdentifier(property.key) && - queriesRegex.test(property.key.name) - ) - .map( - (property: TSESTree.Property) => - (property.key as TSESTree.Identifier).name - ); - - queriesDestructuredInWithinDeclaration.push(...identifiers); + // save the destructured query methods as safe since they are coming + // from within or render + base/container options + saveSafeDestructuredQueries(node); return; } @@ -109,31 +118,33 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ withinDeclaredVariables.push(node.id.name); } }, - [`CallExpression > Identifier[name=/^${ALL_QUERIES_COMBINATIONS_REGEXP}$/]`]( - node: TSESTree.Identifier - ) { + 'CallExpression > Identifier'(node: TSESTree.Identifier) { + if (!helpers.isQuery(node)) { + return; + } + if ( - !queriesDestructuredInWithinDeclaration.some( - (queryName) => queryName === node.name - ) + !safeDestructuredQueries.some((queryName) => queryName === node.name) ) { reportInvalidUsage(node); } }, - [`MemberExpression > Identifier[name=/^${ALL_QUERIES_COMBINATIONS_REGEXP}$/]`]( - node: TSESTree.Identifier - ) { + 'MemberExpression > Identifier'(node: TSESTree.Identifier) { function isIdentifierAllowed(name: string) { return ['screen', ...withinDeclaredVariables].includes(name); } + if (!helpers.isQuery(node)) { + return; + } + if ( ASTUtils.isIdentifier(node) && isMemberExpression(node.parent) && isCallExpression(node.parent.object) && ASTUtils.isIdentifier(node.parent.object.callee) && node.parent.object.callee.name !== 'within' && - node.parent.object.callee.name === 'render' && + helpers.isRenderUtil(node.parent.object.callee) && !usesContainerOrBaseElement(node.parent.object) ) { reportInvalidUsage(node); diff --git a/tests/lib/rules/prefer-screen-queries.test.ts b/tests/lib/rules/prefer-screen-queries.test.ts index cbaa0e44..b1a7f12a 100644 --- a/tests/lib/rules/prefer-screen-queries.test.ts +++ b/tests/lib/rules/prefer-screen-queries.test.ts @@ -1,15 +1,27 @@ import { createRuleTester } from '../test-utils'; import rule, { RULE_NAME } from '../../../lib/rules/prefer-screen-queries'; -import { ALL_QUERIES_COMBINATIONS } from '../../../lib/utils'; +import { + ALL_QUERIES_COMBINATIONS, + ALL_QUERIES_VARIANTS, + combineQueries, +} from '../../../lib/utils'; const ruleTester = createRuleTester(); +const CUSTOM_QUERY_COMBINATIONS = combineQueries(ALL_QUERIES_VARIANTS, [ + 'ByIcon', +]); +const ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS = [ + ...ALL_QUERIES_COMBINATIONS, + ...CUSTOM_QUERY_COMBINATIONS, +]; + ruleTester.run(RULE_NAME, rule, { valid: [ { code: `const baz = () => 'foo'`, }, - ...ALL_QUERIES_COMBINATIONS.map((queryMethod) => ({ + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ code: `screen.${queryMethod}()`, })), { @@ -18,19 +30,19 @@ ruleTester.run(RULE_NAME, rule, { { code: `component.otherFunctionShouldNotThrow()`, }, - ...ALL_QUERIES_COMBINATIONS.map((queryMethod) => ({ + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ code: `within(component).${queryMethod}()`, })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod) => ({ + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ code: `within(screen.${queryMethod}()).${queryMethod}()`, })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod) => ({ + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ code: ` const { ${queryMethod} } = within(screen.getByText('foo')) ${queryMethod}(baz) `, })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod) => ({ + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ code: ` const myWithinVariable = within(foo) myWithinVariable.${queryMethod}('baz') @@ -84,56 +96,158 @@ ruleTester.run(RULE_NAME, rule, { utils.unmount(); `, }, - ...ALL_QUERIES_COMBINATIONS.map((queryMethod: string) => ({ - code: ` + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map( + (queryMethod: string) => ({ + code: ` const { ${queryMethod} } = render(baz, { baseElement: treeA }) expect(${queryMethod}(baz)).toBeDefined() `, - })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod: string) => ({ - code: ` + }) + ), + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map( + (queryMethod: string) => ({ + code: ` const { ${queryMethod}: aliasMethod } = render(baz, { baseElement: treeA }) expect(aliasMethod(baz)).toBeDefined() `, - })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod: string) => ({ - code: ` + }) + ), + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map( + (queryMethod: string) => ({ + code: ` const { ${queryMethod} } = render(baz, { container: treeA }) expect(${queryMethod}(baz)).toBeDefined() `, - })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod: string) => ({ - code: ` + }) + ), + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map( + (queryMethod: string) => ({ + code: ` const { ${queryMethod}: aliasMethod } = render(baz, { container: treeA }) expect(aliasMethod(baz)).toBeDefined() `, - })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod: string) => ({ - code: ` + }) + ), + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map( + (queryMethod: string) => ({ + code: ` const { ${queryMethod} } = render(baz, { baseElement: treeB, container: treeA }) expect(${queryMethod}(baz)).toBeDefined() `, - })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod: string) => ({ - code: ` + }) + ), + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map( + (queryMethod: string) => ({ + code: ` const { ${queryMethod}: aliasMethod } = render(baz, { baseElement: treeB, container: treeA }) expect(aliasMethod(baz)).toBeDefined() `, - })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod: string) => ({ - code: ` + }) + ), + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map( + (queryMethod: string) => ({ + code: ` render(foo, { baseElement: treeA }).${queryMethod}() `, + }) + ), + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render as testUtilRender } from 'test-utils' + import { render } from 'somewhere-else' + const { ${queryMethod} } = render(foo) + ${queryMethod}()`, + })), + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ + settings: { + 'testing-library/custom-renders': ['customRender'], + }, + code: ` + import { anotherRender } from 'whatever' + const { ${queryMethod} } = anotherRender(foo) + ${queryMethod}()`, })), ], invalid: [ - ...ALL_QUERIES_COMBINATIONS.map((queryMethod) => ({ + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ + code: ` + const { ${queryMethod} } = render(foo) + ${queryMethod}()`, + errors: [ + { + messageId: 'preferScreenQueries', + data: { + name: queryMethod, + }, + }, + ], + })), + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render } from 'test-utils' + const { ${queryMethod} } = render(foo) + ${queryMethod}()`, + errors: [ + { + line: 4, + column: 9, + messageId: 'preferScreenQueries', + data: { + name: queryMethod, + }, + }, + ], + })), + + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ + settings: { + 'testing-library/custom-renders': ['customRender'], + }, + code: ` + import { customRender } from 'whatever' + const { ${queryMethod} } = customRender(foo) + ${queryMethod}()`, + errors: [ + { + line: 4, + column: 9, + messageId: 'preferScreenQueries', + data: { + name: queryMethod, + }, + }, + ], + })), + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render as testingLibraryRender} from '@testing-library/react' + const { ${queryMethod} } = testingLibraryRender(foo) + ${queryMethod}()`, + errors: [ + { + line: 4, + column: 9, + messageId: 'preferScreenQueries', + data: { + name: queryMethod, + }, + }, + ], + })), + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ + settings: { 'testing-library/utils-module': 'test-utils' }, code: ` + import { render } from 'test-utils' const { ${queryMethod} } = render(foo) ${queryMethod}()`, errors: [ { + line: 4, + column: 9, messageId: 'preferScreenQueries', data: { name: queryMethod, @@ -141,7 +255,7 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod) => ({ + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ code: `render().${queryMethod}()`, errors: [ { @@ -152,7 +266,7 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod) => ({ + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ code: `render(foo, { hydrate: true }).${queryMethod}()`, errors: [ { @@ -163,7 +277,7 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod) => ({ + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ code: `component.${queryMethod}()`, errors: [ { @@ -174,7 +288,7 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod) => ({ + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ code: ` const { ${queryMethod} } = render() ${queryMethod}(baz) @@ -188,7 +302,7 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod) => ({ + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ code: ` const myRenderVariable = render() myRenderVariable.${queryMethod}(baz) @@ -202,7 +316,7 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod) => ({ + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ code: ` const [myVariable] = render() myVariable.${queryMethod}(baz) @@ -216,7 +330,7 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod) => ({ + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ code: ` const { ${queryMethod} } = render(baz, { hydrate: true }) ${queryMethod}(baz) @@ -230,7 +344,7 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), - ...ALL_QUERIES_COMBINATIONS.map((queryMethod) => ({ + ...ALL_BUILTIN_AND_CUSTOM_QUERIES_COMBINATIONS.map((queryMethod) => ({ code: ` const [myVariable] = within() myVariable.${queryMethod}(baz)