Valid describe callback

README.md

@@ -160,5 +160,6 @@ CLI option\
 | [require-hook](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-hook.md)                               | Require setup and teardown code to be within a hook               |     |     |     |
 | [require-top-level-describe](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-top-level-describe.md)   | Require test cases and hooks to be inside a `test.describe` block |     |     |     |
 | [require-soft-assertions](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-soft-assertions.md)         | Require assertions to use `expect.soft()`                         |     | 🔧  |     |
+| [valid-describe-callback](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-describe-callback.md)         | Enforce valid `describe()` callback                               | ✅  |     |     |
 | [valid-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-expect.md)                               | Enforce valid `expect()` usage                                    | ✅  |     |     |
 | [valid-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-title.md)                                 | Enforce valid titles                                              | ✅  | 🔧  |     |

docs/rules/valid-describe-callback.md

@@ -0,0 +1,53 @@
+# Enforce valid `describe()` callback (`valid-describe-callback`)
+
+Using an improper `describe()` callback function can lead to unexpected test
+errors.
+
+## Rule details
+
+This rule validates that the second parameter of a `describe()` function is a
+callback function. This callback function:
+
+- should not be
+  [async](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function)
+- should not contain any parameters
+- should not contain any `return` statements
+
+The following patterns are considered warnings:
+
+```js
+// Async callback functions are not allowed
+test.describe('myFunction()', async () => {
+  // ...
+});
+
+// Callback function parameters are not allowed
+test.describe('myFunction()', (done) => {
+  // ...
+});
+
+// No return statements are allowed in block of a callback function
+test.describe('myFunction', () => {
+  return Promise.resolve().then(() => {
+    test('breaks', () => {
+      throw new Error('Fail');
+    });
+  });
+});
+
+// Returning a value from a describe block is not allowed
+test.describe('myFunction', () =>
+  test('returns a truthy value', () => {
+    expect(myFunction()).toBeTruthy();
+  }));
+```
+
+The following patterns are **not** considered warnings:
+
+```js
+test.describe('myFunction()', () => {
+  test('returns a truthy value', () => {
+    expect(myFunction()).toBeTruthy();
+  });
+});
+```

src/index.ts

@@ -40,6 +40,7 @@ import preferWebFirstAssertions from './rules/prefer-web-first-assertions';
 import requireHook from './rules/require-hook';
 import requireSoftAssertions from './rules/require-soft-assertions';
 import requireTopLevelDescribe from './rules/require-top-level-describe';
+import validDescribeCallback from './rules/valid-describe-callback';
 import validExpect from './rules/valid-expect';
 import validTitle from './rules/valid-title';
 
@@ -87,6 +88,7 @@ const index = {
     'require-hook': requireHook,
     'require-soft-assertions': requireSoftAssertions,
     'require-top-level-describe': requireTopLevelDescribe,
+    'valid-describe-callback': validDescribeCallback,
     'valid-expect': validExpect,
     'valid-title': validTitle,
   },
@@ -115,6 +117,7 @@ const sharedConfig = {
     'playwright/no-wait-for-selector': 'warn',
     'playwright/no-wait-for-timeout': 'warn',
     'playwright/prefer-web-first-assertions': 'error',
+    'playwright/valid-describe-callback': 'error',
     'playwright/valid-expect': 'error',
     'playwright/valid-title': 'error',
   },

src/rules/valid-describe-callback.test.ts

@@ -0,0 +1,197 @@
+import dedent from 'dedent';
+import { runRuleTester } from '../utils/rule-tester';
+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' }],
+    },
+    {
+      code: 'test.describe("foo", "foo2")',
+      errors: [
+        { column: 15, line: 1, messageId: 'secondArgumentMustBeFunction' },
+      ],
+    },
+    {
+      code: 'test.describe()',
+      errors: [{ column: 1, line: 1, messageId: 'nameAndCallback' }],
+    },
+    {
+      code: 'test.describe("foo", async () => {})',
+      errors: [{ column: 22, line: 1, messageId: 'noAsyncDescribeCallback' }],
+    },
+    {
+      code: 'test.describe("foo", async function () {})',
+      errors: [{ column: 22, line: 1, messageId: 'noAsyncDescribeCallback' }],
+    },
+    {
+      code: 'test.describe.only("foo", async function () {})',
+      errors: [{ column: 27, line: 1, messageId: 'noAsyncDescribeCallback' }],
+    },
+    {
+      code: 'test.describe.skip("foo", async function () {})',
+      errors: [{ column: 27, line: 1, messageId: 'noAsyncDescribeCallback' }],
+    },
+    {
+      code: dedent`
+        test.describe('sample case', () => {
+          test('works', () => {
+            expect(true).toEqual(true);
+          });
+          test.describe('async', async () => {
+            await new Promise(setImmediate);
+            test('breaks', () => {
+              throw new Error('Fail');
+            });
+          });
+        });
+      `,
+      errors: [{ column: 26, line: 5, messageId: 'noAsyncDescribeCallback' }],
+    },
+    {
+      code: dedent`
+        test.describe('foo', function () {
+          return Promise.resolve().then(() => {
+            test('breaks', () => {
+              throw new Error('Fail')
+            })
+          })
+        })
+      `,
+      errors: [{ column: 3, line: 2, messageId: 'unexpectedReturnInDescribe' }],
+    },
+    {
+      code: dedent`
+        test.describe('foo', () => {
+          return Promise.resolve().then(() => {
+            test('breaks', () => {
+              throw new Error('Fail')
+            })
+          })
+          test.describe('nested', () => {
+            return Promise.resolve().then(() => {
+              test('breaks', () => {
+                throw new Error('Fail')
+              })
+            })
+          })
+        })
+      `,
+      errors: [
+        { column: 3, line: 2, messageId: 'unexpectedReturnInDescribe' },
+        { column: 5, line: 8, messageId: 'unexpectedReturnInDescribe' },
+      ],
+    },
+    {
+      code: dedent`
+        test.describe('foo', async () => {
+          await something()
+          test('does something')
+          test.describe('nested', () => {
+            return Promise.resolve().then(() => {
+              test('breaks', () => {
+                throw new Error('Fail')
+              })
+            })
+          })
+        })
+      `,
+      errors: [
+        { column: 22, line: 1, messageId: 'noAsyncDescribeCallback' },
+        { column: 5, line: 5, messageId: 'unexpectedReturnInDescribe' },
+      ],
+    },
+    {
+      code: dedent`
+        test.describe('foo', () =>
+          test('bar', () => {})
+        )
+      `,
+      errors: [
+        { column: 22, line: 1, messageId: 'unexpectedReturnInDescribe' },
+      ],
+    },
+    {
+      code: 'describe("foo", done => {})',
+      errors: [
+        { column: 17, line: 1, messageId: 'unexpectedDescribeArgument' },
+      ],
+    },
+    {
+      code: 'describe("foo", function (done) {})',
+      errors: [
+        { column: 27, line: 1, messageId: 'unexpectedDescribeArgument' },
+      ],
+    },
+    {
+      code: 'describe("foo", function (one, two, three) {})',
+      errors: [
+        { column: 27, line: 1, messageId: 'unexpectedDescribeArgument' },
+      ],
+    },
+    {
+      code: 'describe("foo", async function (done) {})',
+      errors: [
+        { column: 17, line: 1, messageId: 'noAsyncDescribeCallback' },
+        { column: 33, line: 1, messageId: 'unexpectedDescribeArgument' },
+      ],
+    },
+    // Global aliases
+    {
+      code: 'it.describe(() => {})',
+      errors: [{ column: 13, line: 1, messageId: 'nameAndCallback' }],
+      settings: {
+        playwright: {
+          globalAliases: { test: ['it'] },
+        },
+      },
+    },
+  ],
+  valid: [
+    'test.describe("foo", function() {})',
+    'test.describe("foo", () => {})',
+    'test.describe(`foo`, () => {})',
+    'test.describe.only("foo", () => {})',
+    'describe("foo", () => {})',
+    'describe.only("foo", () => {})',
+    dedent`
+      test.describe('foo', () => {
+        test('bar', () => {
+          return Promise.resolve(42).then(value => {
+            expect(value).toBe(42)
+          })
+        })
+      })
+    `,
+    dedent`
+      test.describe('foo', () => {
+        test('bar', async () => {
+          expect(await Promise.resolve(42)).toBe(42)
+        })
+      })
+    `,
+    dedent`
+      if (hasOwnProperty(obj, key)) {
+      }
+    `,
+    // Global aliases
+    {
+      code: 'it.describe("foo", function() {})',
+      settings: {
+        playwright: {
+          globalAliases: { test: ['it'] },
+        },
+      },
+    },
+  ],
+});

src/rules/valid-describe-callback.ts

@@ -0,0 +1,107 @@
+import { Rule } from 'eslint';
+import * as ESTree from 'estree';
+import { getStringValue, isFunction } from '../utils/ast';
+import { parseFnCall } from '../utils/parseFnCall';
+
+const paramsLocation = (
+  params: ESTree.CallExpression['arguments'] | ESTree.Pattern[],
+) => {
+  const [first] = params;
+  const last = params[params.length - 1];
+
+  return {
+    end: last.loc!.end,
+    start: first.loc!.start,
+  };
+};
+
+export default {
+  create(context) {
+    return {
+      CallExpression(node) {
+        const call = parseFnCall(context, node);
+        if (call?.type !== 'describe') return;
+
+        if (node.arguments.length < 1) {
+          return context.report({
+            loc: node.loc!,
+            messageId: 'nameAndCallback',
+          });
+        }
+
+        const [, callback] = node.arguments;
+
+        if (!callback) {
+          context.report({
+            loc: paramsLocation(node.arguments),
+            messageId: 'nameAndCallback',
+          });
+
+          return;
+        }
+
+        if (!isFunction(callback)) {
+          context.report({
+            loc: paramsLocation(node.arguments),
+            messageId: 'secondArgumentMustBeFunction',
+          });
+
+          return;
+        }
+
+        if (callback.async) {
+          context.report({
+            messageId: 'noAsyncDescribeCallback',
+            node: callback,
+          });
+        }
+
+        if (
+          call.members.every((s) => getStringValue(s) !== 'each') &&
+          callback.params.length
+        ) {
+          context.report({
+            loc: paramsLocation(callback.params),
+            messageId: 'unexpectedDescribeArgument',
+          });
+        }
+
+        if (callback.body.type === 'CallExpression') {
+          context.report({
+            messageId: 'unexpectedReturnInDescribe',
+            node: callback,
+          });
+        }
+
+        if (callback.body.type === 'BlockStatement') {
+          callback.body.body.forEach((node) => {
+            if (node.type === 'ReturnStatement') {
+              context.report({
+                messageId: 'unexpectedReturnInDescribe',
+                node,
+              });
+            }
+          });
+        }
+      },
+    };
+  },
+  meta: {
+    docs: {
+      category: 'Possible Errors',
+      description: 'Enforce valid `describe()` callback',
+      recommended: true,
+      url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-describe-callback.md',
+    },
+    messages: {
+      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',
+    },
+    schema: [],
+    type: 'problem',
+  },
+} as Rule.RuleModule;