fix(valid-describe-callback): False positive when using new options a…

…rgument Fixes #266
playwright-community · Feb 28, 2024 · c25d200 · c25d200
1 parent 99f11c7
commit c25d200
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 12 deletions.
diff --git a/src/rules/valid-describe-callback.test.ts b/src/rules/valid-describe-callback.test.ts
@@ -16,11 +16,17 @@ runRuleTester('valid-describe-callback', rule, {
       code: 'test.describe("foo")',
       errors: [{ column: 15, line: 1, messageId: 'nameAndCallback' }],
     },
+    {
+      code: 'test.describe("foo", { tag: ["@slow"] });',
+      errors: [{ column: 15, line: 1, messageId: 'invalidCallback' }],
+    },
     {
       code: 'test.describe("foo", "foo2")',
-      errors: [
-        { column: 15, line: 1, messageId: 'secondArgumentMustBeFunction' },
-      ],
+      errors: [{ column: 15, line: 1, messageId: 'invalidCallback' }],
+    },
+    {
+      code: 'test.describe("foo", foo2)',
+      errors: [{ column: 15, line: 1, messageId: 'invalidCallback' }],
     },
     {
       code: 'test.describe()',
@@ -165,6 +171,7 @@ runRuleTester('valid-describe-callback', rule, {
     'test.describe("foo", function() {})',
     'test.describe("foo", () => {})',
     'test.describe(`foo`, () => {})',
+    'test.describe("another suite", { tag: ["@slow"] }, () => {});',
     'test.describe.only("foo", () => {})',
     'describe("foo", () => {})',
     'describe.only("foo", () => {})',

diff --git a/src/rules/valid-describe-callback.ts b/src/rules/valid-describe-callback.ts
@@ -15,6 +15,14 @@ const paramsLocation = (
   };
 };
 
+function parseArgs(node: ESTree.CallExpression) {
+  const [name, b, c] = node.arguments;
+  const options = node.arguments.length === 2 ? b : undefined;
+  const callback = node.arguments.length === 3 ? c : b;
+
+  return [name, options, callback] as const;
+}
+
 export default {
   create(context) {
     return {
@@ -27,16 +35,16 @@ export default {
           return;
         }
 
+        const [name, _, callback] = parseArgs(node);
+
         if (node.arguments.length < 1) {
           return context.report({
             loc: node.loc!,
             messageId: 'nameAndCallback',
           });
         }
 
-        const [, callback] = node.arguments;
-
-        if (!callback) {
+        if (!name || !callback) {
           context.report({
             loc: paramsLocation(node.arguments),
             messageId: 'nameAndCallback',
@@ -48,7 +56,7 @@ export default {
         if (!isFunction(callback)) {
           context.report({
             loc: paramsLocation(node.arguments),
-            messageId: 'secondArgumentMustBeFunction',
+            messageId: 'invalidCallback',
           });
 
           return;
@@ -61,10 +69,7 @@ export default {
           });
         }
 
-        if (
-          call.members.every((s) => getStringValue(s) !== 'each') &&
-          callback.params.length
-        ) {
+        if (callback.params.length) {
           context.report({
             loc: paramsLocation(callback.params),
             messageId: 'unexpectedDescribeArgument',
@@ -99,9 +104,9 @@ export default {
       url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-describe-callback.md',
     },
     messages: {
+      invalidCallback: 'Callback argument must be a function',
       nameAndCallback: 'Describe requires name and callback arguments',
       noAsyncDescribeCallback: 'No async describe callback',
-      secondArgumentMustBeFunction: 'Second argument must be function',
       unexpectedDescribeArgument: 'Unexpected argument(s) in describe callback',
       unexpectedReturnInDescribe:
         'Unexpected return statement in describe callback',