fix(no-conditional-expect): check for expects in catchs on promises (…

…#819)
jest-community · Apr 26, 2021 · 1fee973 · 1fee973
1 parent 040c605
commit 1fee973
Show file tree

Hide file tree

Showing 3 changed files with 145 additions and 1 deletion.
diff --git 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
@@ -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
@@ -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,