Skip to content

Commit 63c9d7b

Browse files
Belco90Belco90
authored
refactor: improve logic to detect if testing librar imported (#239)
* refactor: check testing library imports automatically * feat: add new shared setting for testing library module * test: increase coverage for create testing library rule * refactor: extract common enhanced rule create types * docs: add jsdoc to detection helpers * docs: update old comments related to ImportDeclaration * test: check rule listener are combined properly
1 parent ce4770f commit 63c9d7b

7 files changed

+292
-45
lines changed

‎jest.config.js

-6
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,6 @@ module.exports = {
1111
statements: 100,
1212
},
1313
// TODO drop this custom threshold in v4
14-
'./lib/detect-testing-library-utils.ts': {
15-
branches: 50,
16-
functions: 90,
17-
lines: 90,
18-
statements: 90,
19-
},
2014
'./lib/node-utils.ts': {
2115
branches: 90,
2216
functions: 90,

‎lib/create-testing-library-rule.ts

+3-7
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { ESLintUtils, TSESLint } from '@typescript-eslint/experimental-utils';
22
import { getDocsUrl } from './utils';
33
import {
44
detectTestingLibraryUtils,
5-
DetectionHelpers,
5+
EnhancedRuleCreate,
66
} from './detect-testing-library-utils';
77

88
// These 2 types are copied from @typescript-eslint/experimental-utils
@@ -20,13 +20,9 @@ export function createTestingLibraryRule<
2020
name: string;
2121
meta: CreateRuleMeta<TMessageIds>;
2222
defaultOptions: Readonly<TOptions>;
23-
create: (
24-
context: Readonly<TSESLint.RuleContext<TMessageIds, TOptions>>,
25-
optionsWithDefault: Readonly<TOptions>,
26-
detectionHelpers: Readonly<DetectionHelpers>
27-
) => TRuleListener;
23+
create: EnhancedRuleCreate<TOptions, TMessageIds, TRuleListener>;
2824
}>
29-
): TSESLint.RuleModule<TMessageIds, TOptions, TRuleListener> {
25+
): TSESLint.RuleModule<TMessageIds, TOptions> {
3026
const { create, ...remainingConfig } = config;
3127

3228
return ESLintUtils.RuleCreator(getDocsUrl)({

‎lib/detect-testing-library-utils.ts

+90-25
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,31 @@
11
import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
22

3+
export type TestingLibrarySettings = {
4+
'testing-library/module'?: string;
5+
};
6+
7+
export type TestingLibraryContext<
8+
TOptions extends readonly unknown[],
9+
TMessageIds extends string
10+
> = Readonly<
11+
TSESLint.RuleContext<TMessageIds, TOptions> & {
12+
settings: TestingLibrarySettings;
13+
}
14+
>;
15+
16+
export type EnhancedRuleCreate<
17+
TOptions extends readonly unknown[],
18+
TMessageIds extends string,
19+
TRuleListener extends TSESLint.RuleListener = TSESLint.RuleListener
20+
> = (
21+
context: TestingLibraryContext<TOptions, TMessageIds>,
22+
optionsWithDefault: Readonly<TOptions>,
23+
detectionHelpers: Readonly<DetectionHelpers>
24+
) => TRuleListener;
25+
326
export type DetectionHelpers = {
4-
getIsImportingTestingLibrary: () => boolean;
27+
getIsTestingLibraryImported: () => boolean;
28+
canReportErrors: () => boolean;
529
};
630

731
/**
@@ -11,50 +35,91 @@ export function detectTestingLibraryUtils<
1135
TOptions extends readonly unknown[],
1236
TMessageIds extends string,
1337
TRuleListener extends TSESLint.RuleListener = TSESLint.RuleListener
14-
>(
15-
ruleCreate: (
16-
context: Readonly<TSESLint.RuleContext<TMessageIds, TOptions>>,
17-
optionsWithDefault: Readonly<TOptions>,
18-
detectionHelpers: Readonly<DetectionHelpers>
19-
) => TRuleListener
20-
) {
38+
>(ruleCreate: EnhancedRuleCreate<TOptions, TMessageIds, TRuleListener>) {
2139
return (
22-
context: Readonly<TSESLint.RuleContext<TMessageIds, TOptions>>,
40+
context: TestingLibraryContext<TOptions, TMessageIds>,
2341
optionsWithDefault: Readonly<TOptions>
24-
): TRuleListener => {
25-
let isImportingTestingLibrary = false;
42+
): TSESLint.RuleListener => {
43+
let isImportingTestingLibraryModule = false;
44+
let isImportingCustomModule = false;
2645

27-
// TODO: init here options based on shared ESLint config
46+
// Init options based on shared ESLint settings
47+
const customModule = context.settings['testing-library/module'];
2848

29-
// helpers for Testing Library detection
49+
// Helpers for Testing Library detection.
3050
const helpers: DetectionHelpers = {
31-
getIsImportingTestingLibrary() {
32-
return isImportingTestingLibrary;
51+
/**
52+
* Gets if Testing Library is considered as imported or not.
53+
*
54+
* By default, it is ALWAYS considered as imported. This is what we call
55+
* "aggressive reporting" so we don't miss TL utils reexported from
56+
* custom modules.
57+
*
58+
* However, there is a setting to customize the module where TL utils can
59+
* be imported from: "testing-library/module". If this setting is enabled,
60+
* then this method will return `true` ONLY IF a testing-library package
61+
* or custom module are imported.
62+
*/
63+
getIsTestingLibraryImported() {
64+
if (!customModule) {
65+
return true;
66+
}
67+
68+
return isImportingTestingLibraryModule || isImportingCustomModule;
69+
},
70+
71+
/**
72+
* Wraps all conditions that must be met to report rules.
73+
*/
74+
canReportErrors() {
75+
return this.getIsTestingLibraryImported();
3376
},
3477
};
3578

36-
// instructions for Testing Library detection
79+
// Instructions for Testing Library detection.
3780
const detectionInstructions: TSESLint.RuleListener = {
81+
/**
82+
* This ImportDeclaration rule listener will check if Testing Library related
83+
* modules are loaded. Since imports happen first thing in a file, it's
84+
* safe to use `isImportingTestingLibraryModule` and `isImportingCustomModule`
85+
* since they will have corresponding value already updated when reporting other
86+
* parts of the file.
87+
*/
3888
ImportDeclaration(node: TSESTree.ImportDeclaration) {
39-
isImportingTestingLibrary = /testing-library/g.test(
40-
node.source.value as string
41-
);
89+
if (!isImportingTestingLibraryModule) {
90+
// check only if testing library import not found yet so we avoid
91+
// to override isImportingTestingLibraryModule after it's found
92+
isImportingTestingLibraryModule = /testing-library/g.test(
93+
node.source.value as string
94+
);
95+
}
96+
97+
if (!isImportingCustomModule) {
98+
// check only if custom module import not found yet so we avoid
99+
// to override isImportingCustomModule after it's found
100+
const importName = String(node.source.value);
101+
isImportingCustomModule = importName.endsWith(customModule);
102+
}
42103
},
43104
};
44105

45106
// update given rule to inject Testing Library detection
46107
const ruleInstructions = ruleCreate(context, optionsWithDefault, helpers);
47-
const enhancedRuleInstructions = Object.assign({}, ruleInstructions);
108+
const enhancedRuleInstructions: TSESLint.RuleListener = {};
109+
110+
const allKeys = new Set(
111+
Object.keys(detectionInstructions).concat(Object.keys(ruleInstructions))
112+
);
48113

49-
Object.keys(detectionInstructions).forEach((instruction) => {
50-
(enhancedRuleInstructions as TSESLint.RuleListener)[instruction] = (
51-
node
52-
) => {
114+
// Iterate over ALL instructions keys so we can override original rule instructions
115+
// to prevent their execution if conditions to report errors are not met.
116+
allKeys.forEach((instruction) => {
117+
enhancedRuleInstructions[instruction] = (node) => {
53118
if (instruction in detectionInstructions) {
54119
detectionInstructions[instruction](node);
55120
}
56121

57-
if (ruleInstructions[instruction]) {
122+
if (helpers.canReportErrors() && ruleInstructions[instruction]) {
58123
return ruleInstructions[instruction](node);
59124
}
60125
};

‎lib/rules/no-node-access.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,10 @@ export default createTestingLibraryRule<Options, MessageIds>({
2525
},
2626
defaultOptions: [],
2727

28-
create: (context, _, helpers) => {
28+
create(context) {
2929
function showErrorForNodeAccess(node: TSESTree.MemberExpression) {
3030
isIdentifier(node.property) &&
3131
ALL_RETURNING_NODES.includes(node.property.name) &&
32-
helpers.getIsImportingTestingLibrary() &&
3332
context.report({
3433
node: node,
3534
loc: node.property.loc.start,

‎tests/create-testing-library-rule.test.ts

+122
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
import { createRuleTester } from './lib/test-utils';
2+
import rule, { RULE_NAME } from './fake-rule';
3+
4+
const ruleTester = createRuleTester({
5+
ecmaFeatures: {
6+
jsx: true,
7+
},
8+
});
9+
10+
ruleTester.run(RULE_NAME, rule, {
11+
valid: [
12+
{
13+
code: `
14+
// case: nothing related to Testing Library at all
15+
import { shallow } from 'enzyme';
16+
17+
const wrapper = shallow(<MyComponent />);
18+
`,
19+
},
20+
{
21+
code: `
22+
// case: render imported from other than custom module
23+
import { render } from '@somewhere/else'
24+
25+
const utils = render();
26+
`,
27+
settings: {
28+
'testing-library/module': 'test-utils',
29+
},
30+
},
31+
{
32+
code: `
33+
// case: prevent import which should trigger an error since it's imported
34+
// from other than custom module
35+
import { foo } from 'report-me'
36+
`,
37+
settings: {
38+
'testing-library/module': 'test-utils',
39+
},
40+
},
41+
],
42+
invalid: [
43+
{
44+
code: `
45+
// case: import module forced to be reported
46+
import { foo } from 'report-me'
47+
`,
48+
errors: [{ line: 3, column: 7, messageId: 'fakeError' }],
49+
},
50+
{
51+
code: `
52+
// case: render imported from any module by default (aggressive reporting)
53+
import { render } from '@somewhere/else'
54+
import { somethingElse } from 'another-module'
55+
56+
const utils = render();
57+
`,
58+
errors: [
59+
{
60+
line: 6,
61+
column: 21,
62+
messageId: 'fakeError',
63+
},
64+
],
65+
},
66+
{
67+
code: `
68+
// case: render imported from Testing Library module
69+
import { render } from '@testing-library/react'
70+
import { somethingElse } from 'another-module'
71+
72+
const utils = render();
73+
`,
74+
errors: [
75+
{
76+
line: 6,
77+
column: 21,
78+
messageId: 'fakeError',
79+
},
80+
],
81+
},
82+
{
83+
code: `
84+
// case: render imported from config custom module
85+
import { render } from 'test-utils'
86+
import { somethingElse } from 'another-module'
87+
88+
const utils = render();
89+
`,
90+
settings: {
91+
'testing-library/module': 'test-utils',
92+
},
93+
errors: [
94+
{
95+
line: 6,
96+
column: 21,
97+
messageId: 'fakeError',
98+
},
99+
],
100+
},
101+
{
102+
code: `
103+
// case: render imported from Testing Library module if
104+
// custom module setup
105+
import { render } from '@testing-library/react'
106+
import { somethingElse } from 'another-module'
107+
108+
const utils = render();
109+
`,
110+
settings: {
111+
'testing-library/module': 'test-utils',
112+
},
113+
errors: [
114+
{
115+
line: 7,
116+
column: 21,
117+
messageId: 'fakeError',
118+
},
119+
],
120+
},
121+
],
122+
});

‎tests/fake-rule.ts

+55
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/**
2+
* @file Fake rule to be able to test createTestingLibraryRule and
3+
* detectTestingLibraryUtils properly
4+
*/
5+
import { TSESTree } from '@typescript-eslint/experimental-utils';
6+
import { createTestingLibraryRule } from '../lib/create-testing-library-rule';
7+
8+
export const RULE_NAME = 'fake-rule';
9+
type Options = [];
10+
type MessageIds = 'fakeError';
11+
12+
export default createTestingLibraryRule<Options, MessageIds>({
13+
name: RULE_NAME,
14+
meta: {
15+
type: 'problem',
16+
docs: {
17+
description: 'Fake rule to test rule maker and detection helpers',
18+
category: 'Possible Errors',
19+
recommended: false,
20+
},
21+
messages: {
22+
fakeError: 'fake error reported',
23+
},
24+
fixable: null,
25+
schema: [],
26+
},
27+
defaultOptions: [],
28+
create(context) {
29+
const reportRenderIdentifier = (node: TSESTree.Identifier) => {
30+
if (node.name === 'render') {
31+
context.report({
32+
node,
33+
messageId: 'fakeError',
34+
});
35+
}
36+
};
37+
38+
const checkImportDeclaration = (node: TSESTree.ImportDeclaration) => {
39+
// This is just to check that defining an `ImportDeclaration` doesn't
40+
// override `ImportDeclaration` from `detectTestingLibraryUtils`
41+
42+
if (node.source.value === 'report-me') {
43+
context.report({
44+
node,
45+
messageId: 'fakeError',
46+
});
47+
}
48+
};
49+
50+
return {
51+
'CallExpression Identifier': reportRenderIdentifier,
52+
ImportDeclaration: checkImportDeclaration,
53+
};
54+
},
55+
});

0 commit comments

Comments
 (0)