From 7db6f181a31ae20add7e8fc1ae1b1e269d7779eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Tue, 3 Nov 2020 10:53:08 +0100 Subject: [PATCH 01/10] refactor(no-manual-cleanup): use custom rule creator --- lib/rules/no-manual-cleanup.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/rules/no-manual-cleanup.ts b/lib/rules/no-manual-cleanup.ts index 81bdab6c..0dcd749e 100644 --- a/lib/rules/no-manual-cleanup.ts +++ b/lib/rules/no-manual-cleanup.ts @@ -1,14 +1,14 @@ -import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; -import { getDocsUrl } from '../utils'; +import { TSESTree } from '@typescript-eslint/experimental-utils'; import { + isIdentifier, isImportDefaultSpecifier, + isImportSpecifier, isLiteral, - isIdentifier, + isMemberExpression, isObjectPattern, isProperty, - isMemberExpression, - isImportSpecifier, } from '../node-utils'; +import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'no-manual-cleanup'; export type MessageIds = 'noManualCleanup'; @@ -16,7 +16,7 @@ type Options = []; const CLEANUP_LIBRARY_REGEX = /(@testing-library\/(preact|react|svelte|vue))|@marko\/testing-library/; -export default ESLintUtils.RuleCreator(getDocsUrl)({ +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'problem', From dac0e91d33c14c8be14cea6d43edd2653d0e533f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Tue, 3 Nov 2020 11:56:45 +0100 Subject: [PATCH 02/10] refactor: extract detection utils for import module name --- lib/detect-testing-library-utils.ts | 19 +++++++++----- lib/node-utils.ts | 26 +++++++++++++++++++ lib/rules/no-dom-import.ts | 40 ++++++++--------------------- tests/fake-rule.ts | 6 ++--- 4 files changed, 52 insertions(+), 39 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 633c0241..70b3588d 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -1,5 +1,9 @@ import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; -import { isLiteral } from './node-utils'; +import { + getImportModuleName, + isLiteral, + ModuleImportation, +} from './node-utils'; export type TestingLibrarySettings = { 'testing-library/module'?: string; @@ -25,14 +29,11 @@ export type EnhancedRuleCreate< detectionHelpers: Readonly ) => TRuleListener; -type ModuleImportation = - | TSESTree.ImportDeclaration - | TSESTree.CallExpression - | null; - export type DetectionHelpers = { getTestingLibraryImportNode: () => ModuleImportation; getCustomModuleImportNode: () => ModuleImportation; + getTestingLibraryImportName: () => string | undefined; + getCustomModuleImportName: () => string | undefined; getIsTestingLibraryImported: () => boolean; getIsValidFilename: () => boolean; canReportErrors: () => boolean; @@ -69,6 +70,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..772ee36c 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -220,3 +220,29 @@ export function isRenderVariableDeclarator( return false; } + +export type ModuleImportation = + | TSESTree.ImportDeclaration + | TSESTree.CallExpression + | null; + +export function getImportModuleName( + node: ModuleImportation | undefined +): string | undefined { + // import node of shape: import { foo } from 'bar' + if ( + node?.type === AST_NODE_TYPES.ImportDeclaration && + typeof node.source.value === 'string' + ) { + return node.source.value; + } + + // import node of shape: const { foo } = require('bar') + if ( + node?.type === AST_NODE_TYPES.CallExpression && + isLiteral(node.arguments[0]) && + typeof node.arguments[0].value === 'string' + ) { + return node.arguments[0].value; + } +} diff --git a/lib/rules/no-dom-import.ts b/lib/rules/no-dom-import.ts index 80db79aa..4964c29f 100644 --- a/lib/rules/no-dom-import.ts +++ b/lib/rules/no-dom-import.ts @@ -2,7 +2,6 @@ import { AST_NODE_TYPES, TSESTree, } from '@typescript-eslint/experimental-utils'; -import { isIdentifier, isLiteral } from '../node-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'no-dom-import'; @@ -40,11 +39,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 +51,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 (node.type === AST_NODE_TYPES.CallExpression) { + 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 +60,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 +78,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/tests/fake-rule.ts b/tests/fake-rule.ts index 4281eeed..b1af6aee 100644 --- a/tests/fake-rule.ts +++ b/tests/fake-rule.ts @@ -55,14 +55,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', From fcf145215f3aee2eebe1ef9bff89307c26ec36d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Tue, 3 Nov 2020 13:27:35 +0100 Subject: [PATCH 03/10] refactor(no-manual-cleanup): use detection helpers for imported modules --- lib/rules/no-manual-cleanup.ts | 124 ++++++++++++++------------------- 1 file changed, 51 insertions(+), 73 deletions(-) diff --git a/lib/rules/no-manual-cleanup.ts b/lib/rules/no-manual-cleanup.ts index 0dcd749e..a1f44136 100644 --- a/lib/rules/no-manual-cleanup.ts +++ b/lib/rules/no-manual-cleanup.ts @@ -1,9 +1,11 @@ -import { TSESTree } from '@typescript-eslint/experimental-utils'; +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; import { isIdentifier, isImportDefaultSpecifier, isImportSpecifier, - isLiteral, isMemberExpression, isObjectPattern, isProperty, @@ -14,7 +16,7 @@ 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 createTestingLibraryRule({ name: RULE_NAME, @@ -34,12 +36,7 @@ export default createTestingLibraryRule({ }, defaultOptions: [], - create(context) { - let defaultImportFromTestingLibrary: TSESTree.ImportDeclaration; - let defaultRequireFromTestingLibrary: - | TSESTree.Identifier - | TSESTree.ArrayPattern; - + create(context, _, helpers) { // can't find the right type? // eslint-disable-next-line @typescript-eslint/no-explicit-any function reportImportReferences(references: any[]) { @@ -61,85 +58,66 @@ export default createTestingLibraryRule({ } return { - ImportDeclaration(node) { - const value = node.source.value as string; - const testingLibraryWithCleanup = value.match(CLEANUP_LIBRARY_REGEX); + 'Program:exit'() { + const moduleName = helpers.getTestingLibraryImportName(); + const moduleNode = helpers.getTestingLibraryImportNode(); - // Early return if the library doesn't support `cleanup` - if (!testingLibraryWithCleanup) { + if (!moduleNode) { return; } - if (isImportDefaultSpecifier(node.specifiers[0])) { - defaultImportFromTestingLibrary = node; - } - - const cleanupSpecifier = node.specifiers.find( - (specifier) => - isImportSpecifier(specifier) && - specifier.imported && - specifier.imported.name === 'cleanup' - ); - - if (cleanupSpecifier) { - context.report({ - node: cleanupSpecifier, - 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) { + // Early return if the library doesn't support `cleanup` + if (!moduleName.match(CLEANUP_LIBRARY_REGEXP)) { return; } - const declaratorNode = node.parent - .parent as TSESTree.VariableDeclarator; + if (moduleNode.type === AST_NODE_TYPES.ImportDeclaration) { + // case: import utils from 'testing-library-module' + if (isImportDefaultSpecifier(moduleNode.specifiers[0])) { + const references = context.getDeclaredVariables(moduleNode)[0] + ?.references; + + reportImportReferences(references); + } - if (isObjectPattern(declaratorNode.id)) { - const cleanupProperty = declaratorNode.id.properties.find( - (property) => - isProperty(property) && - isIdentifier(property.key) && - property.key.name === 'cleanup' + // case: import { cleanup } from 'testing-library-module' + const cleanupSpecifier = moduleNode.specifiers.find( + (specifier) => + isImportSpecifier(specifier) && + specifier.imported.name === 'cleanup' ); - if (cleanupProperty) { + if (cleanupSpecifier) { context.report({ - node: cleanupProperty, + node: cleanupSpecifier, messageId: 'noManualCleanup', }); } } else { - defaultRequireFromTestingLibrary = declaratorNode.id; - } - }, - 'Program:exit'() { - if (defaultImportFromTestingLibrary) { - const references = context.getDeclaredVariables( - defaultImportFromTestingLibrary - )[0].references; - - reportImportReferences(references); - } - - if (defaultRequireFromTestingLibrary) { - const references = context - .getDeclaredVariables(defaultRequireFromTestingLibrary.parent)[0] - .references.slice(1); - - reportImportReferences(references); + 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) && + property.key.name === 'cleanup' + ); + + if (cleanupProperty) { + context.report({ + node: cleanupProperty, + messageId: 'noManualCleanup', + }); + } + } else { + // case: const utils = require('testing-library-module') + const references = context + .getDeclaredVariables(declaratorNode)[0] + ?.references?.slice(1); + reportImportReferences(references); + } } }, }; From 9e48245353cd69a4863c26b8add989399aeb54f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Tue, 3 Nov 2020 16:00:58 +0100 Subject: [PATCH 04/10] refactor(no-manual-cleanup): small improvements --- lib/node-utils.ts | 10 ++++-- lib/rules/no-manual-cleanup.ts | 43 +++++++++++------------ tests/lib/rules/no-manual-cleanup.test.ts | 12 +++++-- 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 772ee36c..f6c6e7e1 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -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 { @@ -176,7 +180,7 @@ export function getVariableReferences( ): TSESLint.Scope.Reference[] { return ( (isVariableDeclarator(node) && - context.getDeclaredVariables(node)[0].references.slice(1)) || + context.getDeclaredVariables(node)[0]?.references?.slice(1)) || [] ); } diff --git a/lib/rules/no-manual-cleanup.ts b/lib/rules/no-manual-cleanup.ts index a1f44136..436ad81a 100644 --- a/lib/rules/no-manual-cleanup.ts +++ b/lib/rules/no-manual-cleanup.ts @@ -1,9 +1,11 @@ import { AST_NODE_TYPES, + ASTUtils, TSESTree, + TSESLint, } from '@typescript-eslint/experimental-utils'; import { - isIdentifier, + getVariableReferences, isImportDefaultSpecifier, isImportSpecifier, isMemberExpression, @@ -39,22 +41,20 @@ export default createTestingLibraryRule({ create(context, _, helpers) { // 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', - }); - } - }); - } + 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 { @@ -74,8 +74,7 @@ export default createTestingLibraryRule({ if (moduleNode.type === AST_NODE_TYPES.ImportDeclaration) { // case: import utils from 'testing-library-module' if (isImportDefaultSpecifier(moduleNode.specifiers[0])) { - const references = context.getDeclaredVariables(moduleNode)[0] - ?.references; + const { references } = context.getDeclaredVariables(moduleNode)[0]; reportImportReferences(references); } @@ -101,7 +100,7 @@ export default createTestingLibraryRule({ const cleanupProperty = declaratorNode.id.properties.find( (property) => isProperty(property) && - isIdentifier(property.key) && + ASTUtils.isIdentifier(property.key) && property.key.name === 'cleanup' ); @@ -113,9 +112,7 @@ export default createTestingLibraryRule({ } } else { // case: const utils = require('testing-library-module') - const references = context - .getDeclaredVariables(declaratorNode)[0] - ?.references?.slice(1); + const references = getVariableReferences(context, declaratorNode); reportImportReferences(references); } } diff --git a/tests/lib/rules/no-manual-cleanup.test.ts b/tests/lib/rules/no-manual-cleanup.test.ts index 69c4f933..f5db8891 100644 --- a/tests/lib/rules/no-manual-cleanup.test.ts +++ b/tests/lib/rules/no-manual-cleanup.test.ts @@ -13,12 +13,18 @@ const ALL_TESTING_LIBRARIES_WITH_CLEANUP = [ ruleTester.run(RULE_NAME, rule, { valid: [ - ...ALL_TESTING_LIBRARIES_WITH_CLEANUP.map((lib) => ({ - code: `import { render } from "${lib}"`, - })), + { + code: `import "@testing-library/react"`, + }, { code: `import { cleanup } from "any-other-library"`, }, + { + code: `import { cleanup } from "@testing-library/angular"`, + }, + ...ALL_TESTING_LIBRARIES_WITH_CLEANUP.map((lib) => ({ + code: `import { render } from "${lib}"`, + })), ...ALL_TESTING_LIBRARIES_WITH_CLEANUP.map((lib) => ({ code: `import utils from "${lib}"`, })), From 8641739e8cbe31d49b25ed5c5c292b9b4274b1d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Tue, 3 Nov 2020 16:07:48 +0100 Subject: [PATCH 05/10] test(no-manual-cleanup): include more variants --- tests/lib/rules/no-manual-cleanup.test.ts | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/lib/rules/no-manual-cleanup.test.ts b/tests/lib/rules/no-manual-cleanup.test.ts index f5db8891..9154b5e8 100644 --- a/tests/lib/rules/no-manual-cleanup.test.ts +++ b/tests/lib/rules/no-manual-cleanup.test.ts @@ -53,6 +53,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) => ({ @@ -65,6 +73,20 @@ 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', + }, + ], + })), ...ALL_TESTING_LIBRARIES_WITH_CLEANUP.map((lib) => ({ code: `import { cleanup as myCustomCleanup } from "${lib}"`, errors: [ From 0dd98b8942eb351b77efd5f6cd9139bc32fba5d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Tue, 3 Nov 2020 16:33:57 +0100 Subject: [PATCH 06/10] feat(no-manual-cleanup): report custom module --- lib/detect-testing-library-utils.ts | 8 +- lib/node-utils.ts | 6 +- lib/rules/no-manual-cleanup.ts | 95 ++++++++++++----------- tests/lib/rules/no-manual-cleanup.test.ts | 49 +++++++++++- 4 files changed, 106 insertions(+), 52 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 70b3588d..bd5acf96 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -30,8 +30,8 @@ export type EnhancedRuleCreate< ) => TRuleListener; export type DetectionHelpers = { - getTestingLibraryImportNode: () => ModuleImportation; - getCustomModuleImportNode: () => ModuleImportation; + getTestingLibraryImportNode: () => ModuleImportation | null; + getCustomModuleImportNode: () => ModuleImportation | null; getTestingLibraryImportName: () => string | undefined; getCustomModuleImportName: () => string | undefined; getIsTestingLibraryImported: () => boolean; @@ -53,8 +53,8 @@ export function detectTestingLibraryUtils< context: TestingLibraryContext, optionsWithDefault: Readonly ): TSESLint.RuleListener => { - let importedTestingLibraryNode: ModuleImportation = null; - let importedCustomModuleNode: ModuleImportation = null; + let importedTestingLibraryNode: ModuleImportation | null = null; + let importedCustomModuleNode: ModuleImportation | null = null; // Init options based on shared ESLint settings const customModule = context.settings['testing-library/module']; diff --git a/lib/node-utils.ts b/lib/node-utils.ts index f6c6e7e1..9c14fc9f 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -225,13 +225,13 @@ export function isRenderVariableDeclarator( return false; } +// TODO: extract into types file? export type ModuleImportation = | TSESTree.ImportDeclaration - | TSESTree.CallExpression - | null; + | TSESTree.CallExpression; export function getImportModuleName( - node: ModuleImportation | undefined + node: ModuleImportation | undefined | null ): string | undefined { // import node of shape: import { foo } from 'bar' if ( diff --git a/lib/rules/no-manual-cleanup.ts b/lib/rules/no-manual-cleanup.ts index 436ad81a..d02166df 100644 --- a/lib/rules/no-manual-cleanup.ts +++ b/lib/rules/no-manual-cleanup.ts @@ -11,6 +11,7 @@ import { isMemberExpression, isObjectPattern, isProperty, + ModuleImportation, } from '../node-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; @@ -57,64 +58,70 @@ export default createTestingLibraryRule({ }); } - return { - 'Program:exit'() { - const moduleName = helpers.getTestingLibraryImportName(); - const moduleNode = helpers.getTestingLibraryImportNode(); + function reportCandidateModule(moduleNode: ModuleImportation) { + if (moduleNode.type === AST_NODE_TYPES.ImportDeclaration) { + // case: import utils from 'testing-library-module' + if (isImportDefaultSpecifier(moduleNode.specifiers[0])) { + const { references } = context.getDeclaredVariables(moduleNode)[0]; - if (!moduleNode) { - return; + reportImportReferences(references); } - // Early return if the library doesn't support `cleanup` - if (!moduleName.match(CLEANUP_LIBRARY_REGEXP)) { - return; - } + // case: import { cleanup } from 'testing-library-module' + const cleanupSpecifier = moduleNode.specifiers.find( + (specifier) => + isImportSpecifier(specifier) && + specifier.imported.name === 'cleanup' + ); - if (moduleNode.type === AST_NODE_TYPES.ImportDeclaration) { - // case: import utils from 'testing-library-module' - if (isImportDefaultSpecifier(moduleNode.specifiers[0])) { - const { references } = context.getDeclaredVariables(moduleNode)[0]; - - reportImportReferences(references); - } + if (cleanupSpecifier) { + context.report({ + node: cleanupSpecifier, + messageId: 'noManualCleanup', + }); + } + } else { + const declaratorNode = moduleNode.parent as TSESTree.VariableDeclarator; - // case: import { cleanup } from 'testing-library-module' - const cleanupSpecifier = moduleNode.specifiers.find( - (specifier) => - isImportSpecifier(specifier) && - specifier.imported.name === 'cleanup' + if (isObjectPattern(declaratorNode.id)) { + // case: const { cleanup } = require('testing-library-module') + const cleanupProperty = declaratorNode.id.properties.find( + (property) => + isProperty(property) && + ASTUtils.isIdentifier(property.key) && + property.key.name === 'cleanup' ); - if (cleanupSpecifier) { + if (cleanupProperty) { context.report({ - node: cleanupSpecifier, + node: cleanupProperty, messageId: 'noManualCleanup', }); } } else { - const declaratorNode = moduleNode.parent as TSESTree.VariableDeclarator; + // case: const utils = require('testing-library-module') + const references = getVariableReferences(context, declaratorNode); + reportImportReferences(references); + } + } + } - if (isObjectPattern(declaratorNode.id)) { - // case: const { cleanup } = require('testing-library-module') - const cleanupProperty = declaratorNode.id.properties.find( - (property) => - isProperty(property) && - ASTUtils.isIdentifier(property.key) && - property.key.name === 'cleanup' - ); + return { + 'Program:exit'() { + const testingLibraryImportName = helpers.getTestingLibraryImportName(); + const testingLibraryImportNode = helpers.getTestingLibraryImportNode(); + const customModuleImportNode = helpers.getCustomModuleImportNode(); - if (cleanupProperty) { - context.report({ - node: cleanupProperty, - messageId: 'noManualCleanup', - }); - } - } else { - // case: const utils = require('testing-library-module') - const references = getVariableReferences(context, declaratorNode); - reportImportReferences(references); - } + if ( + testingLibraryImportName && + testingLibraryImportNode && + testingLibraryImportName.match(CLEANUP_LIBRARY_REGEXP) + ) { + reportCandidateModule(testingLibraryImportNode); + } + + if (customModuleImportNode) { + reportCandidateModule(customModuleImportNode); } }, }; diff --git a/tests/lib/rules/no-manual-cleanup.test.ts b/tests/lib/rules/no-manual-cleanup.test.ts index 9154b5e8..f50e036c 100644 --- a/tests/lib/rules/no-manual-cleanup.test.ts +++ b/tests/lib/rules/no-manual-cleanup.test.ts @@ -17,9 +17,10 @@ ruleTester.run(RULE_NAME, rule, { code: `import "@testing-library/react"`, }, { - code: `import { cleanup } from "any-other-library"`, + 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) => ({ @@ -87,6 +88,15 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), + { + 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: [ @@ -97,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: [ @@ -107,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}" @@ -120,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}" @@ -143,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}") From 9e77544f8b93a3a6cb153164ef5b54d11193fe8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Tue, 3 Nov 2020 23:08:11 +0100 Subject: [PATCH 07/10] refactor: rename type for import module node --- lib/detect-testing-library-utils.ts | 14 +++++--------- lib/node-utils.ts | 4 ++-- lib/rules/no-manual-cleanup.ts | 4 ++-- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index bd5acf96..7ca83d1e 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -1,9 +1,5 @@ import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; -import { - getImportModuleName, - isLiteral, - ModuleImportation, -} from './node-utils'; +import { getImportModuleName, isLiteral, ImportModuleNode } from './node-utils'; export type TestingLibrarySettings = { 'testing-library/module'?: string; @@ -30,8 +26,8 @@ export type EnhancedRuleCreate< ) => TRuleListener; export type DetectionHelpers = { - getTestingLibraryImportNode: () => ModuleImportation | null; - getCustomModuleImportNode: () => ModuleImportation | null; + getTestingLibraryImportNode: () => ImportModuleNode | null; + getCustomModuleImportNode: () => ImportModuleNode | null; getTestingLibraryImportName: () => string | undefined; getCustomModuleImportName: () => string | undefined; getIsTestingLibraryImported: () => boolean; @@ -53,8 +49,8 @@ export function detectTestingLibraryUtils< context: TestingLibraryContext, optionsWithDefault: Readonly ): TSESLint.RuleListener => { - let importedTestingLibraryNode: ModuleImportation | null = null; - let importedCustomModuleNode: ModuleImportation | null = 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']; diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 9c14fc9f..2006463f 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -226,12 +226,12 @@ export function isRenderVariableDeclarator( } // TODO: extract into types file? -export type ModuleImportation = +export type ImportModuleNode = | TSESTree.ImportDeclaration | TSESTree.CallExpression; export function getImportModuleName( - node: ModuleImportation | undefined | null + node: ImportModuleNode | undefined | null ): string | undefined { // import node of shape: import { foo } from 'bar' if ( diff --git a/lib/rules/no-manual-cleanup.ts b/lib/rules/no-manual-cleanup.ts index d02166df..bc3242b2 100644 --- a/lib/rules/no-manual-cleanup.ts +++ b/lib/rules/no-manual-cleanup.ts @@ -11,7 +11,7 @@ import { isMemberExpression, isObjectPattern, isProperty, - ModuleImportation, + ImportModuleNode, } from '../node-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; @@ -58,7 +58,7 @@ export default createTestingLibraryRule({ }); } - function reportCandidateModule(moduleNode: ModuleImportation) { + function reportCandidateModule(moduleNode: ImportModuleNode) { if (moduleNode.type === AST_NODE_TYPES.ImportDeclaration) { // case: import utils from 'testing-library-module' if (isImportDefaultSpecifier(moduleNode.specifiers[0])) { From 07c0be269f810b42ddc0621423ac35b110db0b3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Wed, 4 Nov 2020 09:58:34 +0100 Subject: [PATCH 08/10] refactor: use node utils to know node type --- lib/node-utils.ts | 17 ++++++++++------- lib/rules/no-container.ts | 2 +- lib/rules/no-dom-import.ts | 3 ++- lib/rules/no-manual-cleanup.ts | 3 ++- tests/fake-rule.ts | 5 +---- 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 2006463f..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( @@ -154,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) || @@ -234,16 +240,13 @@ export function getImportModuleName( node: ImportModuleNode | undefined | null ): string | undefined { // import node of shape: import { foo } from 'bar' - if ( - node?.type === AST_NODE_TYPES.ImportDeclaration && - typeof node.source.value === 'string' - ) { + if (isImportDeclaration(node) && typeof node.source.value === 'string') { return node.source.value; } // import node of shape: const { foo } = require('bar') if ( - node?.type === AST_NODE_TYPES.CallExpression && + isCallExpression(node) && isLiteral(node.arguments[0]) && typeof node.arguments[0].value === 'string' ) { 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 4964c29f..1586bb3d 100644 --- a/lib/rules/no-dom-import.ts +++ b/lib/rules/no-dom-import.ts @@ -3,6 +3,7 @@ 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'; @@ -51,7 +52,7 @@ export default createTestingLibraryRule({ module: correctModuleName, }, fix(fixer) { - if (node.type === AST_NODE_TYPES.CallExpression) { + 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 diff --git a/lib/rules/no-manual-cleanup.ts b/lib/rules/no-manual-cleanup.ts index bc3242b2..dcaa3409 100644 --- a/lib/rules/no-manual-cleanup.ts +++ b/lib/rules/no-manual-cleanup.ts @@ -12,6 +12,7 @@ import { isObjectPattern, isProperty, ImportModuleNode, + isImportDeclaration, } from '../node-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; @@ -59,7 +60,7 @@ export default createTestingLibraryRule({ } function reportCandidateModule(moduleNode: ImportModuleNode) { - if (moduleNode.type === AST_NODE_TYPES.ImportDeclaration) { + if (isImportDeclaration(moduleNode)) { // case: import utils from 'testing-library-module' if (isImportDefaultSpecifier(moduleNode.specifiers[0])) { const { references } = context.getDeclaredVariables(moduleNode)[0]; diff --git a/tests/fake-rule.ts b/tests/fake-rule.ts index b1af6aee..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'; From ab353584f7e6102bb0251cfc7211434debc67c43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Wed, 4 Nov 2020 10:00:46 +0100 Subject: [PATCH 09/10] refactor: remove unused imports --- lib/rules/no-dom-import.ts | 5 +---- lib/rules/no-manual-cleanup.ts | 1 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/rules/no-dom-import.ts b/lib/rules/no-dom-import.ts index 1586bb3d..1246447d 100644 --- a/lib/rules/no-dom-import.ts +++ b/lib/rules/no-dom-import.ts @@ -1,7 +1,4 @@ -import { - AST_NODE_TYPES, - TSESTree, -} from '@typescript-eslint/experimental-utils'; +import { TSESTree } from '@typescript-eslint/experimental-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; import { isCallExpression } from '../node-utils'; diff --git a/lib/rules/no-manual-cleanup.ts b/lib/rules/no-manual-cleanup.ts index dcaa3409..90e57a75 100644 --- a/lib/rules/no-manual-cleanup.ts +++ b/lib/rules/no-manual-cleanup.ts @@ -1,5 +1,4 @@ import { - AST_NODE_TYPES, ASTUtils, TSESTree, TSESLint, From 50a3cef729ec127fd0113ebc14e43517b2b8c99a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Wed, 4 Nov 2020 10:02:05 +0100 Subject: [PATCH 10/10] refactor: remove outdated comment line --- lib/rules/no-manual-cleanup.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/rules/no-manual-cleanup.ts b/lib/rules/no-manual-cleanup.ts index 90e57a75..841cfc47 100644 --- a/lib/rules/no-manual-cleanup.ts +++ b/lib/rules/no-manual-cleanup.ts @@ -40,8 +40,6 @@ export default createTestingLibraryRule({ defaultOptions: [], create(context, _, helpers) { - // can't find the right type? - // eslint-disable-next-line @typescript-eslint/no-explicit-any function reportImportReferences(references: TSESLint.Scope.Reference[]) { references.forEach((reference) => { const utilsUsage = reference.identifier.parent;