diff --git a/.eslintignore b/.eslintignore index 404abb22..d64c4ca2 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1 +1,2 @@ coverage/ +dist/ diff --git a/.lintstagedrc b/.lintstagedrc index 172628eb..59919a0f 100644 --- a/.lintstagedrc +++ b/.lintstagedrc @@ -1,8 +1,8 @@ { - "*.{js,ts}": [ + "*.{js,ts}": [ "eslint --fix", "prettier --write", - "jest --findRelatedTests", + "jest --findRelatedTests" ], "*.md": ["prettier --write"] } diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 31b4c809..633c0241 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -1,8 +1,9 @@ import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; +import { isLiteral } from './node-utils'; export type TestingLibrarySettings = { 'testing-library/module'?: string; - 'testing-library/file-name'?: string; + 'testing-library/filename-pattern'?: string; }; export type TestingLibraryContext< @@ -24,13 +25,20 @@ export type EnhancedRuleCreate< detectionHelpers: Readonly ) => TRuleListener; +type ModuleImportation = + | TSESTree.ImportDeclaration + | TSESTree.CallExpression + | null; + export type DetectionHelpers = { + getTestingLibraryImportNode: () => ModuleImportation; + getCustomModuleImportNode: () => ModuleImportation; getIsTestingLibraryImported: () => boolean; - getIsValidFileName: () => boolean; + getIsValidFilename: () => boolean; canReportErrors: () => boolean; }; -const DEFAULT_FILE_NAME_PATTERN = '^.*\\.(test|spec)\\.[jt]sx?$'; +const DEFAULT_FILENAME_PATTERN = '^.*\\.(test|spec)\\.[jt]sx?$'; /** * Enhances a given rule `create` with helpers to detect Testing Library utils. @@ -44,17 +52,23 @@ export function detectTestingLibraryUtils< context: TestingLibraryContext, optionsWithDefault: Readonly ): TSESLint.RuleListener => { - let isImportingTestingLibraryModule = false; - let isImportingCustomModule = false; + let importedTestingLibraryNode: ModuleImportation = null; + let importedCustomModuleNode: ModuleImportation = null; // Init options based on shared ESLint settings const customModule = context.settings['testing-library/module']; - const fileNamePattern = - context.settings['testing-library/file-name'] ?? - DEFAULT_FILE_NAME_PATTERN; + const filenamePattern = + context.settings['testing-library/filename-pattern'] ?? + DEFAULT_FILENAME_PATTERN; // Helpers for Testing Library detection. const helpers: DetectionHelpers = { + getTestingLibraryImportNode() { + return importedTestingLibraryNode; + }, + getCustomModuleImportNode() { + return importedCustomModuleNode; + }, /** * Gets if Testing Library is considered as imported or not. * @@ -72,24 +86,24 @@ export function detectTestingLibraryUtils< return true; } - return isImportingTestingLibraryModule || isImportingCustomModule; + return !!importedTestingLibraryNode || !!importedCustomModuleNode; }, /** - * Gets if name of the file being analyzed is valid or not. + * Gets if filename being analyzed is valid or not. * - * This is based on "testing-library/file-name" setting. + * This is based on "testing-library/filename-pattern" setting. */ - getIsValidFileName() { + getIsValidFilename() { const fileName = context.getFilename(); - return !!fileName.match(fileNamePattern); + return !!fileName.match(filenamePattern); }, /** * Wraps all conditions that must be met to report rules. */ canReportErrors() { - return this.getIsTestingLibraryImported() && this.getIsValidFileName(); + return this.getIsTestingLibraryImported() && this.getIsValidFilename(); }, }; @@ -97,25 +111,60 @@ export function detectTestingLibraryUtils< const detectionInstructions: TSESLint.RuleListener = { /** * This ImportDeclaration rule listener will check if Testing Library related - * modules are loaded. Since imports happen first thing in a file, it's + * modules are imported. Since imports happen first thing in a file, it's * safe to use `isImportingTestingLibraryModule` and `isImportingCustomModule` * since they will have corresponding value already updated when reporting other * parts of the file. */ ImportDeclaration(node: TSESTree.ImportDeclaration) { - if (!isImportingTestingLibraryModule) { - // check only if testing library import not found yet so we avoid - // to override isImportingTestingLibraryModule after it's found - isImportingTestingLibraryModule = /testing-library/g.test( - node.source.value as string - ); + // 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 as string) + ) { + importedTestingLibraryNode = node; + } + + // check only if custom module import not found yet so we avoid + // to override importedCustomModuleNode after it's found + if ( + !importedCustomModuleNode && + String(node.source.value).endsWith(customModule) + ) { + importedCustomModuleNode = node; + } + }, + + // Check if Testing Library related modules are loaded with required. + [`CallExpression > Identifier[name="require"]`]( + node: TSESTree.Identifier + ) { + const callExpression = node.parent as TSESTree.CallExpression; + const { arguments: args } = callExpression; + + if ( + !importedTestingLibraryNode && + args.some( + (arg) => + isLiteral(arg) && + typeof arg.value === 'string' && + /testing-library/g.test(arg.value) + ) + ) { + importedTestingLibraryNode = callExpression; } - if (!isImportingCustomModule) { - // check only if custom module import not found yet so we avoid - // to override isImportingCustomModule after it's found - const importName = String(node.source.value); - isImportingCustomModule = importName.endsWith(customModule); + if ( + !importedCustomModuleNode && + args.some( + (arg) => + isLiteral(arg) && + typeof arg.value === 'string' && + arg.value.endsWith(customModule) + ) + ) { + importedCustomModuleNode = callExpression; } }, }; diff --git a/lib/node-utils.ts b/lib/node-utils.ts index d4220381..a4376732 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -27,8 +27,10 @@ export function isMemberExpression( return node && node.type === AST_NODE_TYPES.MemberExpression; } -export function isLiteral(node: TSESTree.Node): node is TSESTree.Literal { - return node && node.type === AST_NODE_TYPES.Literal; +export function isLiteral( + node: TSESTree.Node | null | undefined +): node is TSESTree.Literal { + return node?.type === AST_NODE_TYPES.Literal; } export function isImportSpecifier( diff --git a/lib/rules/no-dom-import.ts b/lib/rules/no-dom-import.ts index 09ad88ea..80db79aa 100644 --- a/lib/rules/no-dom-import.ts +++ b/lib/rules/no-dom-import.ts @@ -1,6 +1,9 @@ -import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; -import { getDocsUrl } from '../utils'; -import { isLiteral, isIdentifier } from '../node-utils'; +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'; export type MessageIds = 'noDomImport' | 'noDomImportFramework'; @@ -11,7 +14,7 @@ const DOM_TESTING_LIBRARY_MODULES = [ '@testing-library/dom', ]; -export default ESLintUtils.RuleCreator(getDocsUrl)({ +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'problem', @@ -35,7 +38,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ }, defaultOptions: [''], - create(context, [framework]) { + create(context, [framework], helpers) { function report( node: TSESTree.ImportDeclaration | TSESTree.Identifier, moduleName: string @@ -76,33 +79,38 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ }); } } + return { - ImportDeclaration(node) { - const value = node.source.value; - const domModuleName = DOM_TESTING_LIBRARY_MODULES.find( - (module) => module === value - ); + 'Program:exit'() { + const importNode = helpers.getTestingLibraryImportNode(); - if (domModuleName) { - report(node, domModuleName); + if (!importNode) { + return; } - }, - [`CallExpression > Identifier[name="require"]`]( - node: TSESTree.Identifier - ) { - const callExpression = node.parent as TSESTree.CallExpression; - const { arguments: args } = callExpression; + // 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 + ); + + domModuleName && report(importNode, domModuleName); + } - const literalNodeDomModuleName = args.find( - (args) => - isLiteral(args) && - typeof args.value === 'string' && - DOM_TESTING_LIBRARY_MODULES.includes(args.value) - ) as TSESTree.Literal; + // 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; - if (literalNodeDomModuleName) { - report(node, literalNodeDomModuleName.value as string); + literalNodeDomModuleName && + report( + importNode.callee as TSESTree.Identifier, + literalNodeDomModuleName.value as string + ); } }, }; diff --git a/tests/create-testing-library-rule.test.ts b/tests/create-testing-library-rule.test.ts index 1073b370..6ca48128 100644 --- a/tests/create-testing-library-rule.test.ts +++ b/tests/create-testing-library-rule.test.ts @@ -5,6 +5,7 @@ const ruleTester = createRuleTester(); ruleTester.run(RULE_NAME, rule, { valid: [ + // Test Cases for Imports & Filename { code: ` // case: nothing related to Testing Library at all @@ -13,6 +14,14 @@ ruleTester.run(RULE_NAME, rule, { const wrapper = shallow(); `, }, + { + code: ` + // case: nothing related to Testing Library at all (require version) + const { shallow } = require('enzyme'); + + const wrapper = shallow(); + `, + }, { code: ` // case: render imported from other than custom module @@ -24,10 +33,21 @@ ruleTester.run(RULE_NAME, rule, { 'testing-library/module': 'test-utils', }, }, + { + code: ` + // case: render imported from other than custom module (require version) + const { render } = require('@somewhere/else') + + const utils = render(); + `, + settings: { + 'testing-library/module': 'test-utils', + }, + }, { code: ` // case: prevent import which should trigger an error since it's imported - // from other than custom module + // from other than settings custom module import { foo } from 'report-me' `, settings: { @@ -36,15 +56,42 @@ ruleTester.run(RULE_NAME, rule, { }, { code: ` - // case: import module forced to be reported but not matching file name + // case: prevent import which should trigger an error since it's imported + // from other than settings custom module (require version) + const { foo } = require('report-me') + `, + settings: { + 'testing-library/module': 'test-utils', + }, + }, + { + code: ` + // case: import module forced to be reported but not matching settings filename import { foo } from 'report-me' `, settings: { - 'testing-library/file-name': 'testing-library\\.js', + 'testing-library/filename-pattern': 'testing-library\\.js', }, }, + { + code: ` + // case: import module forced to be reported but not matching settings filename + // (require version) + const { foo } = require('report-me') + `, + settings: { + 'testing-library/filename-pattern': 'testing-library\\.js', + }, + }, + { + code: ` + // case: import custom module forced to be reported without custom module setting + import { foo } from 'custom-module-forced-report' + `, + }, ], invalid: [ + // Test Cases for Imports & Filename { code: ` // case: import module forced to be reported @@ -67,7 +114,7 @@ ruleTester.run(RULE_NAME, rule, { import { foo } from 'report-me' `, settings: { - 'testing-library/file-name': 'testing-library\\.js', + 'testing-library/filename-pattern': 'testing-library\\.js', }, errors: [{ line: 3, column: 7, messageId: 'fakeError' }], }, @@ -92,12 +139,30 @@ ruleTester.run(RULE_NAME, rule, { // case: render imported from Testing Library module import { render } from '@testing-library/react' import { somethingElse } from 'another-module' + const foo = require('bar') const utils = render(); `, errors: [ { - line: 6, + line: 7, + column: 21, + messageId: 'fakeError', + }, + ], + }, + { + code: ` + // case: render imported from Testing Library module (require version) + const { render } = require('@testing-library/react') + import { somethingElse } from 'another-module' + const foo = require('bar') + + const utils = render(); + `, + errors: [ + { + line: 7, column: 21, messageId: 'fakeError', }, @@ -105,9 +170,10 @@ ruleTester.run(RULE_NAME, rule, { }, { code: ` - // case: render imported from config custom module + // case: render imported from settings custom module import { render } from 'test-utils' import { somethingElse } from 'another-module' + const foo = require('bar') const utils = render(); `, @@ -116,7 +182,7 @@ ruleTester.run(RULE_NAME, rule, { }, errors: [ { - line: 6, + line: 7, column: 21, messageId: 'fakeError', }, @@ -124,10 +190,10 @@ ruleTester.run(RULE_NAME, rule, { }, { code: ` - // case: render imported from Testing Library module if - // custom module setup - import { render } from '@testing-library/react' + // case: render imported from settings custom module (require version) + const { render } = require('test-utils') import { somethingElse } from 'another-module' + const foo = require('bar') const utils = render(); `, @@ -142,5 +208,57 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + { + code: ` + // case: render imported from Testing Library module with + // settings custom module + import { render } from '@testing-library/react' + import { somethingElse } from 'another-module' + const foo = require('bar') + + const utils = render(); + `, + settings: { + 'testing-library/module': 'test-utils', + }, + errors: [ + { + line: 8, + column: 21, + messageId: 'fakeError', + }, + ], + }, + { + code: ` + // case: render imported from Testing Library module with + // settings custom module (require version) + const { render } = require('@testing-library/react') + import { somethingElse } from 'another-module' + const foo = require('bar') + + const utils = render(); + `, + settings: { + 'testing-library/module': 'test-utils', + }, + errors: [ + { + line: 8, + column: 21, + messageId: 'fakeError', + }, + ], + }, + { + settings: { + 'testing-library/module': 'custom-module-forced-report', + }, + code: ` + // case: import custom module forced to be reported with custom module setting + import { foo } from 'custom-module-forced-report' + `, + errors: [{ line: 3, column: 7, messageId: 'fakeError' }], + }, ], }); diff --git a/tests/fake-rule.ts b/tests/fake-rule.ts index 1bad0a53..4281eeed 100644 --- a/tests/fake-rule.ts +++ b/tests/fake-rule.ts @@ -2,7 +2,10 @@ * @file Fake rule to be able to test createTestingLibraryRule and * detectTestingLibraryUtils properly */ -import { TSESTree } from '@typescript-eslint/experimental-utils'; +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; import { createTestingLibraryRule } from '../lib/create-testing-library-rule'; export const RULE_NAME = 'fake-rule'; @@ -25,7 +28,7 @@ export default createTestingLibraryRule({ schema: [], }, defaultOptions: [], - create(context) { + create(context, _, helpers) { const reportRenderIdentifier = (node: TSESTree.Identifier) => { if (node.name === 'render') { context.report({ @@ -50,6 +53,22 @@ export default createTestingLibraryRule({ return { 'CallExpression Identifier': reportRenderIdentifier, ImportDeclaration: checkImportDeclaration, + 'Program:exit'() { + const importNode = helpers.getCustomModuleImportNode(); + if (!importNode) { + return; + } + + if ( + importNode.type === AST_NODE_TYPES.ImportDeclaration && + importNode.source.value === 'custom-module-forced-report' + ) { + context.report({ + node: importNode, + messageId: 'fakeError', + }); + } + }, }; }, }); diff --git a/tests/lib/rules/no-dom-import.test.ts b/tests/lib/rules/no-dom-import.test.ts index 429c8789..01ba26c1 100644 --- a/tests/lib/rules/no-dom-import.test.ts +++ b/tests/lib/rules/no-dom-import.test.ts @@ -5,22 +5,58 @@ const ruleTester = createRuleTester(); ruleTester.run(RULE_NAME, rule, { valid: [ - { code: 'import { foo } from "foo"' }, - { code: 'import "foo"' }, - { code: 'import { fireEvent } from "react-testing-library"' }, - { code: 'import * as testing from "react-testing-library"' }, - { code: 'import { fireEvent } from "@testing-library/react"' }, - { code: 'import * as testing from "@testing-library/react"' }, - { code: 'import "react-testing-library"' }, - { code: 'import "@testing-library/react"' }, - { code: 'const { foo } = require("foo")' }, - { code: 'require("foo")' }, - { code: 'require("")' }, - { code: 'require()' }, - { code: 'const { fireEvent } = require("react-testing-library")' }, - { code: 'const { fireEvent } = require("@testing-library/react")' }, - { code: 'require("react-testing-library")' }, - { code: 'require("@testing-library/react")' }, + 'import { foo } from "foo"', + 'import "foo"', + 'import { fireEvent } from "react-testing-library"', + 'import * as testing from "react-testing-library"', + 'import { fireEvent } from "@testing-library/react"', + 'import * as testing from "@testing-library/react"', + 'import "react-testing-library"', + 'import "@testing-library/react"', + 'const { foo } = require("foo")', + 'require("foo")', + 'require("")', + 'require()', + 'const { fireEvent } = require("react-testing-library")', + 'const { fireEvent } = require("@testing-library/react")', + 'require("react-testing-library")', + 'require("@testing-library/react")', + { + code: 'import { fireEvent } from "test-utils"', + settings: { 'testing-library/module': 'test-utils' }, + }, + { + code: 'import { fireEvent } from "dom-testing-library"', + filename: 'filename.not-matching.js', + }, + { + code: 'import { fireEvent } from "dom-testing-library"', + settings: { 'testing-library/filename-pattern': '^.*\\.(nope)\\.js$' }, + }, + { + code: 'const { fireEvent } = require("dom-testing-library")', + filename: 'filename.not-matching.js', + }, + { + code: 'const { fireEvent } = require("dom-testing-library")', + settings: { 'testing-library/filename-pattern': '^.*\\.(nope)\\.js$' }, + }, + { + code: 'import { fireEvent } from "@testing-library/dom"', + filename: 'filename.not-matching.js', + }, + { + code: 'import { fireEvent } from "@testing-library/dom"', + settings: { 'testing-library/filename-pattern': '^.*\\.(nope)\\.js$' }, + }, + { + code: 'const { fireEvent } = require("@testing-library/dom")', + filename: 'filename.not-matching.js', + }, + { + code: 'const { fireEvent } = require("@testing-library/dom")', + settings: { 'testing-library/filename-pattern': '^.*\\.(nope)\\.js$' }, + }, ], invalid: [ { @@ -32,6 +68,23 @@ ruleTester.run(RULE_NAME, rule, { ], output: 'import { fireEvent } from "dom-testing-library"', }, + { + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + // case: dom-testing-library imported with custom module setting + import { fireEvent } from "dom-testing-library"`, + errors: [ + { + line: 3, + messageId: 'noDomImport', + }, + ], + output: ` + // case: dom-testing-library imported with custom module setting + import { fireEvent } from "dom-testing-library"`, + }, { code: 'import { fireEvent } from "dom-testing-library"', options: ['react'], @@ -67,6 +120,20 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + { + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + // case: dom-testing-library wildcard imported with custom module setting + import * as testing from "dom-testing-library"`, + errors: [ + { + line: 3, + messageId: 'noDomImport', + }, + ], + }, { code: 'import { fireEvent } from "@testing-library/dom"', errors: [ @@ -75,6 +142,20 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + { + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + // case: @testing-library/dom imported with custom module setting + import { fireEvent } from "@testing-library/dom"`, + errors: [ + { + line: 3, + messageId: 'noDomImport', + }, + ], + }, { code: 'import * as testing from "@testing-library/dom"', errors: [ @@ -107,6 +188,20 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + { + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + // case: dom-testing-library required with custom module setting + const { fireEvent } = require("dom-testing-library")`, + errors: [ + { + line: 3, + messageId: 'noDomImport', + }, + ], + }, { code: 'const { fireEvent } = require("@testing-library/dom")', errors: [ @@ -128,6 +223,26 @@ ruleTester.run(RULE_NAME, rule, { ], output: 'const { fireEvent } = require("@testing-library/vue")', }, + { + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + // case: @testing-library/dom required with custom module setting + const { fireEvent } = require("@testing-library/dom")`, + options: ['vue'], + errors: [ + { + messageId: 'noDomImportFramework', + data: { + module: '@testing-library/vue', + }, + }, + ], + output: ` + // case: @testing-library/dom required with custom module setting + const { fireEvent } = require("@testing-library/vue")`, + }, { code: 'require("dom-testing-library")', errors: [ diff --git a/tests/lib/rules/no-node-access.test.ts b/tests/lib/rules/no-node-access.test.ts index 83dcf04c..d8693ee4 100644 --- a/tests/lib/rules/no-node-access.test.ts +++ b/tests/lib/rules/no-node-access.test.ts @@ -54,7 +54,7 @@ ruleTester.run(RULE_NAME, rule, { } `, settings: { - 'testing-library/file-name': 'testing-library\\.js', + 'testing-library/filename-pattern': 'testing-library\\.js', }, }, {