fix(valid-describe-callback): Support unnamed describe

playwright-community · Mar 2, 2024 · b3768f8 · b3768f8
1 parent 3b272af
commit b3768f8
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 33 deletions.
diff --git a/src/rules/valid-describe-callback.test.ts b/src/rules/valid-describe-callback.test.ts
@@ -4,17 +4,9 @@ import rule from './valid-describe-callback'
 
 runRuleTester('valid-describe-callback', rule, {
   invalid: [
-    {
-      code: 'test.describe(() => {})',
-      errors: [{ column: 15, line: 1, messageId: 'nameAndCallback' }],
-    },
-    {
-      code: 'describe(() => {})',
-      errors: [{ column: 10, line: 1, messageId: 'nameAndCallback' }],
-    },
     {
       code: 'test.describe("foo")',
-      errors: [{ column: 15, line: 1, messageId: 'nameAndCallback' }],
+      errors: [{ column: 15, line: 1, messageId: 'missingCallback' }],
     },
     {
       code: 'test.describe("foo", { tag: ["@slow"] });',
@@ -30,7 +22,7 @@ runRuleTester('valid-describe-callback', rule, {
     },
     {
       code: 'test.describe()',
-      errors: [{ column: 1, line: 1, messageId: 'nameAndCallback' }],
+      errors: [{ column: 1, line: 1, messageId: 'missingCallback' }],
     },
     {
       code: 'test.describe("foo", async () => {})',
@@ -154,8 +146,10 @@ runRuleTester('valid-describe-callback', rule, {
     },
     // Global aliases
     {
-      code: 'it.describe(() => {})',
-      errors: [{ column: 13, line: 1, messageId: 'nameAndCallback' }],
+      code: 'it.describe("foo", done => {})',
+      errors: [
+        { column: 20, line: 1, messageId: 'unexpectedDescribeArgument' },
+      ],
       settings: {
         playwright: {
           globalAliases: { test: ['it'] },
@@ -164,9 +158,11 @@ runRuleTester('valid-describe-callback', rule, {
     },
   ],
   valid: [
+    'describe(() => {})',
     'describe.configure({ timeout: 600_000 })',
     'describe("foo", function() {})',
     'describe("foo", () => {})',
+    'test.describe(() => {})',
     'test.describe.configure({ timeout: 600_000 })',
     'test.describe("foo", function() {})',
     'test.describe("foo", () => {})',

diff --git a/src/rules/valid-describe-callback.ts b/src/rules/valid-describe-callback.ts
@@ -1,6 +1,6 @@
 import { Rule } from 'eslint'
 import * as ESTree from 'estree'
-import { getStringValue, isFunction } from '../utils/ast'
+import { getStringValue, isFunction, isStringLiteral } from '../utils/ast'
 import { parseFnCall } from '../utils/parseFnCall'
 
 const paramsLocation = (
@@ -15,14 +15,6 @@ 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 {
@@ -35,47 +27,49 @@ export default {
           return
         }
 
-        const [name, _, callback] = parseArgs(node)
+        const callback = node.arguments.at(-1)
 
-        if (node.arguments.length < 1) {
+        // e.g., test.describe()
+        if (!callback) {
           return context.report({
             loc: node.loc!,
-            messageId: 'nameAndCallback',
+            messageId: 'missingCallback',
           })
         }
 
-        if (!name || !callback) {
-          context.report({
+        // e.g., test.describe("foo")
+        if (node.arguments.length === 1 && isStringLiteral(callback)) {
+          return context.report({
             loc: paramsLocation(node.arguments),
-            messageId: 'nameAndCallback',
+            messageId: 'missingCallback',
           })
-
-          return
         }
 
+        // e.g., test.describe("foo", "foo2");
         if (!isFunction(callback)) {
-          context.report({
+          return context.report({
             loc: paramsLocation(node.arguments),
             messageId: 'invalidCallback',
           })
-
-          return
         }
 
+        // e.g., test.describe("foo", async () => {});
         if (callback.async) {
           context.report({
             messageId: 'noAsyncDescribeCallback',
             node: callback,
           })
         }
 
+        // e.g., test.describe("foo", (done) => {});
         if (callback.params.length) {
           context.report({
             loc: paramsLocation(callback.params),
             messageId: 'unexpectedDescribeArgument',
           })
         }
 
+        // e.g., test.describe("foo", () => { return; });
         if (callback.body.type === 'CallExpression') {
           context.report({
             messageId: 'unexpectedReturnInDescribe',
@@ -105,7 +99,7 @@ export default {
     },
     messages: {
       invalidCallback: 'Callback argument must be a function',
-      nameAndCallback: 'Describe requires name and callback arguments',
+      missingCallback: 'Describe requires a callback',
       noAsyncDescribeCallback: 'No async describe callback',
       unexpectedDescribeArgument: 'Unexpected argument(s) in describe callback',
       unexpectedReturnInDescribe: