diff --git a/README.md b/README.md index 8c2377a37..853b14e40 100644 --- a/README.md +++ b/README.md @@ -351,6 +351,7 @@ Manually fixable by | [no-standalone-expect](docs/rules/no-standalone-expect.md) | Disallow using `expect` outside of `it` or `test` blocks | ✅ | | | | | [no-test-prefixes](docs/rules/no-test-prefixes.md) | Require using `.only` and `.skip` over `f` and `x` | ✅ | | 🔧 | | | [no-test-return-statement](docs/rules/no-test-return-statement.md) | Disallow explicitly returning from tests | | | | | +| [no-unneeded-async-expect-function](docs/rules/no-unneeded-async-expect-function.md) | Disallow unnecessary async function wrapper for expected promises | | | 🔧 | | | [no-untyped-mock-factory](docs/rules/no-untyped-mock-factory.md) | Disallow using `jest.mock()` factories without an explicit type parameter | | | 🔧 | | | [padding-around-after-all-blocks](docs/rules/padding-around-after-all-blocks.md) | Enforce padding around `afterAll` blocks | | | 🔧 | | | [padding-around-after-each-blocks](docs/rules/padding-around-after-each-blocks.md) | Enforce padding around `afterEach` blocks | | | 🔧 | | diff --git a/docs/rules/no-unneeded-async-expect-function.md b/docs/rules/no-unneeded-async-expect-function.md new file mode 100644 index 000000000..492b50511 --- /dev/null +++ b/docs/rules/no-unneeded-async-expect-function.md @@ -0,0 +1,39 @@ +# Disallow unnecessary async function wrapper for expected promises (`no-unneeded-async-expect-function`) + +🔧 This rule is automatically fixable by the +[`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix). + + + +`Jest` can handle fulfilled/rejected promisified function call normally but +occassionally, engineers wrap said function in another `async` function that is +excessively verbose and make the tests harder to read. + +## Rule details + +This rule triggers a warning if `expect` is passed with an an `async` function +that has a single `await` call. + +Examples of **incorrect** code for this rule + +```js +it('wrong1', async () => { + await expect(async () => { + await doSomethingAsync(); + }).rejects.toThrow(); +}); + +it('wrong2', async () => { + await expect(async function () { + await doSomethingAsync(); + }).rejects.toThrow(); +}); +``` + +Examples of **correct** code for this rule + +```js +it('right1', async () => { + await expect(doSomethingAsync()).rejects.toThrow(); +}); +``` diff --git a/src/__tests__/__snapshots__/rules.test.ts.snap b/src/__tests__/__snapshots__/rules.test.ts.snap index 7ce8670db..307015837 100644 --- a/src/__tests__/__snapshots__/rules.test.ts.snap +++ b/src/__tests__/__snapshots__/rules.test.ts.snap @@ -36,6 +36,7 @@ exports[`rules should export configs that refer to actual rules 1`] = ` "jest/no-standalone-expect": "error", "jest/no-test-prefixes": "error", "jest/no-test-return-statement": "error", + "jest/no-unneeded-async-expect-function": "error", "jest/no-untyped-mock-factory": "error", "jest/padding-around-after-all-blocks": "error", "jest/padding-around-after-each-blocks": "error", @@ -131,6 +132,7 @@ exports[`rules should export configs that refer to actual rules 1`] = ` "jest/no-standalone-expect": "error", "jest/no-test-prefixes": "error", "jest/no-test-return-statement": "error", + "jest/no-unneeded-async-expect-function": "error", "jest/no-untyped-mock-factory": "error", "jest/padding-around-after-all-blocks": "error", "jest/padding-around-after-each-blocks": "error", diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index 953f86d55..763bff5ac 100644 --- a/src/__tests__/rules.test.ts +++ b/src/__tests__/rules.test.ts @@ -2,7 +2,7 @@ import { existsSync } from 'fs'; import { resolve } from 'path'; import plugin from '../'; -const numberOfRules = 66; +const numberOfRules = 67; const ruleNames = Object.keys(plugin.rules); const deprecatedRules = Object.entries(plugin.rules) .filter(([, rule]) => rule.meta.deprecated) diff --git a/src/rules/__tests__/no-unneeded-async-expect-function.test.ts b/src/rules/__tests__/no-unneeded-async-expect-function.test.ts new file mode 100644 index 000000000..aa4b2c6dc --- /dev/null +++ b/src/rules/__tests__/no-unneeded-async-expect-function.test.ts @@ -0,0 +1,230 @@ +import dedent from 'dedent'; +import rule from '../no-unneeded-async-expect-function'; +import { FlatCompatRuleTester as RuleTester, espreeParser } from './test-utils'; + +const ruleTester = new RuleTester({ + parser: espreeParser, + parserOptions: { + ecmaVersion: 2017, + }, +}); + +ruleTester.run('no-unneeded-async-expect-function', rule, { + valid: [ + 'expect.hasAssertions()', + dedent` + it('pass', async () => { + expect(); + }) + `, + dedent` + it('pass', async () => { + await expect(doSomethingAsync()).rejects.toThrow(); + }) + `, + dedent` + it('pass', async () => { + await expect(doSomethingAsync(1, 2)).resolves.toBe(1); + }) + `, + dedent` + it('pass', async () => { + await expect(async () => { + await doSomethingAsync(); + await doSomethingTwiceAsync(1, 2); + }).rejects.toThrow(); + }) + `, + { + code: dedent` + import { expect as pleaseExpect } from '@jest/globals'; + it('pass', async () => { + await pleaseExpect(doSomethingAsync()).rejects.toThrow(); + }) + `, + parserOptions: { sourceType: 'module' }, + }, + dedent` + it('pass', async () => { + await expect(async () => { + doSomethingAsync(); + }).rejects.toThrow(); + }) + `, + dedent` + it('pass', async () => { + await expect(async () => { + const a = 1; + await doSomethingAsync(a); + }).rejects.toThrow(); + }) + `, + dedent` + it('pass for non-async expect', async () => { + await expect(() => { + doSomethingSync(a); + }).rejects.toThrow(); + }) + `, + dedent` + it('pass for await in expect', async () => { + await expect(await doSomethingAsync()).rejects.toThrow(); + }) + `, + dedent` + it('pass for different matchers', async () => { + await expect(await doSomething()).not.toThrow(); + await expect(await doSomething()).toHaveLength(2); + await expect(await doSomething()).toHaveReturned(); + await expect(await doSomething()).not.toHaveBeenCalled(); + await expect(await doSomething()).not.toBeDefined(); + await expect(await doSomething()).toEqual(2); + }) + `, + dedent` + it('pass for using await within for-loop', async () => { + const b = [async () => Promise.resolve(1), async () => Promise.reject(2)]; + await expect(async() => { + for (const a of b) { + await b(); + } + }).rejects.toThrow(); + }) + `, + dedent` + it('pass for using await within array', async () => { + await expect(async() => [await Promise.reject(2)]).rejects.toThrow(2); + }) + `, + ], + invalid: [ + { + code: dedent` + it('should be fixed', async () => { + await expect(async () => { + await doSomethingAsync(); + }).rejects.toThrow(); + }) + `, + output: dedent` + it('should be fixed', async () => { + await expect(doSomethingAsync()).rejects.toThrow(); + }) + `, + errors: [ + { + endColumn: 4, + column: 16, + messageId: 'noAsyncWrapperForExpectedPromise', + }, + ], + }, + { + code: dedent` + it('should be fixed', async () => { + await expect(async function () { + await doSomethingAsync(); + }).rejects.toThrow(); + }) + `, + output: dedent` + it('should be fixed', async () => { + await expect(doSomethingAsync()).rejects.toThrow(); + }) + `, + errors: [ + { + endColumn: 4, + column: 16, + messageId: 'noAsyncWrapperForExpectedPromise', + }, + ], + }, + { + code: dedent` + it('should be fixed for async arrow function', async () => { + await expect(async () => { + await doSomethingAsync(1, 2); + }).rejects.toThrow(); + }) + `, + output: dedent` + it('should be fixed for async arrow function', async () => { + await expect(doSomethingAsync(1, 2)).rejects.toThrow(); + }) + `, + errors: [ + { + endColumn: 4, + column: 16, + messageId: 'noAsyncWrapperForExpectedPromise', + }, + ], + }, + { + code: dedent` + it('should be fixed for async normal function', async () => { + await expect(async function () { + await doSomethingAsync(1, 2); + }).rejects.toThrow(); + }) + `, + output: dedent` + it('should be fixed for async normal function', async () => { + await expect(doSomethingAsync(1, 2)).rejects.toThrow(); + }) + `, + errors: [ + { + endColumn: 4, + column: 16, + messageId: 'noAsyncWrapperForExpectedPromise', + }, + ], + }, + { + code: dedent` + it('should be fixed for Promise.all', async () => { + await expect(async function () { + await Promise.all([doSomethingAsync(1, 2), doSomethingAsync()]); + }).rejects.toThrow(); + }) + `, + output: dedent` + it('should be fixed for Promise.all', async () => { + await expect(Promise.all([doSomethingAsync(1, 2), doSomethingAsync()])).rejects.toThrow(); + }) + `, + errors: [ + { + endColumn: 4, + column: 16, + messageId: 'noAsyncWrapperForExpectedPromise', + }, + ], + }, + { + code: dedent` + it('should be fixed for async ref to expect', async () => { + const a = async () => { await doSomethingAsync() }; + await expect(async () => { + await a(); + }).rejects.toThrow(); + }) + `, + output: dedent` + it('should be fixed for async ref to expect', async () => { + const a = async () => { await doSomethingAsync() }; + await expect(a()).rejects.toThrow(); + }) + `, + errors: [ + { + endColumn: 4, + column: 16, + messageId: 'noAsyncWrapperForExpectedPromise', + }, + ], + }, + ], +}); diff --git a/src/rules/no-unneeded-async-expect-function.ts b/src/rules/no-unneeded-async-expect-function.ts new file mode 100644 index 000000000..51cd35738 --- /dev/null +++ b/src/rules/no-unneeded-async-expect-function.ts @@ -0,0 +1,73 @@ +import { AST_NODE_TYPES, type TSESTree } from '@typescript-eslint/utils'; +import { createRule, isFunction, parseJestFnCall } from './utils'; + +export default createRule({ + name: __filename, + meta: { + docs: { + description: + 'Disallow unnecessary async function wrapper for expected promises', + }, + fixable: 'code', + messages: { + noAsyncWrapperForExpectedPromise: 'Unnecessary async function wrapper', + }, + schema: [], + type: 'suggestion', + }, + defaultOptions: [], + create(context) { + return { + CallExpression(node: TSESTree.CallExpression) { + const jestFnCall = parseJestFnCall(node, context); + + if (jestFnCall?.type !== 'expect') { + return; + } + + const { parent } = jestFnCall.head.node; + + if (parent?.type !== AST_NODE_TYPES.CallExpression) { + return; + } + + const [awaitNode] = parent.arguments; + + if ( + !awaitNode || + !isFunction(awaitNode) || + !awaitNode?.async || + awaitNode.body.type !== AST_NODE_TYPES.BlockStatement || + awaitNode.body.body.length !== 1 + ) { + return; + } + + const [callback] = awaitNode.body.body; + + if ( + callback.type === AST_NODE_TYPES.ExpressionStatement && + callback.expression.type === AST_NODE_TYPES.AwaitExpression && + callback.expression.argument.type === AST_NODE_TYPES.CallExpression + ) { + const innerAsyncFuncCall = callback.expression.argument; + + context.report({ + node: awaitNode, + messageId: 'noAsyncWrapperForExpectedPromise', + fix(fixer) { + const { sourceCode } = context; + + return [ + fixer.replaceTextRange( + [awaitNode.range[0], awaitNode.range[1]], + sourceCode.getText(innerAsyncFuncCall), + ), + ]; + }, + }); + } + }, + }; + }, +});