From ad3fc06b134141e1976576ec439483cbd0ccd785 Mon Sep 17 00:00:00 2001 From: Nimalan Date: Sat, 12 Oct 2019 07:00:59 +0530 Subject: [PATCH] feat(rules): prefer-hooks-on-top (#425) * feat(rules): prefer-hooks-on-top * chore: review changes * fix: failing test --- README.md | 2 + docs/rules/prefer-hooks-on-top.md | 96 +++++++++ src/__tests__/rules.test.ts | 2 +- .../__tests__/prefer-hooks-on-top.test.ts | 188 ++++++++++++++++++ src/rules/prefer-hooks-on-top.ts | 38 ++++ 5 files changed, 325 insertions(+), 1 deletion(-) create mode 100644 docs/rules/prefer-hooks-on-top.md create mode 100644 src/rules/__tests__/prefer-hooks-on-top.test.ts create mode 100644 src/rules/prefer-hooks-on-top.ts diff --git a/README.md b/README.md index 742e91168..a5a006fc3 100644 --- a/README.md +++ b/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 diff --git a/docs/rules/prefer-hooks-on-top.md b/docs/rules/prefer-hooks-on-top.md new file mode 100644 index 000000000..a403dab48 --- /dev/null +++ b/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(); + }); + }); +}); +``` diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index 787c3c4cb..6b6e12fcc 100644 --- a/src/__tests__/rules.test.ts +++ b/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', () => { diff --git a/src/rules/__tests__/prefer-hooks-on-top.test.ts b/src/rules/__tests__/prefer-hooks-on-top.test.ts new file mode 100644 index 000000000..3a01066ad --- /dev/null +++ b/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, + }, + ], + }, + ], +}); diff --git a/src/rules/prefer-hooks-on-top.ts b/src/rules/prefer-hooks-on-top.ts new file mode 100644 index 000000000..abb2e5e0e --- /dev/null +++ b/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(); + }, + }; + }, +});