fix(valid-expect-in-promise): bailout if done callback is present

jest-community · Sep 26, 2021 · 5ae02ab · 5ae02ab
1 parent d1516eb
commit 5ae02ab
Show file tree

Hide file tree

Showing 2 changed files with 85 additions and 24 deletions.
diff --git a/src/rules/__tests__/valid-expect-in-promise.test.ts b/src/rules/__tests__/valid-expect-in-promise.test.ts
@@ -12,16 +12,15 @@ const ruleTester = new TSESLint.RuleTester({
 
 ruleTester.run('valid-expect-in-promise', rule, {
   valid: [
-    // todo: done callback
-    // dedent`
-    //   it('it1', () => new Promise((done) => {
-    //     test()
-    //       .then(() => {
-    //         expect(someThing).toEqual(true);
-    //         done();
-    //       });
-    //   }));
-    // `,
+    dedent`
+      it('it1', () => new Promise((done) => {
+        test()
+          .then(() => {
+            expect(someThing).toEqual(true);
+            done();
+          });
+      }));
+    `,
     dedent`
       it('passes', () => {
         Promise.resolve().then(() => {
@@ -280,20 +279,30 @@ ruleTester.run('valid-expect-in-promise', rule, {
       }))
     `,
     'it("it1", () => somePromise.then(() => expect(someThing).toEqual(true)))',
-    // todo: done callback
-    // dedent`
-    //   it('promise test with done', (done) => {
-    //     const promise = getPromise();
-    //     promise.then(() => expect(someThing).toEqual(true));
-    //   });
-    // `,
-    // todo: done callback
-    // dedent`
-    //   it('name of done param does not matter', (nameDoesNotMatter) => {
-    //     const promise = getPromise();
-    //     promise.then(() => expect(someThing).toEqual(true));
-    //   });
-    // `,
+    dedent`
+      it('promise test with done', (done) => {
+        const promise = getPromise();
+        promise.then(() => expect(someThing).toEqual(true));
+      });
+    `,
+    dedent`
+      it('name of done param does not matter', (nameDoesNotMatter) => {
+        const promise = getPromise();
+        promise.then(() => expect(someThing).toEqual(true));
+      });
+    `,
+    dedent`
+      it.each([])('name of done param does not matter', (nameDoesNotMatter) => {
+        const promise = getPromise();
+        promise.then(() => expect(someThing).toEqual(true));
+      });
+    `,
+    dedent`
+      it.each\`\`('name of done param does not matter', ({}, nameDoesNotMatter) => {
+        const promise = getPromise();
+        promise.then(() => expect(someThing).toEqual(true));
+      });
+    `,
   ],
   invalid: [
     {

diff --git a/src/rules/valid-expect-in-promise.ts b/src/rules/valid-expect-in-promise.ts
@@ -7,7 +7,9 @@ import {
   KnownCallExpression,
   createRule,
   getAccessorValue,
+  getNodeName,
   isExpectCall,
+  isFunction,
   isSupportedAccessor,
   isTestCaseCall,
 } from './utils';
@@ -76,6 +78,41 @@ const findTopOfBodyNode = (
   return null;
 };
 
+const isTestCaseCallWithCallbackArg = (
+  node: TSESTree.CallExpression,
+): boolean => {
+  if (!isTestCaseCall(node)) {
+    return false;
+  }
+
+  const isJestEach = getNodeName(node).endsWith('.each');
+
+  if (
+    isJestEach &&
+    node.callee.type !== AST_NODE_TYPES.TaggedTemplateExpression
+  ) {
+    // isJestEach but not a TaggedTemplateExpression, so this must be
+    // the `jest.each([])()` syntax which this rule doesn't support due
+    // to its complexity (see jest-community/eslint-plugin-jest#710)
+    // so we return true to trigger bailout
+    return true;
+  }
+
+  if (isJestEach || node.arguments.length >= 2) {
+    const [, callback] = node.arguments;
+
+    const callbackArgIndex = Number(isJestEach);
+
+    return (
+      callback &&
+      isFunction(callback) &&
+      callback.params.length === 1 + callbackArgIndex
+    );
+  }
+
+  return false;
+};
+
 export default createRule<unknown[], MessageIds>({
   name: __filename,
   meta: {
@@ -93,11 +130,18 @@ export default createRule<unknown[], MessageIds>({
   },
   defaultOptions: [],
   create(context) {
+    let inTestCaseWithDoneCallback = false;
     let inPromiseChain = false;
     let hasExpectCall = false;
 
     return {
       CallExpression(node) {
+        if (isTestCaseCallWithCallbackArg(node)) {
+          inTestCaseWithDoneCallback = true;
+
+          return;
+        }
+
         if (isThenOrCatchCall(node)) {
           inPromiseChain = true;
 
@@ -115,6 +159,14 @@ export default createRule<unknown[], MessageIds>({
         }
       },
       'CallExpression:exit'(node: TSESTree.CallExpression) {
+        if (inTestCaseWithDoneCallback) {
+          if (isTestCaseCall(node)) {
+            inTestCaseWithDoneCallback = false;
+          }
+
+          return;
+        }
+
         if (isThenOrCatchCall(node)) {
           inPromiseChain = false;