fix(prefer-hooks-on-top): improve message & docs (#999)

* test(prefer-hooks-on-top): add case for expressions before hooks

* test(prefer-hooks-on-top): make cases a bit more readable

* fix(prefer-hooks-on-top): improve working of lint message

* docs(prefer-hooks-on-top): improve code example

* docs(prefer-hooks-on-top): improve rule description

Author: G-Rath

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

@@ -1,6 +1,10 @@
 # Suggest having hooks before any test cases (`prefer-hooks-on-top`)
 
-All hooks should be defined before the start of the tests
+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
 
@@ -11,47 +15,51 @@ Examples of **incorrect** code for this rule
 
 describe('foo', () => {
   beforeEach(() => {
-    //some hook code
-  });
-  test('bar', () => {
-    some_fn();
+    seedMyDatabase();
   });
-  beforeAll(() => {
-    //some hook code
-  });
-  test('bar', () => {
-    some_fn();
+
+  it('accepts this input', () => {
+    // ...
   });
-});
 
-// Nested describe scenario
-describe('foo', () => {
   beforeAll(() => {
-    //some hook code
+    createMyDatabase();
   });
-  test('bar', () => {
-    some_fn();
+
+  it('returns that value', () => {
+    // ...
   });
-  describe('inner_foo', () => {
+
+  describe('when the database has specific values', () => {
+    const specificValue = '...';
+
     beforeEach(() => {
-      //some hook code
+      seedMyDatabase(specificValue);
     });
-    test('inner bar', () => {
-      some_fn();
+
+    it('accepts that input', () => {
+      // ...
     });
-    test('inner bar', () => {
-      some_fn();
+
+    it('throws an error', () => {
+      // ...
     });
-    beforeAll(() => {
-      //some hook code
+
+    afterEach(() => {
+      clearLogger();
     });
-    afterAll(() => {
-      //some hook code
+    beforeEach(() => {
+      mockLogger();
     });
-    test('inner bar', () => {
-      some_fn();
+
+    it('logs a message', () => {
+      // ...
     });
   });
+
+  afterAll(() => {
+    removeMyDatabase();
+  });
 });
 ```
 
@@ -61,35 +69,51 @@ Examples of **correct** code for this rule
 /* eslint jest/prefer-hooks-on-top: "error" */
 
 describe('foo', () => {
-  beforeEach(() => {
-    //some hook code
+  beforeAll(() => {
+    createMyDatabase();
   });
 
-  // Not affected by rule
-  someSetup();
-
-  afterEach(() => {
-    //some hook code
+  beforeEach(() => {
+    seedMyDatabase();
   });
-  test('bar', () => {
-    some_fn();
+
+  afterAll(() => {
+    clearMyDatabase();
   });
-});
 
-// Nested describe scenario
-describe('foo', () => {
-  beforeEach(() => {
-    //some hook code
+  it('accepts this input', () => {
+    // ...
   });
-  test('bar', () => {
-    some_fn();
+
+  it('returns that value', () => {
+    // ...
   });
-  describe('inner_foo', () => {
+
+  describe('when the database has specific values', () => {
+    const specificValue = '...';
+
     beforeEach(() => {
-      //some hook code
+      seedMyDatabase(specificValue);
     });
-    test('inner bar', () => {
-      some_fn();
+
+    beforeEach(() => {
+      mockLogger();
+    });
+
+    afterEach(() => {
+      clearLogger();
+    });
+
+    it('accepts that input', () => {
+      // ...
+    });
+
+    it('throws an error', () => {
+      // ...
+    });
+
+    it('logs a message', () => {
+      // ...
     });
   });
 });

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

@@ -17,6 +17,18 @@ ruleTester.run('basic describe block', rule, {
         beforeEach(() => {});
         someSetupFn();
         afterEach(() => {});
+
+        test('bar', () => {
+          someFn();
+        });
+      });
+    `,
+    dedent`
+      describe('foo', () => {
+        someSetupFn();
+        beforeEach(() => {});
+        afterEach(() => {});
+
         test('bar', () => {
           someFn();
         });
@@ -31,6 +43,7 @@ ruleTester.run('basic describe block', rule, {
           test('bar', () => {
             someFn();
           });
+
           beforeAll(() => {});
           test('bar', () => {
             someFn();
@@ -41,7 +54,7 @@ ruleTester.run('basic describe block', rule, {
         {
           messageId: 'noHookOnTop',
           column: 3,
-          line: 6,
+          line: 7,
         },
       ],
     },
@@ -52,6 +65,7 @@ ruleTester.run('basic describe block', rule, {
           test.each\`\`('bar', () => {
             someFn();
           });
+
           beforeAll(() => {});
           test.only('bar', () => {
             someFn();
@@ -62,7 +76,7 @@ ruleTester.run('basic describe block', rule, {
         {
           messageId: 'noHookOnTop',
           column: 3,
-          line: 6,
+          line: 7,
         },
       ],
     },
@@ -73,6 +87,7 @@ ruleTester.run('basic describe block', rule, {
           test.only.each\`\`('bar', () => {
             someFn();
           });
+
           beforeAll(() => {});
           test.only('bar', () => {
             someFn();
@@ -83,7 +98,7 @@ ruleTester.run('basic describe block', rule, {
         {
           messageId: 'noHookOnTop',
           column: 3,
-          line: 6,
+          line: 7,
         },
       ],
     },
@@ -96,19 +111,21 @@ ruleTester.run('multiple describe blocks', rule, {
       describe.skip('foo', () => {
         beforeEach(() => {});
         beforeAll(() => {});
+
         test('bar', () => {
           someFn();
         });
       });
+
       describe('foo', () => {
         beforeEach(() => {});
+
         test('bar', () => {
           someFn();
         });
       });
     `,
   ],
-
   invalid: [
     {
       code: dedent`
@@ -117,6 +134,7 @@ ruleTester.run('multiple describe blocks', rule, {
           test('bar', () => {
             someFn();
           });
+
           beforeAll(() => {});
           test('bar', () => {
             someFn();
@@ -126,14 +144,17 @@ ruleTester.run('multiple describe blocks', rule, {
           beforeEach(() => {});
           beforeEach(() => {});
           beforeAll(() => {});
+
           test('bar', () => {
             someFn();
           });
         });
+
         describe('foo', () => {
           test('bar', () => {
             someFn();
           });
+
           beforeEach(() => {});
           beforeEach(() => {});
           beforeAll(() => {});
@@ -143,22 +164,22 @@ ruleTester.run('multiple describe blocks', rule, {
         {
           messageId: 'noHookOnTop',
           column: 3,
-          line: 6,
+          line: 7,
         },
         {
           messageId: 'noHookOnTop',
           column: 3,
-          line: 23,
+          line: 27,
         },
         {
           messageId: 'noHookOnTop',
           column: 3,
-          line: 24,
+          line: 28,
         },
         {
           messageId: 'noHookOnTop',
           column: 3,
-          line: 25,
+          line: 29,
         },
       ],
     },
@@ -173,6 +194,7 @@ ruleTester.run('nested describe blocks', rule, {
         test('bar', () => {
           someFn();
         });
+
         describe('inner_foo', () => {
           beforeEach(() => {});
           test('inner bar', () => {
@@ -182,7 +204,6 @@ ruleTester.run('nested describe blocks', rule, {
       });
     `,
   ],
-
   invalid: [
     {
       code: dedent`
@@ -191,14 +212,17 @@ ruleTester.run('nested describe blocks', rule, {
           test('bar', () => {
             someFn();
           });
+
           describe('inner_foo', () => {
             beforeEach(() => {});
             test('inner bar', () => {
               someFn();
             });
+
             test('inner bar', () => {
               someFn();
             });
+
             beforeAll(() => {});
             afterAll(() => {});
             test('inner bar', () => {
@@ -211,12 +235,12 @@ ruleTester.run('nested describe blocks', rule, {
         {
           messageId: 'noHookOnTop',
           column: 5,
-          line: 14,
+          line: 17,
         },
         {
           messageId: 'noHookOnTop',
           column: 5,
-          line: 15,
+          line: 18,
         },
       ],
     },

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

@@ -9,7 +9,7 @@ export default createRule({
       recommended: false,
     },
     messages: {
-      noHookOnTop: 'Move all hooks before test cases',
+      noHookOnTop: 'Hooks should come before test cases',
     },
     schema: [],
     type: 'suggestion',