Skip to content

Commit c92e7ff

Browse files
zaicevaszaicevas
authored
feat(prefer-explicit-assert): report on findBy* queries too (#421)
* feat: extend prefer-explicit-assert to report standalone findBy* queries * feat: add comments to make if branches more clear * feat: update README * fix: add tests and fix bug when return statements are reported * refactor: rule description, add TODO Co-authored-by: Mario Beltrán Alarcón <[email protected]> Relates to #409
1 parent e0da981 commit c92e7ff

File tree

3 files changed

+199
-6
lines changed

3 files changed

+199
-6
lines changed

‎README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ To enable this configuration use the `extends` property in your
204204
| [`testing-library/no-wait-for-multiple-assertions`](./docs/rules/no-wait-for-multiple-assertions.md) | Disallow the use of multiple `expect` calls inside `waitFor` | | |
205205
| [`testing-library/no-wait-for-side-effects`](./docs/rules/no-wait-for-side-effects.md) | Disallow the use of side effects in `waitFor` | | |
206206
| [`testing-library/no-wait-for-snapshot`](./docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | | |
207-
| [`testing-library/prefer-explicit-assert`](./docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |
207+
| [`testing-library/prefer-explicit-assert`](./docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than standalone queries | | |
208208
| [`testing-library/prefer-find-by`](./docs/rules/prefer-find-by.md) | Suggest using `find(All)By*` query instead of `waitFor` + `get(All)By*` to wait for elements | 🔧 | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] |
209209
| [`testing-library/prefer-presence-queries`](./docs/rules/prefer-presence-queries.md) | Ensure appropriate `get*`/`query*` queries are used with their respective matchers | | |
210210
| [`testing-library/prefer-query-by-disappearance`](./docs/rules/prefer-query-by-disappearance.md) | Suggest using `queryBy*` queries when waiting for disappearance | | |

‎lib/rules/prefer-explicit-assert.ts

+78-5
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { TSESTree, ASTUtils } from '@typescript-eslint/experimental-utils';
22

33
import { createTestingLibraryRule } from '../create-testing-library-rule';
4-
import { findClosestCallNode, isMemberExpression } from '../node-utils';
4+
import {
5+
findClosestCallNode,
6+
isCallExpression,
7+
isMemberExpression,
8+
} from '../node-utils';
59
import { PRESENCE_MATCHERS, ABSENCE_MATCHERS } from '../utils';
610

711
export const RULE_NAME = 'prefer-explicit-assert';
@@ -15,15 +19,55 @@ type Options = [
1519
];
1620

1721
const isAtTopLevel = (node: TSESTree.Node) =>
18-
!!node.parent?.parent && node.parent.parent.type === 'ExpressionStatement';
22+
(!!node.parent?.parent &&
23+
node.parent.parent.type === 'ExpressionStatement') ||
24+
(node.parent?.parent?.type === 'AwaitExpression' &&
25+
!!node.parent.parent.parent &&
26+
node.parent.parent.parent.type === 'ExpressionStatement');
27+
28+
const isVariableDeclaration = (node: TSESTree.Node) => {
29+
if (
30+
isCallExpression(node.parent) &&
31+
ASTUtils.isAwaitExpression(node.parent.parent) &&
32+
ASTUtils.isVariableDeclarator(node.parent.parent.parent)
33+
) {
34+
return true; // const quxElement = await findByLabelText('qux')
35+
}
36+
37+
if (
38+
isCallExpression(node.parent) &&
39+
ASTUtils.isVariableDeclarator(node.parent.parent)
40+
) {
41+
return true; // const quxElement = findByLabelText('qux')
42+
}
43+
44+
if (
45+
isMemberExpression(node.parent) &&
46+
isCallExpression(node.parent.parent) &&
47+
ASTUtils.isAwaitExpression(node.parent.parent.parent) &&
48+
ASTUtils.isVariableDeclarator(node.parent.parent.parent.parent)
49+
) {
50+
return true; // const quxElement = await screen.findByLabelText('qux')
51+
}
52+
53+
if (
54+
isMemberExpression(node.parent) &&
55+
isCallExpression(node.parent.parent) &&
56+
ASTUtils.isVariableDeclarator(node.parent.parent.parent)
57+
) {
58+
return true; // const quxElement = screen.findByLabelText('qux')
59+
}
60+
61+
return false;
62+
};
1963

2064
export default createTestingLibraryRule<Options, MessageIds>({
2165
name: RULE_NAME,
2266
meta: {
2367
type: 'suggestion',
2468
docs: {
2569
description:
26-
'Suggest using explicit assertions rather than just `getBy*` queries',
70+
'Suggest using explicit assertions rather than standalone queries',
2771
category: 'Best Practices',
2872
recommendedConfig: {
2973
dom: false,
@@ -34,9 +78,9 @@ export default createTestingLibraryRule<Options, MessageIds>({
3478
},
3579
messages: {
3680
preferExplicitAssert:
37-
'Wrap stand-alone `getBy*` query with `expect` function for better explicit assertion',
81+
'Wrap stand-alone `{{queryType}}` query with `expect` function for better explicit assertion',
3882
preferExplicitAssertAssertion:
39-
'`getBy*` queries must be asserted with `{{assertion}}`',
83+
'`getBy*` queries must be asserted with `{{assertion}}`', // TODO: support findBy* queries as well
4084
},
4185
schema: [
4286
{
@@ -55,14 +99,40 @@ export default createTestingLibraryRule<Options, MessageIds>({
5599
create(context, [options], helpers) {
56100
const { assertion } = options;
57101
const getQueryCalls: TSESTree.Identifier[] = [];
102+
const findQueryCalls: TSESTree.Identifier[] = [];
58103

59104
return {
60105
'CallExpression Identifier'(node: TSESTree.Identifier) {
61106
if (helpers.isGetQueryVariant(node)) {
62107
getQueryCalls.push(node);
63108
}
109+
110+
if (helpers.isFindQueryVariant(node)) {
111+
findQueryCalls.push(node);
112+
}
64113
},
65114
'Program:exit'() {
115+
findQueryCalls.forEach((queryCall) => {
116+
const memberExpression = isMemberExpression(queryCall.parent)
117+
? queryCall.parent
118+
: queryCall;
119+
120+
if (
121+
isVariableDeclaration(queryCall) ||
122+
!isAtTopLevel(memberExpression)
123+
) {
124+
return;
125+
}
126+
127+
context.report({
128+
node: queryCall,
129+
messageId: 'preferExplicitAssert',
130+
data: {
131+
queryType: 'findBy*',
132+
},
133+
});
134+
});
135+
66136
getQueryCalls.forEach((queryCall) => {
67137
const node = isMemberExpression(queryCall.parent)
68138
? queryCall.parent
@@ -72,6 +142,9 @@ export default createTestingLibraryRule<Options, MessageIds>({
72142
context.report({
73143
node: queryCall,
74144
messageId: 'preferExplicitAssert',
145+
data: {
146+
queryType: 'getBy*',
147+
},
75148
});
76149
}
77150

‎tests/lib/rules/prefer-explicit-assert.test.ts

+120
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,52 @@ ruleTester.run(RULE_NAME, rule, {
5454
...COMBINED_QUERIES_METHODS.map((queryMethod) => ({
5555
code: `const quxElement = get${queryMethod}('qux')`,
5656
})),
57+
...COMBINED_QUERIES_METHODS.map((queryMethod) => ({
58+
code: `
59+
async () => {
60+
const quxElement = await find${queryMethod}('qux')
61+
}`,
62+
})),
63+
...COMBINED_QUERIES_METHODS.map((queryMethod) => ({
64+
code: `const quxElement = find${queryMethod}('qux')`,
65+
})),
66+
...COMBINED_QUERIES_METHODS.map((queryMethod) => ({
67+
code: `const quxElement = screen.find${queryMethod}('qux')`,
68+
})),
69+
...COMBINED_QUERIES_METHODS.map((queryMethod) => ({
70+
code: `
71+
async () => {
72+
const quxElement = await screen.find${queryMethod}('qux')
73+
}`,
74+
})),
75+
...COMBINED_QUERIES_METHODS.map((queryMethod) => ({
76+
code: `
77+
function findBySubmit() {
78+
return screen.find${queryMethod}('foo')
79+
}`,
80+
})),
81+
...COMBINED_QUERIES_METHODS.map((queryMethod) => ({
82+
code: `
83+
function findBySubmit() {
84+
return find${queryMethod}('foo')
85+
}`,
86+
})),
87+
...COMBINED_QUERIES_METHODS.map((queryMethod) => ({
88+
code: `
89+
() => { return screen.find${queryMethod}('foo') }`,
90+
})),
91+
...COMBINED_QUERIES_METHODS.map((queryMethod) => ({
92+
code: `
93+
() => { return find${queryMethod}('foo') }`,
94+
})),
95+
...COMBINED_QUERIES_METHODS.map((queryMethod) => ({
96+
code: `
97+
() => screen.find${queryMethod}('foo')`,
98+
})),
99+
...COMBINED_QUERIES_METHODS.map((queryMethod) => ({
100+
code: `
101+
() => find${queryMethod}('foo')`,
102+
})),
57103
...COMBINED_QUERIES_METHODS.map((queryMethod) => ({
58104
code: `() => { return get${queryMethod}('foo') }`,
59105
})),
@@ -106,6 +152,65 @@ ruleTester.run(RULE_NAME, rule, {
106152
errors: [
107153
{
108154
messageId: 'preferExplicitAssert',
155+
data: {
156+
queryType: 'getBy*',
157+
},
158+
},
159+
],
160+
} as const)
161+
),
162+
...COMBINED_QUERIES_METHODS.map(
163+
(queryMethod) =>
164+
({
165+
code: `find${queryMethod}('foo')`,
166+
errors: [
167+
{
168+
messageId: 'preferExplicitAssert',
169+
data: { queryType: 'findBy*' },
170+
},
171+
],
172+
} as const)
173+
),
174+
...COMBINED_QUERIES_METHODS.map(
175+
(queryMethod) =>
176+
({
177+
code: `screen.find${queryMethod}('foo')`,
178+
errors: [
179+
{
180+
messageId: 'preferExplicitAssert',
181+
data: { queryType: 'findBy*' },
182+
},
183+
],
184+
} as const)
185+
),
186+
...COMBINED_QUERIES_METHODS.map(
187+
(queryMethod) =>
188+
({
189+
code: `
190+
async () => {
191+
await screen.find${queryMethod}('foo')
192+
}
193+
`,
194+
errors: [
195+
{
196+
messageId: 'preferExplicitAssert',
197+
data: { queryType: 'findBy*' },
198+
},
199+
],
200+
} as const)
201+
),
202+
...COMBINED_QUERIES_METHODS.map(
203+
(queryMethod) =>
204+
({
205+
code: `
206+
async () => {
207+
await find${queryMethod}('foo')
208+
}
209+
`,
210+
errors: [
211+
{
212+
messageId: 'preferExplicitAssert',
213+
data: { queryType: 'findBy*' },
109214
},
110215
],
111216
} as const)
@@ -122,6 +227,9 @@ ruleTester.run(RULE_NAME, rule, {
122227
messageId: 'preferExplicitAssert',
123228
line: 3,
124229
column: 15,
230+
data: {
231+
queryType: 'getBy*',
232+
},
125233
},
126234
],
127235
} as const)
@@ -135,6 +243,9 @@ ruleTester.run(RULE_NAME, rule, {
135243
messageId: 'preferExplicitAssert',
136244
line: 1,
137245
column: 8,
246+
data: {
247+
queryType: 'getBy*',
248+
},
138249
},
139250
],
140251
} as const)
@@ -155,10 +266,16 @@ ruleTester.run(RULE_NAME, rule, {
155266
{
156267
messageId: 'preferExplicitAssert',
157268
line: 3,
269+
data: {
270+
queryType: 'getBy*',
271+
},
158272
},
159273
{
160274
messageId: 'preferExplicitAssert',
161275
line: 6,
276+
data: {
277+
queryType: 'getBy*',
278+
},
162279
},
163280
],
164281
} as const)
@@ -176,6 +293,9 @@ ruleTester.run(RULE_NAME, rule, {
176293
errors: [
177294
{
178295
messageId: 'preferExplicitAssert',
296+
data: {
297+
queryType: 'getBy*',
298+
},
179299
},
180300
],
181301
} as const)

0 commit comments

Comments
 (0)