Skip to content

Commit 49205c2

Browse files
Belco90Belco90
authored
Merge branch 'main' into dependabot/npm_and_yarn/commitlint/cli-19.8.1
2 parents b243080 + 5e7b9a3 commit 49205c2

File tree

9 files changed

+879
-199
lines changed

9 files changed

+879
-199
lines changed

‎docs/rules/await-async-queries.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ problems in the tests. The promise will be considered as handled when:
2121

2222
- using the `await` operator
2323
- wrapped within `Promise.all` or `Promise.allSettled` methods
24-
- chaining the `then` method
24+
- chaining the `then` or `catch` method
2525
- chaining `resolves` or `rejects` from jest
2626
- chaining `toResolve()` or `toReject()` from [jest-extended](https://github.com/jest-community/jest-extended#promise)
2727
- chaining jasmine [async matchers](https://jasmine.github.io/api/edge/async-matchers.html)

‎docs/rules/await-async-utils.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ problems in the tests. The promise will be considered as handled when:
1919

2020
- using the `await` operator
2121
- wrapped within `Promise.all` or `Promise.allSettled` methods
22-
- chaining the `then` method
22+
- chaining the `then` or `catch` method
2323
- chaining `resolves` or `rejects` from jest
2424
- chaining `toResolve()` or `toReject()` from [jest-extended](https://github.com/jest-community/jest-extended#promise)
2525
- chaining jasmine [async matchers](https://jasmine.github.io/api/edge/async-matchers.html)

‎lib/node-utils/index.ts‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -151,16 +151,24 @@ export function hasThenProperty(node: TSESTree.Node): boolean {
151151
);
152152
}
153153

154-
export function hasChainedThen(node: TSESTree.Node): boolean {
154+
export function hasPromiseHandlerProperty(node: TSESTree.Node): boolean {
155+
return (
156+
isMemberExpression(node) &&
157+
ASTUtils.isIdentifier(node.property) &&
158+
['then', 'catch'].includes(node.property.name)
159+
);
160+
}
161+
162+
export function hasChainedPromiseHandler(node: TSESTree.Node): boolean {
155163
const parent = node.parent;
156164

157-
// wait(...).then(...)
165+
// wait(...).then(...) or wait(...).catch(...)
158166
if (isCallExpression(parent) && parent.parent) {
159-
return hasThenProperty(parent.parent);
167+
return hasPromiseHandlerProperty(parent.parent);
160168
}
161169

162-
// promise.then(...)
163-
return !!parent && hasThenProperty(parent);
170+
// promise.then(...) or promise.catch(...)
171+
return !!parent && hasPromiseHandlerProperty(parent);
164172
}
165173

166174
export function isPromiseIdentifier(
@@ -214,7 +222,7 @@ export function isPromisesArrayResolved(node: TSESTree.Node): boolean {
214222
* - it belongs to the `await` expression
215223
* - it belongs to the `Promise.all` method
216224
* - it belongs to the `Promise.allSettled` method
217-
* - it's chained with the `then` method
225+
* - it's chained with the `then` or `catch` method
218226
* - it's returned from a function
219227
* - has `resolves` or `rejects` jest methods
220228
* - has `toResolve` or `toReject` jest-extended matchers
@@ -243,7 +251,7 @@ export function isPromiseHandled(nodeIdentifier: TSESTree.Identifier): boolean {
243251
)
244252
return true;
245253
if (hasClosestExpectHandlesPromise(node.parent)) return true;
246-
if (hasChainedThen(node)) return true;
254+
if (hasChainedPromiseHandler(node)) return true;
247255
if (isPromisesArrayResolved(node)) return true;
248256
});
249257
}

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

Lines changed: 22 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,15 @@
1-
import {
2-
DefinitionType,
3-
type ScopeVariable,
4-
} from '@typescript-eslint/scope-manager';
51
import { TSESTree, ASTUtils } from '@typescript-eslint/utils';
62

73
import { createTestingLibraryRule } from '../create-testing-library-rule';
84
import {
95
getDeepestIdentifierNode,
10-
getPropertyIdentifierNode,
11-
isCallExpression,
6+
isLiteral,
127
isMemberExpression,
138
} from '../node-utils';
149
import {
10+
ALL_QUERIES_COMBINATIONS,
1511
ALL_RETURNING_NODES,
1612
EVENT_HANDLER_METHODS,
17-
getScope,
1813
resolveToTestingLibraryFn,
1914
} from '../utils';
2015

@@ -60,8 +55,6 @@ export default createTestingLibraryRule<Options, MessageIds>({
6055
],
6156

6257
create(context, [{ allowContainerFirstChild = false }], helpers) {
63-
const userEventInstanceNames = new Set<string>();
64-
6558
function showErrorForNodeAccess(node: TSESTree.MemberExpression) {
6659
// This rule is so aggressive that can cause tons of false positives outside test files when Aggressive Reporting
6760
// is enabled. Because of that, this rule will skip this mechanism and report only if some Testing Library package
@@ -99,94 +92,44 @@ export default createTestingLibraryRule<Options, MessageIds>({
9992
}
10093
}
10194

102-
function detectTestingLibraryFn(
103-
node: TSESTree.CallExpression,
104-
variable: ScopeVariable | null
95+
function getProperty(
96+
node: TSESTree.PrivateIdentifier | TSESTree.Expression
10597
) {
106-
if (variable && variable.defs.length > 0) {
107-
const def = variable.defs[0];
108-
if (
109-
def.type === DefinitionType.Variable &&
110-
isCallExpression(def.node.init)
111-
) {
112-
return resolveToTestingLibraryFn(def.node.init, context);
113-
}
98+
if (isLiteral(node)) {
99+
return node;
114100
}
115101

116-
return resolveToTestingLibraryFn(node, context);
102+
return getDeepestIdentifierNode(node);
117103
}
118104

119105
return {
120106
CallExpression(node: TSESTree.CallExpression) {
121-
const property = getDeepestIdentifierNode(node);
122-
const identifier = getPropertyIdentifierNode(node);
123-
124-
const isEventHandlerMethod = EVENT_HANDLER_METHODS.some(
125-
(method) => method === property?.name
126-
);
127-
const hasUserEventInstanceName = userEventInstanceNames.has(
128-
identifier?.name ?? ''
129-
);
130-
131-
const variable = identifier
132-
? ASTUtils.findVariable(getScope(context, node), identifier)
133-
: null;
134-
const testingLibraryFn = detectTestingLibraryFn(node, variable);
107+
if (!isMemberExpression(node.callee)) return;
135108

109+
const { callee } = node;
136110
if (
137-
!testingLibraryFn &&
138-
isEventHandlerMethod &&
139-
!hasUserEventInstanceName
111+
!EVENT_HANDLER_METHODS.some(
112+
(method) => method === ASTUtils.getPropertyName(callee)
113+
)
140114
) {
141-
context.report({
142-
node,
143-
loc: property?.loc.start,
144-
messageId: 'noNodeAccess',
145-
});
146-
}
147-
},
148-
VariableDeclarator(node: TSESTree.VariableDeclarator) {
149-
const { init, id } = node;
150-
151-
if (!isCallExpression(init)) {
152115
return;
153116
}
117+
const identifier = getDeepestIdentifierNode(callee.object);
154118

155119
if (
156-
!isMemberExpression(init.callee) ||
157-
!ASTUtils.isIdentifier(init.callee.object)
120+
!identifier ||
121+
!ALL_QUERIES_COMBINATIONS.includes(identifier.name)
158122
) {
159123
return;
160124
}
161125

162-
const testingLibraryFn = resolveToTestingLibraryFn(init, context);
163-
if (
164-
init.callee.object.name === testingLibraryFn?.local &&
165-
ASTUtils.isIdentifier(init.callee.property) &&
166-
init.callee.property.name === 'setup' &&
167-
ASTUtils.isIdentifier(id)
168-
) {
169-
userEventInstanceNames.add(id.name);
170-
}
171-
},
172-
AssignmentExpression(node: TSESTree.AssignmentExpression) {
173-
if (
174-
ASTUtils.isIdentifier(node.left) &&
175-
isCallExpression(node.right) &&
176-
isMemberExpression(node.right.callee) &&
177-
ASTUtils.isIdentifier(node.right.callee.object)
178-
) {
179-
const testingLibraryFn = resolveToTestingLibraryFn(
180-
node.right,
181-
context
182-
);
183-
if (
184-
node.right.callee.object.name === testingLibraryFn?.local &&
185-
ASTUtils.isIdentifier(node.right.callee.property) &&
186-
node.right.callee.property.name === 'setup'
187-
) {
188-
userEventInstanceNames.add(node.left.name);
189-
}
126+
if (resolveToTestingLibraryFn(node, context)) {
127+
const property = getProperty(callee.property);
128+
context.report({
129+
node,
130+
loc: property?.loc.start,
131+
messageId: 'noNodeAccess',
132+
});
190133
}
191134
},
192135
'ExpressionStatement MemberExpression': showErrorForNodeAccess,

‎tests/lib/rules/await-async-events.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,16 @@ ruleTester.run(RULE_NAME, rule, {
9696
fireEvent.${eventMethod}(getByLabelText('username')).then(() => { done() })
9797
})
9898
})
99+
`,
100+
options: [{ eventModule: 'fireEvent' }] as const,
101+
})),
102+
...FIRE_EVENT_ASYNC_FUNCTIONS.map((eventMethod) => ({
103+
code: `
104+
import { fireEvent } from '${testingFramework}'
105+
test('chain catch method to promise from event method is valid', async (done) => {
106+
fireEvent.${eventMethod}(getByLabelText('username'))
107+
.catch((error) => { done() })
108+
})
99109
`,
100110
options: [{ eventModule: 'fireEvent' }] as const,
101111
})),
@@ -330,6 +340,16 @@ ruleTester.run(RULE_NAME, rule, {
330340
...USER_EVENT_ASYNC_FUNCTIONS.map((eventMethod) => ({
331341
code: `
332342
import userEvent from '${testingFramework}'
343+
test('chain catch method to promise from event method is valid', async (done) => {
344+
userEvent.${eventMethod}(getByLabelText('username'))
345+
.catch((error) => { done() })
346+
})
347+
`,
348+
options: [{ eventModule: 'userEvent' }] as const,
349+
})),
350+
...USER_EVENT_ASYNC_FUNCTIONS.map((eventMethod) => ({
351+
code: `
352+
import userEvent from '${testingFramework}'
333353
test('chain then method to several promises from event methods is valid', async (done) => {
334354
userEvent.${eventMethod}(getByLabelText('username')).then(() => {
335355
userEvent.${eventMethod}(getByLabelText('username')).then(() => { done() })

‎tests/lib/rules/await-async-queries.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,15 @@ ruleTester.run(RULE_NAME, rule, {
160160
`
161161
),
162162

163+
// async queries are valid with catch
164+
...createTestCase(
165+
(query) => `
166+
${query}('foo').catch((error) => {
167+
expect(error.message).toMatch(/my error message/)
168+
})
169+
`
170+
),
171+
163172
// async queries are valid when wrapped within Promise.all + await expression
164173
...createTestCase(
165174
(query) => `

‎tests/lib/rules/await-async-utils.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,15 @@ ruleTester.run(RULE_NAME, rule, {
4848
doSomethingElse();
4949
${asyncUtil}(() => getByLabelText('email')).then(() => { console.log('done') });
5050
});
51+
`,
52+
})),
53+
...ASYNC_UTILS.map((asyncUtil) => ({
54+
code: `
55+
import { ${asyncUtil} } from '${testingFramework}';
56+
test('${asyncUtil} util directly chained with catch is valid', () => {
57+
doSomethingElse();
58+
${asyncUtil}(() => getByLabelText('email')).catch((error) => { console.log('done') });
59+
});
5160
`,
5261
})),
5362
...ASYNC_UTILS.map((asyncUtil) => ({

0 commit comments

Comments
 (0)