Skip to content

Commit bd60704

Browse files
gndeliagndelia
authored
refactor(prefer-find-by) migrate to v4 (#270)
* refactor: migrate prefer-find-by to v4 * refactor: applied pr suggestions
1 parent d0c76d4 commit bd60704

File tree

5 files changed

+256
-109
lines changed

5 files changed

+256
-109
lines changed

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,12 @@ import {
1414
isMemberExpression,
1515
isProperty,
1616
} from './node-utils';
17-
import { ABSENCE_MATCHERS, ASYNC_UTILS, PRESENCE_MATCHERS } from './utils';
17+
import {
18+
ABSENCE_MATCHERS,
19+
ASYNC_UTILS,
20+
PRESENCE_MATCHERS,
21+
ALL_QUERIES_COMBINATIONS,
22+
} from './utils';
1823

1924
export type TestingLibrarySettings = {
2025
'testing-library/module'?: string;
@@ -52,6 +57,7 @@ export type DetectionHelpers = {
5257
isFindByQuery: (node: TSESTree.Identifier) => boolean;
5358
isSyncQuery: (node: TSESTree.Identifier) => boolean;
5459
isAsyncQuery: (node: TSESTree.Identifier) => boolean;
60+
isCustomQuery: (node: TSESTree.Identifier) => boolean;
5561
isAsyncUtil: (node: TSESTree.Identifier) => boolean;
5662
isFireEventMethod: (node: TSESTree.Identifier) => boolean;
5763
isPresenceAssert: (node: TSESTree.MemberExpression) => boolean;
@@ -180,6 +186,13 @@ export function detectTestingLibraryUtils<
180186
return isFindByQuery(node);
181187
};
182188

189+
const isCustomQuery: DetectionHelpers['isCustomQuery'] = (node) => {
190+
return (
191+
(isSyncQuery(node) || isAsyncQuery(node)) &&
192+
!ALL_QUERIES_COMBINATIONS.includes(node.name)
193+
);
194+
};
195+
183196
/**
184197
* Determines whether a given node is async util or not.
185198
*/
@@ -356,6 +369,7 @@ export function detectTestingLibraryUtils<
356369
isFindByQuery,
357370
isSyncQuery,
358371
isAsyncQuery,
372+
isCustomQuery,
359373
isAsyncUtil,
360374
isFireEventMethod,
361375
isPresenceAssert,

‎lib/rules/prefer-find-by.ts‎

Lines changed: 72 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
import {
2-
ESLintUtils,
3-
TSESTree,
4-
ASTUtils,
5-
} from '@typescript-eslint/experimental-utils';
1+
import { TSESTree, ASTUtils } from '@typescript-eslint/experimental-utils';
62
import {
73
ReportFixFunction,
84
RuleFix,
@@ -15,7 +11,7 @@ import {
1511
isObjectPattern,
1612
isProperty,
1713
} from '../node-utils';
18-
import { getDocsUrl, SYNC_QUERIES_COMBINATIONS } from '../utils';
14+
import { createTestingLibraryRule } from '../create-testing-library-rule';
1915

2016
export const RULE_NAME = 'prefer-find-by';
2117
export type MessageIds = 'preferFindBy';
@@ -51,7 +47,7 @@ function findRenderDefinitionDeclaration(
5147
return findRenderDefinitionDeclaration(scope.upper, query);
5248
}
5349

54-
export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
50+
export default createTestingLibraryRule<Options, MessageIds>({
5551
name: RULE_NAME,
5652
meta: {
5753
type: 'suggestion',
@@ -70,7 +66,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
7066
},
7167
defaultOptions: [],
7268

73-
create(context) {
69+
create(context, _, helpers) {
7470
const sourceCode = context.getSourceCode();
7571

7672
/**
@@ -126,7 +122,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
126122
isMemberExpression(argument.body.callee) &&
127123
ASTUtils.isIdentifier(argument.body.callee.property) &&
128124
ASTUtils.isIdentifier(argument.body.callee.object) &&
129-
SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.property.name)
125+
helpers.isSyncQuery(argument.body.callee.property)
130126
) {
131127
// shape of () => screen.getByText
132128
const fullQueryMethod = argument.body.callee.property.name;
@@ -139,6 +135,11 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
139135
queryMethod,
140136
queryVariant,
141137
fix(fixer) {
138+
const property = ((argument.body as TSESTree.CallExpression)
139+
.callee as TSESTree.MemberExpression).property;
140+
if (helpers.isCustomQuery(property as TSESTree.Identifier)) {
141+
return;
142+
}
142143
const newCode = `${caller}.${queryVariant}${queryMethod}(${callArguments
143144
.map((node) => sourceCode.getText(node))
144145
.join(', ')})`;
@@ -148,65 +149,74 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
148149
return;
149150
}
150151
if (
151-
ASTUtils.isIdentifier(argument.body.callee) &&
152-
SYNC_QUERIES_COMBINATIONS.includes(argument.body.callee.name)
152+
!ASTUtils.isIdentifier(argument.body.callee) ||
153+
!helpers.isSyncQuery(argument.body.callee)
153154
) {
154-
// shape of () => getByText
155-
const fullQueryMethod = argument.body.callee.name;
156-
const queryMethod = fullQueryMethod.split('By')[1];
157-
const queryVariant = getFindByQueryVariant(fullQueryMethod);
158-
const callArguments = argument.body.arguments;
155+
return;
156+
}
157+
// shape of () => getByText
158+
const fullQueryMethod = argument.body.callee.name;
159+
const queryMethod = fullQueryMethod.split('By')[1];
160+
const queryVariant = getFindByQueryVariant(fullQueryMethod);
161+
const callArguments = argument.body.arguments;
159162

160-
reportInvalidUsage(node, {
161-
queryMethod,
162-
queryVariant,
163-
fix(fixer) {
164-
const findByMethod = `${queryVariant}${queryMethod}`;
165-
const allFixes: RuleFix[] = [];
166-
// this updates waitFor with findBy*
167-
const newCode = `${findByMethod}(${callArguments
168-
.map((node) => sourceCode.getText(node))
169-
.join(', ')})`;
170-
allFixes.push(fixer.replaceText(node, newCode));
163+
reportInvalidUsage(node, {
164+
queryMethod,
165+
queryVariant,
166+
fix(fixer) {
167+
// we know from above callee is an Identifier
168+
if (
169+
helpers.isCustomQuery(
170+
(argument.body as TSESTree.CallExpression)
171+
.callee as TSESTree.Identifier
172+
)
173+
) {
174+
return;
175+
}
176+
const findByMethod = `${queryVariant}${queryMethod}`;
177+
const allFixes: RuleFix[] = [];
178+
// this updates waitFor with findBy*
179+
const newCode = `${findByMethod}(${callArguments
180+
.map((node) => sourceCode.getText(node))
181+
.join(', ')})`;
182+
allFixes.push(fixer.replaceText(node, newCode));
171183

172-
// this adds the findBy* declaration - adding it to the list of destructured variables { findBy* } = render()
173-
const definition = findRenderDefinitionDeclaration(
174-
context.getScope(),
175-
fullQueryMethod
176-
);
177-
// I think it should always find it, otherwise code should not be valid (it'd be using undeclared variables)
178-
if (!definition) {
184+
// this adds the findBy* declaration - adding it to the list of destructured variables { findBy* } = render()
185+
const definition = findRenderDefinitionDeclaration(
186+
context.getScope(),
187+
fullQueryMethod
188+
);
189+
// I think it should always find it, otherwise code should not be valid (it'd be using undeclared variables)
190+
if (!definition) {
191+
return allFixes;
192+
}
193+
// check the declaration is part of a destructuring
194+
if (isObjectPattern(definition.parent.parent)) {
195+
const allVariableDeclarations = definition.parent.parent;
196+
// verify if the findBy* method was already declared
197+
if (
198+
allVariableDeclarations.properties.some(
199+
(p) =>
200+
isProperty(p) &&
201+
ASTUtils.isIdentifier(p.key) &&
202+
p.key.name === findByMethod
203+
)
204+
) {
179205
return allFixes;
180206
}
181-
// check the declaration is part of a destructuring
182-
if (isObjectPattern(definition.parent.parent)) {
183-
const allVariableDeclarations = definition.parent.parent;
184-
// verify if the findBy* method was already declared
185-
if (
186-
allVariableDeclarations.properties.some(
187-
(p) =>
188-
isProperty(p) &&
189-
ASTUtils.isIdentifier(p.key) &&
190-
p.key.name === findByMethod
191-
)
192-
) {
193-
return allFixes;
194-
}
195-
// the last character of a destructuring is always a "}", so we should replace it with the findBy* declaration
196-
const textDestructuring = sourceCode.getText(
197-
allVariableDeclarations
198-
);
199-
const text =
200-
textDestructuring.substring(0, textDestructuring.length - 2) +
201-
`, ${findByMethod} }`;
202-
allFixes.push(fixer.replaceText(allVariableDeclarations, text));
203-
}
207+
// the last character of a destructuring is always a "}", so we should replace it with the findBy* declaration
208+
const textDestructuring = sourceCode.getText(
209+
allVariableDeclarations
210+
);
211+
const text =
212+
textDestructuring.substring(0, textDestructuring.length - 2) +
213+
`, ${findByMethod} }`;
214+
allFixes.push(fixer.replaceText(allVariableDeclarations, text));
215+
}
204216

205-
return allFixes;
206-
},
207-
});
208-
return;
209-
}
217+
return allFixes;
218+
},
219+
});
210220
},
211221
};
212222
},

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -468,21 +468,21 @@ ruleTester.run(RULE_NAME, rule, {
468468
// case: custom "getBy*" query reported without import (aggressive reporting)
469469
getByIcon('search')
470470
`,
471-
errors: [{ line: 3, column: 7, messageId: 'getByError' }],
471+
errors: [{ line: 3, column: 7, messageId: 'customQueryError' }],
472472
},
473473
{
474474
code: `
475475
// case: custom "queryBy*" query reported without import (aggressive reporting)
476476
queryByIcon('search')
477477
`,
478-
errors: [{ line: 3, column: 7, messageId: 'queryByError' }],
478+
errors: [{ line: 3, column: 7, messageId: 'customQueryError' }],
479479
},
480480
{
481481
code: `
482482
// case: custom "findBy*" query reported without import (aggressive reporting)
483483
findByIcon('search')
484484
`,
485-
errors: [{ line: 3, column: 7, messageId: 'findByError' }],
485+
errors: [{ line: 3, column: 7, messageId: 'customQueryError' }],
486486
},
487487
{
488488
settings: {
@@ -564,7 +564,7 @@ ruleTester.run(RULE_NAME, rule, {
564564
import { render } from '@testing-library/react'
565565
getByIcon('search')
566566
`,
567-
errors: [{ line: 4, column: 7, messageId: 'getByError' }],
567+
errors: [{ line: 4, column: 7, messageId: 'customQueryError' }],
568568
},
569569
{
570570
filename: 'MyComponent.spec.js',
@@ -576,7 +576,7 @@ ruleTester.run(RULE_NAME, rule, {
576576
import { render } from '@testing-library/framework'
577577
queryByIcon('search')
578578
`,
579-
errors: [{ line: 4, column: 7, messageId: 'queryByError' }],
579+
errors: [{ line: 4, column: 7, messageId: 'customQueryError' }],
580580
},
581581
{
582582
filename: 'MyComponent.spec.js',
@@ -588,7 +588,7 @@ ruleTester.run(RULE_NAME, rule, {
588588
import { render } from '@testing-library/framework'
589589
findByIcon('search')
590590
`,
591-
errors: [{ line: 4, column: 7, messageId: 'findByError' }],
591+
errors: [{ line: 4, column: 7, messageId: 'customQueryError' }],
592592
},
593593
{
594594
settings: {
@@ -599,7 +599,7 @@ ruleTester.run(RULE_NAME, rule, {
599599
import { render } from 'test-utils'
600600
getByIcon('search')
601601
`,
602-
errors: [{ line: 4, column: 7, messageId: 'getByError' }],
602+
errors: [{ line: 4, column: 7, messageId: 'customQueryError' }],
603603
},
604604
{
605605
filename: 'MyComponent.spec.js',
@@ -611,7 +611,7 @@ ruleTester.run(RULE_NAME, rule, {
611611
import { render } from 'test-utils'
612612
queryByIcon('search')
613613
`,
614-
errors: [{ line: 4, column: 7, messageId: 'queryByError' }],
614+
errors: [{ line: 4, column: 7, messageId: 'customQueryError' }],
615615
},
616616
{
617617
filename: 'MyComponent.spec.js',
@@ -623,7 +623,7 @@ ruleTester.run(RULE_NAME, rule, {
623623
import { render } from 'test-utils'
624624
findByIcon('search')
625625
`,
626-
errors: [{ line: 4, column: 7, messageId: 'findByError' }],
626+
errors: [{ line: 4, column: 7, messageId: 'customQueryError' }],
627627
},
628628
{
629629
settings: {

‎tests/fake-rule.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ type MessageIds =
1212
| 'getByError'
1313
| 'queryByError'
1414
| 'findByError'
15+
| 'customQueryError'
1516
| 'presenceAssertError'
1617
| 'absenceAssertError';
1718

@@ -29,6 +30,7 @@ export default createTestingLibraryRule<Options, MessageIds>({
2930
getByError: 'some error related to getBy reported',
3031
queryByError: 'some error related to queryBy reported',
3132
findByError: 'some error related to findBy reported',
33+
customQueryError: 'some error related to a customQuery reported',
3234
presenceAssertError: 'some error related to presence assert reported',
3335
absenceAssertError: 'some error related to absence assert reported',
3436
},
@@ -43,6 +45,10 @@ export default createTestingLibraryRule<Options, MessageIds>({
4345
return context.report({ node, messageId: 'fakeError' });
4446
}
4547

48+
if (helpers.isCustomQuery(node)) {
49+
return context.report({ node, messageId: 'customQueryError' });
50+
}
51+
4652
// force queries to be reported
4753
if (helpers.isGetByQuery(node)) {
4854
return context.report({ node, messageId: 'getByError' });

0 commit comments

Comments
 (0)