From 78a3d4e549a0f6e2d4647407c379d48c04a23f9f Mon Sep 17 00:00:00 2001 From: Senja Jarva Date: Sat, 1 Oct 2022 18:08:46 +0300 Subject: [PATCH 1/2] fix: refactor helper functions to keep a list testing-library imports Previously only first testing-library import was saved, now all of them are saved. --- .../detect-testing-library-utils.ts | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/lib/create-testing-library-rule/detect-testing-library-utils.ts b/lib/create-testing-library-rule/detect-testing-library-utils.ts index 93922a06..e8fa2996 100644 --- a/lib/create-testing-library-rule/detect-testing-library-utils.ts +++ b/lib/create-testing-library-rule/detect-testing-library-utils.ts @@ -54,6 +54,7 @@ export type EnhancedRuleCreate< // Helpers methods type GetTestingLibraryImportNodeFn = () => ImportModuleNode | null; +type GetTestingLibraryImportNodesFn = () => ImportModuleNode[]; type GetCustomModuleImportNodeFn = () => ImportModuleNode | null; type GetTestingLibraryImportNameFn = () => string | undefined; type GetCustomModuleImportNameFn = () => string | undefined; @@ -95,6 +96,7 @@ type IsNodeComingFromTestingLibraryFn = ( export interface DetectionHelpers { getTestingLibraryImportNode: GetTestingLibraryImportNodeFn; + getAllTestingLibraryImportNodes: GetTestingLibraryImportNodesFn; getCustomModuleImportNode: GetCustomModuleImportNodeFn; getTestingLibraryImportName: GetTestingLibraryImportNameFn; getCustomModuleImportName: GetCustomModuleImportNameFn; @@ -158,7 +160,7 @@ export function detectTestingLibraryUtils< context: TestingLibraryContext, optionsWithDefault: Readonly ): TSESLint.RuleListener => { - let importedTestingLibraryNode: ImportModuleNode | null = null; + const importedTestingLibraryNodes: ImportModuleNode[] = []; let importedCustomModuleNode: ImportModuleNode | null = null; let importedUserEventLibraryNode: ImportModuleNode | null = null; let importedReactDomTestUtilsNode: ImportModuleNode | null = null; @@ -299,15 +301,20 @@ export function detectTestingLibraryUtils< // Helpers for Testing Library detection. const getTestingLibraryImportNode: GetTestingLibraryImportNodeFn = () => { - return importedTestingLibraryNode; + return importedTestingLibraryNodes[0]; }; + const getAllTestingLibraryImportNodes: GetTestingLibraryImportNodesFn = + () => { + return importedTestingLibraryNodes; + }; + const getCustomModuleImportNode: GetCustomModuleImportNodeFn = () => { return importedCustomModuleNode; }; const getTestingLibraryImportName: GetTestingLibraryImportNameFn = () => { - return getImportModuleName(importedTestingLibraryNode); + return getImportModuleName(importedTestingLibraryNodes[0]); }; const getCustomModuleImportName: GetCustomModuleImportNameFn = () => { @@ -331,7 +338,7 @@ export function detectTestingLibraryUtils< isStrict = false ) => { const isSomeModuleImported = - !!importedTestingLibraryNode || !!importedCustomModuleNode; + importedTestingLibraryNodes.length !== 0 || !!importedCustomModuleNode; return ( (!isStrict && isAggressiveModuleReportingEnabled()) || @@ -945,6 +952,7 @@ export function detectTestingLibraryUtils< const helpers: DetectionHelpers = { getTestingLibraryImportNode, + getAllTestingLibraryImportNodes, getCustomModuleImportNode, getTestingLibraryImportName, getCustomModuleImportName, @@ -989,12 +997,9 @@ export function detectTestingLibraryUtils< return; } // check only if testing library import not found yet so we avoid - // to override importedTestingLibraryNode after it's found - if ( - !importedTestingLibraryNode && - /testing-library/g.test(node.source.value) - ) { - importedTestingLibraryNode = node; + // to override importedTestingLibraryNodes after it's found + if (/testing-library/g.test(node.source.value)) { + importedTestingLibraryNodes.push(node); } // check only if custom module import not found yet so we avoid @@ -1035,7 +1040,6 @@ export function detectTestingLibraryUtils< const { arguments: args } = callExpression; if ( - !importedTestingLibraryNode && args.some( (arg) => isLiteral(arg) && @@ -1043,7 +1047,7 @@ export function detectTestingLibraryUtils< /testing-library/g.test(arg.value) ) ) { - importedTestingLibraryNode = callExpression; + importedTestingLibraryNodes.push(callExpression); } const customModule = getCustomModule(); From 6e2df311b135c6eb619be6dcce76f2af74dc1ec2 Mon Sep 17 00:00:00 2001 From: Senja Jarva Date: Sat, 1 Oct 2022 18:12:55 +0300 Subject: [PATCH 2/2] fix: false negatives for no-dom-import with several imports Use the new refactored utils function to get a list of all testing-library imports and make sure that none of them are a dom import. Ref: #586 --- docs/rules/no-dom-import.md | 5 +++++ lib/rules/no-dom-import.ts | 26 +++++++++++++------------- tests/lib/rules/no-dom-import.test.ts | 12 ++++++++++++ 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/docs/rules/no-dom-import.md b/docs/rules/no-dom-import.md index 2e84b17a..c61cd405 100644 --- a/docs/rules/no-dom-import.md +++ b/docs/rules/no-dom-import.md @@ -31,6 +31,11 @@ import { fireEvent } from 'dom-testing-library'; import { fireEvent } from '@testing-library/dom'; ``` +```js +import { render } from '@testing-library/react'; // Okay, no error +import { screen } from '@testing-library/dom'; // Error, unnecessary import from @testing-library/dom +``` + ```js const { fireEvent } = require('dom-testing-library'); ``` diff --git a/lib/rules/no-dom-import.ts b/lib/rules/no-dom-import.ts index 887075f1..4b34e21b 100644 --- a/lib/rules/no-dom-import.ts +++ b/lib/rules/no-dom-import.ts @@ -1,7 +1,7 @@ import { TSESTree } from '@typescript-eslint/utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; -import { isCallExpression } from '../node-utils'; +import { isCallExpression, getImportModuleName } from '../node-utils'; export const RULE_NAME = 'no-dom-import'; export type MessageIds = 'noDomImport' | 'noDomImportFramework'; @@ -84,22 +84,22 @@ export default createTestingLibraryRule({ return { 'Program:exit'() { - const importName = helpers.getTestingLibraryImportName(); - const importNode = helpers.getTestingLibraryImportNode(); + let importName: string | undefined; + const allImportNodes = helpers.getAllTestingLibraryImportNodes(); - if (!importNode) { - return; - } + allImportNodes.forEach((importNode) => { + importName = getImportModuleName(importNode); - const domModuleName = DOM_TESTING_LIBRARY_MODULES.find( - (module) => module === importName - ); + const domModuleName = DOM_TESTING_LIBRARY_MODULES.find( + (module) => module === importName + ); - if (!domModuleName) { - return; - } + if (!domModuleName) { + return; + } - report(importNode, domModuleName); + report(importNode, domModuleName); + }); }, }; }, diff --git a/tests/lib/rules/no-dom-import.test.ts b/tests/lib/rules/no-dom-import.test.ts index 7336bbd6..fd493ad6 100644 --- a/tests/lib/rules/no-dom-import.test.ts +++ b/tests/lib/rules/no-dom-import.test.ts @@ -203,5 +203,17 @@ ruleTester.run(RULE_NAME, rule, { code: 'require("@testing-library/dom")', errors: [{ messageId: 'noDomImport' }], }, + { + code: ` + require("@testing-library/dom"); + require("@testing-library/react");`, + errors: [{ line: 2, messageId: 'noDomImport' }], + }, + { + code: ` + import { render } from '@testing-library/react'; + import { screen } from '@testing-library/dom';`, + errors: [{ line: 3, messageId: 'noDomImport' }], + }, ], });