From 1fee973429a74c60b14eead6a335623b4349b5f2 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Mon, 26 Apr 2021 12:10:26 +1200 Subject: [PATCH] fix(no-conditional-expect): check for expects in `catch`s on promises (#819) --- docs/rules/no-conditional-expect.md | 11 ++ .../__tests__/no-conditional-expect.test.ts | 106 ++++++++++++++++++ src/rules/no-conditional-expect.ts | 29 ++++- 3 files changed, 145 insertions(+), 1 deletion(-) diff --git a/docs/rules/no-conditional-expect.md b/docs/rules/no-conditional-expect.md index e10b563ea..716708699 100644 --- a/docs/rules/no-conditional-expect.md +++ b/docs/rules/no-conditional-expect.md @@ -3,6 +3,9 @@ This rule prevents the use of `expect` in conditional blocks, such as `if`s & `catch`s. +This includes using `expect` in callbacks to functions named `catch`, which are +assumed to be promises. + ## Rule Details Jest considered a test to have failed if it throws an error, rather than on if @@ -37,6 +40,10 @@ it('baz', async () => { expect(err).toMatchObject({ code: 'MODULE_NOT_FOUND' }); } }); + +it('throws an error', async () => { + await foo().catch(error => expect(error).toBeInstanceOf(error)); +}); ``` The following patterns are not warnings: @@ -67,4 +74,8 @@ it('validates the request', () => { expect(validRequest).toHaveBeenCalledWith(request); } }); + +it('throws an error', async () => { + await expect(foo).rejects.toThrow(Error); +}); ``` diff --git a/src/rules/__tests__/no-conditional-expect.test.ts b/src/rules/__tests__/no-conditional-expect.test.ts index 5b3637208..8ef0487d1 100644 --- a/src/rules/__tests__/no-conditional-expect.test.ts +++ b/src/rules/__tests__/no-conditional-expect.test.ts @@ -1,4 +1,5 @@ import { TSESLint } from '@typescript-eslint/experimental-utils'; +import dedent from 'dedent'; import resolveFrom from 'resolve-from'; import rule from '../no-conditional-expect'; @@ -584,3 +585,108 @@ ruleTester.run('catch conditions', rule, { }, ], }); + +ruleTester.run('promises', rule, { + valid: [ + ` + it('works', async () => { + try { + await Promise.resolve().then(() => { + throw new Error('oh noes!'); + }); + } catch { + // ignore errors + } finally { + expect(something).toHaveBeenCalled(); + } + }); + `, + ` + it('works', async () => { + await doSomething().catch(error => error); + + expect(error).toBeInstanceOf(Error); + }); + `, + ` + it('works', async () => { + try { + await Promise.resolve().then(() => { + throw new Error('oh noes!'); + }); + } catch { + // ignore errors + } + + expect(something).toHaveBeenCalled(); + }); + `, + ], + invalid: [ + { + code: dedent` + it('works', async () => { + await Promise.resolve() + .then(() => { throw new Error('oh noes!'); }) + .catch(error => expect(error).toBeInstanceOf(Error)); + }); + `, + errors: [{ messageId: 'conditionalExpect' }], + }, + { + code: dedent` + it('works', async () => { + await Promise.resolve() + .then(() => { throw new Error('oh noes!'); }) + .catch(error => expect(error).toBeInstanceOf(Error)) + .then(() => { throw new Error('oh noes!'); }) + .catch(error => expect(error).toBeInstanceOf(Error)) + .then(() => { throw new Error('oh noes!'); }) + .catch(error => expect(error).toBeInstanceOf(Error)); + }); + `, + errors: [{ messageId: 'conditionalExpect' }], + }, + { + code: dedent` + it('works', async () => { + await Promise.resolve() + .catch(error => expect(error).toBeInstanceOf(Error)) + .catch(error => expect(error).toBeInstanceOf(Error)) + .catch(error => expect(error).toBeInstanceOf(Error)); + }); + `, + errors: [{ messageId: 'conditionalExpect' }], + }, + { + code: dedent` + it('works', async () => { + await Promise.resolve() + .catch(error => expect(error).toBeInstanceOf(Error)) + .then(() => { throw new Error('oh noes!'); }) + .then(() => { throw new Error('oh noes!'); }) + .then(() => { throw new Error('oh noes!'); }); + }); + `, + errors: [{ messageId: 'conditionalExpect' }], + }, + { + code: dedent` + it('works', async () => { + await somePromise + .then(() => { throw new Error('oh noes!'); }) + .catch(error => expect(error).toBeInstanceOf(Error)); + }); + `, + errors: [{ messageId: 'conditionalExpect' }], + }, + { + code: dedent` + it('works', async () => { + await somePromise.catch(error => expect(error).toBeInstanceOf(Error)); + }); + `, + errors: [{ messageId: 'conditionalExpect' }], + }, + ], +}); diff --git a/src/rules/no-conditional-expect.ts b/src/rules/no-conditional-expect.ts index 189ea0812..f49c6ea8e 100644 --- a/src/rules/no-conditional-expect.ts +++ b/src/rules/no-conditional-expect.ts @@ -1,11 +1,22 @@ -import { TSESTree } from '@typescript-eslint/experimental-utils'; import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import { + KnownCallExpression, createRule, getTestCallExpressionsFromDeclaredVariables, isExpectCall, + isSupportedAccessor, isTestCaseCall, } from './utils'; +const isCatchCall = ( + node: TSESTree.CallExpression, +): node is KnownCallExpression<'catch'> => + node.callee.type === AST_NODE_TYPES.MemberExpression && + isSupportedAccessor(node.callee.property, 'catch'); + export default createRule({ name: __filename, meta: { @@ -24,6 +35,7 @@ export default createRule({ create(context) { let conditionalDepth = 0; let inTestCase = false; + let inPromiseCatch = false; const increaseConditionalDepth = () => inTestCase && conditionalDepth++; const decreaseConditionalDepth = () => inTestCase && conditionalDepth--; @@ -44,17 +56,32 @@ export default createRule({ inTestCase = true; } + if (isCatchCall(node)) { + inPromiseCatch = true; + } + if (inTestCase && isExpectCall(node) && conditionalDepth > 0) { context.report({ messageId: 'conditionalExpect', node, }); } + + if (inPromiseCatch && isExpectCall(node)) { + context.report({ + messageId: 'conditionalExpect', + node, + }); + } }, 'CallExpression:exit'(node) { if (isTestCaseCall(node)) { inTestCase = false; } + + if (isCatchCall(node)) { + inPromiseCatch = false; + } }, CatchClause: increaseConditionalDepth, 'CatchClause:exit': decreaseConditionalDepth,