From 0fec322aa61aa8c3ab338c709035db54bd7ef9f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Mon, 26 Oct 2020 18:19:17 +0100 Subject: [PATCH 01/16] feat: new setting for customizing file name pattern to report --- lib/detect-testing-library-utils.ts | 19 +++++++++- tests/create-testing-library-rule.test.ts | 43 ++++++++++++++++++++--- tests/lib/rules/no-node-access.test.ts | 31 ++++++++++++---- tests/lib/test-utils.ts | 4 +++ 4 files changed, 84 insertions(+), 13 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index cf5ca51f..0ca45a53 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -2,6 +2,7 @@ import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; export type TestingLibrarySettings = { 'testing-library/module'?: string; + 'testing-library/file-name'?: string; }; export type TestingLibraryContext< @@ -25,9 +26,12 @@ export type EnhancedRuleCreate< export type DetectionHelpers = { getIsTestingLibraryImported: () => boolean; + getIsValidFileName: () => boolean; canReportErrors: () => boolean; }; +const DEFAULT_FILE_NAME_PATTERN = '^.*\\.(test|spec)\\.[jt]sx?$'; + /** * Enhances a given rule `create` with helpers to detect Testing Library utils. */ @@ -45,6 +49,9 @@ export function detectTestingLibraryUtils< // 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; // Helpers for Testing Library detection. const helpers: DetectionHelpers = { @@ -68,11 +75,21 @@ export function detectTestingLibraryUtils< return isImportingTestingLibraryModule || isImportingCustomModule; }, + /** + * Gets if file name being analyzed is valid or not. + * + * This is based on "testing-library/file-name" setting. + */ + getIsValidFileName() { + const fileName = context.getFilename(); + return !!fileName.match(fileNamePattern); + }, + /** * Wraps all conditions that must be met to report rules. */ canReportErrors() { - return this.getIsTestingLibraryImported(); + return this.getIsTestingLibraryImported() && this.getIsValidFileName(); }, }; diff --git a/tests/create-testing-library-rule.test.ts b/tests/create-testing-library-rule.test.ts index 14977c72..566eb318 100644 --- a/tests/create-testing-library-rule.test.ts +++ b/tests/create-testing-library-rule.test.ts @@ -1,15 +1,12 @@ import { createRuleTester } from './lib/test-utils'; import rule, { RULE_NAME } from './fake-rule'; -const ruleTester = createRuleTester({ - ecmaFeatures: { - jsx: true, - }, -}); +const ruleTester = createRuleTester(); ruleTester.run(RULE_NAME, rule, { valid: [ { + filename: 'MyComponent.test.js', code: ` // case: nothing related to Testing Library at all import { shallow } from 'enzyme'; @@ -18,6 +15,7 @@ ruleTester.run(RULE_NAME, rule, { `, }, { + filename: 'MyComponent.test.js', code: ` // case: render imported from other than custom module import { render } from '@somewhere/else' @@ -29,6 +27,7 @@ ruleTester.run(RULE_NAME, rule, { }, }, { + filename: 'MyComponent.test.js', code: ` // case: prevent import which should trigger an error since it's imported // from other than custom module @@ -38,9 +37,20 @@ ruleTester.run(RULE_NAME, rule, { 'testing-library/module': 'test-utils', }, }, + { + filename: 'MyComponent.test.js', + code: ` + // case: import module forced to be reported but not matching file name + import { foo } from 'report-me' + `, + settings: { + 'testing-library/file-name': 'testing-library\\.js', + }, + }, ], invalid: [ { + filename: 'MyComponent.test.js', code: ` // case: import module forced to be reported import { foo } from 'report-me' @@ -48,6 +58,26 @@ ruleTester.run(RULE_NAME, rule, { errors: [{ line: 3, column: 7, messageId: 'fakeError' }], }, { + filename: 'MyComponent.spec.js', + code: ` + // case: import module forced to be reported but from .spec.js named file + import { foo } from 'report-me' + `, + errors: [{ line: 3, column: 7, messageId: 'fakeError' }], + }, + { + filename: 'MyComponent.testing-library.js', + code: ` + // case: import module forced to be reported with custom file name + import { foo } from 'report-me' + `, + settings: { + 'testing-library/file-name': 'testing-library\\.js', + }, + errors: [{ line: 3, column: 7, messageId: 'fakeError' }], + }, + { + filename: 'MyComponent.test.js', code: ` // case: render imported from any module by default (aggressive reporting) import { render } from '@somewhere/else' @@ -64,6 +94,7 @@ ruleTester.run(RULE_NAME, rule, { ], }, { + filename: 'MyComponent.test.js', code: ` // case: render imported from Testing Library module import { render } from '@testing-library/react' @@ -80,6 +111,7 @@ ruleTester.run(RULE_NAME, rule, { ], }, { + filename: 'MyComponent.test.js', code: ` // case: render imported from config custom module import { render } from 'test-utils' @@ -99,6 +131,7 @@ ruleTester.run(RULE_NAME, rule, { ], }, { + filename: 'MyComponent.test.js', code: ` // case: render imported from Testing Library module if // custom module setup diff --git a/tests/lib/rules/no-node-access.test.ts b/tests/lib/rules/no-node-access.test.ts index 9c7cbcc4..54ec18fc 100644 --- a/tests/lib/rules/no-node-access.test.ts +++ b/tests/lib/rules/no-node-access.test.ts @@ -10,6 +10,7 @@ const ruleTester = createRuleTester({ ruleTester.run(RULE_NAME, rule, { valid: [ { + filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -17,6 +18,7 @@ ruleTester.run(RULE_NAME, rule, { `, }, { + filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -26,6 +28,7 @@ ruleTester.run(RULE_NAME, rule, { `, }, { + filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -34,6 +37,7 @@ ruleTester.run(RULE_NAME, rule, { `, }, { + filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -43,6 +47,7 @@ ruleTester.run(RULE_NAME, rule, { `, }, { + filename: 'MyComponent.test.js', code: ` import { render, within } from '@testing-library/react'; @@ -51,19 +56,19 @@ ruleTester.run(RULE_NAME, rule, { within(signinModal).getByPlaceholderText('Username'); `, }, - /*{ - // TODO: this one should be valid indeed. Rule implementation must be improved - // to track where the nodes are coming from. This one wasn't reported before - // just because this code is not importing TL module, but that's a really - // brittle check. Instead, this one shouldn't be reported since `children` - // it's just a property not related to a node + { + filename: 'MyComponent.test.js', code: ` const Component = props => { return
{props.children}
} `, - },*/ + settings: { + 'testing-library/file-name': 'testing-library\\.js', + }, + }, { + filename: 'MyComponent.test.js', code: ` // case: importing custom module const closestButton = document.getElementById('submit-btn').closest('button'); @@ -76,6 +81,7 @@ ruleTester.run(RULE_NAME, rule, { ], invalid: [ { + filename: 'MyComponent.test.js', code: ` // case: without importing TL (aggressive reporting) const closestButton = document.getElementById('submit-btn') @@ -84,6 +90,7 @@ ruleTester.run(RULE_NAME, rule, { errors: [{ messageId: 'noNodeAccess', line: 3 }], }, { + filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -99,6 +106,7 @@ ruleTester.run(RULE_NAME, rule, { ], }, { + filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -111,6 +119,7 @@ ruleTester.run(RULE_NAME, rule, { ], }, { + filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -126,6 +135,7 @@ ruleTester.run(RULE_NAME, rule, { ], }, { + filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -138,6 +148,7 @@ ruleTester.run(RULE_NAME, rule, { ], }, { + filename: 'MyComponent.test.js', code: ` import { render } from '@testing-library/react'; @@ -151,6 +162,7 @@ ruleTester.run(RULE_NAME, rule, { ], }, { + filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -175,6 +187,7 @@ ruleTester.run(RULE_NAME, rule, { ], }, { + filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -188,6 +201,7 @@ ruleTester.run(RULE_NAME, rule, { ], }, { + filename: 'MyComponent.test.js', code: ` import { render } from '@testing-library/react'; @@ -202,6 +216,7 @@ ruleTester.run(RULE_NAME, rule, { ], }, { + filename: 'MyComponent.test.js', code: ` import { render } from '@testing-library/react'; @@ -215,6 +230,7 @@ ruleTester.run(RULE_NAME, rule, { ], }, { + filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -245,6 +261,7 @@ ruleTester.run(RULE_NAME, rule, { ], }, { + filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; diff --git a/tests/lib/test-utils.ts b/tests/lib/test-utils.ts index 391f5844..b318e0b2 100644 --- a/tests/lib/test-utils.ts +++ b/tests/lib/test-utils.ts @@ -9,6 +9,10 @@ export const createRuleTester = ( parserOptions: { ecmaVersion: 2018, sourceType: 'module', + // TODO: we should deep merge here + ecmaFeatures: { + jsx: true, + }, ...parserOptions, }, }); From 4ec2a687e921f014103fb8f7bbfcd6c710df3152 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Tue, 27 Oct 2020 18:27:57 +0100 Subject: [PATCH 02/16] test: add custom rule tester for testing library --- lib/detect-testing-library-utils.ts | 2 +- tests/create-testing-library-rule.test.ts | 9 ----- tests/lib/test-utils.ts | 40 +++++++++++++++++++++-- tsconfig.json | 2 +- 4 files changed, 39 insertions(+), 14 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 0ca45a53..31b4c809 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -76,7 +76,7 @@ export function detectTestingLibraryUtils< }, /** - * Gets if file name being analyzed is valid or not. + * Gets if name of the file being analyzed is valid or not. * * This is based on "testing-library/file-name" setting. */ diff --git a/tests/create-testing-library-rule.test.ts b/tests/create-testing-library-rule.test.ts index 566eb318..1073b370 100644 --- a/tests/create-testing-library-rule.test.ts +++ b/tests/create-testing-library-rule.test.ts @@ -6,7 +6,6 @@ const ruleTester = createRuleTester(); ruleTester.run(RULE_NAME, rule, { valid: [ { - filename: 'MyComponent.test.js', code: ` // case: nothing related to Testing Library at all import { shallow } from 'enzyme'; @@ -15,7 +14,6 @@ ruleTester.run(RULE_NAME, rule, { `, }, { - filename: 'MyComponent.test.js', code: ` // case: render imported from other than custom module import { render } from '@somewhere/else' @@ -27,7 +25,6 @@ ruleTester.run(RULE_NAME, rule, { }, }, { - filename: 'MyComponent.test.js', code: ` // case: prevent import which should trigger an error since it's imported // from other than custom module @@ -38,7 +35,6 @@ ruleTester.run(RULE_NAME, rule, { }, }, { - filename: 'MyComponent.test.js', code: ` // case: import module forced to be reported but not matching file name import { foo } from 'report-me' @@ -50,7 +46,6 @@ ruleTester.run(RULE_NAME, rule, { ], invalid: [ { - filename: 'MyComponent.test.js', code: ` // case: import module forced to be reported import { foo } from 'report-me' @@ -77,7 +72,6 @@ ruleTester.run(RULE_NAME, rule, { errors: [{ line: 3, column: 7, messageId: 'fakeError' }], }, { - filename: 'MyComponent.test.js', code: ` // case: render imported from any module by default (aggressive reporting) import { render } from '@somewhere/else' @@ -94,7 +88,6 @@ ruleTester.run(RULE_NAME, rule, { ], }, { - filename: 'MyComponent.test.js', code: ` // case: render imported from Testing Library module import { render } from '@testing-library/react' @@ -111,7 +104,6 @@ ruleTester.run(RULE_NAME, rule, { ], }, { - filename: 'MyComponent.test.js', code: ` // case: render imported from config custom module import { render } from 'test-utils' @@ -131,7 +123,6 @@ ruleTester.run(RULE_NAME, rule, { ], }, { - filename: 'MyComponent.test.js', code: ` // case: render imported from Testing Library module if // custom module setup diff --git a/tests/lib/test-utils.ts b/tests/lib/test-utils.ts index b318e0b2..263cc6e7 100644 --- a/tests/lib/test-utils.ts +++ b/tests/lib/test-utils.ts @@ -1,18 +1,52 @@ import { resolve } from 'path'; import { TSESLint } from '@typescript-eslint/experimental-utils'; +// TODO: remove ecmaFeatures object from other tests files + +const DEFAULT_TEST_CASE_CONFIG = { + filename: 'MyComponent.test.js', +}; + +class TestingLibraryRuleTester extends TSESLint.RuleTester { + run>( + ruleName: string, + rule: TSESLint.RuleModule, + tests: TSESLint.RunTests + ): void { + const { valid, invalid } = tests; + + const finalValid = valid.map((testCase) => { + if (typeof testCase === 'string') { + return { + ...DEFAULT_TEST_CASE_CONFIG, + code: testCase, + }; + } + + return { ...DEFAULT_TEST_CASE_CONFIG, ...testCase }; + }); + const finalInvalid = invalid.map((testCase) => ({ + ...DEFAULT_TEST_CASE_CONFIG, + ...testCase, + })); + + super.run(ruleName, rule, { valid: finalValid, invalid: finalInvalid }); + } +} + export const createRuleTester = ( parserOptions: Partial = {} -): TSESLint.RuleTester => - new TSESLint.RuleTester({ +): TSESLint.RuleTester => { + return new TestingLibraryRuleTester({ parser: resolve('./node_modules/@typescript-eslint/parser'), parserOptions: { ecmaVersion: 2018, sourceType: 'module', - // TODO: we should deep merge here + // TODO: should we deep merge here? ecmaFeatures: { jsx: true, }, ...parserOptions, }, }); +}; diff --git a/tsconfig.json b/tsconfig.json index cddbaa37..1856daa6 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,6 +1,6 @@ { "compilerOptions": { - "target": "es5", + "target": "es6", "module": "commonjs", "moduleResolution": "node", "esModuleInterop": true, From 6f92a9058a2a5d1406dff79f824fccb8cbc14d29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Wed, 28 Oct 2020 10:23:21 +0100 Subject: [PATCH 03/16] refactor: use common rule tester config --- .../lib/rules/consistent-data-testid.test.ts | 6 +---- tests/lib/rules/no-container.test.ts | 6 +---- tests/lib/rules/no-debug.test.ts | 6 +---- .../no-multiple-assertions-wait-for.test.ts | 6 +---- tests/lib/rules/no-node-access.test.ts | 25 +------------------ tests/lib/rules/no-render-in-setup.test.ts | 6 +---- .../rules/no-side-effects-wait-for.test.ts | 6 +---- tests/lib/rules/prefer-find-by.test.ts | 6 +---- .../render-result-naming-convention.test.ts | 6 +---- tests/lib/test-utils.ts | 3 --- 10 files changed, 9 insertions(+), 67 deletions(-) diff --git a/tests/lib/rules/consistent-data-testid.test.ts b/tests/lib/rules/consistent-data-testid.test.ts index cb7ba92c..dd0892f2 100644 --- a/tests/lib/rules/consistent-data-testid.test.ts +++ b/tests/lib/rules/consistent-data-testid.test.ts @@ -1,11 +1,7 @@ import { createRuleTester } from '../test-utils'; import rule, { RULE_NAME } from '../../../lib/rules/consistent-data-testid'; -const ruleTester = createRuleTester({ - ecmaFeatures: { - jsx: true, - }, -}); +const ruleTester = createRuleTester(); ruleTester.run(RULE_NAME, rule, { valid: [ diff --git a/tests/lib/rules/no-container.test.ts b/tests/lib/rules/no-container.test.ts index a6a6116a..4823e9a6 100644 --- a/tests/lib/rules/no-container.test.ts +++ b/tests/lib/rules/no-container.test.ts @@ -1,11 +1,7 @@ import { createRuleTester } from '../test-utils'; import rule, { RULE_NAME } from '../../../lib/rules/no-container'; -const ruleTester = createRuleTester({ - ecmaFeatures: { - jsx: true, - }, -}); +const ruleTester = createRuleTester(); ruleTester.run(RULE_NAME, rule, { valid: [ diff --git a/tests/lib/rules/no-debug.test.ts b/tests/lib/rules/no-debug.test.ts index ad175bac..3e26b414 100644 --- a/tests/lib/rules/no-debug.test.ts +++ b/tests/lib/rules/no-debug.test.ts @@ -1,11 +1,7 @@ import { createRuleTester } from '../test-utils'; import rule, { RULE_NAME } from '../../../lib/rules/no-debug'; -const ruleTester = createRuleTester({ - ecmaFeatures: { - jsx: true, - }, -}); +const ruleTester = createRuleTester(); ruleTester.run(RULE_NAME, rule, { valid: [ diff --git a/tests/lib/rules/no-multiple-assertions-wait-for.test.ts b/tests/lib/rules/no-multiple-assertions-wait-for.test.ts index 44bef16a..4398662a 100644 --- a/tests/lib/rules/no-multiple-assertions-wait-for.test.ts +++ b/tests/lib/rules/no-multiple-assertions-wait-for.test.ts @@ -3,11 +3,7 @@ import rule, { RULE_NAME, } from '../../../lib/rules/no-multiple-assertions-wait-for'; -const ruleTester = createRuleTester({ - ecmaFeatures: { - jsx: true, - }, -}); +const ruleTester = createRuleTester(); ruleTester.run(RULE_NAME, rule, { valid: [ diff --git a/tests/lib/rules/no-node-access.test.ts b/tests/lib/rules/no-node-access.test.ts index 54ec18fc..83dcf04c 100644 --- a/tests/lib/rules/no-node-access.test.ts +++ b/tests/lib/rules/no-node-access.test.ts @@ -1,16 +1,11 @@ import { createRuleTester } from '../test-utils'; import rule, { RULE_NAME } from '../../../lib/rules/no-node-access'; -const ruleTester = createRuleTester({ - ecmaFeatures: { - jsx: true, - }, -}); +const ruleTester = createRuleTester(); ruleTester.run(RULE_NAME, rule, { valid: [ { - filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -18,7 +13,6 @@ ruleTester.run(RULE_NAME, rule, { `, }, { - filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -28,7 +22,6 @@ ruleTester.run(RULE_NAME, rule, { `, }, { - filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -37,7 +30,6 @@ ruleTester.run(RULE_NAME, rule, { `, }, { - filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -47,7 +39,6 @@ ruleTester.run(RULE_NAME, rule, { `, }, { - filename: 'MyComponent.test.js', code: ` import { render, within } from '@testing-library/react'; @@ -57,7 +48,6 @@ ruleTester.run(RULE_NAME, rule, { `, }, { - filename: 'MyComponent.test.js', code: ` const Component = props => { return
{props.children}
@@ -68,7 +58,6 @@ ruleTester.run(RULE_NAME, rule, { }, }, { - filename: 'MyComponent.test.js', code: ` // case: importing custom module const closestButton = document.getElementById('submit-btn').closest('button'); @@ -81,7 +70,6 @@ ruleTester.run(RULE_NAME, rule, { ], invalid: [ { - filename: 'MyComponent.test.js', code: ` // case: without importing TL (aggressive reporting) const closestButton = document.getElementById('submit-btn') @@ -90,7 +78,6 @@ ruleTester.run(RULE_NAME, rule, { errors: [{ messageId: 'noNodeAccess', line: 3 }], }, { - filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -106,7 +93,6 @@ ruleTester.run(RULE_NAME, rule, { ], }, { - filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -119,7 +105,6 @@ ruleTester.run(RULE_NAME, rule, { ], }, { - filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -135,7 +120,6 @@ ruleTester.run(RULE_NAME, rule, { ], }, { - filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -148,7 +132,6 @@ ruleTester.run(RULE_NAME, rule, { ], }, { - filename: 'MyComponent.test.js', code: ` import { render } from '@testing-library/react'; @@ -162,7 +145,6 @@ ruleTester.run(RULE_NAME, rule, { ], }, { - filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -187,7 +169,6 @@ ruleTester.run(RULE_NAME, rule, { ], }, { - filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -201,7 +182,6 @@ ruleTester.run(RULE_NAME, rule, { ], }, { - filename: 'MyComponent.test.js', code: ` import { render } from '@testing-library/react'; @@ -216,7 +196,6 @@ ruleTester.run(RULE_NAME, rule, { ], }, { - filename: 'MyComponent.test.js', code: ` import { render } from '@testing-library/react'; @@ -230,7 +209,6 @@ ruleTester.run(RULE_NAME, rule, { ], }, { - filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; @@ -261,7 +239,6 @@ ruleTester.run(RULE_NAME, rule, { ], }, { - filename: 'MyComponent.test.js', code: ` import { screen } from '@testing-library/react'; diff --git a/tests/lib/rules/no-render-in-setup.test.ts b/tests/lib/rules/no-render-in-setup.test.ts index 3c8fbc57..1cf9867a 100644 --- a/tests/lib/rules/no-render-in-setup.test.ts +++ b/tests/lib/rules/no-render-in-setup.test.ts @@ -2,11 +2,7 @@ import { createRuleTester } from '../test-utils'; import { TESTING_FRAMEWORK_SETUP_HOOKS } from '../../../lib/utils'; import rule, { RULE_NAME } from '../../../lib/rules/no-render-in-setup'; -const ruleTester = createRuleTester({ - ecmaFeatures: { - jsx: true, - }, -}); +const ruleTester = createRuleTester(); ruleTester.run(RULE_NAME, rule, { valid: [ diff --git a/tests/lib/rules/no-side-effects-wait-for.test.ts b/tests/lib/rules/no-side-effects-wait-for.test.ts index 664e5db2..6b3fadf2 100644 --- a/tests/lib/rules/no-side-effects-wait-for.test.ts +++ b/tests/lib/rules/no-side-effects-wait-for.test.ts @@ -1,11 +1,7 @@ import { createRuleTester } from '../test-utils'; import rule, { RULE_NAME } from '../../../lib/rules/no-side-effects-wait-for'; -const ruleTester = createRuleTester({ - ecmaFeatures: { - jsx: true, - }, -}); +const ruleTester = createRuleTester(); ruleTester.run(RULE_NAME, rule, { valid: [ diff --git a/tests/lib/rules/prefer-find-by.test.ts b/tests/lib/rules/prefer-find-by.test.ts index eacef738..da743103 100644 --- a/tests/lib/rules/prefer-find-by.test.ts +++ b/tests/lib/rules/prefer-find-by.test.ts @@ -14,11 +14,7 @@ import rule, { MessageIds, } from '../../../lib/rules/prefer-find-by'; -const ruleTester = createRuleTester({ - ecmaFeatures: { - jsx: true, - }, -}); +const ruleTester = createRuleTester(); function buildFindByMethod(queryMethod: string) { return `${getFindByQueryVariant(queryMethod)}${queryMethod.split('By')[1]}`; diff --git a/tests/lib/rules/render-result-naming-convention.test.ts b/tests/lib/rules/render-result-naming-convention.test.ts index 3e34c524..7f51163c 100644 --- a/tests/lib/rules/render-result-naming-convention.test.ts +++ b/tests/lib/rules/render-result-naming-convention.test.ts @@ -3,11 +3,7 @@ import rule, { RULE_NAME, } from '../../../lib/rules/render-result-naming-convention'; -const ruleTester = createRuleTester({ - ecmaFeatures: { - jsx: true, - }, -}); +const ruleTester = createRuleTester(); ruleTester.run(RULE_NAME, rule, { valid: [ diff --git a/tests/lib/test-utils.ts b/tests/lib/test-utils.ts index 263cc6e7..88c1b778 100644 --- a/tests/lib/test-utils.ts +++ b/tests/lib/test-utils.ts @@ -1,8 +1,6 @@ import { resolve } from 'path'; import { TSESLint } from '@typescript-eslint/experimental-utils'; -// TODO: remove ecmaFeatures object from other tests files - const DEFAULT_TEST_CASE_CONFIG = { filename: 'MyComponent.test.js', }; @@ -42,7 +40,6 @@ export const createRuleTester = ( parserOptions: { ecmaVersion: 2018, sourceType: 'module', - // TODO: should we deep merge here? ecmaFeatures: { jsx: true, }, From bbe10df32d9b6ae6407b17b6623bfe512ecbafcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Wed, 28 Oct 2020 22:44:50 +0100 Subject: [PATCH 04/16] refactor(no-dom-import): use createTestingLibraryRule --- lib/rules/no-dom-import.ts | 8 ++-- tests/lib/rules/no-dom-import.test.ts | 68 ++++++++++++++++++++------- 2 files changed, 56 insertions(+), 20 deletions(-) diff --git a/lib/rules/no-dom-import.ts b/lib/rules/no-dom-import.ts index 09ad88ea..30b55fd0 100644 --- a/lib/rules/no-dom-import.ts +++ b/lib/rules/no-dom-import.ts @@ -1,6 +1,6 @@ -import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; -import { getDocsUrl } from '../utils'; -import { isLiteral, isIdentifier } from '../node-utils'; +import { 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 +11,7 @@ const DOM_TESTING_LIBRARY_MODULES = [ '@testing-library/dom', ]; -export default ESLintUtils.RuleCreator(getDocsUrl)({ +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'problem', diff --git a/tests/lib/rules/no-dom-import.test.ts b/tests/lib/rules/no-dom-import.test.ts index 429c8789..706fb406 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/file-name': '^.*\\.(nope)\\.js$' }, + }, + { + code: 'const { fireEvent } = require("dom-testing-library")', + filename: 'filename.not-matching.js', + }, + { + code: 'const { fireEvent } = require("dom-testing-library")', + settings: { 'testing-library/file-name': '^.*\\.(nope)\\.js$' }, + }, + { + code: 'import { fireEvent } from "@testing-library/dom"', + filename: 'filename.not-matching.js', + }, + { + code: 'import { fireEvent } from "@testing-library/dom"', + settings: { 'testing-library/file-name': '^.*\\.(nope)\\.js$' }, + }, + { + code: 'const { fireEvent } = require("@testing-library/dom")', + filename: 'filename.not-matching.js', + }, + { + code: 'const { fireEvent } = require("@testing-library/dom")', + settings: { 'testing-library/file-name': '^.*\\.(nope)\\.js$' }, + }, ], invalid: [ { From 6a10b4d3ee1a777badfb893b6131d50183f6e253 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sat, 31 Oct 2020 21:17:29 +0100 Subject: [PATCH 05/16] feat(detection-helpers): check imports with require --- .lintstagedrc | 5 +- lib/detect-testing-library-utils.ts | 46 ++++++-- lib/node-utils.ts | 6 +- lib/rules/no-dom-import.ts | 1 + tests/create-testing-library-rule.test.ts | 122 ++++++++++++++++++++-- tests/lib/rules/no-dom-import.test.ts | 10 +- tests/lib/rules/no-node-access.test.ts | 2 +- 7 files changed, 163 insertions(+), 29 deletions(-) diff --git a/.lintstagedrc b/.lintstagedrc index 172628eb..584baca7 100644 --- a/.lintstagedrc +++ b/.lintstagedrc @@ -1,8 +1,9 @@ { - "*.{js,ts}": [ + "*.ts": "tsc -p tsconfig.json --noEmit", + "*.{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..49b7965e 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'?: string; }; export type TestingLibraryContext< @@ -26,11 +27,11 @@ export type EnhancedRuleCreate< export type DetectionHelpers = { 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. @@ -50,8 +51,7 @@ export function detectTestingLibraryUtils< // 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; + context.settings['testing-library/filename'] ?? DEFAULT_FILENAME_PATTERN; // Helpers for Testing Library detection. const helpers: DetectionHelpers = { @@ -76,11 +76,11 @@ export function detectTestingLibraryUtils< }, /** - * 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" setting. */ - getIsValidFileName() { + getIsValidFilename() { const fileName = context.getFilename(); return !!fileName.match(fileNamePattern); }, @@ -89,7 +89,7 @@ export function detectTestingLibraryUtils< * Wraps all conditions that must be met to report rules. */ canReportErrors() { - return this.getIsTestingLibraryImported() && this.getIsValidFileName(); + return this.getIsTestingLibraryImported() && this.getIsValidFilename(); }, }; @@ -97,7 +97,7 @@ 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. @@ -118,6 +118,32 @@ export function detectTestingLibraryUtils< isImportingCustomModule = importName.endsWith(customModule); } }, + + // 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 (!isImportingTestingLibraryModule) { + isImportingTestingLibraryModule = args.some( + (arg) => + isLiteral(arg) && + typeof arg.value === 'string' && + /testing-library/g.test(arg.value) + ); + } + + if (!isImportingCustomModule) { + isImportingCustomModule = args.some( + (arg) => + isLiteral(arg) && + typeof arg.value === 'string' && + arg.value.endsWith(customModule) + ); + } + }, }; // update given rule to inject Testing Library detection 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 30b55fd0..e12d5db9 100644 --- a/lib/rules/no-dom-import.ts +++ b/lib/rules/no-dom-import.ts @@ -76,6 +76,7 @@ export default createTestingLibraryRule({ }); } } + return { ImportDeclaration(node) { const value = node.source.value; diff --git a/tests/create-testing-library-rule.test.ts b/tests/create-testing-library-rule.test.ts index 1073b370..0e497787 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,36 @@ 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': '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': 'testing-library\\.js', }, }, ], invalid: [ + // Test Cases for Imports & Filename { code: ` // case: import module forced to be reported @@ -67,7 +108,7 @@ ruleTester.run(RULE_NAME, rule, { import { foo } from 'report-me' `, settings: { - 'testing-library/file-name': 'testing-library\\.js', + 'testing-library/filename': 'testing-library\\.js', }, errors: [{ line: 3, column: 7, messageId: 'fakeError' }], }, @@ -92,12 +133,13 @@ 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', }, @@ -105,9 +147,27 @@ ruleTester.run(RULE_NAME, rule, { }, { code: ` - // case: render imported from config custom module + // 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', + }, + ], + }, + { + code: ` + // 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 +176,7 @@ ruleTester.run(RULE_NAME, rule, { }, errors: [ { - line: 6, + line: 7, column: 21, messageId: 'fakeError', }, @@ -124,10 +184,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 +202,47 @@ 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', + }, + ], + }, ], }); diff --git a/tests/lib/rules/no-dom-import.test.ts b/tests/lib/rules/no-dom-import.test.ts index 706fb406..df886f6e 100644 --- a/tests/lib/rules/no-dom-import.test.ts +++ b/tests/lib/rules/no-dom-import.test.ts @@ -31,7 +31,7 @@ ruleTester.run(RULE_NAME, rule, { }, { code: 'import { fireEvent } from "dom-testing-library"', - settings: { 'testing-library/file-name': '^.*\\.(nope)\\.js$' }, + settings: { 'testing-library/filename': '^.*\\.(nope)\\.js$' }, }, { code: 'const { fireEvent } = require("dom-testing-library")', @@ -39,7 +39,7 @@ ruleTester.run(RULE_NAME, rule, { }, { code: 'const { fireEvent } = require("dom-testing-library")', - settings: { 'testing-library/file-name': '^.*\\.(nope)\\.js$' }, + settings: { 'testing-library/filename': '^.*\\.(nope)\\.js$' }, }, { code: 'import { fireEvent } from "@testing-library/dom"', @@ -47,7 +47,7 @@ ruleTester.run(RULE_NAME, rule, { }, { code: 'import { fireEvent } from "@testing-library/dom"', - settings: { 'testing-library/file-name': '^.*\\.(nope)\\.js$' }, + settings: { 'testing-library/filename': '^.*\\.(nope)\\.js$' }, }, { code: 'const { fireEvent } = require("@testing-library/dom")', @@ -55,10 +55,12 @@ ruleTester.run(RULE_NAME, rule, { }, { code: 'const { fireEvent } = require("@testing-library/dom")', - settings: { 'testing-library/file-name': '^.*\\.(nope)\\.js$' }, + settings: { 'testing-library/filename': '^.*\\.(nope)\\.js$' }, }, ], invalid: [ + // TODO: add invalid cases: + // - custom filename importing from dom-testing-library { code: 'import { fireEvent } from "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..ddda12ba 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': 'testing-library\\.js', }, }, { From 370e1334b768c937491619c093f4def418da1305 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sun, 1 Nov 2020 12:38:27 +0100 Subject: [PATCH 06/16] test(no-dom-import): include test cases for custom module setting --- tests/lib/rules/no-dom-import.test.ts | 78 ++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 2 deletions(-) diff --git a/tests/lib/rules/no-dom-import.test.ts b/tests/lib/rules/no-dom-import.test.ts index df886f6e..79e314a7 100644 --- a/tests/lib/rules/no-dom-import.test.ts +++ b/tests/lib/rules/no-dom-import.test.ts @@ -59,8 +59,6 @@ ruleTester.run(RULE_NAME, rule, { }, ], invalid: [ - // TODO: add invalid cases: - // - custom filename importing from dom-testing-library { code: 'import { fireEvent } from "dom-testing-library"', errors: [ @@ -70,6 +68,22 @@ 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: 'import { fireEvent } from "dom-testing-library"', + }, { code: 'import { fireEvent } from "dom-testing-library"', options: ['react'], @@ -105,6 +119,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: [ @@ -113,6 +141,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: [ @@ -145,6 +187,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: [ @@ -166,6 +222,24 @@ 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: 'const { fireEvent } = require("@testing-library/vue")', + }, { code: 'require("dom-testing-library")', errors: [ From a5cf7ecb59bad1512973972507fd303984d3f3d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sun, 1 Nov 2020 12:38:27 +0100 Subject: [PATCH 07/16] test(no-dom-import): include test cases for custom module setting --- .lintstagedrc | 1 - tests/lib/rules/no-dom-import.test.ts | 81 ++++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/.lintstagedrc b/.lintstagedrc index 584baca7..59919a0f 100644 --- a/.lintstagedrc +++ b/.lintstagedrc @@ -1,5 +1,4 @@ { - "*.ts": "tsc -p tsconfig.json --noEmit", "*.{js,ts}": [ "eslint --fix", "prettier --write", diff --git a/tests/lib/rules/no-dom-import.test.ts b/tests/lib/rules/no-dom-import.test.ts index df886f6e..62dcde97 100644 --- a/tests/lib/rules/no-dom-import.test.ts +++ b/tests/lib/rules/no-dom-import.test.ts @@ -59,8 +59,6 @@ ruleTester.run(RULE_NAME, rule, { }, ], invalid: [ - // TODO: add invalid cases: - // - custom filename importing from dom-testing-library { code: 'import { fireEvent } from "dom-testing-library"', errors: [ @@ -70,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'], @@ -105,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: [ @@ -113,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: [ @@ -145,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: [ @@ -166,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: [ From 0e4f3dd44e17368286fd5d4c3ece18b2863bba2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sun, 1 Nov 2020 13:13:57 +0100 Subject: [PATCH 08/16] chore: fix merge --- tests/lib/rules/no-dom-import.test.ts | 62 --------------------------- 1 file changed, 62 deletions(-) diff --git a/tests/lib/rules/no-dom-import.test.ts b/tests/lib/rules/no-dom-import.test.ts index b8f544d8..62dcde97 100644 --- a/tests/lib/rules/no-dom-import.test.ts +++ b/tests/lib/rules/no-dom-import.test.ts @@ -68,22 +68,6 @@ 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: 'import { fireEvent } from "dom-testing-library"', - }, { settings: { 'testing-library/module': 'test-utils', @@ -172,20 +156,6 @@ 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: [ @@ -232,20 +202,6 @@ 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: [ @@ -267,24 +223,6 @@ 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: 'const { fireEvent } = require("@testing-library/vue")', - }, { settings: { 'testing-library/module': 'test-utils', From 93bbbb76e4fbff9f843cf707ee6a992cdf0d713d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sun, 1 Nov 2020 13:58:46 +0100 Subject: [PATCH 09/16] refactor(no-dom-import): extract detection helpers for import nodes --- .eslintignore | 1 + lib/detect-testing-library-utils.ts | 62 +++++++++++++++++++---------- lib/rules/no-dom-import.ts | 53 +++++++++++++----------- 3 files changed, 73 insertions(+), 43 deletions(-) diff --git a/.eslintignore b/.eslintignore index 404abb22..d64c4ca2 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1 +1,2 @@ coverage/ +dist/ diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 49b7965e..af9e417b 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -25,7 +25,14 @@ export type EnhancedRuleCreate< detectionHelpers: Readonly ) => TRuleListener; +type ModuleImportation = + | TSESTree.ImportDeclaration + | TSESTree.CallExpression + | null; + export type DetectionHelpers = { + getTestingLibraryImportNode: () => ModuleImportation; + getCustomModuleImportNode: () => ModuleImportation; getIsTestingLibraryImported: () => boolean; getIsValidFilename: () => boolean; canReportErrors: () => boolean; @@ -45,8 +52,8 @@ 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']; @@ -55,6 +62,12 @@ export function detectTestingLibraryUtils< // 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,7 +85,7 @@ export function detectTestingLibraryUtils< return true; } - return isImportingTestingLibraryModule || isImportingCustomModule; + return !!importedTestingLibraryNode || !!importedCustomModuleNode; }, /** @@ -103,19 +116,22 @@ export function detectTestingLibraryUtils< * 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; } - 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); + // 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; } }, @@ -126,22 +142,28 @@ export function detectTestingLibraryUtils< const callExpression = node.parent as TSESTree.CallExpression; const { arguments: args } = callExpression; - if (!isImportingTestingLibraryModule) { - isImportingTestingLibraryModule = args.some( + if ( + !importedTestingLibraryNode && + args.some( (arg) => isLiteral(arg) && typeof arg.value === 'string' && /testing-library/g.test(arg.value) - ); + ) + ) { + importedTestingLibraryNode = callExpression; } - if (!isImportingCustomModule) { - isImportingCustomModule = args.some( + if ( + !importedCustomModuleNode && + args.some( (arg) => isLiteral(arg) && typeof arg.value === 'string' && arg.value.endsWith(customModule) - ); + ) + ) { + importedCustomModuleNode = callExpression; } }, }; diff --git a/lib/rules/no-dom-import.ts b/lib/rules/no-dom-import.ts index e12d5db9..80db79aa 100644 --- a/lib/rules/no-dom-import.ts +++ b/lib/rules/no-dom-import.ts @@ -1,4 +1,7 @@ -import { TSESTree } from '@typescript-eslint/experimental-utils'; +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; import { isIdentifier, isLiteral } from '../node-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; @@ -35,7 +38,7 @@ export default createTestingLibraryRule({ }, defaultOptions: [''], - create(context, [framework]) { + create(context, [framework], helpers) { function report( node: TSESTree.ImportDeclaration | TSESTree.Identifier, moduleName: string @@ -78,32 +81,36 @@ export default createTestingLibraryRule({ } 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 + ); } }, }; From 20f2c6afe0264acaafde67b3a40d667a11ba1a10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sun, 1 Nov 2020 14:17:50 +0100 Subject: [PATCH 10/16] test: increase coverage --- tests/create-testing-library-rule.test.ts | 16 ++++++++++++++++ tests/fake-rule.ts | 23 +++++++++++++++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/tests/create-testing-library-rule.test.ts b/tests/create-testing-library-rule.test.ts index 0e497787..9fb78648 100644 --- a/tests/create-testing-library-rule.test.ts +++ b/tests/create-testing-library-rule.test.ts @@ -83,6 +83,12 @@ ruleTester.run(RULE_NAME, rule, { 'testing-library/filename': '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 @@ -244,5 +250,15 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + { + 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', + }); + } + }, }; }, }); From 12569d6575a240ad55a452f08138acb9fe1187f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sun, 1 Nov 2020 19:08:48 +0100 Subject: [PATCH 11/16] refactor: rename setting for filename pattern --- lib/detect-testing-library-utils.ts | 11 ++++++----- tests/create-testing-library-rule.test.ts | 6 +++--- tests/lib/rules/no-dom-import.test.ts | 8 ++++---- tests/lib/rules/no-node-access.test.ts | 2 +- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index af9e417b..633c0241 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -3,7 +3,7 @@ import { isLiteral } from './node-utils'; export type TestingLibrarySettings = { 'testing-library/module'?: string; - 'testing-library/filename'?: string; + 'testing-library/filename-pattern'?: string; }; export type TestingLibraryContext< @@ -57,8 +57,9 @@ export function detectTestingLibraryUtils< // Init options based on shared ESLint settings const customModule = context.settings['testing-library/module']; - const fileNamePattern = - context.settings['testing-library/filename'] ?? DEFAULT_FILENAME_PATTERN; + const filenamePattern = + context.settings['testing-library/filename-pattern'] ?? + DEFAULT_FILENAME_PATTERN; // Helpers for Testing Library detection. const helpers: DetectionHelpers = { @@ -91,11 +92,11 @@ export function detectTestingLibraryUtils< /** * Gets if filename being analyzed is valid or not. * - * This is based on "testing-library/filename" setting. + * This is based on "testing-library/filename-pattern" setting. */ getIsValidFilename() { const fileName = context.getFilename(); - return !!fileName.match(fileNamePattern); + return !!fileName.match(filenamePattern); }, /** diff --git a/tests/create-testing-library-rule.test.ts b/tests/create-testing-library-rule.test.ts index 9fb78648..6ca48128 100644 --- a/tests/create-testing-library-rule.test.ts +++ b/tests/create-testing-library-rule.test.ts @@ -70,7 +70,7 @@ ruleTester.run(RULE_NAME, rule, { import { foo } from 'report-me' `, settings: { - 'testing-library/filename': 'testing-library\\.js', + 'testing-library/filename-pattern': 'testing-library\\.js', }, }, { @@ -80,7 +80,7 @@ ruleTester.run(RULE_NAME, rule, { const { foo } = require('report-me') `, settings: { - 'testing-library/filename': 'testing-library\\.js', + 'testing-library/filename-pattern': 'testing-library\\.js', }, }, { @@ -114,7 +114,7 @@ ruleTester.run(RULE_NAME, rule, { import { foo } from 'report-me' `, settings: { - 'testing-library/filename': 'testing-library\\.js', + 'testing-library/filename-pattern': 'testing-library\\.js', }, errors: [{ line: 3, column: 7, messageId: 'fakeError' }], }, diff --git a/tests/lib/rules/no-dom-import.test.ts b/tests/lib/rules/no-dom-import.test.ts index 62dcde97..01ba26c1 100644 --- a/tests/lib/rules/no-dom-import.test.ts +++ b/tests/lib/rules/no-dom-import.test.ts @@ -31,7 +31,7 @@ ruleTester.run(RULE_NAME, rule, { }, { code: 'import { fireEvent } from "dom-testing-library"', - settings: { 'testing-library/filename': '^.*\\.(nope)\\.js$' }, + settings: { 'testing-library/filename-pattern': '^.*\\.(nope)\\.js$' }, }, { code: 'const { fireEvent } = require("dom-testing-library")', @@ -39,7 +39,7 @@ ruleTester.run(RULE_NAME, rule, { }, { code: 'const { fireEvent } = require("dom-testing-library")', - settings: { 'testing-library/filename': '^.*\\.(nope)\\.js$' }, + settings: { 'testing-library/filename-pattern': '^.*\\.(nope)\\.js$' }, }, { code: 'import { fireEvent } from "@testing-library/dom"', @@ -47,7 +47,7 @@ ruleTester.run(RULE_NAME, rule, { }, { code: 'import { fireEvent } from "@testing-library/dom"', - settings: { 'testing-library/filename': '^.*\\.(nope)\\.js$' }, + settings: { 'testing-library/filename-pattern': '^.*\\.(nope)\\.js$' }, }, { code: 'const { fireEvent } = require("@testing-library/dom")', @@ -55,7 +55,7 @@ ruleTester.run(RULE_NAME, rule, { }, { code: 'const { fireEvent } = require("@testing-library/dom")', - settings: { 'testing-library/filename': '^.*\\.(nope)\\.js$' }, + settings: { 'testing-library/filename-pattern': '^.*\\.(nope)\\.js$' }, }, ], invalid: [ diff --git a/tests/lib/rules/no-node-access.test.ts b/tests/lib/rules/no-node-access.test.ts index ddda12ba..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/filename': 'testing-library\\.js', + 'testing-library/filename-pattern': 'testing-library\\.js', }, }, { From f1afc6fd95069c84df5ac14fe0927e75b176e8f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sun, 1 Nov 2020 21:10:11 +0100 Subject: [PATCH 12/16] refactor: add new detection option to skip reporting checks --- lib/create-testing-library-rule.ts | 7 +++++-- lib/detect-testing-library-utils.ts | 32 +++++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/lib/create-testing-library-rule.ts b/lib/create-testing-library-rule.ts index 96f47d66..7fcca8b9 100644 --- a/lib/create-testing-library-rule.ts +++ b/lib/create-testing-library-rule.ts @@ -1,6 +1,7 @@ import { ESLintUtils, TSESLint } from '@typescript-eslint/experimental-utils'; import { getDocsUrl } from './utils'; import { + DetectionOptions, detectTestingLibraryUtils, EnhancedRuleCreate, } from './detect-testing-library-utils'; @@ -21,14 +22,16 @@ export function createTestingLibraryRule< meta: CreateRuleMeta; defaultOptions: Readonly; create: EnhancedRuleCreate; + detectionOptions?: Partial; }> ): TSESLint.RuleModule { - const { create, ...remainingConfig } = config; + const { create, detectionOptions, ...remainingConfig } = config; return ESLintUtils.RuleCreator(getDocsUrl)({ ...remainingConfig, create: detectTestingLibraryUtils( - create + create, + detectionOptions ), }); } diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 633c0241..375f70c3 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -38,8 +38,23 @@ export type DetectionHelpers = { canReportErrors: () => boolean; }; +export type DetectionOptions = { + /** + * If true, force `detectTestingLibraryUtils` to skip `canReportErrors` + * so it doesn't opt-out rule listener. + * + * Useful when some rule apply to files other than testing ones + * (e.g. `consistent-data-testid`) + */ + skipRuleReportingCheck?: boolean; +}; + const DEFAULT_FILENAME_PATTERN = '^.*\\.(test|spec)\\.[jt]sx?$'; +const DEFAULT_DETECTION_OPTIONS: DetectionOptions = { + skipRuleReportingCheck: false, +}; + /** * Enhances a given rule `create` with helpers to detect Testing Library utils. */ @@ -47,11 +62,19 @@ export function detectTestingLibraryUtils< TOptions extends readonly unknown[], TMessageIds extends string, TRuleListener extends TSESLint.RuleListener = TSESLint.RuleListener ->(ruleCreate: EnhancedRuleCreate) { +>( + ruleCreate: EnhancedRuleCreate, + options?: Partial +) { return ( context: TestingLibraryContext, optionsWithDefault: Readonly ): TSESLint.RuleListener => { + const { skipRuleReportingCheck } = Object.assign( + {}, + DEFAULT_DETECTION_OPTIONS, + options + ); let importedTestingLibraryNode: ModuleImportation = null; let importedCustomModuleNode: ModuleImportation = null; @@ -101,9 +124,14 @@ export function detectTestingLibraryUtils< /** * Wraps all conditions that must be met to report rules. + * + * A rule can skip it if necessary. */ canReportErrors() { - return this.getIsTestingLibraryImported() && this.getIsValidFilename(); + return ( + skipRuleReportingCheck || + (this.getIsTestingLibraryImported() && this.getIsValidFilename()) + ); }, }; From a002ba9ef1e37ea911e33ed875dd65d0303318f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sun, 1 Nov 2020 21:11:03 +0100 Subject: [PATCH 13/16] refactor(consistent-data-testid): use createTestingLibraryRule --- lib/rules/consistent-data-testid.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/rules/consistent-data-testid.ts b/lib/rules/consistent-data-testid.ts index 8b14e67c..2bf4ce11 100644 --- a/lib/rules/consistent-data-testid.ts +++ b/lib/rules/consistent-data-testid.ts @@ -1,6 +1,5 @@ -import { getDocsUrl } from '../utils'; -import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; import { isJSXAttribute, isLiteral } from '../node-utils'; +import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'consistent-data-testid'; export type MessageIds = 'consistentDataTestId'; @@ -13,7 +12,7 @@ type Options = [ const FILENAME_PLACEHOLDER = '{fileName}'; -export default ESLintUtils.RuleCreator(getDocsUrl)({ +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'suggestion', @@ -60,6 +59,9 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ testIdAttribute: 'data-testid', }, ], + detectionOptions: { + skipRuleReportingCheck: true, + }, create(context, [options]) { const { getFilename } = context; @@ -89,7 +91,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ } return { - [`JSXIdentifier`]: (node: TSESTree.JSXIdentifier) => { + JSXIdentifier: (node) => { if ( !isJSXAttribute(node.parent) || !isLiteral(node.parent.value) || From 15eb73b205289e4a7b6bcec53ba4e2532dfcfcca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Mon, 2 Nov 2020 13:13:02 +0100 Subject: [PATCH 14/16] revert: refactor consistent-data-testid --- lib/rules/consistent-data-testid.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/rules/consistent-data-testid.ts b/lib/rules/consistent-data-testid.ts index 2bf4ce11..8b14e67c 100644 --- a/lib/rules/consistent-data-testid.ts +++ b/lib/rules/consistent-data-testid.ts @@ -1,5 +1,6 @@ +import { getDocsUrl } from '../utils'; +import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; import { isJSXAttribute, isLiteral } from '../node-utils'; -import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'consistent-data-testid'; export type MessageIds = 'consistentDataTestId'; @@ -12,7 +13,7 @@ type Options = [ const FILENAME_PLACEHOLDER = '{fileName}'; -export default createTestingLibraryRule({ +export default ESLintUtils.RuleCreator(getDocsUrl)({ name: RULE_NAME, meta: { type: 'suggestion', @@ -59,9 +60,6 @@ export default createTestingLibraryRule({ testIdAttribute: 'data-testid', }, ], - detectionOptions: { - skipRuleReportingCheck: true, - }, create(context, [options]) { const { getFilename } = context; @@ -91,7 +89,7 @@ export default createTestingLibraryRule({ } return { - JSXIdentifier: (node) => { + [`JSXIdentifier`]: (node: TSESTree.JSXIdentifier) => { if ( !isJSXAttribute(node.parent) || !isLiteral(node.parent.value) || From b7aa7cacc34f8dc2d2a98729458fe7f8cb722636 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Mon, 2 Nov 2020 13:13:51 +0100 Subject: [PATCH 15/16] revert: detection options --- lib/create-testing-library-rule.ts | 7 ++----- lib/detect-testing-library-utils.ts | 32 ++--------------------------- 2 files changed, 4 insertions(+), 35 deletions(-) diff --git a/lib/create-testing-library-rule.ts b/lib/create-testing-library-rule.ts index 7fcca8b9..96f47d66 100644 --- a/lib/create-testing-library-rule.ts +++ b/lib/create-testing-library-rule.ts @@ -1,7 +1,6 @@ import { ESLintUtils, TSESLint } from '@typescript-eslint/experimental-utils'; import { getDocsUrl } from './utils'; import { - DetectionOptions, detectTestingLibraryUtils, EnhancedRuleCreate, } from './detect-testing-library-utils'; @@ -22,16 +21,14 @@ export function createTestingLibraryRule< meta: CreateRuleMeta; defaultOptions: Readonly; create: EnhancedRuleCreate; - detectionOptions?: Partial; }> ): TSESLint.RuleModule { - const { create, detectionOptions, ...remainingConfig } = config; + const { create, ...remainingConfig } = config; return ESLintUtils.RuleCreator(getDocsUrl)({ ...remainingConfig, create: detectTestingLibraryUtils( - create, - detectionOptions + create ), }); } diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 375f70c3..633c0241 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -38,23 +38,8 @@ export type DetectionHelpers = { canReportErrors: () => boolean; }; -export type DetectionOptions = { - /** - * If true, force `detectTestingLibraryUtils` to skip `canReportErrors` - * so it doesn't opt-out rule listener. - * - * Useful when some rule apply to files other than testing ones - * (e.g. `consistent-data-testid`) - */ - skipRuleReportingCheck?: boolean; -}; - const DEFAULT_FILENAME_PATTERN = '^.*\\.(test|spec)\\.[jt]sx?$'; -const DEFAULT_DETECTION_OPTIONS: DetectionOptions = { - skipRuleReportingCheck: false, -}; - /** * Enhances a given rule `create` with helpers to detect Testing Library utils. */ @@ -62,19 +47,11 @@ export function detectTestingLibraryUtils< TOptions extends readonly unknown[], TMessageIds extends string, TRuleListener extends TSESLint.RuleListener = TSESLint.RuleListener ->( - ruleCreate: EnhancedRuleCreate, - options?: Partial -) { +>(ruleCreate: EnhancedRuleCreate) { return ( context: TestingLibraryContext, optionsWithDefault: Readonly ): TSESLint.RuleListener => { - const { skipRuleReportingCheck } = Object.assign( - {}, - DEFAULT_DETECTION_OPTIONS, - options - ); let importedTestingLibraryNode: ModuleImportation = null; let importedCustomModuleNode: ModuleImportation = null; @@ -124,14 +101,9 @@ export function detectTestingLibraryUtils< /** * Wraps all conditions that must be met to report rules. - * - * A rule can skip it if necessary. */ canReportErrors() { - return ( - skipRuleReportingCheck || - (this.getIsTestingLibraryImported() && this.getIsValidFilename()) - ); + return this.getIsTestingLibraryImported() && this.getIsValidFilename(); }, }; From bbb3dfa0d1e1a1d56ea96b293551b61cbdd001d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Mon, 2 Nov 2020 13:16:47 +0100 Subject: [PATCH 16/16] docs(consistent-data-testid): add clarification about rule creation --- lib/rules/consistent-data-testid.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/rules/consistent-data-testid.ts b/lib/rules/consistent-data-testid.ts index 8b14e67c..6ca2220d 100644 --- a/lib/rules/consistent-data-testid.ts +++ b/lib/rules/consistent-data-testid.ts @@ -1,5 +1,5 @@ import { getDocsUrl } from '../utils'; -import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; +import { ESLintUtils } from '@typescript-eslint/experimental-utils'; import { isJSXAttribute, isLiteral } from '../node-utils'; export const RULE_NAME = 'consistent-data-testid'; @@ -13,6 +13,11 @@ type Options = [ const FILENAME_PLACEHOLDER = '{fileName}'; +/** + * This rule is not created with `createTestingLibraryRule` since: + * - it doesn't need any detection helper + * - it doesn't apply to testing files but component files + */ export default ESLintUtils.RuleCreator(getDocsUrl)({ name: RULE_NAME, meta: { @@ -89,7 +94,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ } return { - [`JSXIdentifier`]: (node: TSESTree.JSXIdentifier) => { + JSXIdentifier: (node) => { if ( !isJSXAttribute(node.parent) || !isLiteral(node.parent.value) ||