Merge pull request #224 from playwright-community/prefer-hooks-on-top

Add `prefer-hooks-on-top` rule

Author: mskelton

README.md

@@ -142,6 +142,7 @@ command line option.\
 | ✔  | 🔧  |     | [no-useless-not](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-useless-not.md)                           | Disallow usage of `not` matchers when a specific matcher exists   |
 | ✔  |     | 💡  | [no-wait-for-selector](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-selector.md)               | Disallow usage of `page.waitForSelector()`                        |
 | ✔  |     | 💡  | [no-wait-for-timeout](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-timeout.md)                 | Disallow usage of `page.waitForTimeout()`                         |
+|     |     |     | [prefer-hooks-on-top](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-on-top.md)                 | Suggest having hooks before any test cases                        |
 |     |     | 💡  | [prefer-strict-equal](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-strict-equal.md)                 | Suggest using `toStrictEqual()`                                   |
 |     | 🔧  |     | [prefer-lowercase-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-lowercase-title.md)           | Enforce lowercase test names                                      |
 |     | 🔧  |     | [prefer-to-be](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-be.md)                               | Suggest using `toBe()`                                            |

docs/rules/prefer-hooks-on-top.md

@@ -0,0 +1,121 @@
+# Suggest having hooks before any test cases (`prefer-hooks-on-top`)
+
+While hooks can be setup anywhere in a test file, they are always called in a
+specific order, which means it can be confusing if they're intermixed with test
+cases.
+
+This rule helps to ensure that hooks are always defined before test cases.
+
+## Rule details
+
+Examples of **incorrect** code for this rule
+
+```js
+/* eslint playwright/prefer-hooks-on-top: "error" */
+
+test.describe('foo', () => {
+  test.beforeEach(() => {
+    seedMyDatabase();
+  });
+
+  test('accepts this input', () => {
+    // ...
+  });
+
+  test.beforeAll(() => {
+    createMyDatabase();
+  });
+
+  test('returns that value', () => {
+    // ...
+  });
+
+  test.describe('when the database has specific values', () => {
+    const specificValue = '...';
+
+    test.beforeEach(() => {
+      seedMyDatabase(specificValue);
+    });
+
+    test('accepts that input', () => {
+      // ...
+    });
+
+    test('throws an error', () => {
+      // ...
+    });
+
+    test.afterEach(() => {
+      clearLogger();
+    });
+
+    test.beforeEach(() => {
+      mockLogger();
+    });
+
+    test('logs a message', () => {
+      // ...
+    });
+  });
+
+  test.afterAll(() => {
+    removeMyDatabase();
+  });
+});
+```
+
+Examples of **correct** code for this rule
+
+```js
+/* eslint playwright/prefer-hooks-on-top: "error" */
+
+test.describe('foo', () => {
+  test.beforeAll(() => {
+    createMyDatabase();
+  });
+
+  test.beforeEach(() => {
+    seedMyDatabase();
+  });
+
+  test.afterAll(() => {
+    clearMyDatabase();
+  });
+
+  test('accepts this input', () => {
+    // ...
+  });
+
+  test('returns that value', () => {
+    // ...
+  });
+
+  test.describe('when the database has specific values', () => {
+    const specificValue = '...';
+
+    beforeEach(() => {
+      seedMyDatabase(specificValue);
+    });
+
+    beforeEach(() => {
+      mockLogger();
+    });
+
+    afterEach(() => {
+      clearLogger();
+    });
+
+    test('accepts that input', () => {
+      // ...
+    });
+
+    test('throws an error', () => {
+      // ...
+    });
+
+    test('logs a message', () => {
+      // ...
+    });
+  });
+});
+```

src/index.ts

@@ -22,6 +22,7 @@ import noUselessAwait from './rules/no-useless-await';
 import noUselessNot from './rules/no-useless-not';
 import noWaitForSelector from './rules/no-wait-for-selector';
 import noWaitForTimeout from './rules/no-wait-for-timeout';
+import preferHooksOnTop from './rules/prefer-hooks-on-top';
 import preferLowercaseTitle from './rules/prefer-lowercase-title';
 import preferStrictEqual from './rules/prefer-strict-equal';
 import preferToBe from './rules/prefer-to-be';
@@ -60,6 +61,7 @@ const index = {
     'no-useless-not': noUselessNot,
     'no-wait-for-selector': noWaitForSelector,
     'no-wait-for-timeout': noWaitForTimeout,
+    'prefer-hooks-on-top': preferHooksOnTop,
     'prefer-lowercase-title': preferLowercaseTitle,
     'prefer-strict-equal': preferStrictEqual,
     'prefer-to-be': preferToBe,

src/rules/prefer-hooks-on-top.ts

@@ -0,0 +1,40 @@
+import { Rule } from 'eslint';
+import { isTestCall, isTestHook } from '../utils/ast';
+
+export default {
+  create(context) {
+    const stack = [false];
+
+    return {
+      CallExpression(node) {
+        if (isTestCall(context, node)) {
+          stack[stack.length - 1] = true;
+        }
+
+        if (stack.at(-1) && isTestHook(context, node)) {
+          context.report({
+            messageId: 'noHookOnTop',
+            node,
+          });
+        }
+
+        stack.push(false);
+      },
+      'CallExpression:exit'() {
+        stack.pop();
+      },
+    };
+  },
+  meta: {
+    docs: {
+      category: 'Best Practices',
+      description: 'Suggest having hooks before any test cases',
+      recommended: false,
+      url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-on-top.md',
+    },
+    messages: {
+      noHookOnTop: 'Hooks should come before test cases',
+    },
+    type: 'suggestion',
+  },
+} as Rule.RuleModule;