From 880adcdfb886bd2a37b7aae097faf00edf31cba2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Tue, 13 Apr 2021 19:58:05 +0200 Subject: [PATCH 1/3] test(no-node-access): improve current invalid cases asserts --- tests/lib/rules/no-node-access.test.ts | 30 ++++++++++++-------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/tests/lib/rules/no-node-access.test.ts b/tests/lib/rules/no-node-access.test.ts index 4b8afb36..e3a28f91 100644 --- a/tests/lib/rules/no-node-access.test.ts +++ b/tests/lib/rules/no-node-access.test.ts @@ -65,7 +65,7 @@ ruleTester.run(RULE_NAME, rule, { const closestButton = document.getElementById('submit-btn') expect(closestButton).toBeInTheDocument(); `, - errors: [{ messageId: 'noNodeAccess', line: 3 }], + errors: [{ line: 3, column: 38, messageId: 'noNodeAccess' }], }, { code: ` @@ -75,9 +75,13 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 4, + column: 33, messageId: 'noNodeAccess', }, { + line: 4, + column: 62, messageId: 'noNodeAccess', }, ], @@ -90,6 +94,8 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 4, + column: 18, messageId: 'noNodeAccess', }, ], @@ -117,6 +123,8 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 4, + column: 43, messageId: 'noNodeAccess', }, ], @@ -128,11 +136,7 @@ ruleTester.run(RULE_NAME, rule, { const { getByText } = render() getByText('submit').closest('button'); `, - errors: [ - { - messageId: 'noNodeAccess', - }, - ], + errors: [{ line: 5, column: 29, messageId: 'noNodeAccess' }], }, { code: ` @@ -165,11 +169,7 @@ ruleTester.run(RULE_NAME, rule, { const buttonText = screen.getByText('submit'); const button = buttonText.closest('button'); `, - errors: [ - { - messageId: 'noNodeAccess', - }, - ], + errors: [{ line: 5, column: 35, messageId: 'noNodeAccess' }], }, { code: ` @@ -181,6 +181,8 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [ { + line: 6, + column: 35, messageId: 'noNodeAccess', }, ], @@ -192,11 +194,7 @@ ruleTester.run(RULE_NAME, rule, { const { getByText } = render() const button = getByText('submit').closest('button'); `, - errors: [ - { - messageId: 'noNodeAccess', - }, - ], + errors: [{ line: 5, column: 44, messageId: 'noNodeAccess' }], }, { code: ` From 67792dfe64e28b64433135226f30d36b9d5ad134 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Tue, 13 Apr 2021 20:18:41 +0200 Subject: [PATCH 2/3] fix(no-node-access): skip Aggressive Reporting --- lib/detect-testing-library-utils.ts | 19 +++++++---- lib/rules/no-node-access.ts | 9 +++++- tests/lib/rules/no-node-access.test.ts | 44 +++++++++++++++++++++----- 3 files changed, 56 insertions(+), 16 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 33ca3350..8d5effff 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -56,7 +56,7 @@ type GetTestingLibraryImportNodeFn = () => ImportModuleNode | null; type GetCustomModuleImportNodeFn = () => ImportModuleNode | null; type GetTestingLibraryImportNameFn = () => string | undefined; type GetCustomModuleImportNameFn = () => string | undefined; -type IsTestingLibraryImportedFn = () => boolean; +type IsTestingLibraryImportedFn = (isStrict?: boolean) => boolean; type IsGetQueryVariantFn = (node: TSESTree.Identifier) => boolean; type IsQueryQueryVariantFn = (node: TSESTree.Identifier) => boolean; type IsFindQueryVariantFn = (node: TSESTree.Identifier) => boolean; @@ -242,12 +242,17 @@ export function detectTestingLibraryUtils< * then this method will return `true` ONLY IF a testing-library package * or custom module are imported. */ - const isTestingLibraryImported: IsTestingLibraryImportedFn = () => { - return ( - isAggressiveModuleReportingEnabled() || - !!importedTestingLibraryNode || - !!importedCustomModuleNode - ); + const isTestingLibraryImported: IsTestingLibraryImportedFn = ( + isStrict = false + ) => { + const isSomeModuleImported = + !!importedTestingLibraryNode || !!importedCustomModuleNode; + + if (isStrict) { + return isSomeModuleImported; + } + + return isAggressiveModuleReportingEnabled() || isSomeModuleImported; }; /** diff --git a/lib/rules/no-node-access.ts b/lib/rules/no-node-access.ts index ca3acf59..52de053f 100644 --- a/lib/rules/no-node-access.ts +++ b/lib/rules/no-node-access.ts @@ -23,8 +23,15 @@ export default createTestingLibraryRule({ }, defaultOptions: [], - create(context) { + create(context, _, helpers) { function showErrorForNodeAccess(node: TSESTree.MemberExpression) { + // This rule is so aggressive that can cause tons of false positives outside test files when Aggressive Reporting + // is enabled. Because of that, this rule will skip this mechanism and report only if some Testing Library package + // or custom one (set in utils-module Shared Setting) is found. + if (!helpers.isTestingLibraryImported(true)) { + return; + } + ASTUtils.isIdentifier(node.property) && ALL_RETURNING_NODES.includes(node.property.name) && context.report({ diff --git a/tests/lib/rules/no-node-access.test.ts b/tests/lib/rules/no-node-access.test.ts index e3a28f91..4e58bb7b 100644 --- a/tests/lib/rules/no-node-access.test.ts +++ b/tests/lib/rules/no-node-access.test.ts @@ -31,7 +31,7 @@ ruleTester.run(RULE_NAME, rule, { }, { code: ` - import { screen } from '@testing-library/react'; + import { screen } from '@testing-library/react'; const { getByText } = screen; const button = getByRole('button'); @@ -43,29 +43,57 @@ ruleTester.run(RULE_NAME, rule, { import { render, within } from '@testing-library/react'; const { getByLabelText } = render(); - const signinModal = getByLabelText('Sign In'); - within(signinModal).getByPlaceholderText('Username'); + const signInModal = getByLabelText('Sign In'); + within(signInModal).getByPlaceholderText('Username'); `, }, { code: ` - // case: importing custom module - const closestButton = document.getElementById('submit-btn').closest('button'); - expect(closestButton).toBeInTheDocument(); + // case: code not related to testing library at all + ReactDOM.render( + + + + }> + + + + , + + document.getElementById('root') + ); `, + }, + { settings: { 'testing-library/utils-module': 'test-utils', }, + code: ` + // case: custom module set but not imported (aggressive reporting limited) + const closestButton = document.getElementById('submit-btn').closest('button'); + expect(closestButton).toBeInTheDocument(); + `, + }, + { + code: ` + // case: without importing TL (aggressive reporting skipped) + const closestButton = document.getElementById('submit-btn') + expect(closestButton).toBeInTheDocument(); + `, }, ], invalid: [ { + settings: { + 'testing-library/utils-module': 'test-utils', + }, code: ` - // case: without importing TL (aggressive reporting) + // case: importing from custom module (aggressive reporting limited) + import 'test-utils'; const closestButton = document.getElementById('submit-btn') expect(closestButton).toBeInTheDocument(); `, - errors: [{ line: 3, column: 38, messageId: 'noNodeAccess' }], + errors: [{ line: 4, column: 38, messageId: 'noNodeAccess' }], }, { code: ` From 5bbd0524a87544a47b816ed7a5bae5a2dc723d30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Tue, 13 Apr 2021 20:25:28 +0200 Subject: [PATCH 3/3] refactor: simplify check for strict module imported --- lib/detect-testing-library-utils.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 8d5effff..261e2609 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -248,11 +248,10 @@ export function detectTestingLibraryUtils< const isSomeModuleImported = !!importedTestingLibraryNode || !!importedCustomModuleNode; - if (isStrict) { - return isSomeModuleImported; - } - - return isAggressiveModuleReportingEnabled() || isSomeModuleImported; + return ( + (!isStrict && isAggressiveModuleReportingEnabled()) || + isSomeModuleImported + ); }; /**