From 013df69f31edc488ea61d874a63a4e69da466cf0 Mon Sep 17 00:00:00 2001 From: Gonzalo D'Elia Date: Tue, 20 Apr 2021 12:35:00 -0300 Subject: [PATCH 1/5] fix: allow console.debug() in no-debug --- lib/rules/no-debug.ts | 9 +++++++-- tests/lib/rules/no-debug.test.ts | 9 +++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/rules/no-debug.ts b/lib/rules/no-debug.ts index 5e2222e5..87bc1708 100644 --- a/lib/rules/no-debug.ts +++ b/lib/rules/no-debug.ts @@ -6,6 +6,7 @@ import { getReferenceNode, isObjectPattern, isProperty, + isMemberExpression, } from '../node-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; @@ -50,7 +51,7 @@ export default createTestingLibraryRule({ } const initIdentifierNode = getDeepestIdentifierNode(node.init); - if (!initIdentifierNode) { + if (!initIdentifierNode || initIdentifierNode.name === 'console') { return; } @@ -107,7 +108,11 @@ export default createTestingLibraryRule({ return; } - const isDebugUtil = helpers.isDebugUtil(callExpressionIdentifier); + const isDebugUtil = + helpers.isDebugUtil(callExpressionIdentifier) && + (!isMemberExpression(node.callee) || + !ASTUtils.isIdentifier(node.callee.object) || + node.callee.object.name !== 'console'); const isDeclaredDebugVariable = suspiciousDebugVariableNames.includes( callExpressionIdentifier.name ); diff --git a/tests/lib/rules/no-debug.test.ts b/tests/lib/rules/no-debug.test.ts index fda5faab..d7a8979d 100644 --- a/tests/lib/rules/no-debug.test.ts +++ b/tests/lib/rules/no-debug.test.ts @@ -54,6 +54,15 @@ ruleTester.run(RULE_NAME, rule, { settings: { 'testing-library/utils-module': 'test-utils' }, code: `screen.debug()`, }, + { + code: `console.debug()`, + }, + { + code: ` + const consoleDebug = console.debug + consoleDebug() + `, + }, { code: ` const { screen } = require('@testing-library/dom') From eb5748f4a475954648335e648bf422067517ee6c Mon Sep 17 00:00:00 2001 From: Gonzalo D'Elia Date: Tue, 20 Apr 2021 18:07:35 -0300 Subject: [PATCH 2/5] refactor: check for built-in console in isDebugUtil --- lib/detect-testing-library-utils.ts | 29 ++++++++++++++++++++--------- lib/rules/no-debug.ts | 7 +------ 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index a5faa373..5f28c018 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -78,7 +78,10 @@ type IsRenderUtilFn = (node: TSESTree.Identifier) => boolean; type IsRenderVariableDeclaratorFn = ( node: TSESTree.VariableDeclarator ) => boolean; -type IsDebugUtilFn = (node: TSESTree.Identifier) => boolean; +type IsDebugUtilFn = ( + identifierNode: TSESTree.Identifier, + callExpressionNode: TSESTree.CallExpression +) => boolean; type IsPresenceAssertFn = (node: TSESTree.MemberExpression) => boolean; type IsAbsenceAssertFn = (node: TSESTree.MemberExpression) => boolean; type CanReportErrorsFn = () => boolean; @@ -580,14 +583,22 @@ export function detectTestingLibraryUtils< return isRenderUtil(initIdentifierNode); }; - const isDebugUtil: IsDebugUtilFn = (node) => { - return isTestingLibraryUtil( - node, - (identifierNodeName, originalNodeName) => { - return [identifierNodeName, originalNodeName] - .filter(Boolean) - .includes('debug'); - } + const isDebugUtil: IsDebugUtilFn = (identifierNode, callExpressionNode) => { + const isBuiltInConsole = + isMemberExpression(callExpressionNode.callee) && + ASTUtils.isIdentifier(callExpressionNode.callee.object) && + callExpressionNode.callee.object.name === 'console'; + + return ( + !isBuiltInConsole && + isTestingLibraryUtil( + identifierNode, + (identifierNodeName, originalNodeName) => { + return [identifierNodeName, originalNodeName] + .filter(Boolean) + .includes('debug'); + } + ) ); }; diff --git a/lib/rules/no-debug.ts b/lib/rules/no-debug.ts index 87bc1708..b3665600 100644 --- a/lib/rules/no-debug.ts +++ b/lib/rules/no-debug.ts @@ -6,7 +6,6 @@ import { getReferenceNode, isObjectPattern, isProperty, - isMemberExpression, } from '../node-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; @@ -108,11 +107,7 @@ export default createTestingLibraryRule({ return; } - const isDebugUtil = - helpers.isDebugUtil(callExpressionIdentifier) && - (!isMemberExpression(node.callee) || - !ASTUtils.isIdentifier(node.callee.object) || - node.callee.object.name !== 'console'); + const isDebugUtil = helpers.isDebugUtil(callExpressionIdentifier, node); const isDeclaredDebugVariable = suspiciousDebugVariableNames.includes( callExpressionIdentifier.name ); From 268a598efa31af9b2b64f80644891c1f294f887b Mon Sep 17 00:00:00 2001 From: Gonzalo D'Elia Date: Tue, 20 Apr 2021 19:15:28 -0300 Subject: [PATCH 3/5] fix: support destructuring from console object --- lib/rules/no-debug.ts | 22 ++++++++++++++++++++-- tests/lib/rules/no-debug.test.ts | 12 ++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/rules/no-debug.ts b/lib/rules/no-debug.ts index b3665600..523d1747 100644 --- a/lib/rules/no-debug.ts +++ b/lib/rules/no-debug.ts @@ -34,6 +34,7 @@ export default createTestingLibraryRule({ const suspiciousDebugVariableNames: string[] = []; const suspiciousReferenceNodes: TSESTree.Identifier[] = []; const renderWrapperNames: string[] = []; + const builtInConsoleNodes: TSESTree.VariableDeclarator[] = []; function detectRenderWrapper(node: TSESTree.Identifier): void { const innerFunction = getInnermostReturningFunction(context, node); @@ -50,7 +51,12 @@ export default createTestingLibraryRule({ } const initIdentifierNode = getDeepestIdentifierNode(node.init); - if (!initIdentifierNode || initIdentifierNode.name === 'console') { + if (!initIdentifierNode) { + return; + } + + if (initIdentifierNode.name === 'console') { + builtInConsoleNodes.push(node); return; } @@ -120,7 +126,19 @@ export default createTestingLibraryRule({ } ); - if (isDebugUtil || isDeclaredDebugVariable || isChainedReferenceDebug) { + const isVariableFromBuiltInConsole = builtInConsoleNodes.some( + (variableDeclarator) => { + const variables = context.getDeclaredVariables(variableDeclarator); + return variables.some( + ({ name }) => name === callExpressionIdentifier.name + ); + } + ); + + if ( + !isVariableFromBuiltInConsole && + (isDebugUtil || isDeclaredDebugVariable || isChainedReferenceDebug) + ) { context.report({ node: callExpressionIdentifier, messageId: 'noDebug', diff --git a/tests/lib/rules/no-debug.test.ts b/tests/lib/rules/no-debug.test.ts index d7a8979d..bbaa67b0 100644 --- a/tests/lib/rules/no-debug.test.ts +++ b/tests/lib/rules/no-debug.test.ts @@ -63,6 +63,18 @@ ruleTester.run(RULE_NAME, rule, { consoleDebug() `, }, + { + code: ` + const { debug } = console + debug() + `, + }, + { + code: ` + const { debug: consoleDebug } = console + consoleDebug() + `, + }, { code: ` const { screen } = require('@testing-library/dom') From 1778fa8ed6771e4869acbe0de254e1435cecab2a Mon Sep 17 00:00:00 2001 From: Gonzalo D'Elia Date: Wed, 21 Apr 2021 15:25:47 -0300 Subject: [PATCH 4/5] fix: no-debug check correctly for declared variable --- lib/detect-testing-library-utils.ts | 17 +++++++++-------- lib/rules/no-debug.ts | 7 +++++-- tests/lib/rules/no-debug.test.ts | 18 ++++++++++++++++++ 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 5f28c018..2e21229a 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -16,6 +16,7 @@ import { isImportNamespaceSpecifier, isImportSpecifier, isLiteral, + isCallExpression, isMemberExpression, isObjectPattern, isProperty, @@ -78,10 +79,7 @@ type IsRenderUtilFn = (node: TSESTree.Identifier) => boolean; type IsRenderVariableDeclaratorFn = ( node: TSESTree.VariableDeclarator ) => boolean; -type IsDebugUtilFn = ( - identifierNode: TSESTree.Identifier, - callExpressionNode: TSESTree.CallExpression -) => boolean; +type IsDebugUtilFn = (identifierNode: TSESTree.Identifier) => boolean; type IsPresenceAssertFn = (node: TSESTree.MemberExpression) => boolean; type IsAbsenceAssertFn = (node: TSESTree.MemberExpression) => boolean; type CanReportErrorsFn = () => boolean; @@ -583,11 +581,14 @@ export function detectTestingLibraryUtils< return isRenderUtil(initIdentifierNode); }; - const isDebugUtil: IsDebugUtilFn = (identifierNode, callExpressionNode) => { + const isDebugUtil: IsDebugUtilFn = (identifierNode) => { const isBuiltInConsole = - isMemberExpression(callExpressionNode.callee) && - ASTUtils.isIdentifier(callExpressionNode.callee.object) && - callExpressionNode.callee.object.name === 'console'; + isMemberExpression(identifierNode.parent) && + identifierNode.parent.parent && + isCallExpression(identifierNode.parent.parent) && + isMemberExpression(identifierNode.parent.parent.callee) && + ASTUtils.isIdentifier(identifierNode.parent.parent.callee.object) && + identifierNode.parent.parent.callee.object.name === 'console'; return ( !isBuiltInConsole && diff --git a/lib/rules/no-debug.ts b/lib/rules/no-debug.ts index 523d1747..71e0f8f1 100644 --- a/lib/rules/no-debug.ts +++ b/lib/rules/no-debug.ts @@ -4,6 +4,7 @@ import { getInnermostReturningFunction, getPropertyIdentifierNode, getReferenceNode, + isCallExpression, isObjectPattern, isProperty, } from '../node-utils'; @@ -113,7 +114,7 @@ export default createTestingLibraryRule({ return; } - const isDebugUtil = helpers.isDebugUtil(callExpressionIdentifier, node); + const isDebugUtil = helpers.isDebugUtil(callExpressionIdentifier); const isDeclaredDebugVariable = suspiciousDebugVariableNames.includes( callExpressionIdentifier.name ); @@ -130,7 +131,9 @@ export default createTestingLibraryRule({ (variableDeclarator) => { const variables = context.getDeclaredVariables(variableDeclarator); return variables.some( - ({ name }) => name === callExpressionIdentifier.name + ({ name }) => + name === callExpressionIdentifier.name && + isCallExpression(callExpressionIdentifier.parent) ); } ); diff --git a/tests/lib/rules/no-debug.test.ts b/tests/lib/rules/no-debug.test.ts index bbaa67b0..7606b915 100644 --- a/tests/lib/rules/no-debug.test.ts +++ b/tests/lib/rules/no-debug.test.ts @@ -537,5 +537,23 @@ ruleTester.run(RULE_NAME, rule, { `, 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) + const { debug: renamedDestructuredDebug } = console + const { debug } = console + const assignedDebug = console.debug + console.debug('debugging') + debug('destructured') + assignedDebug('foo') + // the following line is the one that fails + utils.debug() + renamedDestructuredDebug('foo') + `, + errors: [{ line: 12, column: 13, messageId: 'noDebug' }], + }, ], }); From c89f34c575c415a68156348f6881a980a979785c Mon Sep 17 00:00:00 2001 From: Gonzalo D'Elia Date: Wed, 21 Apr 2021 16:42:06 -0300 Subject: [PATCH 5/5] refactor: simplify code --- lib/detect-testing-library-utils.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 2e21229a..0c81fa6e 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -16,7 +16,6 @@ import { isImportNamespaceSpecifier, isImportSpecifier, isLiteral, - isCallExpression, isMemberExpression, isObjectPattern, isProperty, @@ -584,11 +583,8 @@ export function detectTestingLibraryUtils< const isDebugUtil: IsDebugUtilFn = (identifierNode) => { const isBuiltInConsole = isMemberExpression(identifierNode.parent) && - identifierNode.parent.parent && - isCallExpression(identifierNode.parent.parent) && - isMemberExpression(identifierNode.parent.parent.callee) && - ASTUtils.isIdentifier(identifierNode.parent.parent.callee.object) && - identifierNode.parent.parent.callee.object.name === 'console'; + ASTUtils.isIdentifier(identifierNode.parent.object) && + identifierNode.parent.object.name === 'console'; return ( !isBuiltInConsole &&