fix: proper support for it.each (#722)

* fix: proper it.each support

* fix: apply code review suggestions

Co-authored-by: Gareth Jones <[email protected]>

Co-authored-by: Gareth Jones <[email protected]>

Author: k-yle

src/rules/__tests__/consistent-test-it.test.ts

@@ -144,6 +144,20 @@ ruleTester.run('consistent-test-it with fn=test', rule, {
         },
       ],
     },
+    {
+      code: 'it.each``("foo")',
+      output: 'test.each``("foo")',
+      options: [{ fn: TestCaseName.test }],
+      errors: [
+        {
+          messageId: 'consistentMethod',
+          data: {
+            testKeyword: TestCaseName.test,
+            oppositeTestKeyword: TestCaseName.it,
+          },
+        },
+      ],
+    },
     {
       code: 'describe("suite", () => { it("foo") })',
       output: 'describe("suite", () => { test("foo") })',
@@ -285,6 +299,20 @@ ruleTester.run('consistent-test-it with fn=it', rule, {
         },
       ],
     },
+    {
+      code: 'test.each``("foo")',
+      output: 'it.each``("foo")',
+      options: [{ fn: TestCaseName.it }],
+      errors: [
+        {
+          messageId: 'consistentMethod',
+          data: {
+            testKeyword: TestCaseName.it,
+            oppositeTestKeyword: TestCaseName.test,
+          },
+        },
+      ],
+    },
     {
       code: 'describe("suite", () => { test("foo") })',
       output: 'describe("suite", () => { it("foo") })',

src/rules/__tests__/no-disabled-tests.test.ts

@@ -89,6 +89,22 @@ ruleTester.run('no-disabled-tests', rule, {
       code: 'test.skip("foo", function () {})',
       errors: [{ messageId: 'skippedTest', column: 1, line: 1 }],
     },
+    {
+      code: 'it.skip.each``("foo", function () {})',
+      errors: [{ messageId: 'skippedTest', column: 1, line: 1 }],
+    },
+    {
+      code: 'test.skip.each``("foo", function () {})',
+      errors: [{ messageId: 'skippedTest', column: 1, line: 1 }],
+    },
+    {
+      code: 'it.skip.each([])("foo", function () {})',
+      errors: [{ messageId: 'skippedTest', column: 1, line: 1 }],
+    },
+    {
+      code: 'test.skip.each([])("foo", function () {})',
+      errors: [{ messageId: 'skippedTest', column: 1, line: 1 }],
+    },
     {
       code: 'test.concurrent.skip("foo", function () {})',
       errors: [{ messageId: 'skippedTest', column: 1, line: 1 }],
@@ -109,6 +125,22 @@ ruleTester.run('no-disabled-tests', rule, {
       code: 'xtest("foo", function () {})',
       errors: [{ messageId: 'disabledTest', column: 1, line: 1 }],
     },
+    {
+      code: 'xit.each``("foo", function () {})',
+      errors: [{ messageId: 'skippedTest', column: 1, line: 1 }],
+    },
+    {
+      code: 'xtest.each``("foo", function () {})',
+      errors: [{ messageId: 'skippedTest', column: 1, line: 1 }],
+    },
+    {
+      code: 'xit.each([])("foo", function () {})',
+      errors: [{ messageId: 'skippedTest', column: 1, line: 1 }],
+    },
+    {
+      code: 'xtest.each([])("foo", function () {})',
+      errors: [{ messageId: 'skippedTest', column: 1, line: 1 }],
+    },
     {
       code: 'it("has title but no callback")',
       errors: [{ messageId: 'missingFunction', column: 1, line: 1 }],

src/rules/__tests__/no-done-callback.test.ts

@@ -20,6 +20,8 @@ ruleTester.run('no-done-callback', rule, {
     'it.each()("something", ({ a, b }) => {})',
     'it.each([])("something", (a, b) => {})',
     'it.each``("something", ({ a, b }) => {})',
+    'it.each([])("something", (a, b) => { a(); b(); })',
+    'it.each``("something", ({ a, b }) => { a(); b(); })',
     'test("something", async function () {})',
     'test("something", someArg)',
     'beforeEach(() => {})',
@@ -390,5 +392,25 @@ ruleTester.run('no-done-callback', rule, {
         },
       ],
     },
+    {
+      code: 'test.each``("something", ({ a, b }, done) => { done(); })',
+      errors: [
+        {
+          messageId: 'noDoneCallback',
+          line: 1,
+          column: 37,
+        },
+      ],
+    },
+    {
+      code: 'it.each``("something", ({ a, b }, done) => { done(); })',
+      errors: [
+        {
+          messageId: 'noDoneCallback',
+          line: 1,
+          column: 35,
+        },
+      ],
+    },
   ],
 });

src/rules/__tests__/no-test-prefixes.test.ts

@@ -106,5 +106,55 @@ ruleTester.run('no-test-prefixes', rule, {
         },
       ],
     },
+    {
+      code: 'xit.each``("foo", function () {})',
+      output: 'it.skip.each``("foo", function () {})',
+      parserOptions: { ecmaVersion: 6 },
+      errors: [
+        {
+          messageId: 'usePreferredName',
+          data: { preferredNodeName: 'it.skip.each' },
+          column: 1,
+          line: 1,
+        },
+      ],
+    },
+    {
+      code: 'xtest.each``("foo", function () {})',
+      output: 'test.skip.each``("foo", function () {})',
+      parserOptions: { ecmaVersion: 6 },
+      errors: [
+        {
+          messageId: 'usePreferredName',
+          data: { preferredNodeName: 'test.skip.each' },
+          column: 1,
+          line: 1,
+        },
+      ],
+    },
+    {
+      code: 'xit.each([])("foo", function () {})',
+      output: 'it.skip.each([])("foo", function () {})',
+      errors: [
+        {
+          messageId: 'usePreferredName',
+          data: { preferredNodeName: 'it.skip.each' },
+          column: 1,
+          line: 1,
+        },
+      ],
+    },
+    {
+      code: 'xtest.each([])("foo", function () {})',
+      output: 'test.skip.each([])("foo", function () {})',
+      errors: [
+        {
+          messageId: 'usePreferredName',
+          data: { preferredNodeName: 'test.skip.each' },
+          column: 1,
+          line: 1,
+        },
+      ],
+    },
   ],
 });

src/rules/consistent-test-it.ts

@@ -82,6 +82,11 @@ export default createRule<
           describeNestingLevel++;
         }
 
+        const funcNode =
+          node.callee.type === AST_NODE_TYPES.TaggedTemplateExpression
+            ? node.callee.tag
+            : node.callee;
+
         if (
           isTestCase(node) &&
           describeNestingLevel === 0 &&
@@ -93,7 +98,7 @@ export default createRule<
             messageId: 'consistentMethod',
             node: node.callee,
             data: { testKeyword, oppositeTestKeyword },
-            fix: buildFixer(node.callee, nodeName, testKeyword),
+            fix: buildFixer(funcNode, nodeName, testKeyword),
           });
         }
 
@@ -110,7 +115,7 @@ export default createRule<
             messageId: 'consistentMethodWithinDescribe',
             node: node.callee,
             data: { testKeywordWithinDescribe, oppositeTestKeyword },
-            fix: buildFixer(node.callee, nodeName, testKeywordWithinDescribe),
+            fix: buildFixer(funcNode, nodeName, testKeywordWithinDescribe),
           });
         }
       },

src/rules/no-disabled-tests.ts

@@ -39,6 +39,11 @@ export default createRule({
       CallExpression(node) {
         const functionName = getNodeName(node.callee);
 
+        // prevent duplicate warnings for it.each()()
+        if (node.callee.type === 'CallExpression') {
+          return;
+        }
+
         switch (functionName) {
           case 'describe.skip':
             context.report({ messageId: 'skippedTestSuite', node });
@@ -48,6 +53,10 @@ export default createRule({
           case 'it.concurrent.skip':
           case 'test.skip':
           case 'test.concurrent.skip':
+          case 'it.skip.each':
+          case 'test.skip.each':
+          case 'xit.each':
+          case 'xtest.each':
             context.report({ messageId: 'skippedTest', node });
             break;
         }

src/rules/no-done-callback.ts

@@ -2,11 +2,22 @@ import {
   AST_NODE_TYPES,
   TSESTree,
 } from '@typescript-eslint/experimental-utils';
-import { createRule, isFunction, isHook, isTestCase } from './utils';
+import {
+  createRule,
+  getNodeName,
+  isFunction,
+  isHook,
+  isTestCase,
+} from './utils';
 
 const findCallbackArg = (
   node: TSESTree.CallExpression,
+  isJestEach: boolean,
 ): TSESTree.CallExpression['arguments'][0] | null => {
+  if (isJestEach) {
+    return node.arguments[1];
+  }
+
   if (isHook(node) && node.arguments.length >= 1) {
     return node.arguments[0];
   }
@@ -41,17 +52,31 @@ export default createRule({
   create(context) {
     return {
       CallExpression(node) {
-        const callback = findCallbackArg(node);
+        // done is the second argument for it.each, not the first
+        const isJestEach = getNodeName(node.callee)?.endsWith('.each') ?? false;
+
+        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)
+          return;
+        }
+
+        const callback = findCallbackArg(node, isJestEach);
+        const callbackArgIndex = Number(isJestEach);
 
         if (
           !callback ||
           !isFunction(callback) ||
-          callback.params.length !== 1
+          callback.params.length !== 1 + callbackArgIndex
         ) {
           return;
         }
 
-        const [argument] = callback.params;
+        const argument = callback.params[callbackArgIndex];
 
         if (argument.type !== AST_NODE_TYPES.Identifier) {
           context.report({

src/rules/no-test-prefixes.ts

@@ -1,3 +1,4 @@
+import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils';
 import { createRule, getNodeName, isDescribe, isTestCase } from './utils';
 
 export default createRule({
@@ -27,12 +28,17 @@ export default createRule({
 
         if (!preferredNodeName) return;
 
+        const funcNode =
+          node.callee.type === AST_NODE_TYPES.TaggedTemplateExpression
+            ? node.callee.tag
+            : node.callee;
+
         context.report({
           messageId: 'usePreferredName',
           node: node.callee,
           data: { preferredNodeName },
           fix(fixer) {
-            return [fixer.replaceText(node.callee, preferredNodeName)];
+            return [fixer.replaceText(funcNode, preferredNodeName)];
           },
         });
       },
@@ -43,12 +49,14 @@ export default createRule({
 function getPreferredNodeName(nodeName: string) {
   const firstChar = nodeName.charAt(0);
 
+  const suffix = nodeName.endsWith('.each') ? '.each' : '';
+
   if (firstChar === 'f') {
-    return `${nodeName.slice(1)}.only`;
+    return `${nodeName.slice(1).replace('.each', '')}.only${suffix}`;
   }
 
   if (firstChar === 'x') {
-    return `${nodeName.slice(1)}.skip`;
+    return `${nodeName.slice(1).replace('.each', '')}.skip${suffix}`;
   }
 
   return null;

src/rules/utils.ts

@@ -580,10 +580,16 @@ export interface JestFunctionCallExpressionWithIdentifierCallee<
   callee: JestFunctionIdentifier<FunctionName>;
 }
 
+interface JestFunctionCallExpressionWithTaggedTemplateCallee
+  extends TSESTree.CallExpression {
+  callee: TSESTree.TaggedTemplateExpression;
+}
+
 export type JestFunctionCallExpression<
   FunctionName extends JestFunctionName = JestFunctionName
 > =
   | JestFunctionCallExpressionWithMemberExpressionCallee<FunctionName>
+  | JestFunctionCallExpressionWithTaggedTemplateCallee
   | JestFunctionCallExpressionWithIdentifierCallee<FunctionName>;
 
 const joinNames = (a: string | null, b: string | null): string | null =>
@@ -592,7 +598,8 @@ const joinNames = (a: string | null, b: string | null): string | null =>
 export function getNodeName(
   node:
     | JestFunctionMemberExpression<JestFunctionName>
-    | JestFunctionIdentifier<JestFunctionName>,
+    | JestFunctionIdentifier<JestFunctionName>
+    | TSESTree.TaggedTemplateExpression,
 ): string;
 export function getNodeName(node: TSESTree.Node): string | null;
 export function getNodeName(node: TSESTree.Node): string | null {
@@ -601,6 +608,8 @@ export function getNodeName(node: TSESTree.Node): string | null {
   }
 
   switch (node.type) {
+    case AST_NODE_TYPES.TaggedTemplateExpression:
+      return getNodeName(node.tag);
     case AST_NODE_TYPES.MemberExpression:
       return joinNames(getNodeName(node.object), getNodeName(node.property));
     case AST_NODE_TYPES.NewExpression:
@@ -651,6 +660,11 @@ export const isTestCase = (
 ): node is JestFunctionCallExpression<TestCaseName> =>
   (node.callee.type === AST_NODE_TYPES.Identifier &&
     TestCaseName.hasOwnProperty(node.callee.name)) ||
+  // e.g. it.each``()
+  (node.callee.type === AST_NODE_TYPES.TaggedTemplateExpression &&
+    node.callee.tag.type === AST_NODE_TYPES.MemberExpression &&
+    isSupportedAccessor(node.callee.tag.property, TestCaseProperty.each)) ||
+  // e.g. it.concurrent.{skip,only}
   (node.callee.type === AST_NODE_TYPES.MemberExpression &&
     node.callee.property.type === AST_NODE_TYPES.Identifier &&
     TestCaseProperty.hasOwnProperty(node.callee.property.name) &&