diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 633c0241..7ca83d1e 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -1,5 +1,5 @@ import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; -import { isLiteral } from './node-utils'; +import { getImportModuleName, isLiteral, ImportModuleNode } from './node-utils'; export type TestingLibrarySettings = { 'testing-library/module'?: string; @@ -25,14 +25,11 @@ export type EnhancedRuleCreate< detectionHelpers: Readonly ) => TRuleListener; -type ModuleImportation = - | TSESTree.ImportDeclaration - | TSESTree.CallExpression - | null; - export type DetectionHelpers = { - getTestingLibraryImportNode: () => ModuleImportation; - getCustomModuleImportNode: () => ModuleImportation; + getTestingLibraryImportNode: () => ImportModuleNode | null; + getCustomModuleImportNode: () => ImportModuleNode | null; + getTestingLibraryImportName: () => string | undefined; + getCustomModuleImportName: () => string | undefined; getIsTestingLibraryImported: () => boolean; getIsValidFilename: () => boolean; canReportErrors: () => boolean; @@ -52,8 +49,8 @@ export function detectTestingLibraryUtils< context: TestingLibraryContext, optionsWithDefault: Readonly ): TSESLint.RuleListener => { - let importedTestingLibraryNode: ModuleImportation = null; - let importedCustomModuleNode: ModuleImportation = null; + let importedTestingLibraryNode: ImportModuleNode | null = null; + let importedCustomModuleNode: ImportModuleNode | null = null; // Init options based on shared ESLint settings const customModule = context.settings['testing-library/module']; @@ -69,6 +66,12 @@ export function detectTestingLibraryUtils< getCustomModuleImportNode() { return importedCustomModuleNode; }, + getTestingLibraryImportName() { + return getImportModuleName(importedTestingLibraryNode); + }, + getCustomModuleImportName() { + return getImportModuleName(importedCustomModuleNode); + }, /** * Gets if Testing Library is considered as imported or not. * diff --git a/lib/node-utils.ts b/lib/node-utils.ts index a4376732..a7c90332 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -6,9 +6,9 @@ import { import { RuleContext } from '@typescript-eslint/experimental-utils/dist/ts-eslint'; export function isCallExpression( - node: TSESTree.Node + node: TSESTree.Node | null | undefined ): node is TSESTree.CallExpression { - return node && node.type === AST_NODE_TYPES.CallExpression; + return node?.type === AST_NODE_TYPES.CallExpression; } export function isNewExpression( @@ -17,6 +17,7 @@ export function isNewExpression( return node && node.type === 'NewExpression'; } +// TODO: remove this one and use ASTUtils one instead export function isIdentifier(node: TSESTree.Node): node is TSESTree.Identifier { return node && node.type === AST_NODE_TYPES.Identifier; } @@ -69,8 +70,10 @@ export function isObjectPattern( return node && node.type === AST_NODE_TYPES.ObjectPattern; } -export function isProperty(node: TSESTree.Node): node is TSESTree.Property { - return node && node.type === AST_NODE_TYPES.Property; +export function isProperty( + node: TSESTree.Node | null | undefined +): node is TSESTree.Property { + return node?.type === AST_NODE_TYPES.Property; } export function isJSXAttribute( @@ -126,6 +129,7 @@ export function hasThenProperty(node: TSESTree.Node): boolean { ); } +// TODO: remove this one and use ASTUtils one instead export function isAwaitExpression( node: TSESTree.Node ): node is TSESTree.AwaitExpression { @@ -150,6 +154,12 @@ export function isArrayExpression( return node?.type === AST_NODE_TYPES.ArrayExpression; } +export function isImportDeclaration( + node: TSESTree.Node | null | undefined +): node is TSESTree.ImportDeclaration { + return node?.type === AST_NODE_TYPES.ImportDeclaration; +} + export function isAwaited(node: TSESTree.Node): boolean { return ( isAwaitExpression(node) || @@ -176,7 +186,7 @@ export function getVariableReferences( ): TSESLint.Scope.Reference[] { return ( (isVariableDeclarator(node) && - context.getDeclaredVariables(node)[0].references.slice(1)) || + context.getDeclaredVariables(node)[0]?.references?.slice(1)) || [] ); } @@ -220,3 +230,26 @@ export function isRenderVariableDeclarator( return false; } + +// TODO: extract into types file? +export type ImportModuleNode = + | TSESTree.ImportDeclaration + | TSESTree.CallExpression; + +export function getImportModuleName( + node: ImportModuleNode | undefined | null +): string | undefined { + // import node of shape: import { foo } from 'bar' + if (isImportDeclaration(node) && typeof node.source.value === 'string') { + return node.source.value; + } + + // import node of shape: const { foo } = require('bar') + if ( + isCallExpression(node) && + isLiteral(node.arguments[0]) && + typeof node.arguments[0].value === 'string' + ) { + return node.arguments[0].value; + } +} diff --git a/lib/rules/no-container.ts b/lib/rules/no-container.ts index b2d86c43..89000731 100644 --- a/lib/rules/no-container.ts +++ b/lib/rules/no-container.ts @@ -105,7 +105,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ } }, - CallExpression(node: TSESTree.CallExpression) { + CallExpression(node) { if (isMemberExpression(node.callee)) { showErrorIfChainedContainerMethod(node.callee); } else { diff --git a/lib/rules/no-dom-import.ts b/lib/rules/no-dom-import.ts index 80db79aa..1246447d 100644 --- a/lib/rules/no-dom-import.ts +++ b/lib/rules/no-dom-import.ts @@ -1,9 +1,6 @@ -import { - AST_NODE_TYPES, - TSESTree, -} from '@typescript-eslint/experimental-utils'; -import { isIdentifier, isLiteral } from '../node-utils'; +import { TSESTree } from '@typescript-eslint/experimental-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; +import { isCallExpression } from '../node-utils'; export const RULE_NAME = 'no-dom-import'; export type MessageIds = 'noDomImport' | 'noDomImportFramework'; @@ -40,11 +37,10 @@ export default createTestingLibraryRule({ create(context, [framework], helpers) { function report( - node: TSESTree.ImportDeclaration | TSESTree.Identifier, + node: TSESTree.ImportDeclaration | TSESTree.CallExpression, moduleName: string ) { if (framework) { - const isRequire = isIdentifier(node) && node.name === 'require'; const correctModuleName = moduleName.replace('dom', framework); context.report({ node, @@ -53,9 +49,8 @@ export default createTestingLibraryRule({ module: correctModuleName, }, fix(fixer) { - if (isRequire) { - const callExpression = node.parent as TSESTree.CallExpression; - const name = callExpression.arguments[0] as TSESTree.Literal; + if (isCallExpression(node)) { + const name = node.arguments[0] as TSESTree.Literal; // Replace the module name with the raw module name as we can't predict which punctuation the user is going to use return fixer.replaceText( @@ -63,8 +58,7 @@ export default createTestingLibraryRule({ name.raw.replace(moduleName, correctModuleName) ); } else { - const importDeclaration = node as TSESTree.ImportDeclaration; - const name = importDeclaration.source; + const name = node.source; return fixer.replaceText( name, name.raw.replace(moduleName, correctModuleName) @@ -82,36 +76,22 @@ export default createTestingLibraryRule({ return { 'Program:exit'() { + const importName = helpers.getTestingLibraryImportName(); const importNode = helpers.getTestingLibraryImportNode(); if (!importNode) { return; } - // import node of shape: import { foo } from 'bar' - if (importNode.type === AST_NODE_TYPES.ImportDeclaration) { - const domModuleName = DOM_TESTING_LIBRARY_MODULES.find( - (module) => module === importNode.source.value - ); + const domModuleName = DOM_TESTING_LIBRARY_MODULES.find( + (module) => module === importName + ); - domModuleName && report(importNode, domModuleName); + if (!domModuleName) { + return; } - // import node of shape: const { foo } = require('bar') - if (importNode.type === AST_NODE_TYPES.CallExpression) { - const literalNodeDomModuleName = importNode.arguments.find( - (arg) => - isLiteral(arg) && - typeof arg.value === 'string' && - DOM_TESTING_LIBRARY_MODULES.includes(arg.value) - ) as TSESTree.Literal; - - literalNodeDomModuleName && - report( - importNode.callee as TSESTree.Identifier, - literalNodeDomModuleName.value as string - ); - } + report(importNode, domModuleName); }, }; }, diff --git a/lib/rules/no-manual-cleanup.ts b/lib/rules/no-manual-cleanup.ts index 81bdab6c..841cfc47 100644 --- a/lib/rules/no-manual-cleanup.ts +++ b/lib/rules/no-manual-cleanup.ts @@ -1,22 +1,27 @@ -import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; -import { getDocsUrl } from '../utils'; import { + ASTUtils, + TSESTree, + TSESLint, +} from '@typescript-eslint/experimental-utils'; +import { + getVariableReferences, isImportDefaultSpecifier, - isLiteral, - isIdentifier, + isImportSpecifier, + isMemberExpression, isObjectPattern, isProperty, - isMemberExpression, - isImportSpecifier, + ImportModuleNode, + isImportDeclaration, } from '../node-utils'; +import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'no-manual-cleanup'; export type MessageIds = 'noManualCleanup'; type Options = []; -const CLEANUP_LIBRARY_REGEX = /(@testing-library\/(preact|react|svelte|vue))|@marko\/testing-library/; +const CLEANUP_LIBRARY_REGEXP = /(@testing-library\/(preact|react|svelte|vue))|@marko\/testing-library/; -export default ESLintUtils.RuleCreator(getDocsUrl)({ +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'problem', @@ -34,50 +39,36 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ }, defaultOptions: [], - create(context) { - let defaultImportFromTestingLibrary: TSESTree.ImportDeclaration; - let defaultRequireFromTestingLibrary: - | TSESTree.Identifier - | TSESTree.ArrayPattern; - - // can't find the right type? - // eslint-disable-next-line @typescript-eslint/no-explicit-any - function reportImportReferences(references: any[]) { - if (references && references.length > 0) { - references.forEach((reference) => { - const utilsUsage = reference.identifier.parent; - if ( - isMemberExpression(utilsUsage) && - isIdentifier(utilsUsage.property) && - utilsUsage.property.name === 'cleanup' - ) { - context.report({ - node: utilsUsage.property, - messageId: 'noManualCleanup', - }); - } - }); - } + create(context, _, helpers) { + function reportImportReferences(references: TSESLint.Scope.Reference[]) { + references.forEach((reference) => { + const utilsUsage = reference.identifier.parent; + if ( + isMemberExpression(utilsUsage) && + ASTUtils.isIdentifier(utilsUsage.property) && + utilsUsage.property.name === 'cleanup' + ) { + context.report({ + node: utilsUsage.property, + messageId: 'noManualCleanup', + }); + } + }); } - return { - ImportDeclaration(node) { - const value = node.source.value as string; - const testingLibraryWithCleanup = value.match(CLEANUP_LIBRARY_REGEX); - - // Early return if the library doesn't support `cleanup` - if (!testingLibraryWithCleanup) { - return; - } + function reportCandidateModule(moduleNode: ImportModuleNode) { + if (isImportDeclaration(moduleNode)) { + // case: import utils from 'testing-library-module' + if (isImportDefaultSpecifier(moduleNode.specifiers[0])) { + const { references } = context.getDeclaredVariables(moduleNode)[0]; - if (isImportDefaultSpecifier(node.specifiers[0])) { - defaultImportFromTestingLibrary = node; + reportImportReferences(references); } - const cleanupSpecifier = node.specifiers.find( + // case: import { cleanup } from 'testing-library-module' + const cleanupSpecifier = moduleNode.specifiers.find( (specifier) => isImportSpecifier(specifier) && - specifier.imported && specifier.imported.name === 'cleanup' ); @@ -87,31 +78,15 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ messageId: 'noManualCleanup', }); } - }, - [`VariableDeclarator > CallExpression > Identifier[name="require"]`]( - node: TSESTree.Identifier - ) { - const { arguments: args } = node.parent as TSESTree.CallExpression; - - const literalNodeCleanupModuleName = args.find( - (args) => - isLiteral(args) && - typeof args.value === 'string' && - args.value.match(CLEANUP_LIBRARY_REGEX) - ); - - if (!literalNodeCleanupModuleName) { - return; - } - - const declaratorNode = node.parent - .parent as TSESTree.VariableDeclarator; + } else { + const declaratorNode = moduleNode.parent as TSESTree.VariableDeclarator; if (isObjectPattern(declaratorNode.id)) { + // case: const { cleanup } = require('testing-library-module') const cleanupProperty = declaratorNode.id.properties.find( (property) => isProperty(property) && - isIdentifier(property.key) && + ASTUtils.isIdentifier(property.key) && property.key.name === 'cleanup' ); @@ -122,24 +97,29 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ }); } } else { - defaultRequireFromTestingLibrary = declaratorNode.id; - } - }, - 'Program:exit'() { - if (defaultImportFromTestingLibrary) { - const references = context.getDeclaredVariables( - defaultImportFromTestingLibrary - )[0].references; - + // case: const utils = require('testing-library-module') + const references = getVariableReferences(context, declaratorNode); reportImportReferences(references); } + } + } - if (defaultRequireFromTestingLibrary) { - const references = context - .getDeclaredVariables(defaultRequireFromTestingLibrary.parent)[0] - .references.slice(1); + return { + 'Program:exit'() { + const testingLibraryImportName = helpers.getTestingLibraryImportName(); + const testingLibraryImportNode = helpers.getTestingLibraryImportNode(); + const customModuleImportNode = helpers.getCustomModuleImportNode(); + + if ( + testingLibraryImportName && + testingLibraryImportNode && + testingLibraryImportName.match(CLEANUP_LIBRARY_REGEXP) + ) { + reportCandidateModule(testingLibraryImportNode); + } - reportImportReferences(references); + if (customModuleImportNode) { + reportCandidateModule(customModuleImportNode); } }, }; diff --git a/tests/fake-rule.ts b/tests/fake-rule.ts index 4281eeed..829940c3 100644 --- a/tests/fake-rule.ts +++ b/tests/fake-rule.ts @@ -2,10 +2,7 @@ * @file Fake rule to be able to test createTestingLibraryRule and * detectTestingLibraryUtils properly */ -import { - AST_NODE_TYPES, - TSESTree, -} from '@typescript-eslint/experimental-utils'; +import { TSESTree } from '@typescript-eslint/experimental-utils'; import { createTestingLibraryRule } from '../lib/create-testing-library-rule'; export const RULE_NAME = 'fake-rule'; @@ -55,14 +52,12 @@ export default createTestingLibraryRule({ ImportDeclaration: checkImportDeclaration, 'Program:exit'() { const importNode = helpers.getCustomModuleImportNode(); + const importName = helpers.getCustomModuleImportName(); if (!importNode) { return; } - if ( - importNode.type === AST_NODE_TYPES.ImportDeclaration && - importNode.source.value === 'custom-module-forced-report' - ) { + if (importName === 'custom-module-forced-report') { context.report({ node: importNode, messageId: 'fakeError', diff --git a/tests/lib/rules/no-manual-cleanup.test.ts b/tests/lib/rules/no-manual-cleanup.test.ts index 69c4f933..f50e036c 100644 --- a/tests/lib/rules/no-manual-cleanup.test.ts +++ b/tests/lib/rules/no-manual-cleanup.test.ts @@ -13,12 +13,19 @@ const ALL_TESTING_LIBRARIES_WITH_CLEANUP = [ ruleTester.run(RULE_NAME, rule, { valid: [ + { + code: `import "@testing-library/react"`, + }, + { + code: `import { cleanup } from "test-utils"`, + }, + { + // Angular Testing Library doesn't have `cleanup` util + code: `import { cleanup } from "@testing-library/angular"`, + }, ...ALL_TESTING_LIBRARIES_WITH_CLEANUP.map((lib) => ({ code: `import { render } from "${lib}"`, })), - { - code: `import { cleanup } from "any-other-library"`, - }, ...ALL_TESTING_LIBRARIES_WITH_CLEANUP.map((lib) => ({ code: `import utils from "${lib}"`, })), @@ -47,6 +54,14 @@ ruleTester.run(RULE_NAME, rule, { { code: `const utils = require(moduleName)`, }, + { + settings: { + 'testing-library/filename-pattern': 'testing-library\\.js', + }, + code: ` + import { render, cleanup } from "${ALL_TESTING_LIBRARIES_WITH_CLEANUP[0]}" + `, + }, ], invalid: [ ...ALL_TESTING_LIBRARIES_WITH_CLEANUP.map((lib) => ({ @@ -59,6 +74,29 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), + ...ALL_TESTING_LIBRARIES_WITH_CLEANUP.map((lib) => ({ + // official testing-library packages should be reported with custom module setting + settings: { + 'testing-library/module': 'test-utils', + }, + code: `import { cleanup, render } from "${lib}"`, + errors: [ + { + line: 1, + column: 10, // error points to `cleanup` + messageId: 'noManualCleanup', + }, + ], + })), + { + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import { render, cleanup } from 'test-utils' + `, + errors: [{ line: 2, column: 26, messageId: 'noManualCleanup' }], + }, ...ALL_TESTING_LIBRARIES_WITH_CLEANUP.map((lib) => ({ code: `import { cleanup as myCustomCleanup } from "${lib}"`, errors: [ @@ -69,6 +107,15 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), + { + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import { cleanup as myCustomCleanup } from 'test-utils' + `, + errors: [{ line: 2, column: 18, messageId: 'noManualCleanup' }], + }, ...ALL_TESTING_LIBRARIES_WITH_CLEANUP.map((lib) => ({ code: `import utils, { cleanup } from "${lib}"`, errors: [ @@ -79,6 +126,15 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), + { + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import utils, { cleanup } from 'test-utils' + `, + errors: [{ line: 2, column: 25, messageId: 'noManualCleanup' }], + }, ...ALL_TESTING_LIBRARIES_WITH_CLEANUP.map((lib) => ({ code: ` import utils from "${lib}" @@ -92,6 +148,16 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), + { + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import utils from 'test-utils' + afterEach(() => utils.cleanup()) + `, + errors: [{ line: 3, column: 31, messageId: 'noManualCleanup' }], + }, ...ALL_TESTING_LIBRARIES_WITH_CLEANUP.map((lib) => ({ code: ` import utils from "${lib}" @@ -115,6 +181,15 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), + { + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + const { render, cleanup } = require('test-utils') + `, + errors: [{ line: 2, column: 25, messageId: 'noManualCleanup' }], + }, ...ALL_TESTING_LIBRARIES_WITH_CLEANUP.map((lib) => ({ code: ` const utils = require("${lib}")