Merge pull request #225 from playwright-community/prefer-hooks-in-order

Add `prefer-hooks-in-order` 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-in-order](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-in-order.md)             | Prefer having hooks in a consistent order                         |
 |     |     |     | [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                                      |

docs/rules/prefer-hooks-in-order.md

@@ -0,0 +1,129 @@
+# Prefer having hooks in a consistent order (`prefer-hooks-in-order`)
+
+While hooks can be setup in any order, they're always called by `playwright` in
+this specific order:
+
+1. `beforeAll`
+1. `beforeEach`
+1. `afterEach`
+1. `afterAll`
+
+This rule aims to make that more obvious by enforcing grouped hooks be setup in
+that order within tests.
+
+## Rule details
+
+Examples of **incorrect** code for this rule
+
+```js
+/* eslint playwright/prefer-hooks-in-order: "error" */
+
+test.describe('foo', () => {
+  test.beforeEach(() => {
+    seedMyDatabase();
+  });
+
+  test.beforeAll(() => {
+    createMyDatabase();
+  });
+
+  test('accepts this input', () => {
+    // ...
+  });
+
+  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-in-order: "error" */
+
+test.describe('foo', () => {
+  test.beforeAll(() => {
+    createMyDatabase();
+  });
+
+  test.beforeEach(() => {
+    seedMyDatabase();
+  });
+
+  test('accepts this input', () => {
+    // ...
+  });
+
+  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.beforeEach(() => {
+      mockLogger();
+    });
+
+    test.afterEach(() => {
+      clearLogger();
+    });
+
+    test('logs a message', () => {
+      // ...
+    });
+  });
+
+  test.afterAll(() => {
+    removeMyDatabase();
+  });
+});
+```
+
+## Also See
+
+- [`prefer-hooks-on-top`](prefer-hooks-on-top.md)

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 preferHooksInOrder from './rules/prefer-hooks-in-order';
 import preferHooksOnTop from './rules/prefer-hooks-on-top';
 import preferLowercaseTitle from './rules/prefer-lowercase-title';
 import preferStrictEqual from './rules/prefer-strict-equal';
@@ -61,6 +62,7 @@ const index = {
     'no-useless-not': noUselessNot,
     'no-wait-for-selector': noWaitForSelector,
     'no-wait-for-timeout': noWaitForTimeout,
+    'prefer-hooks-in-order': preferHooksInOrder,
     'prefer-hooks-on-top': preferHooksOnTop,
     'prefer-lowercase-title': preferLowercaseTitle,
     'prefer-strict-equal': preferStrictEqual,

src/rules/prefer-hooks-in-order.ts

@@ -0,0 +1,70 @@
+import { Rule } from 'eslint';
+import { getStringValue, isTestHook } from '../utils/ast';
+
+const HooksOrder = ['beforeAll', 'beforeEach', 'afterEach', 'afterAll'];
+
+export default {
+  create(context) {
+    let previousHookIndex = -1;
+    let inHook = false;
+
+    return {
+      CallExpression(node) {
+        // Ignore everything that is passed into a hook
+        if (inHook) return;
+
+        // Reset the previousHookIndex when encountering something different from a hook
+        if (!isTestHook(context, node)) {
+          previousHookIndex = -1;
+          return;
+        }
+
+        inHook = true;
+        const currentHook =
+          node.callee.type === 'MemberExpression'
+            ? getStringValue(node.callee.property)
+            : '';
+        const currentHookIndex = HooksOrder.indexOf(currentHook);
+
+        if (currentHookIndex < previousHookIndex) {
+          return context.report({
+            data: {
+              currentHook,
+              previousHook: HooksOrder[previousHookIndex],
+            },
+            messageId: 'reorderHooks',
+            node,
+          });
+        }
+
+        previousHookIndex = currentHookIndex;
+      },
+      'CallExpression:exit'(node) {
+        if (isTestHook(context, node)) {
+          inHook = false;
+          return;
+        }
+
+        if (inHook) {
+          return;
+        }
+
+        // Reset the previousHookIndex when encountering something different from a hook
+        previousHookIndex = -1;
+      },
+    };
+  },
+  meta: {
+    docs: {
+      category: 'Best Practices',
+      description: 'Prefer having hooks in a consistent order',
+      recommended: false,
+      url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-in-order.md',
+    },
+    messages: {
+      reorderHooks:
+        '`{{ currentHook }}` hooks should be before any `{{ previousHook }}` hooks',
+    },
+    type: 'suggestion',
+  },
+} as Rule.RuleModule;