From 40c3588fa94d9930137dfc6b7f03cb42c19bca0c Mon Sep 17 00:00:00 2001 From: Gonzalo D'Elia Date: Wed, 4 Nov 2020 23:47:36 -0300 Subject: [PATCH 1/3] feat: add new settings for prefer-user-event pt1 --- lib/node-utils.ts | 19 +++++++++++ lib/rules/prefer-user-event.ts | 59 +++++++++++----------------------- 2 files changed, 38 insertions(+), 40 deletions(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index a7c90332..b2090352 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -253,3 +253,22 @@ export function getImportModuleName( return node.arguments[0].value; } } + +export function getSpecifierFromImport( + node: ImportModuleNode, + specifierName: string +) { + if (isImportDeclaration(node)) { + const namedExport = node.specifiers.find( + (node) => isImportSpecifier(node) && node.imported.name === specifierName + ); + // it is "import { foo } from 'baz'"" + if (namedExport) { + return namedExport; + } + // it could be "import * as rtl from 'baz'" + return node.specifiers.find((n) => isImportNamespaceSpecifier(n)); + } else { + // TODO make it work for require + } +} diff --git a/lib/rules/prefer-user-event.ts b/lib/rules/prefer-user-event.ts index b2018ea7..f70d271f 100644 --- a/lib/rules/prefer-user-event.ts +++ b/lib/rules/prefer-user-event.ts @@ -1,9 +1,9 @@ -import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; -import { getDocsUrl, hasTestingLibraryImportModule } from '../utils'; +import { TSESTree } from '@typescript-eslint/experimental-utils'; +import { createTestingLibraryRule } from '../create-testing-library-rule'; import { - isImportSpecifier, isIdentifier, isMemberExpression, + getSpecifierFromImport, } from '../node-utils'; export const RULE_NAME = 'prefer-user-event'; @@ -65,7 +65,7 @@ function buildErrorMessage(fireEventMethod: string) { const fireEventMappedMethods = Object.keys(MappingToUserEvent); -export default ESLintUtils.RuleCreator(getDocsUrl)({ +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'suggestion', @@ -90,59 +90,38 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ }, defaultOptions: [{ allowedMethods: [] }], - create(context, [options]) { + create(context, [options], helpers) { const { allowedMethods } = options; const sourceCode = context.getSourceCode(); - let hasNamedImportedFireEvent = false; - let hasImportedFireEvent = false; - let fireEventAlias: string | undefined; - let wildcardImportName: string | undefined; return { - // checks if import has shape: - // import { fireEvent } from '@testing-library/dom'; - ImportDeclaration(node: TSESTree.ImportDeclaration) { - if (!hasTestingLibraryImportModule(node)) { - return; - } - const fireEventImport = node.specifiers.find( - (node) => - isImportSpecifier(node) && node.imported.name === 'fireEvent' - ); - hasNamedImportedFireEvent = !!fireEventImport; - if (!hasNamedImportedFireEvent) { + ['CallExpression > MemberExpression'](node: TSESTree.MemberExpression) { + if (!helpers.getIsTestingLibraryImported()) { return; } - fireEventAlias = fireEventImport.local.name; - }, + const testingLibraryImportNode = helpers.getTestingLibraryImportNode(); + const fireEventAliasOrWildcard = getSpecifierFromImport( + testingLibraryImportNode, + 'fireEvent' + )?.local.name; - // checks if import has shape: - // import * as dom from '@testing-library/dom'; - 'ImportDeclaration ImportNamespaceSpecifier'( - node: TSESTree.ImportNamespaceSpecifier - ) { - const importDeclarationNode = node.parent as TSESTree.ImportDeclaration; - if (!hasTestingLibraryImportModule(importDeclarationNode)) { + if (!fireEventAliasOrWildcard) { + // testing library was imported, but fireEvent was not imported return; } - hasImportedFireEvent = !!node.local.name; - wildcardImportName = node.local.name; - }, - ['CallExpression > MemberExpression'](node: TSESTree.MemberExpression) { - if (!hasImportedFireEvent && !hasNamedImportedFireEvent) { - return; - } - // check node is fireEvent or it's alias from the named import const fireEventUsed = - isIdentifier(node.object) && node.object.name === fireEventAlias; + isIdentifier(node.object) && + node.object.name === fireEventAliasOrWildcard; + const fireEventFromWildcardUsed = isMemberExpression(node.object) && isIdentifier(node.object.object) && - node.object.object.name === wildcardImportName && + node.object.object.name === fireEventAliasOrWildcard && isIdentifier(node.object.property) && node.object.property.name === 'fireEvent'; if (!fireEventUsed && !fireEventFromWildcardUsed) { + // fireEvent was imported but it was not used return; } From f12acf532720fb60f023e8d433d5ffe0c88d7fea Mon Sep 17 00:00:00 2001 From: Gonzalo D'Elia Date: Sat, 7 Nov 2020 19:41:31 -0300 Subject: [PATCH 2/3] feat: part2 of refactoring user event. improved docs --- docs/rules/prefer-user-event.md | 8 ++ lib/detect-testing-library-utils.ts | 55 ++++++++++- lib/node-utils.ts | 19 ---- lib/rules/prefer-user-event.ts | 18 +--- tests/lib/rules/prefer-user-event.test.ts | 109 ++++++++++++++++++++++ 5 files changed, 175 insertions(+), 34 deletions(-) diff --git a/docs/rules/prefer-user-event.md b/docs/rules/prefer-user-event.md index 25aa69e6..a4f539b3 100644 --- a/docs/rules/prefer-user-event.md +++ b/docs/rules/prefer-user-event.md @@ -18,6 +18,7 @@ Examples of **incorrect** code for this rule: ```ts // a method in fireEvent that has a userEvent equivalent import { fireEvent } from '@testing-library/dom'; +// or const { fireEvent } = require('@testing-library/dom'); fireEvent.click(node); // using fireEvent with an alias @@ -26,6 +27,7 @@ fireEventAliased.click(node); // using fireEvent after importing the entire library import * as dom from '@testing-library/dom'; +// or const dom = require(@testing-library/dom'); dom.fireEvent.click(node); ``` @@ -33,14 +35,18 @@ Examples of **correct** code for this rule: ```ts import userEvent from '@testing-library/user-event'; +// or const userEvent = require('@testing-library/user-event'); // any userEvent method userEvent.click(); // fireEvent method that does not have an alternative in userEvent +import { fireEvent } from '@testing-library/dom'; +// or const { fireEvent } = require('@testing-library/dom'); fireEvent.cut(node); import * as dom from '@testing-library/dom'; +// or const dom = require('@testing-library/dom'); dom.fireEvent.cut(node); ``` @@ -69,6 +75,7 @@ With this configuration example, the following use cases are considered valid ```ts // using a named import import { fireEvent } from '@testing-library/dom'; +// or const { fireEvent } = require('@testing-library/dom'); fireEvent.click(node); fireEvent.change(node, { target: { value: 'foo' } }); @@ -79,6 +86,7 @@ fireEventAliased.change(node, { target: { value: 'foo' } }); // using fireEvent after importing the entire library import * as dom from '@testing-library/dom'; +// or const dom = require('@testing-library/dom'); dom.fireEvent.click(node); dom.fireEvent.change(node, { target: { value: 'foo' } }); ``` diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 7ca83d1e..2e0ca930 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -1,5 +1,14 @@ import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; -import { getImportModuleName, isLiteral, ImportModuleNode } from './node-utils'; +import { + getImportModuleName, + isLiteral, + ImportModuleNode, + isImportDeclaration, + isImportNamespaceSpecifier, + isImportSpecifier, + isIdentifier, + isProperty, +} from './node-utils'; export type TestingLibrarySettings = { 'testing-library/module'?: string; @@ -33,6 +42,9 @@ export type DetectionHelpers = { getIsTestingLibraryImported: () => boolean; getIsValidFilename: () => boolean; canReportErrors: () => boolean; + findImportedUtilSpecifier: ( + specifierName: string + ) => TSESTree.ImportClause | TSESTree.Identifier | undefined; }; const DEFAULT_FILENAME_PATTERN = '^.*\\.(test|spec)\\.[jt]sx?$'; @@ -106,7 +118,46 @@ export function detectTestingLibraryUtils< * Wraps all conditions that must be met to report rules. */ canReportErrors() { - return this.getIsTestingLibraryImported() && this.getIsValidFilename(); + return ( + helpers.getIsTestingLibraryImported() && helpers.getIsValidFilename() + ); + }, + /** + * Gets a string and verifies if it was imported/required by our custom module node + */ + findImportedUtilSpecifier(specifierName: string) { + const node = + helpers.getCustomModuleImportNode() ?? + helpers.getTestingLibraryImportNode(); + if (!node) { + return null; + } + if (isImportDeclaration(node)) { + const namedExport = node.specifiers.find( + (n) => isImportSpecifier(n) && n.imported.name === specifierName + ); + // it is "import { foo [as alias] } from 'baz'"" + if (namedExport) { + return namedExport; + } + // it could be "import * as rtl from 'baz'" + return node.specifiers.find((n) => isImportNamespaceSpecifier(n)); + } else { + const requireNode = node.parent as TSESTree.VariableDeclarator; + if (isIdentifier(requireNode.id)) { + // this is const rtl = require('foo') + return requireNode.id; + } + // this should be const { something } = require('foo') + const destructuring = requireNode.id as TSESTree.ObjectPattern; + const property = destructuring.properties.find( + (n) => + isProperty(n) && + isIdentifier(n.key) && + n.key.name === specifierName + ); + return (property as TSESTree.Property).key as TSESTree.Identifier; + } }, }; diff --git a/lib/node-utils.ts b/lib/node-utils.ts index b2090352..a7c90332 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -253,22 +253,3 @@ export function getImportModuleName( return node.arguments[0].value; } } - -export function getSpecifierFromImport( - node: ImportModuleNode, - specifierName: string -) { - if (isImportDeclaration(node)) { - const namedExport = node.specifiers.find( - (node) => isImportSpecifier(node) && node.imported.name === specifierName - ); - // it is "import { foo } from 'baz'"" - if (namedExport) { - return namedExport; - } - // it could be "import * as rtl from 'baz'" - return node.specifiers.find((n) => isImportNamespaceSpecifier(n)); - } else { - // TODO make it work for require - } -} diff --git a/lib/rules/prefer-user-event.ts b/lib/rules/prefer-user-event.ts index f70d271f..754c289a 100644 --- a/lib/rules/prefer-user-event.ts +++ b/lib/rules/prefer-user-event.ts @@ -1,10 +1,6 @@ import { TSESTree } from '@typescript-eslint/experimental-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; -import { - isIdentifier, - isMemberExpression, - getSpecifierFromImport, -} from '../node-utils'; +import { isIdentifier, isMemberExpression } from '../node-utils'; export const RULE_NAME = 'prefer-user-event'; @@ -96,14 +92,10 @@ export default createTestingLibraryRule({ return { ['CallExpression > MemberExpression'](node: TSESTree.MemberExpression) { - if (!helpers.getIsTestingLibraryImported()) { - return; - } - const testingLibraryImportNode = helpers.getTestingLibraryImportNode(); - const fireEventAliasOrWildcard = getSpecifierFromImport( - testingLibraryImportNode, - 'fireEvent' - )?.local.name; + const util = helpers.findImportedUtilSpecifier('fireEvent'); + const fireEventAliasOrWildcard = isIdentifier(util) + ? util?.name + : util?.local.name; if (!fireEventAliasOrWildcard) { // testing library was imported, but fireEvent was not imported diff --git a/tests/lib/rules/prefer-user-event.test.ts b/tests/lib/rules/prefer-user-event.test.ts index 1ff87da0..24bbe322 100644 --- a/tests/lib/rules/prefer-user-event.test.ts +++ b/tests/lib/rules/prefer-user-event.test.ts @@ -105,6 +105,67 @@ ruleTester.run(RULE_NAME, rule, { fireEvent() `, })), + { + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import { screen } from 'test-utils' + const element = screen.getByText(foo) + `, + }, + { + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import { render } from 'test-utils' + const utils = render(baz) + const element = utils.getByText(foo) + `, + }, + ...UserEventMethods.map((userEventMethod) => ({ + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import userEvent from 'test-utils' + const node = document.createElement(elementType) + userEvent.${userEventMethod}(foo) + `, + })), + ...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({ + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import { fireEvent } from 'test-utils' + const node = document.createElement(elementType) + fireEvent.${fireEventMethod}(foo) + `, + options: [{ allowedMethods: [fireEventMethod] }], + })), + ...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({ + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import { fireEvent as fireEventAliased } from 'test-utils' + const node = document.createElement(elementType) + fireEventAliased.${fireEventMethod}(foo) + `, + options: [{ allowedMethods: [fireEventMethod] }], + })), + ...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({ + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import * as dom from 'test-utils' + dom.fireEvent.${fireEventMethod}(foo) + `, + options: [{ allowedMethods: [fireEventMethod] }], + })), ], invalid: [ ...createScenarioWithImport>( @@ -130,5 +191,53 @@ ruleTester.run(RULE_NAME, rule, { errors: [{ messageId: 'preferUserEvent' }], }) ), + ...createScenarioWithImport>( + (libraryModule: string, fireEventMethod: string) => ({ + code: ` + const { fireEvent } = require('${libraryModule}') + fireEvent.${fireEventMethod}(foo) + `, + errors: [{ messageId: 'preferUserEvent' }], + }) + ), + ...createScenarioWithImport>( + (libraryModule: string, fireEventMethod: string) => ({ + code: ` + const rtl = require('${libraryModule}') + rtl.fireEvent.${fireEventMethod}(foo) + `, + errors: [{ messageId: 'preferUserEvent' }], + }) + ), + ...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({ + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import * as dom from 'test-utils' + dom.fireEvent.${fireEventMethod}(foo) + `, + errors: [{ messageId: 'preferUserEvent' }], + })), + ...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({ + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import { fireEvent } from 'test-utils' + fireEvent.${fireEventMethod}(foo) + `, + errors: [{ messageId: 'preferUserEvent' }], + })), + ...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({ + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import { fireEvent as fireEventAliased } from 'test-utils' + fireEventAliased.${fireEventMethod}(foo) + `, + errors: [{ messageId: 'preferUserEvent' }], + })), ], }); From 06cb9cf45c0a78f963461dcaeb3230840ed657e0 Mon Sep 17 00:00:00 2001 From: Gonzalo D'Elia Date: Sun, 8 Nov 2020 17:34:42 -0300 Subject: [PATCH 3/3] test: improved coverage for prefer-user-event. applied feedback --- lib/rules/prefer-user-event.ts | 10 +++++----- tests/lib/rules/prefer-user-event.test.ts | 14 ++++++++------ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/rules/prefer-user-event.ts b/lib/rules/prefer-user-event.ts index 754c289a..aae27e65 100644 --- a/lib/rules/prefer-user-event.ts +++ b/lib/rules/prefer-user-event.ts @@ -93,14 +93,14 @@ export default createTestingLibraryRule({ return { ['CallExpression > MemberExpression'](node: TSESTree.MemberExpression) { const util = helpers.findImportedUtilSpecifier('fireEvent'); - const fireEventAliasOrWildcard = isIdentifier(util) - ? util?.name - : util?.local.name; - - if (!fireEventAliasOrWildcard) { + if (!util) { // testing library was imported, but fireEvent was not imported return; } + const fireEventAliasOrWildcard = isIdentifier(util) + ? util.name + : util.local.name; + const fireEventUsed = isIdentifier(node.object) && node.object.name === fireEventAliasOrWildcard; diff --git a/tests/lib/rules/prefer-user-event.test.ts b/tests/lib/rules/prefer-user-event.test.ts index 24bbe322..b1a4158a 100644 --- a/tests/lib/rules/prefer-user-event.test.ts +++ b/tests/lib/rules/prefer-user-event.test.ts @@ -178,6 +178,8 @@ ruleTester.run(RULE_NAME, rule, { errors: [ { messageId: 'preferUserEvent', + line: 4, + column: 9, }, ], }) @@ -188,7 +190,7 @@ ruleTester.run(RULE_NAME, rule, { import * as dom from '${libraryModule}' dom.fireEvent.${fireEventMethod}(foo) `, - errors: [{ messageId: 'preferUserEvent' }], + errors: [{ messageId: 'preferUserEvent', line: 3, column: 9 }], }) ), ...createScenarioWithImport>( @@ -197,7 +199,7 @@ ruleTester.run(RULE_NAME, rule, { const { fireEvent } = require('${libraryModule}') fireEvent.${fireEventMethod}(foo) `, - errors: [{ messageId: 'preferUserEvent' }], + errors: [{ messageId: 'preferUserEvent', line: 3, column: 9 }], }) ), ...createScenarioWithImport>( @@ -206,7 +208,7 @@ ruleTester.run(RULE_NAME, rule, { const rtl = require('${libraryModule}') rtl.fireEvent.${fireEventMethod}(foo) `, - errors: [{ messageId: 'preferUserEvent' }], + errors: [{ messageId: 'preferUserEvent', line: 3, column: 9 }], }) ), ...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({ @@ -217,7 +219,7 @@ ruleTester.run(RULE_NAME, rule, { import * as dom from 'test-utils' dom.fireEvent.${fireEventMethod}(foo) `, - errors: [{ messageId: 'preferUserEvent' }], + errors: [{ messageId: 'preferUserEvent', line: 3, column: 9 }], })), ...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({ settings: { @@ -227,7 +229,7 @@ ruleTester.run(RULE_NAME, rule, { import { fireEvent } from 'test-utils' fireEvent.${fireEventMethod}(foo) `, - errors: [{ messageId: 'preferUserEvent' }], + errors: [{ messageId: 'preferUserEvent', line: 3, column: 9 }], })), ...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({ settings: { @@ -237,7 +239,7 @@ ruleTester.run(RULE_NAME, rule, { import { fireEvent as fireEventAliased } from 'test-utils' fireEventAliased.${fireEventMethod}(foo) `, - errors: [{ messageId: 'preferUserEvent' }], + errors: [{ messageId: 'preferUserEvent', line: 3, column: 9 }], })), ], });