Skip to content

Commit 6b8182d

Browse files
hainenberhainenber
authored
feat: create new no-unneeded-async-expect-function rule (#1863)
* feat(rule): disallow unnecessary async function wrapper for single `await` call Signed-off-by: hainenber <[email protected]> * chore: regenerate docs for changed error message Signed-off-by: hainenber <[email protected]> * chore: remove from recommended config Signed-off-by: hainenber <[email protected]> * chore: correct indentation for test code samples + use isFunction utility + rephrase error message Signed-off-by: hainenber <[email protected]> * chore: add valid test case Signed-off-by: hainenber <[email protected]> * chore: resolve merge conflict with `main` branch Signed-off-by: hainenber <[email protected]> * chore: correct English grammar for invalid test case title Signed-off-by: hainenber <[email protected]> * chore: resolve lint issues Signed-off-by: hainenber <[email protected]> * feat: add tests to cover more invalid/valid cases + rename rule Signed-off-by: hainenber <[email protected]> --------- Signed-off-by: hainenber <[email protected]>
1 parent 48e78fc commit 6b8182d

File tree

6 files changed

+346
-1
lines changed

6 files changed

+346
-1
lines changed

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,7 @@ Manually fixable by
351351
| [no-standalone-expect](docs/rules/no-standalone-expect.md) | Disallow using `expect` outside of `it` or `test` blocks | ✅ | | | |
352352
| [no-test-prefixes](docs/rules/no-test-prefixes.md) | Require using `.only` and `.skip` over `f` and `x` | ✅ | | 🔧 | |
353353
| [no-test-return-statement](docs/rules/no-test-return-statement.md) | Disallow explicitly returning from tests | | | | |
354+
| [no-unneeded-async-expect-function](docs/rules/no-unneeded-async-expect-function.md) | Disallow unnecessary async function wrapper for expected promises | | | 🔧 | |
354355
| [no-untyped-mock-factory](docs/rules/no-untyped-mock-factory.md) | Disallow using `jest.mock()` factories without an explicit type parameter | | | 🔧 | |
355356
| [padding-around-after-all-blocks](docs/rules/padding-around-after-all-blocks.md) | Enforce padding around `afterAll` blocks | | | 🔧 | |
356357
| [padding-around-after-each-blocks](docs/rules/padding-around-after-each-blocks.md) | Enforce padding around `afterEach` blocks | | | 🔧 | |

‎docs/rules/no-unneeded-async-expect-function.md‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# Disallow unnecessary async function wrapper for expected promises (`no-unneeded-async-expect-function`)
2+
3+
🔧 This rule is automatically fixable by the
4+
[`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).
5+
6+
<!-- end auto-generated rule header -->
7+
8+
`Jest` can handle fulfilled/rejected promisified function call normally but
9+
occassionally, engineers wrap said function in another `async` function that is
10+
excessively verbose and make the tests harder to read.
11+
12+
## Rule details
13+
14+
This rule triggers a warning if `expect` is passed with an an `async` function
15+
that has a single `await` call.
16+
17+
Examples of **incorrect** code for this rule
18+
19+
```js
20+
it('wrong1', async () => {
21+
await expect(async () => {
22+
await doSomethingAsync();
23+
}).rejects.toThrow();
24+
});
25+
26+
it('wrong2', async () => {
27+
await expect(async function () {
28+
await doSomethingAsync();
29+
}).rejects.toThrow();
30+
});
31+
```
32+
33+
Examples of **correct** code for this rule
34+
35+
```js
36+
it('right1', async () => {
37+
await expect(doSomethingAsync()).rejects.toThrow();
38+
});
39+
```

‎src/__tests__/__snapshots__/rules.test.ts.snap‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ exports[`rules should export configs that refer to actual rules 1`] = `
3636
"jest/no-standalone-expect": "error",
3737
"jest/no-test-prefixes": "error",
3838
"jest/no-test-return-statement": "error",
39+
"jest/no-unneeded-async-expect-function": "error",
3940
"jest/no-untyped-mock-factory": "error",
4041
"jest/padding-around-after-all-blocks": "error",
4142
"jest/padding-around-after-each-blocks": "error",
@@ -131,6 +132,7 @@ exports[`rules should export configs that refer to actual rules 1`] = `
131132
"jest/no-standalone-expect": "error",
132133
"jest/no-test-prefixes": "error",
133134
"jest/no-test-return-statement": "error",
135+
"jest/no-unneeded-async-expect-function": "error",
134136
"jest/no-untyped-mock-factory": "error",
135137
"jest/padding-around-after-all-blocks": "error",
136138
"jest/padding-around-after-each-blocks": "error",

‎src/__tests__/rules.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { existsSync } from 'fs';
22
import { resolve } from 'path';
33
import plugin from '../';
44

5-
const numberOfRules = 66;
5+
const numberOfRules = 67;
66
const ruleNames = Object.keys(plugin.rules);
77
const deprecatedRules = Object.entries(plugin.rules)
88
.filter(([, rule]) => rule.meta.deprecated)

‎src/rules/__tests__/no-unneeded-async-expect-function.test.ts‎

Lines changed: 230 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,230 @@
1+
import dedent from 'dedent';
2+
import rule from '../no-unneeded-async-expect-function';
3+
import { FlatCompatRuleTester as RuleTester, espreeParser } from './test-utils';
4+
5+
const ruleTester = new RuleTester({
6+
parser: espreeParser,
7+
parserOptions: {
8+
ecmaVersion: 2017,
9+
},
10+
});
11+
12+
ruleTester.run('no-unneeded-async-expect-function', rule, {
13+
valid: [
14+
'expect.hasAssertions()',
15+
dedent`
16+
it('pass', async () => {
17+
expect();
18+
})
19+
`,
20+
dedent`
21+
it('pass', async () => {
22+
await expect(doSomethingAsync()).rejects.toThrow();
23+
})
24+
`,
25+
dedent`
26+
it('pass', async () => {
27+
await expect(doSomethingAsync(1, 2)).resolves.toBe(1);
28+
})
29+
`,
30+
dedent`
31+
it('pass', async () => {
32+
await expect(async () => {
33+
await doSomethingAsync();
34+
await doSomethingTwiceAsync(1, 2);
35+
}).rejects.toThrow();
36+
})
37+
`,
38+
{
39+
code: dedent`
40+
import { expect as pleaseExpect } from '@jest/globals';
41+
it('pass', async () => {
42+
await pleaseExpect(doSomethingAsync()).rejects.toThrow();
43+
})
44+
`,
45+
parserOptions: { sourceType: 'module' },
46+
},
47+
dedent`
48+
it('pass', async () => {
49+
await expect(async () => {
50+
doSomethingAsync();
51+
}).rejects.toThrow();
52+
})
53+
`,
54+
dedent`
55+
it('pass', async () => {
56+
await expect(async () => {
57+
const a = 1;
58+
await doSomethingAsync(a);
59+
}).rejects.toThrow();
60+
})
61+
`,
62+
dedent`
63+
it('pass for non-async expect', async () => {
64+
await expect(() => {
65+
doSomethingSync(a);
66+
}).rejects.toThrow();
67+
})
68+
`,
69+
dedent`
70+
it('pass for await in expect', async () => {
71+
await expect(await doSomethingAsync()).rejects.toThrow();
72+
})
73+
`,
74+
dedent`
75+
it('pass for different matchers', async () => {
76+
await expect(await doSomething()).not.toThrow();
77+
await expect(await doSomething()).toHaveLength(2);
78+
await expect(await doSomething()).toHaveReturned();
79+
await expect(await doSomething()).not.toHaveBeenCalled();
80+
await expect(await doSomething()).not.toBeDefined();
81+
await expect(await doSomething()).toEqual(2);
82+
})
83+
`,
84+
dedent`
85+
it('pass for using await within for-loop', async () => {
86+
const b = [async () => Promise.resolve(1), async () => Promise.reject(2)];
87+
await expect(async() => {
88+
for (const a of b) {
89+
await b();
90+
}
91+
}).rejects.toThrow();
92+
})
93+
`,
94+
dedent`
95+
it('pass for using await within array', async () => {
96+
await expect(async() => [await Promise.reject(2)]).rejects.toThrow(2);
97+
})
98+
`,
99+
],
100+
invalid: [
101+
{
102+
code: dedent`
103+
it('should be fixed', async () => {
104+
await expect(async () => {
105+
await doSomethingAsync();
106+
}).rejects.toThrow();
107+
})
108+
`,
109+
output: dedent`
110+
it('should be fixed', async () => {
111+
await expect(doSomethingAsync()).rejects.toThrow();
112+
})
113+
`,
114+
errors: [
115+
{
116+
endColumn: 4,
117+
column: 16,
118+
messageId: 'noAsyncWrapperForExpectedPromise',
119+
},
120+
],
121+
},
122+
{
123+
code: dedent`
124+
it('should be fixed', async () => {
125+
await expect(async function () {
126+
await doSomethingAsync();
127+
}).rejects.toThrow();
128+
})
129+
`,
130+
output: dedent`
131+
it('should be fixed', async () => {
132+
await expect(doSomethingAsync()).rejects.toThrow();
133+
})
134+
`,
135+
errors: [
136+
{
137+
endColumn: 4,
138+
column: 16,
139+
messageId: 'noAsyncWrapperForExpectedPromise',
140+
},
141+
],
142+
},
143+
{
144+
code: dedent`
145+
it('should be fixed for async arrow function', async () => {
146+
await expect(async () => {
147+
await doSomethingAsync(1, 2);
148+
}).rejects.toThrow();
149+
})
150+
`,
151+
output: dedent`
152+
it('should be fixed for async arrow function', async () => {
153+
await expect(doSomethingAsync(1, 2)).rejects.toThrow();
154+
})
155+
`,
156+
errors: [
157+
{
158+
endColumn: 4,
159+
column: 16,
160+
messageId: 'noAsyncWrapperForExpectedPromise',
161+
},
162+
],
163+
},
164+
{
165+
code: dedent`
166+
it('should be fixed for async normal function', async () => {
167+
await expect(async function () {
168+
await doSomethingAsync(1, 2);
169+
}).rejects.toThrow();
170+
})
171+
`,
172+
output: dedent`
173+
it('should be fixed for async normal function', async () => {
174+
await expect(doSomethingAsync(1, 2)).rejects.toThrow();
175+
})
176+
`,
177+
errors: [
178+
{
179+
endColumn: 4,
180+
column: 16,
181+
messageId: 'noAsyncWrapperForExpectedPromise',
182+
},
183+
],
184+
},
185+
{
186+
code: dedent`
187+
it('should be fixed for Promise.all', async () => {
188+
await expect(async function () {
189+
await Promise.all([doSomethingAsync(1, 2), doSomethingAsync()]);
190+
}).rejects.toThrow();
191+
})
192+
`,
193+
output: dedent`
194+
it('should be fixed for Promise.all', async () => {
195+
await expect(Promise.all([doSomethingAsync(1, 2), doSomethingAsync()])).rejects.toThrow();
196+
})
197+
`,
198+
errors: [
199+
{
200+
endColumn: 4,
201+
column: 16,
202+
messageId: 'noAsyncWrapperForExpectedPromise',
203+
},
204+
],
205+
},
206+
{
207+
code: dedent`
208+
it('should be fixed for async ref to expect', async () => {
209+
const a = async () => { await doSomethingAsync() };
210+
await expect(async () => {
211+
await a();
212+
}).rejects.toThrow();
213+
})
214+
`,
215+
output: dedent`
216+
it('should be fixed for async ref to expect', async () => {
217+
const a = async () => { await doSomethingAsync() };
218+
await expect(a()).rejects.toThrow();
219+
})
220+
`,
221+
errors: [
222+
{
223+
endColumn: 4,
224+
column: 16,
225+
messageId: 'noAsyncWrapperForExpectedPromise',
226+
},
227+
],
228+
},
229+
],
230+
});

‎src/rules/no-unneeded-async-expect-function.ts‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import { AST_NODE_TYPES, type TSESTree } from '@typescript-eslint/utils';
2+
import { createRule, isFunction, parseJestFnCall } from './utils';
3+
4+
export default createRule({
5+
name: __filename,
6+
meta: {
7+
docs: {
8+
description:
9+
'Disallow unnecessary async function wrapper for expected promises',
10+
},
11+
fixable: 'code',
12+
messages: {
13+
noAsyncWrapperForExpectedPromise: 'Unnecessary async function wrapper',
14+
},
15+
schema: [],
16+
type: 'suggestion',
17+
},
18+
defaultOptions: [],
19+
create(context) {
20+
return {
21+
CallExpression(node: TSESTree.CallExpression) {
22+
const jestFnCall = parseJestFnCall(node, context);
23+
24+
if (jestFnCall?.type !== 'expect') {
25+
return;
26+
}
27+
28+
const { parent } = jestFnCall.head.node;
29+
30+
if (parent?.type !== AST_NODE_TYPES.CallExpression) {
31+
return;
32+
}
33+
34+
const [awaitNode] = parent.arguments;
35+
36+
if (
37+
!awaitNode ||
38+
!isFunction(awaitNode) ||
39+
!awaitNode?.async ||
40+
awaitNode.body.type !== AST_NODE_TYPES.BlockStatement ||
41+
awaitNode.body.body.length !== 1
42+
) {
43+
return;
44+
}
45+
46+
const [callback] = awaitNode.body.body;
47+
48+
if (
49+
callback.type === AST_NODE_TYPES.ExpressionStatement &&
50+
callback.expression.type === AST_NODE_TYPES.AwaitExpression &&
51+
callback.expression.argument.type === AST_NODE_TYPES.CallExpression
52+
) {
53+
const innerAsyncFuncCall = callback.expression.argument;
54+
55+
context.report({
56+
node: awaitNode,
57+
messageId: 'noAsyncWrapperForExpectedPromise',
58+
fix(fixer) {
59+
const { sourceCode } = context;
60+
61+
return [
62+
fixer.replaceTextRange(
63+
[awaitNode.range[0], awaitNode.range[1]],
64+
sourceCode.getText(innerAsyncFuncCall),
65+
),
66+
];
67+
},
68+
});
69+
}
70+
},
71+
};
72+
},
73+
});

0 commit comments

Comments
 (0)