feat(rules): no-try-expect

README.md

@@ -127,6 +127,7 @@ installations requiring long-term consistency.
 | [no-test-prefixes][]         | Disallow using `f` & `x` prefixes to define focused/skipped tests | ![recommended][] | ![fixable-green][]  |
 | [no-test-return-statement][] | Disallow explicitly returning from tests                          |                  |                     |
 | [no-truthy-falsy][]          | Disallow using `toBeTruthy()` & `toBeFalsy()`                     |                  |                     |
+| [no-try-expect][]            | Prevent `catch` assertions in tests                               |                  |                     |
 | [prefer-expect-assertions][] | Suggest using `expect.assertions()` OR `expect.hasAssertions()`   |                  |                     |
 | [prefer-spy-on][]            | Suggest using `jest.spyOn()`                                      |                  | ![fixable-green][]  |
 | [prefer-strict-equal][]      | Suggest using `toStrictEqual()`                                   |                  | ![fixable-green][]  |
@@ -177,6 +178,7 @@ https://github.com/dangreenisrael/eslint-plugin-jest-formatting
 [no-test-prefixes]: docs/rules/no-test-prefixes.md
 [no-test-return-statement]: docs/rules/no-test-return-statement.md
 [no-truthy-falsy]: docs/rules/no-truthy-falsy.md
+[no-try-expect]: docs/rules/no-try-expect.md
 [prefer-called-with]: docs/rules/prefer-called-with.md
 [prefer-expect-assertions]: docs/rules/prefer-expect-assertions.md
 [prefer-spy-on]: docs/rules/prefer-spy-on.md

docs/rules/no-try-expect.md

@@ -0,0 +1,42 @@
+# Prevent catch assertions in tests (no-try-expect)
+
+This rule prevents the use of `expect` inside `catch` blocks.
+
+## Rule Details
+
+Expectations inside a `catch` block can be silently skipped. While Jest provides
+an `expect.assertions(number)` helper, Shopify rarely uses this feature. Using
+`toThrow` concisely guarantees that an exception was thrown, and that its
+contents match expectations.
+
+The following patterns are warnings:
+
+```js
+it('foo', () => {
+  try {
+    foo(); // `foo` may be refactored to not throw exceptions, yet still appears to be tested here.
+  } catch (err) {
+    expect(err).toMatch(/foo error/);
+  }
+});
+
+it('bar', async () => {
+  try {
+    await foo();
+  } catch (err) {
+    expect(err).toMatch(/foo error/);
+  }
+});
+```
+
+The following patterns are not warnings:
+
+```js
+it('foo', () => {
+  expect(() => foo()).toThrow(/foo error/);
+});
+
+it('bar', async () => {
+  await expect(fooPromise).rejects.toThrow(/foo error/);
+});
+```

src/__tests__/rules.test.js

@@ -3,7 +3,7 @@ import { resolve } from 'path';
 import { rules } from '../';
 
 const ruleNames = Object.keys(rules);
-const numberOfRules = 35;
+const numberOfRules = 36;
 
 describe('rules', () => {
   it('should have a corresponding doc for each rule', () => {

src/rules/__tests__/no-try-expect.test.js

@@ -0,0 +1,66 @@
+import { RuleTester } from 'eslint';
+import rule from '../no-try-expect';
+
+const ruleTester = new RuleTester({
+  parser: require.resolve('babel-eslint'),
+});
+
+const errors = [
+  {
+    messageId: 'noTryExpect',
+  },
+];
+
+ruleTester.run('no-try-catch', rule, {
+  valid: [
+    {
+      code: `it('foo', () => {
+        expect('foo').toEqual('foo');
+      })`,
+    },
+    {
+      code: `
+        it('foo', () => {
+          expect('bar').toEqual('bar');
+        });
+        try {
+
+        } catch {
+          expect('foo').toEqual('foo');
+        }`,
+    },
+    {
+      code: `
+        it.skip('foo');
+        try {
+
+        } catch {
+          expect('foo').toEqual('foo');
+        }`,
+    },
+  ],
+  invalid: [
+    {
+      code: `it('foo', () => {
+        try {
+
+        } catch (err) {
+          expect(err).toMatch('Error');
+        }
+      })`,
+      errors,
+    },
+    {
+      code: `it('foo', async () => {
+        await wrapper('production', async () => {
+          try {
+
+          } catch (err) {
+            expect(err).toMatch('Error');
+          }
+        })
+      })`,
+      errors,
+    },
+  ],
+});

src/rules/no-try-expect.js

@@ -0,0 +1,53 @@
+import { getDocsUrl, isTestCase } from './util';
+
+export default {
+  meta: {
+    docs: {
+      description: 'Prefer using toThrow for exception tests',
+      uri: getDocsUrl(__filename),
+    },
+    messages: {
+      noTryExpect: [
+        'Tests should use Jest‘s exception helpers.',
+        'Use "expect(() => yourFunction()).toThrow()" for synchronous tests,',
+        'or "await expect(yourFunction()).rejects.toThrow()" for async tests',
+      ].join(' '),
+    },
+  },
+  create(context) {
+    let isTest = false;
+    let catchDepth = 0;
+
+    function isThrowExpectCall(node) {
+      return catchDepth > 0 && node.callee.name === 'expect';
+    }
+
+    return {
+      CallExpression(node) {
+        if (isTestCase(node)) {
+          isTest = true;
+        } else if (isTest && isThrowExpectCall(node)) {
+          context.report({
+            messageId: 'noTryExpect',
+            node,
+          });
+        }
+      },
+      CatchClause() {
+        if (isTest) {
+          ++catchDepth;
+        }
+      },
+      'CatchClause:exit'() {
+        if (isTest) {
+          --catchDepth;
+        }
+      },
+      'CallExpression:exit'(node) {
+        if (isTestCase(node)) {
+          isTest = false;
+        }
+      },
+    };
+  },
+};