feat(rules): prefer-hooks-on-top (#425)

* feat(rules): prefer-hooks-on-top

* chore: review changes

* fix: failing test

Author: Mark1626

README.md

@@ -136,6 +136,7 @@ installations requiring long-term consistency.
 | [no-try-expect][]              | Prevent `catch` assertions in tests                               |                  |                     |
 | [prefer-called-with][]         | Suggest using `toBeCalledWith()` OR `toHaveBeenCalledWith()`      |                  |                     |
 | [prefer-expect-assertions][]   | Suggest using `expect.assertions()` OR `expect.hasAssertions()`   |                  |                     |
+| [prefer-hooks-on-top][]        | Suggest to have all hooks at top-level before tests               |                  |                     |
 | [prefer-inline-snapshots][]    | Suggest using `toMatchInlineSnapshot()`                           |                  | ![fixable-green][]  |
 | [prefer-spy-on][]              | Suggest using `jest.spyOn()`                                      |                  | ![fixable-green][]  |
 | [prefer-strict-equal][]        | Suggest using `toStrictEqual()`                                   |                  | ![fixable-green][]  |
@@ -191,6 +192,7 @@ https://github.com/dangreenisrael/eslint-plugin-jest-formatting
 [prefer-called-with]: docs/rules/prefer-called-with.md
 [prefer-expect-assertions]: docs/rules/prefer-expect-assertions.md
 [prefer-inline-snapshots]: docs/rules/prefer-inline-snapshots.md
+[prefer-hooks-on-top]: docs/rules/prefer-hooks-on-top.md
 [prefer-spy-on]: docs/rules/prefer-spy-on.md
 [prefer-strict-equal]: docs/rules/prefer-strict-equal.md
 [prefer-to-be-null]: docs/rules/prefer-to-be-null.md

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

@@ -0,0 +1,96 @@
+# Suggest to have all hooks at top-level before tests (prefer-hooks-on-top)
+
+All hooks should be defined before the start of the tests
+
+## Rule Details
+
+Examples of **incorrect** code for this rule
+
+```js
+/* eslint jest/prefer-hooks-on-top: "error" */
+
+describe("foo" () => {
+  beforeEach(() => {
+    //some hook code
+  });
+  test("bar" () => {
+    some_fn();
+  });
+  beforeAll(() => {
+    //some hook code
+  });
+  test("bar" () => {
+    some_fn();
+  });
+});
+
+// Nested describe scenario
+describe("foo" () => {
+  beforeAll(() => {
+    //some hook code
+  });
+  test("bar" () => {
+    some_fn();
+  });
+  describe("inner_foo" () => {
+    beforeEach(() => {
+      //some hook code
+    });
+    test("inner bar" () => {
+      some_fn();
+    });
+    test("inner bar" () => {
+      some_fn();
+    });
+    beforeAll(() => {
+      //some hook code
+    });
+    afterAll(() => {
+      //some hook code
+    });
+    test("inner bar" () => {
+      some_fn();
+    });
+  });
+});
+```
+
+Examples of **correct** code for this rule
+
+```js
+/* eslint jest/prefer-hooks-on-top: "error" */
+
+describe("foo" () => {
+  beforeEach(() => {
+    //some hook code
+  });
+
+  // Not affected by rule
+  someSetup();
+
+  afterEach(() => {
+    //some hook code
+  });
+  test("bar" () => {
+    some_fn();
+  });
+});
+
+// Nested describe scenario
+describe("foo" () => {
+  beforeEach(() => {
+    //some hook code
+  });
+  test("bar" () => {
+    some_fn();
+  });
+  describe("inner_foo" () => {
+    beforeEach(() => {
+      //some hook code
+    });
+    test("inner bar" () => {
+      some_fn();
+    });
+  });
+});
+```

src/__tests__/rules.test.ts

@@ -3,7 +3,7 @@ import { resolve } from 'path';
 import plugin from '../';
 
 const ruleNames = Object.keys(plugin.rules);
-const numberOfRules = 39;
+const numberOfRules = 40;
 
 describe('rules', () => {
   it('should have a corresponding doc for each rule', () => {

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

@@ -0,0 +1,188 @@
+import { TSESLint } from '@typescript-eslint/experimental-utils';
+import rule from '../prefer-hooks-on-top';
+
+const ruleTester = new TSESLint.RuleTester({
+  parserOptions: {
+    ecmaVersion: 6,
+  },
+});
+
+ruleTester.run('basic describe block', rule, {
+  valid: [
+    `describe("foo", () => {
+        beforeEach(() => {
+        });
+        someSetupFn();
+        afterEach(() => {
+        });
+        test("bar", () => {
+          some_fn();
+        });
+      });`,
+  ],
+  invalid: [
+    {
+      code: `describe("foo", () => {
+          beforeEach(() => {
+          });
+          test("bar", () => {
+            some_fn();
+          });
+          beforeAll(() => {
+          });
+          test("bar", () => {
+            some_fn();
+          });
+        });`,
+      errors: [
+        {
+          messageId: 'noHookOnTop',
+          column: 11,
+          line: 7,
+        },
+      ],
+    },
+  ],
+});
+
+ruleTester.run('multiple describe blocks', rule, {
+  valid: [
+    `describe.skip("foo", () => {
+        beforeEach(() => {
+        });
+        beforeAll(() => {
+        });
+        test("bar", () => {
+          some_fn();
+        });
+      });
+      describe("foo", () => {
+        beforeEach(() => {
+        });
+        test("bar", () => {
+          some_fn();
+        });
+      });`,
+  ],
+
+  invalid: [
+    {
+      code: `describe.skip("foo", () => {
+          beforeEach(() => {
+          });
+          test("bar", () => {
+            some_fn();
+          });
+          beforeAll(() => {
+          });
+          test("bar", () => {
+            some_fn();
+          });
+        });
+        describe("foo", () => {
+          beforeEach(() => {
+          });
+          beforeEach(() => {
+          });
+          beforeAll(() => {
+          });
+          test("bar", () => {
+            some_fn();
+          });
+        });
+        describe("foo", () => {
+          test("bar", () => {
+            some_fn();
+          });
+          beforeEach(() => {
+          });
+          beforeEach(() => {
+          });
+          beforeAll(() => {
+          });
+        });`,
+      errors: [
+        {
+          messageId: 'noHookOnTop',
+          column: 11,
+          line: 7,
+        },
+        {
+          messageId: 'noHookOnTop',
+          column: 11,
+          line: 28,
+        },
+        {
+          messageId: 'noHookOnTop',
+          column: 11,
+          line: 30,
+        },
+        {
+          messageId: 'noHookOnTop',
+          column: 11,
+          line: 32,
+        },
+      ],
+    },
+  ],
+});
+
+ruleTester.run('nested describe blocks', rule, {
+  valid: [
+    `describe("foo", () => {
+        beforeEach(() => {
+        });
+        test("bar", () => {
+          some_fn();
+        });
+        describe("inner_foo" , () => {
+          beforeEach(() => {
+          });
+          test("inner bar", () => {
+            some_fn();
+          });
+        });
+      });`,
+  ],
+
+  invalid: [
+    {
+      code: `describe("foo", () => {
+          beforeAll(() => {
+          });
+          test("bar", () => {
+            some_fn();
+          });
+          describe("inner_foo" , () => {
+            beforeEach(() => {
+            });
+            test("inner bar", () => {
+              some_fn();
+            });
+            test("inner bar", () => {
+              some_fn();
+            });
+            beforeAll(() => {
+            });
+            afterAll(() => {
+            });
+            test("inner bar", () => {
+              some_fn();
+            });
+          });
+        });`,
+      errors: [
+        {
+          messageId: 'noHookOnTop',
+          column: 13,
+          line: 16,
+        },
+        {
+          messageId: 'noHookOnTop',
+          column: 13,
+          line: 18,
+        },
+      ],
+    },
+  ],
+});

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

@@ -0,0 +1,38 @@
+import { createRule, isHook, isTestCase } from './utils';
+
+export default createRule({
+  name: __filename,
+  meta: {
+    docs: {
+      category: 'Best Practices',
+      description: 'Suggest to have all hooks at top level',
+      recommended: false,
+    },
+    messages: {
+      noHookOnTop: 'Move all hooks before test cases',
+    },
+    schema: [],
+    type: 'suggestion',
+  },
+  defaultOptions: [],
+  create(context) {
+    const hooksContext = [false];
+    return {
+      CallExpression(node) {
+        if (!isHook(node) && isTestCase(node)) {
+          hooksContext[hooksContext.length - 1] = true;
+        }
+        if (hooksContext[hooksContext.length - 1] && isHook(node)) {
+          context.report({
+            messageId: 'noHookOnTop',
+            node,
+          });
+        }
+        hooksContext.push(false);
+      },
+      'CallExpression:exit'() {
+        hooksContext.pop();
+      },
+    };
+  },
+});