diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 8b4e0cc9..312c3b63 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -8,6 +8,7 @@ import { getImportModuleName, getPropertyIdentifierNode, getReferenceNode, + hasImportMatch, ImportModuleNode, isImportDeclaration, isImportNamespaceSpecifier, @@ -127,22 +128,41 @@ export function detectTestingLibraryUtils< /** * Small method to extract common checks to determine whether a node is * related to Testing Library or not. + * + * To determine whether a node is a valid Testing Library util, there are + * two conditions to match: + * - it's named in a particular way (decided by given callback) + * - it's imported from valid Testing Library module (depends on aggressive + * reporting) */ function isTestingLibraryUtil( node: TSESTree.Identifier, - isUtilCallback: (identifierNode: TSESTree.Identifier) => boolean + isUtilCallback: ( + identifierNodeName: string, + originalNodeName?: string + ) => boolean ): boolean { - if (!isUtilCallback(node)) { + const referenceNode = getReferenceNode(node); + const referenceNodeIdentifier = getPropertyIdentifierNode(referenceNode); + const importedUtilSpecifier = getImportedUtilSpecifier( + referenceNodeIdentifier + ); + + const originalNodeName = + isImportSpecifier(importedUtilSpecifier) && + importedUtilSpecifier.local.name !== importedUtilSpecifier.imported.name + ? importedUtilSpecifier.imported.name + : undefined; + + if (!isUtilCallback(node.name, originalNodeName)) { return false; } - const referenceNode = getReferenceNode(node); - const referenceNodeIdentifier = getPropertyIdentifierNode(referenceNode); + if (isAggressiveModuleReportingEnabled()) { + return true; + } - return ( - isAggressiveModuleReportingEnabled() || - isNodeComingFromTestingLibrary(referenceNodeIdentifier) - ); + return isNodeComingFromTestingLibrary(referenceNodeIdentifier); } /** @@ -272,8 +292,8 @@ export function detectTestingLibraryUtils< * coming from Testing Library will be considered as valid. */ const isAsyncUtil: IsAsyncUtilFn = (node) => { - return isTestingLibraryUtil(node, (identifierNode) => - ASYNC_UTILS.includes(identifierNode.name) + return isTestingLibraryUtil(node, (identifierNodeName) => + ASYNC_UTILS.includes(identifierNodeName) ); }; @@ -347,13 +367,27 @@ export function detectTestingLibraryUtils< * only those nodes coming from Testing Library will be considered as valid. */ const isRenderUtil: IsRenderUtilFn = (node) => { - return isTestingLibraryUtil(node, (identifierNode) => { - if (isAggressiveRenderReportingEnabled()) { - return identifierNode.name.toLowerCase().includes(RENDER_NAME); + return isTestingLibraryUtil( + node, + (identifierNodeName, originalNodeName) => { + if (isAggressiveRenderReportingEnabled()) { + return identifierNodeName.toLowerCase().includes(RENDER_NAME); + } + + return [RENDER_NAME, ...customRenders].some((validRenderName) => { + let isMatch = false; + + if (validRenderName === identifierNodeName) { + isMatch = true; + } + + if (!!originalNodeName && validRenderName === originalNodeName) { + isMatch = true; + } + return isMatch; + }); } - - return [RENDER_NAME, ...customRenders].includes(identifierNode.name); - }); + ); }; /** @@ -402,25 +436,34 @@ export function detectTestingLibraryUtils< specifierName ) => { const node = getCustomModuleImportNode() ?? getTestingLibraryImportNode(); + if (!node) { return null; } + if (isImportDeclaration(node)) { - const namedExport = node.specifiers.find( - (n) => isImportSpecifier(n) && n.imported.name === specifierName - ); + const namedExport = node.specifiers.find((n) => { + return ( + isImportSpecifier(n) && + [n.imported.name, n.local.name].includes(specifierName) + ); + }); + // it is "import { foo [as alias] } from 'baz'"" if (namedExport) { return namedExport; } + // it could be "import * as rtl from 'baz'" return node.specifiers.find((n) => isImportNamespaceSpecifier(n)); } else { const requireNode = node.parent as TSESTree.VariableDeclarator; + if (ASTUtils.isIdentifier(requireNode.id)) { // this is const rtl = require('foo') return requireNode.id; } + // this should be const { something } = require('foo') const destructuring = requireNode.id as TSESTree.ObjectPattern; const property = destructuring.properties.find( @@ -429,27 +472,48 @@ export function detectTestingLibraryUtils< ASTUtils.isIdentifier(n.key) && n.key.name === specifierName ); + if (!property) { + return undefined; + } return (property as TSESTree.Property).key as TSESTree.Identifier; } }; + const getImportedUtilSpecifier = ( + node: TSESTree.MemberExpression | TSESTree.Identifier + ): TSESTree.ImportClause | TSESTree.Identifier | undefined => { + const identifierName: string | undefined = getPropertyIdentifierNode(node) + .name; + + return findImportedUtilSpecifier(identifierName); + }; + /** * Determines if file inspected meets all conditions to be reported by rules or not. */ const canReportErrors: CanReportErrorsFn = () => { return isTestingLibraryImported() && isValidFilename(); }; + /** - * Takes a MemberExpression or an Identifier and verifies if its name comes from the import in TL - * @param node a MemberExpression (in "foo.property" it would be property) or an Identifier + * Determines whether a node is imported from a valid Testing Library module + * + * This method will try to find any import matching the given node name, + * and also make sure the name is a valid match in case it's been renamed. */ const isNodeComingFromTestingLibrary: IsNodeComingFromTestingLibraryFn = ( node ) => { + const importNode = getImportedUtilSpecifier(node); + + if (!importNode) { + return false; + } + const identifierName: string | undefined = getPropertyIdentifierNode(node) .name; - return !!findImportedUtilSpecifier(identifierName); + return hasImportMatch(importNode, identifierName); }; const helpers: DetectionHelpers = { diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 2fcd4e75..4aba63cd 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -620,3 +620,14 @@ export function getInnermostReturningFunction( return functionScope.block; } + +export function hasImportMatch( + importNode: TSESTree.ImportClause | TSESTree.Identifier, + identifierName: string +): boolean { + if (ASTUtils.isIdentifier(importNode)) { + return importNode.name === identifierName; + } + + return importNode.local.name === identifierName; +} diff --git a/lib/rules/render-result-naming-convention.ts b/lib/rules/render-result-naming-convention.ts index f68e5170..b322a89a 100644 --- a/lib/rules/render-result-naming-convention.ts +++ b/lib/rules/render-result-naming-convention.ts @@ -1,6 +1,11 @@ import { createTestingLibraryRule } from '../create-testing-library-rule'; -import { getDeepestIdentifierNode, isObjectPattern } from '../node-utils'; -import { ASTUtils } from '@typescript-eslint/experimental-utils'; +import { + getDeepestIdentifierNode, + getFunctionName, + getInnermostReturningFunction, + isObjectPattern, +} from '../node-utils'; +import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; export const RULE_NAME = 'render-result-naming-convention'; export type MessageIds = 'renderResultNamingConvention'; @@ -30,7 +35,22 @@ export default createTestingLibraryRule({ defaultOptions: [], create(context, _, helpers) { + const renderWrapperNames: string[] = []; + + function detectRenderWrapper(node: TSESTree.Identifier): void { + const innerFunction = getInnermostReturningFunction(context, node); + + if (innerFunction) { + renderWrapperNames.push(getFunctionName(innerFunction)); + } + } + return { + 'CallExpression Identifier'(node: TSESTree.Identifier) { + if (helpers.isRenderUtil(node)) { + detectRenderWrapper(node); + } + }, VariableDeclarator(node) { const initIdentifierNode = getDeepestIdentifierNode(node.init); @@ -38,7 +58,10 @@ export default createTestingLibraryRule({ return; } - if (!helpers.isRenderUtil(initIdentifierNode)) { + if ( + !helpers.isRenderUtil(initIdentifierNode) && + !renderWrapperNames.includes(initIdentifierNode.name) + ) { return; } diff --git a/tests/create-testing-library-rule.test.ts b/tests/create-testing-library-rule.test.ts index 68c119cb..4907e995 100644 --- a/tests/create-testing-library-rule.test.ts +++ b/tests/create-testing-library-rule.test.ts @@ -126,6 +126,19 @@ ruleTester.run(RULE_NAME, rule, { const utils = render() `, }, + { + settings: { + 'testing-library/utils-module': 'test-utils', + }, + code: ` + // case (render util): aggressive reporting disabled - method with same name + // as TL method but not coming from TL module is valid + import { render as testingLibraryRender } from 'test-utils' + import { render } from 'somewhere-else' + + const utils = render() + `, + }, // Test Cases for presence/absence assertions // cases: asserts not related to presence/absence @@ -287,6 +300,21 @@ ruleTester.run(RULE_NAME, rule, { waitFor() `, }, + { + settings: { + 'testing-library/utils-module': 'test-utils', + }, + code: ` + // case (async util): aggressive reporting disabled - method with same name + // as TL method but not coming from TL module is valid + import { waitFor as testingLibraryWaitFor } from 'test-utils' + import { waitFor } from 'somewhere-else' + + test('this should not be reported', () => { + waitFor() + }); + `, + }, // Test Cases for all settings mixed { @@ -642,26 +670,6 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - { - settings: { - 'testing-library/utils-module': 'test-utils', - }, - code: ` - // case: waitFor from object property shadowed name is checked correctly - import * as tl from 'test-utils' - const obj = { tl } - - obj.module.waitFor(() => {}) - `, - errors: [ - { - line: 6, - column: 20, - messageId: 'asyncUtilError', - data: { utilName: 'waitFor' }, - }, - ], - }, { settings: { 'testing-library/utils-module': 'test-utils', diff --git a/tests/lib/rules/prefer-wait-for.test.ts b/tests/lib/rules/prefer-wait-for.test.ts index e905c328..f9a9ca8b 100644 --- a/tests/lib/rules/prefer-wait-for.test.ts +++ b/tests/lib/rules/prefer-wait-for.test.ts @@ -218,6 +218,21 @@ ruleTester.run(RULE_NAME, rule, { cy.wait(); `, }, + { + settings: { + 'testing-library/utils-module': 'test-utils', + }, + code: ` + // case: aggressive reporting disabled - method named same as invalid method + // but not coming from Testing Library is valid + import { wait as testingLibraryWait } from 'test-utils' + import { wait } from 'somewhere-else' + + async () => { + await wait(); + } + `, + }, { // https://github.com/testing-library/eslint-plugin-testing-library/issues/145 code: `import * as foo from 'imNoTestingLibrary'; diff --git a/tests/lib/rules/render-result-naming-convention.test.ts b/tests/lib/rules/render-result-naming-convention.test.ts index 8dea67cf..9043ca5f 100644 --- a/tests/lib/rules/render-result-naming-convention.test.ts +++ b/tests/lib/rules/render-result-naming-convention.test.ts @@ -134,6 +134,41 @@ ruleTester.run(RULE_NAME, rule, { }); `, }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render as testingLibraryRender } from '@testing-library/react'; + import { render } from '@somewhere/else' + + const setup = () => render(); + + test('aggressive reporting disabled - should not report nested render not related to TL', () => { + const wrapper = setup(); + const button = wrapper.getByText('some button'); + }); + `, + }, + { + settings: { + 'testing-library/utils-module': 'test-utils', + 'testing-library/custom-renders': ['customRender'], + }, + code: ` + import { customRender as myRender } from 'test-utils'; + import { customRender } from 'non-related' + + const setup = () => { + return customRender(); + }; + + test( + 'both render and module aggressive reporting disabled - should not report render result called "wrapper" from nont-related renamed custom render wrapped in a function', + async () => { + const wrapper = setup(); + await wrapper.findByRole('button'); + }); + `, + }, ], invalid: [ { @@ -260,6 +295,55 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render } from '@testing-library/react'; + + const setup = () => render(); + + test('aggressive reporting disabled - should report nested render from TL package', () => { + const wrapper = setup(); + const button = wrapper.getByText('some button'); + }); + `, + errors: [ + { + messageId: 'renderResultNamingConvention', + data: { + renderResultName: 'wrapper', + }, + line: 7, + column: 17, + }, + ], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render } from 'test-utils'; + + function setup() { + doSomethingElse(); + return render() + } + + test('aggressive reporting disabled - should report nested render from custom utils module', () => { + const wrapper = setup(); + const button = wrapper.getByText('some button'); + }); + `, + errors: [ + { + messageId: 'renderResultNamingConvention', + data: { + renderResultName: 'wrapper', + }, + line: 10, + column: 17, + }, + ], + }, { code: ` import { customRender } from 'test-utils'; @@ -343,5 +427,86 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + { + code: ` + import { render as testingLibraryRender } from '@testing-library/react'; + + const setup = () => { + return testingLibraryRender(); + }; + + test('should report render result called "wrapper" from renamed render wrapped in a function', async () => { + const wrapper = setup(); + await wrapper.findByRole('button'); + }); + `, + errors: [ + { + messageId: 'renderResultNamingConvention', + data: { + renderResultName: 'wrapper', + }, + line: 9, + column: 17, + }, + ], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render as testingLibraryRender } from '@testing-library/react'; + + const setup = () => { + return testingLibraryRender(); + }; + + test( + 'aggressive reporting disabled - should report render result called "wrapper" from renamed render wrapped in a function', + async () => { + const wrapper = setup(); + await wrapper.findByRole('button'); + }); + `, + errors: [ + { + messageId: 'renderResultNamingConvention', + data: { + renderResultName: 'wrapper', + }, + line: 11, + column: 17, + }, + ], + }, + { + settings: { + 'testing-library/utils-module': 'test-utils', + 'testing-library/custom-renders': ['customRender'], + }, + code: ` + import { customRender as myRender } from 'test-utils'; + + const setup = () => { + return myRender(); + }; + + test( + 'both render and module aggressive reporting disabled - should report render result called "wrapper" from renamed custom render wrapped in a function', + async () => { + const wrapper = setup(); + await wrapper.findByRole('button'); + }); + `, + errors: [ + { + messageId: 'renderResultNamingConvention', + data: { + renderResultName: 'wrapper', + }, + line: 11, + column: 17, + }, + ], + }, ], });