diff --git a/docs/rules/no-container.md b/docs/rules/no-container.md index 658879e2..2089fad5 100644 --- a/docs/rules/no-container.md +++ b/docs/rules/no-container.md @@ -32,12 +32,6 @@ render(); screen.getByRole('button', { name: /click me/i }); ``` -If you use [custom render functions](https://testing-library.com/docs/example-react-redux) then you can set a config option in your `.eslintrc` to look for these. - -``` -"testing-library/no-container": ["error", {"renderFunctions":["renderWithRedux", "renderWithRouter"]}], -``` - ## Further Reading - [about the `container` element](https://testing-library.com/docs/react-testing-library/api#container-1) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 316accf7..503729c1 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -5,6 +5,7 @@ import { } from '@typescript-eslint/experimental-utils'; import { getAssertNodeInfo, + getDeepestIdentifierNode, getImportModuleName, getPropertyIdentifierNode, getReferenceNode, @@ -69,6 +70,9 @@ type IsAsyncUtilFn = ( ) => boolean; type IsFireEventMethodFn = (node: TSESTree.Identifier) => boolean; type IsRenderUtilFn = (node: TSESTree.Identifier) => boolean; +type IsRenderVariableDeclaratorFn = ( + node: TSESTree.VariableDeclarator +) => boolean; type IsDebugUtilFn = (node: TSESTree.Identifier) => boolean; type IsPresenceAssertFn = (node: TSESTree.MemberExpression) => boolean; type IsAbsenceAssertFn = (node: TSESTree.MemberExpression) => boolean; @@ -97,6 +101,7 @@ export interface DetectionHelpers { isAsyncUtil: IsAsyncUtilFn; isFireEventMethod: IsFireEventMethodFn; isRenderUtil: IsRenderUtilFn; + isRenderVariableDeclarator: IsRenderVariableDeclaratorFn; isDebugUtil: IsDebugUtilFn; isPresenceAssert: IsPresenceAssertFn; isAbsenceAssert: IsAbsenceAssertFn; @@ -149,6 +154,10 @@ export function detectTestingLibraryUtils< originalNodeName?: string ) => boolean ): boolean { + if (!node) { + return false; + } + const referenceNode = getReferenceNode(node); const referenceNodeIdentifier = getPropertyIdentifierNode(referenceNode); const importedUtilSpecifier = getImportedUtilSpecifier( @@ -408,6 +417,12 @@ export function detectTestingLibraryUtils< ); }; + const isRenderVariableDeclarator: IsRenderVariableDeclaratorFn = (node) => { + const initIdentifierNode = getDeepestIdentifierNode(node.init); + + return isRenderUtil(initIdentifierNode); + }; + const isDebugUtil: IsDebugUtilFn = (node) => { return isTestingLibraryUtil( node, @@ -562,6 +577,7 @@ export function detectTestingLibraryUtils< isAsyncUtil, isFireEventMethod, isRenderUtil, + isRenderVariableDeclarator, isDebugUtil, isPresenceAssert, isAbsenceAssert, diff --git a/lib/node-utils.ts b/lib/node-utils.ts index c6a76253..4cda2a2e 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -477,31 +477,6 @@ export function isRenderFunction( }); } -// TODO: should be removed after v4 is finished -export function isRenderVariableDeclarator( - node: TSESTree.VariableDeclarator, - renderFunctions: string[] -): boolean { - if (node.init) { - if (ASTUtils.isAwaitExpression(node.init)) { - return ( - node.init.argument && - isRenderFunction( - node.init.argument as TSESTree.CallExpression, - renderFunctions - ) - ); - } else { - return ( - isCallExpression(node.init) && - isRenderFunction(node.init, renderFunctions) - ); - } - } - - return false; -} - // TODO: extract into types file? export type ImportModuleNode = | TSESTree.ImportDeclaration diff --git a/lib/rules/no-container.ts b/lib/rules/no-container.ts index 7970ec9c..d50923c2 100644 --- a/lib/rules/no-container.ts +++ b/lib/rules/no-container.ts @@ -1,21 +1,19 @@ +import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; import { - ESLintUtils, - TSESTree, - ASTUtils, -} from '@typescript-eslint/experimental-utils'; -import { getDocsUrl } from '../utils'; -import { + getDeepestIdentifierNode, + getFunctionName, + getInnermostReturningFunction, isMemberExpression, isObjectPattern, isProperty, - isRenderVariableDeclarator, } from '../node-utils'; +import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'no-container'; export type MessageIds = 'noContainer'; -type Options = [{ renderFunctions?: string[] }]; +type Options = []; -export default ESLintUtils.RuleCreator(getDocsUrl)({ +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'problem', @@ -29,48 +27,52 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ 'Avoid using container methods. Prefer using the methods from Testing Library, such as "getByRole()"', }, fixable: null, - schema: [ - { - type: 'object', - properties: { - renderFunctions: { - type: 'array', - }, - }, - }, - ], + schema: [], }, - defaultOptions: [ - { - renderFunctions: [], - }, - ], + defaultOptions: [], - create(context, [options]) { - const { renderFunctions } = options; + create(context, [], helpers) { const destructuredContainerPropNames: string[] = []; - let renderWrapperName: string = null; + const renderWrapperNames: string[] = []; + let renderResultVarName: string = null; let containerName: string = null; let containerCallsMethod = false; + function detectRenderWrapper(node: TSESTree.Identifier): void { + const innerFunction = getInnermostReturningFunction(context, node); + + if (innerFunction) { + renderWrapperNames.push(getFunctionName(innerFunction)); + } + } + function showErrorIfChainedContainerMethod( innerNode: TSESTree.MemberExpression ) { if (isMemberExpression(innerNode)) { if (ASTUtils.isIdentifier(innerNode.object)) { const isContainerName = innerNode.object.name === containerName; - const isRenderWrapper = innerNode.object.name === renderWrapperName; + if (isContainerName) { + context.report({ + node: innerNode, + messageId: 'noContainer', + }); + return; + } + + const isRenderWrapper = innerNode.object.name === renderResultVarName; containerCallsMethod = ASTUtils.isIdentifier(innerNode.property) && innerNode.property.name === 'container' && isRenderWrapper; - if (isContainerName || containerCallsMethod) { + if (containerCallsMethod) { context.report({ - node: innerNode, + node: innerNode.property, messageId: 'noContainer', }); + return; } } showErrorIfChainedContainerMethod( @@ -80,35 +82,12 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ } return { - VariableDeclarator(node) { - if (isRenderVariableDeclarator(node, ['render', ...renderFunctions])) { - if (isObjectPattern(node.id)) { - const containerIndex = node.id.properties.findIndex( - (property) => - isProperty(property) && - ASTUtils.isIdentifier(property.key) && - property.key.name === 'container' - ); - const nodeValue = - containerIndex !== -1 && node.id.properties[containerIndex].value; - if (ASTUtils.isIdentifier(nodeValue)) { - containerName = nodeValue.name; - } else { - isObjectPattern(nodeValue) && - nodeValue.properties.forEach( - (property) => - isProperty(property) && - ASTUtils.isIdentifier(property.key) && - destructuredContainerPropNames.push(property.key.name) - ); - } - } else { - renderWrapperName = ASTUtils.isIdentifier(node.id) && node.id.name; - } + CallExpression(node) { + const callExpressionIdentifier = getDeepestIdentifierNode(node); + if (helpers.isRenderUtil(callExpressionIdentifier)) { + detectRenderWrapper(callExpressionIdentifier); } - }, - CallExpression(node) { if (isMemberExpression(node.callee)) { showErrorIfChainedContainerMethod(node.callee); } else { @@ -120,6 +99,47 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ }); } }, + + VariableDeclarator(node) { + const initIdentifierNode = getDeepestIdentifierNode(node.init); + + const isRenderWrapperVariableDeclarator = initIdentifierNode + ? renderWrapperNames.includes(initIdentifierNode.name) + : false; + + if ( + !helpers.isRenderVariableDeclarator(node) && + !isRenderWrapperVariableDeclarator + ) { + return; + } + + if (isObjectPattern(node.id)) { + const containerIndex = node.id.properties.findIndex( + (property) => + isProperty(property) && + ASTUtils.isIdentifier(property.key) && + property.key.name === 'container' + ); + + const nodeValue = + containerIndex !== -1 && node.id.properties[containerIndex].value; + + if (ASTUtils.isIdentifier(nodeValue)) { + containerName = nodeValue.name; + } else { + isObjectPattern(nodeValue) && + nodeValue.properties.forEach( + (property) => + isProperty(property) && + ASTUtils.isIdentifier(property.key) && + destructuredContainerPropNames.push(property.key.name) + ); + } + } else { + renderResultVarName = ASTUtils.isIdentifier(node.id) && node.id.name; + } + }, }; }, }); diff --git a/lib/rules/no-debug.ts b/lib/rules/no-debug.ts index f03f0a33..3f5bb64e 100644 --- a/lib/rules/no-debug.ts +++ b/lib/rules/no-debug.ts @@ -1,5 +1,7 @@ import { getDeepestIdentifierNode, + getFunctionName, + getInnermostReturningFunction, getPropertyIdentifierNode, getReferenceNode, isObjectPattern, @@ -41,12 +43,28 @@ export default createTestingLibraryRule({ create(context, [], helpers) { const suspiciousDebugVariableNames: string[] = []; const suspiciousReferenceNodes: TSESTree.Identifier[] = []; + const renderWrapperNames: string[] = []; + + function detectRenderWrapper(node: TSESTree.Identifier): void { + const innerFunction = getInnermostReturningFunction(context, node); + + if (innerFunction) { + renderWrapperNames.push(getFunctionName(innerFunction)); + } + } return { VariableDeclarator(node) { const initIdentifierNode = getDeepestIdentifierNode(node.init); - if (!helpers.isRenderUtil(initIdentifierNode)) { + const isRenderWrapperVariableDeclarator = initIdentifierNode + ? renderWrapperNames.includes(initIdentifierNode.name) + : false; + + if ( + !helpers.isRenderVariableDeclarator(node) && + !isRenderWrapperVariableDeclarator + ) { return; } @@ -74,6 +92,10 @@ export default createTestingLibraryRule({ }, CallExpression(node) { const callExpressionIdentifier = getDeepestIdentifierNode(node); + if (helpers.isRenderUtil(callExpressionIdentifier)) { + detectRenderWrapper(callExpressionIdentifier); + } + const referenceNode = getReferenceNode(node); const referenceIdentifier = getPropertyIdentifierNode(referenceNode); diff --git a/lib/rules/render-result-naming-convention.ts b/lib/rules/render-result-naming-convention.ts index b322a89a..9fb7adbb 100644 --- a/lib/rules/render-result-naming-convention.ts +++ b/lib/rules/render-result-naming-convention.ts @@ -59,7 +59,7 @@ export default createTestingLibraryRule({ } if ( - !helpers.isRenderUtil(initIdentifierNode) && + !helpers.isRenderVariableDeclarator(node) && !renderWrapperNames.includes(initIdentifierNode.name) ) { return; diff --git a/tests/lib/rules/no-container.test.ts b/tests/lib/rules/no-container.test.ts index 4823e9a6..0c15d5f4 100644 --- a/tests/lib/rules/no-container.test.ts +++ b/tests/lib/rules/no-container.test.ts @@ -48,6 +48,25 @@ ruleTester.run(RULE_NAME, rule, { expect(firstChild).toBeDefined(); `, }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render as renamed } from '@testing-library/react' + import { render } from 'somewhere-else' + const { container } = render(); + const button = container.querySelector('.btn-primary'); + `, + }, + { + settings: { + 'testing-library/custom-renders': ['customRender', 'renderWithRedux'], + }, + code: ` + import { otherRender } from 'somewhere-else' + const { container } = otherRender(); + const button = container.querySelector('.btn-primary'); + `, + }, ], invalid: [ { @@ -57,6 +76,56 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 3, + column: 24, + messageId: 'noContainer', + }, + ], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render } from 'test-utils' + const { container } = render(); + const button = container.querySelector('.btn-primary'); + `, + errors: [ + { + line: 4, + column: 24, + messageId: 'noContainer', + }, + ], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render as testingRender } from '@testing-library/react' + const { container: renamed } = testingRender(); + const button = renamed.querySelector('.btn-primary'); + `, + errors: [ + { + line: 4, + column: 24, + messageId: 'noContainer', + }, + ], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render } from '@testing-library/react' + + const setup = () => render() + + const { container } = setup() + const button = container.querySelector('.btn-primary'); + `, + errors: [ + { + line: 7, + column: 24, messageId: 'noContainer', }, ], @@ -68,6 +137,8 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 3, + column: 9, messageId: 'noContainer', }, ], @@ -79,6 +150,8 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 3, + column: 9, messageId: 'noContainer', }, ], @@ -90,6 +163,8 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 3, + column: 29, messageId: 'noContainer', }, ], @@ -101,22 +176,39 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 3, + column: 9, messageId: 'noContainer', }, ], }, { + settings: { 'testing-library/utils-module': 'test-utils' }, code: ` - const { container } = renderWithRedux(); - container.querySelector(); + import { render } from '@testing-library/react' + const { container: { querySelector } } = render(); + querySelector('foo'); `, - options: [ + errors: [ { - renderFunctions: ['renderWithRedux'], + line: 4, + column: 9, + messageId: 'noContainer', }, ], + }, + { + settings: { + 'testing-library/custom-renders': ['customRender', 'renderWithRedux'], + }, + code: ` + const { container } = renderWithRedux(); + container.querySelector(); + `, errors: [ { + line: 3, + column: 9, messageId: 'noContainer', }, ], diff --git a/tests/lib/rules/no-debug.test.ts b/tests/lib/rules/no-debug.test.ts index 2de323df..d2e3f589 100644 --- a/tests/lib/rules/no-debug.test.ts +++ b/tests/lib/rules/no-debug.test.ts @@ -211,6 +211,24 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render } from 'test-utils' + + const setup = () => render() + + const utils = setup() + utils.debug() + `, + errors: [ + { + line: 7, + column: 15, + messageId: 'noDebug', + }, + ], + }, { settings: { 'testing-library/utils-module': 'test-utils' }, code: `// aggressive reporting disabled