diff --git a/docs/rules/no-debug.md b/docs/rules/no-debug.md index ef848d85..e3278738 100644 --- a/docs/rules/no-debug.md +++ b/docs/rules/no-debug.md @@ -1,6 +1,6 @@ # Disallow the use of `debug` (no-debug) -Just like `console.log` statements pollutes the browser's output, debug statements also pollutes the tests if one of your team mates forgot to remove it. `debug` statements should be used when you actually want to debug your tests but should not be pushed to the codebase. +Just like `console.log` statements pollutes the browser's output, debug statements also pollutes the tests if one of your teammates forgot to remove it. `debug` statements should be used when you actually want to debug your tests but should not be pushed to the codebase. ## Rule Details @@ -28,12 +28,6 @@ const { screen } = require('@testing-library/react'); screen.debug(); ``` -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-debug": ["error", {"renderFunctions":["renderWithRedux", "renderWithRouter"]}], -``` - ## Further Reading - [debug API in React Testing Library](https://testing-library.com/docs/react-testing-library/api#debug) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 7559ae8f..316accf7 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -69,6 +69,7 @@ type IsAsyncUtilFn = ( ) => boolean; type IsFireEventMethodFn = (node: TSESTree.Identifier) => boolean; type IsRenderUtilFn = (node: TSESTree.Identifier) => boolean; +type IsDebugUtilFn = (node: TSESTree.Identifier) => boolean; type IsPresenceAssertFn = (node: TSESTree.MemberExpression) => boolean; type IsAbsenceAssertFn = (node: TSESTree.MemberExpression) => boolean; type CanReportErrorsFn = () => boolean; @@ -96,6 +97,7 @@ export interface DetectionHelpers { isAsyncUtil: IsAsyncUtilFn; isFireEventMethod: IsFireEventMethodFn; isRenderUtil: IsRenderUtilFn; + isDebugUtil: IsDebugUtilFn; isPresenceAssert: IsPresenceAssertFn; isAbsenceAssert: IsAbsenceAssertFn; canReportErrors: CanReportErrorsFn; @@ -406,6 +408,17 @@ export function detectTestingLibraryUtils< ); }; + const isDebugUtil: IsDebugUtilFn = (node) => { + return isTestingLibraryUtil( + node, + (identifierNodeName, originalNodeName) => { + return [identifierNodeName, originalNodeName] + .filter(Boolean) + .includes('debug'); + } + ); + }; + /** * Determines whether a given MemberExpression node is a presence assert * @@ -549,6 +562,7 @@ export function detectTestingLibraryUtils< isAsyncUtil, isFireEventMethod, isRenderUtil, + isDebugUtil, isPresenceAssert, isAbsenceAssert, canReportErrors, diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 4aba63cd..c6a76253 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -392,7 +392,7 @@ export function getPropertyIdentifierNode( } /** - * Gets the deepest identifier node from a given node. + * Gets the deepest identifier node in the expression from a given node. * * Opposite of {@link getReferenceNode} * @@ -416,11 +416,15 @@ export function getDeepestIdentifierNode( return getDeepestIdentifierNode(node.callee); } + if (ASTUtils.isAwaitExpression(node)) { + return getDeepestIdentifierNode(node.argument); + } + return null; } /** - * Gets the farthest node from a given node. + * Gets the farthest node in the expression from a given node. * * Opposite of {@link getDeepestIdentifierNode} diff --git a/lib/rules/no-debug.ts b/lib/rules/no-debug.ts index e2023e91..f03f0a33 100644 --- a/lib/rules/no-debug.ts +++ b/lib/rules/no-debug.ts @@ -1,28 +1,18 @@ import { - ESLintUtils, - TSESTree, - ASTUtils, -} from '@typescript-eslint/experimental-utils'; -import { - getDocsUrl, - LIBRARY_MODULES, - hasTestingLibraryImportModule, -} from '../utils'; -import { + getDeepestIdentifierNode, + getPropertyIdentifierNode, + getReferenceNode, isObjectPattern, isProperty, - isCallExpression, - isLiteral, - isMemberExpression, - isImportSpecifier, - isRenderVariableDeclarator, } from '../node-utils'; +import { createTestingLibraryRule } from '../create-testing-library-rule'; +import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; export const RULE_NAME = 'no-debug'; export type MessageIds = 'noDebug'; -type Options = [{ renderFunctions?: string[] }]; +type Options = []; -export default ESLintUtils.RuleCreator(getDocsUrl)({ +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'problem', @@ -46,154 +36,67 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ }, ], }, - defaultOptions: [ - { - renderFunctions: [], - }, - ], - - create(context, [options]) { - let hasDestructuredDebugStatement = false; - const renderVariableDeclarators: TSESTree.VariableDeclarator[] = []; - - const { renderFunctions } = options; + defaultOptions: [], - let hasImportedScreen = false; - let wildcardImportName: string = null; + create(context, [], helpers) { + const suspiciousDebugVariableNames: string[] = []; + const suspiciousReferenceNodes: TSESTree.Identifier[] = []; return { VariableDeclarator(node) { - if (isRenderVariableDeclarator(node, ['render', ...renderFunctions])) { - if ( - isObjectPattern(node.id) && - node.id.properties.some( - (property) => - isProperty(property) && - ASTUtils.isIdentifier(property.key) && - property.key.name === 'debug' - ) - ) { - hasDestructuredDebugStatement = true; - } - - if (node.id.type === 'Identifier') { - renderVariableDeclarators.push(node); - } - } - }, - [`VariableDeclarator > CallExpression > Identifier[name="require"]`]( - node: TSESTree.Identifier - ) { - const { arguments: args } = node.parent as TSESTree.CallExpression; + const initIdentifierNode = getDeepestIdentifierNode(node.init); - const literalNodeScreenModuleName = args.find( - (args) => - isLiteral(args) && - typeof args.value === 'string' && - LIBRARY_MODULES.includes(args.value) - ); - - if (!literalNodeScreenModuleName) { + if (!helpers.isRenderUtil(initIdentifierNode)) { return; } - const declaratorNode = node.parent - .parent as TSESTree.VariableDeclarator; - - hasImportedScreen = - isObjectPattern(declaratorNode.id) && - declaratorNode.id.properties.some( - (property) => + // find debug obtained from render and save their name, like: + // const { debug } = render(); + if (isObjectPattern(node.id)) { + for (const property of node.id.properties) { + if ( isProperty(property) && ASTUtils.isIdentifier(property.key) && - property.key.name === 'screen' - ); - }, - // checks if import has shape: - // import { screen } from '@testing-library/dom'; - ImportDeclaration(node: TSESTree.ImportDeclaration) { - if (!hasTestingLibraryImportModule(node)) { - return; - } - - hasImportedScreen = node.specifiers.some( - (s) => isImportSpecifier(s) && s.imported.name === 'screen' - ); - }, - // checks if import has shape: - // import * as dtl from '@testing-library/dom'; - 'ImportDeclaration ImportNamespaceSpecifier'( - node: TSESTree.ImportNamespaceSpecifier - ) { - const importDeclarationNode = node.parent as TSESTree.ImportDeclaration; - if (!hasTestingLibraryImportModule(importDeclarationNode)) { - return; + property.key.name === 'debug' + ) { + suspiciousDebugVariableNames.push( + getDeepestIdentifierNode(property.value).name + ); + } + } } - wildcardImportName = node.local && node.local.name; - }, - [`CallExpression > Identifier[name="debug"]`](node: TSESTree.Identifier) { - if (hasDestructuredDebugStatement) { - context.report({ - node, - messageId: 'noDebug', - }); + // find utils kept from render and save their node, like: + // const utils = render(); + if (ASTUtils.isIdentifier(node.id)) { + suspiciousReferenceNodes.push(node.id); } }, - [`CallExpression > MemberExpression > Identifier[name="debug"]`]( - node: TSESTree.Identifier - ) { - const memberExpression = node.parent as TSESTree.MemberExpression; - const identifier = memberExpression.object as TSESTree.Identifier; - const memberExpressionName = identifier.name; - /* - check if `debug` used following the pattern: - - import { screen } from '@testing-library/dom'; - ... - screen.debug(); - */ - const isScreenDebugUsed = - hasImportedScreen && memberExpressionName === 'screen'; - - /* - check if `debug` used following the pattern: - - import * as dtl from '@testing-library/dom'; - ... - dtl.debug(); - */ - const isNamespaceDebugUsed = - wildcardImportName && memberExpressionName === wildcardImportName; + CallExpression(node) { + const callExpressionIdentifier = getDeepestIdentifierNode(node); + const referenceNode = getReferenceNode(node); + const referenceIdentifier = getPropertyIdentifierNode(referenceNode); + + const isDebugUtil = helpers.isDebugUtil(callExpressionIdentifier); + const isDeclaredDebugVariable = suspiciousDebugVariableNames.includes( + callExpressionIdentifier.name + ); + const isChainedReferenceDebug = suspiciousReferenceNodes.some( + (suspiciousReferenceIdentifier) => { + return ( + callExpressionIdentifier.name === 'debug' && + suspiciousReferenceIdentifier.name === referenceIdentifier.name + ); + } + ); - if (isScreenDebugUsed || isNamespaceDebugUsed) { + if (isDebugUtil || isDeclaredDebugVariable || isChainedReferenceDebug) { context.report({ - node, + node: callExpressionIdentifier, messageId: 'noDebug', }); } }, - 'Program:exit'() { - renderVariableDeclarators.forEach((renderVar) => { - const renderVarReferences = context - .getDeclaredVariables(renderVar)[0] - .references.slice(1); - renderVarReferences.forEach((ref) => { - const parent = ref.identifier.parent; - if ( - isMemberExpression(parent) && - ASTUtils.isIdentifier(parent.property) && - parent.property.name === 'debug' && - isCallExpression(parent.parent) - ) { - context.report({ - node: parent.property, - messageId: 'noDebug', - }); - } - }); - }); - }, }; }, }); diff --git a/tests/lib/rules/no-debug.test.ts b/tests/lib/rules/no-debug.test.ts index 3e26b414..2de323df 100644 --- a/tests/lib/rules/no-debug.test.ts +++ b/tests/lib/rules/no-debug.test.ts @@ -6,9 +6,18 @@ const ruleTester = createRuleTester(); ruleTester.run(RULE_NAME, rule, { valid: [ { + settings: { 'testing-library/utils-module': 'test-utils' }, code: `debug()`, }, { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { screen } from 'somewhere-else' + screen.debug() + `, + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, code: `() => { const somethingElse = {} const { debug } = foo() @@ -16,6 +25,7 @@ ruleTester.run(RULE_NAME, rule, { }`, }, { + settings: { 'testing-library/utils-module': 'test-utils' }, code: ` let foo const debug = require('debug') @@ -41,6 +51,7 @@ ruleTester.run(RULE_NAME, rule, { `, }, { + settings: { 'testing-library/utils-module': 'test-utils' }, code: `screen.debug()`, }, { @@ -64,6 +75,7 @@ ruleTester.run(RULE_NAME, rule, { `, }, { + settings: { 'testing-library/utils-module': 'test-utils' }, code: ` import * as foo from '@somewhere/else'; foo.debug(); @@ -73,12 +85,14 @@ ruleTester.run(RULE_NAME, rule, { code: `import { queries } from '@testing-library/dom'`, }, { + settings: { 'testing-library/utils-module': 'test-utils' }, code: ` const { screen } = require('something-else') screen.debug() `, }, { + settings: { 'testing-library/utils-module': 'test-utils' }, code: ` import { screen } from 'something-else' screen.debug() @@ -91,9 +105,70 @@ ruleTester.run(RULE_NAME, rule, { } `, }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { debug as testingDebug } from 'test-utils' + import { debug } from 'somewhere-else' + + debug() + `, + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render as testingRender } from '@testing-library/react' + import { render } from 'somewhere-else' + + const { debug } = render(element) + + somethingElse() + debug() + `, + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render as testingRender } from '@testing-library/react' + import { render } from 'somewhere-else' + + const { debug } = render(element) + const { debug: testingDebug } = testingRender(element) + + somethingElse() + debug() + `, + }, ], invalid: [ + { + code: `debug()`, + errors: [{ line: 1, column: 1, messageId: 'noDebug' }], + }, + { + code: ` + import { screen } from 'aggressive-reporting' + screen.debug() + `, + errors: [{ line: 3, column: 14, messageId: 'noDebug' }], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { screen } from 'test-utils' + screen.debug() + `, + errors: [{ line: 3, column: 14, messageId: 'noDebug' }], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { debug as testingDebug } from 'test-utils' + testingDebug() + `, + errors: [{ line: 3, column: 7, messageId: 'noDebug' }], + }, { code: ` const { debug } = render() @@ -101,33 +176,52 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 3, + column: 9, messageId: 'noDebug', }, ], }, { + settings: { + 'testing-library/custom-renders': ['customRender', 'renderWithRedux'], + }, code: ` const { debug } = renderWithRedux() debug() `, - options: [ + errors: [ { - renderFunctions: ['renderWithRedux'], + line: 3, + column: 9, + messageId: 'noDebug', }, ], + }, + { + code: ` + const utils = render() + utils.debug() + `, errors: [ { + line: 3, + column: 15, messageId: 'noDebug', }, ], }, { - code: ` + settings: { 'testing-library/utils-module': 'test-utils' }, + code: `// aggressive reporting disabled + import { render } from 'test-utils' const utils = render() utils.debug() `, errors: [ { + line: 4, + column: 15, messageId: 'noDebug', }, ], @@ -141,9 +235,35 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 3, + column: 15, messageId: 'noDebug', }, { + line: 5, + column: 15, + messageId: 'noDebug', + }, + ], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: `// aggressive reporting disabled + import { render } from 'test-utils' + const utils = render() + utils.debug() + utils.foo() + utils.debug() + `, + errors: [ + { + line: 4, + column: 15, + messageId: 'noDebug', + }, + { + line: 6, + column: 15, messageId: 'noDebug', }, ], @@ -158,6 +278,26 @@ ruleTester.run(RULE_NAME, rule, { })`, errors: [ { + line: 5, + column: 11, + messageId: 'noDebug', + }, + ], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: `// aggressive reporting disabled + import { render } from 'test-utils' + describe(() => { + test(async () => { + const { debug } = await render("foo") + debug() + }) + })`, + errors: [ + { + line: 6, + column: 11, messageId: 'noDebug', }, ], @@ -172,6 +312,26 @@ ruleTester.run(RULE_NAME, rule, { })`, errors: [ { + line: 5, + column: 17, + messageId: 'noDebug', + }, + ], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: `// aggressive reporting disabled + import { render } from 'test-utils' + describe(() => { + test(async () => { + const utils = await render("foo") + utils.debug() + }) + })`, + errors: [ + { + line: 6, + column: 17, messageId: 'noDebug', }, ], @@ -183,6 +343,22 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 3, + column: 16, + messageId: 'noDebug', + }, + ], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: `// aggressive reporting disabled + const { screen } = require('@testing-library/dom') + screen.debug() + `, + errors: [ + { + line: 3, + column: 16, messageId: 'noDebug', }, ], @@ -194,6 +370,22 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 3, + column: 16, + messageId: 'noDebug', + }, + ], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: `// aggressive reporting disabled + import { screen } from '@testing-library/dom' + screen.debug() + `, + errors: [ + { + line: 3, + column: 16, messageId: 'noDebug', }, ], @@ -206,11 +398,28 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 3, + column: 16, + messageId: 'noDebug', + }, + ], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: `// aggressive reporting disabled + import { screen, render } from '@testing-library/dom' + screen.debug() + `, + errors: [ + { + line: 3, + column: 16, messageId: 'noDebug', }, ], }, { + settings: { 'testing-library/utils-module': 'test-utils' }, code: ` import * as dtl from '@testing-library/dom'; dtl.debug(); @@ -223,5 +432,67 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + { + code: ` + import { render } from 'aggressive-reporting' + + const { debug } = render(element) + + somethingElse() + debug() + `, + errors: [{ line: 7, column: 7, messageId: 'noDebug' }], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render } from '@testing-library/react' + + const { debug } = render(element) + + somethingElse() + debug() + `, + errors: [{ line: 7, column: 7, messageId: 'noDebug' }], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render } from 'test-utils' + + const { debug: renamed } = render(element) + + somethingElse() + renamed() + `, + errors: [{ line: 7, column: 7, messageId: 'noDebug' }], + }, + { + settings: { 'testing-library/utils-module': 'test-utils' }, + code: ` + import { render } from '@testing-library/react' + + const utils = render(element) + + somethingElse() + utils.debug() + `, + errors: [{ line: 7, column: 13, messageId: 'noDebug' }], + }, + { + settings: { + 'testing-library/utils-module': 'test-utils', + 'testing-library/custom-renders': ['testingRender'], + }, + code: `// aggressive reporting disabled, custom render set + import { testingRender } from 'test-utils' + + const { debug: renamedDebug } = testingRender(element) + + somethingElse() + renamedDebug() + `, + errors: [{ line: 7, column: 7, messageId: 'noDebug' }], + }, ], });