From c33f78e8257d5678872e0dc3ff249d47c725f24a Mon Sep 17 00:00:00 2001 From: Andrey Nelyubin Date: Sat, 13 Nov 2021 11:40:58 +0300 Subject: [PATCH] fix(require-hook): added optional settings --- docs/rules/require-hook.md | 90 ++++++++++++++++ src/rules/__tests__/require-hook.test.ts | 126 +++++++++++++++++++++++ src/rules/require-hook.ts | 75 ++++++++++++-- 3 files changed, 284 insertions(+), 7 deletions(-) diff --git a/docs/rules/require-hook.md b/docs/rules/require-hook.md index 350f5b5ef..ec4aa7d77 100644 --- a/docs/rules/require-hook.md +++ b/docs/rules/require-hook.md @@ -148,3 +148,93 @@ afterEach(() => { clearCityDatabase(); }); ``` + +## Options + +Some test utils provides methods which takes hook as an argument +and should be executed outside a hook. + +For example https://vue-test-utils.vuejs.org/api/#enableautodestroy-hook +which takes the hook as an argument. To exclude them you can update settings + +```json +{ + "jest/require-hook": [ + "error", + { + "excludedFunctions": [ + "enableAutoDestroy" + ] + } + ] +} +``` + +For additional control you might need to reduce possible hooks which can be +passed as an argument (by default it is `beforeAll`, `beforeEach`, `afterAll`, `afterEach`) + +```json +{ + "jest/require-hook": [ + "error", + { + "excludedFunctions": [ + "enableAutoDestroy" + ], + "allowedHooks": ["beforeEach"] + } + ] +} +``` + + +Examples of **incorrect** code for the `{ "excludedFunctions": ["expect"], "allowedHooks": ["beforeEach"] }` +option: + +```js +/* eslint jest/require-hook: ["error", { "excludedFunctions": ["expect"], "allowedHooks": ["beforeEach"] }] */ + +import { + enableAutoDestroy, + resetAutoDestroyState, + mount +} from '@vue/test-utils'; +import initDatabase from './initDatabase'; + +enableAutoDestroy(afterEach); +afterAll(resetAutoDestroyState); // this will throw a linting error +initDatabase(); // this will too + +describe('Foo', () => { + test('always returns 42', () => { + expect(global.getAnswer()).toBe(42); + }) +}) +``` + + +Examples of **correct** code for the `{ "excludedFunctions": ["expect"], "allowedHooks": ["beforeEach", "afterAll"] }` +option: + +```js +/* eslint jest/require-hook: ["error", { "excludedFunctions": ["expect"], "allowedHooks": ["beforeEach", "afterAll"] }] */ + +import { + enableAutoDestroy, + resetAutoDestroyState, + mount +} from '@vue/test-utils'; +import {initDatabase, tearDownDatabase} from './databaseUtils'; + +enableAutoDestroy(afterEach); +afterAll(resetAutoDestroyState); + +beforeEach(initDatabase); +afterEach(tearDownDatabase); + +describe('Foo', () => { + test('always returns 42', () => { + expect(global.getAnswer()).toBe(42); + }); +}); +``` \ No newline at end of file diff --git a/src/rules/__tests__/require-hook.test.ts b/src/rules/__tests__/require-hook.test.ts index 5c6f83ec3..75d92247a 100644 --- a/src/rules/__tests__/require-hook.test.ts +++ b/src/rules/__tests__/require-hook.test.ts @@ -152,6 +152,54 @@ ruleTester.run('require-hook', rule, { }); }); `, + { + code: dedent` + enableAutoDestroy(afterEach); + + describe('some tests', () => { + it('is false', () => { + expect(true).toBe(true); + }); + }); + `, + options: [{ excludedFunctions: ['enableAutoDestroy'] }], + }, + { + code: dedent` + enableAutoDestroy(beforeEach); + + describe('some tests', () => { + it('is false', () => { + expect(true).toBe(true); + }); + }); + `, + options: [{ excludedFunctions: ['enableAutoDestroy'] }], + }, + { + code: dedent` + enableAutoDestroy(afterAll); + + describe('some tests', () => { + it('is false', () => { + expect(true).toBe(true); + }); + }); + `, + options: [{ excludedFunctions: ['enableAutoDestroy'] }], + }, + { + code: dedent` + enableAutoDestroy(beforeAll); + + describe('some tests', () => { + it('is false', () => { + expect(true).toBe(true); + }); + }); + `, + options: [{ excludedFunctions: ['enableAutoDestroy'] }], + }, ], invalid: [ { @@ -374,6 +422,84 @@ ruleTester.run('require-hook', rule, { }, ], }, + { + code: dedent` + enableAutoDestroy(afterEach); + + describe('some tests', () => { + it('is false', () => { + expect(true).toBe(true); + }); + }); + `, + options: [{ excludedFunctions: ['someOtherName'] }], + errors: [ + { + messageId: 'useHook', + line: 1, + column: 1, + }, + ], + }, + { + code: dedent` + someOtherName(afterUnknownHook); + + describe('some tests', () => { + it('is false', () => { + expect(true).toBe(true); + }); + }); + `, + options: [{ excludedFunctions: ['someOtherName'] }], + errors: [ + { + messageId: 'useHook', + line: 1, + column: 1, + }, + ], + }, + { + code: dedent` + someOtherName(afterEach); + + describe('some tests', () => { + it('is false', () => { + expect(true).toBe(true); + }); + }); + `, + options: [ + { excludedFunctions: ['someOtherName'], allowedHooks: ['beforeAll'] }, + ], + errors: [ + { + messageId: 'useHook', + line: 1, + column: 1, + }, + ], + }, + { + code: dedent` + enableAutoDestroy(); + + describe('some tests', () => { + it('is false', () => { + expect(true).toBe(true); + }); + }); + `, + options: [{ excludedFunctions: ['enableAutoDestroy'] }], + errors: [ + { + messageId: 'useHook', + line: 1, + column: 1, + }, + ], + }, ], }); diff --git a/src/rules/require-hook.ts b/src/rules/require-hook.ts index 179e554a8..8277d9f7c 100644 --- a/src/rules/require-hook.ts +++ b/src/rules/require-hook.ts @@ -12,6 +12,34 @@ import { isTestCaseCall, } from './utils'; +interface RequireHooksOptions { + excludedFunctions?: readonly string[]; + allowedHooks?: readonly string[]; +} + +const isExcludedFnCall = ( + node: TSESTree.CallExpression, + options: RequireHooksOptions, +): boolean => { + const [firstArgument] = node.arguments; + + if (firstArgument === undefined) { + return false; + } + + const nodeName = getNodeName(node); + const argumentNodeName = getNodeName(firstArgument); + + if (nodeName === null || argumentNodeName === null) { + return false; + } + + return ( + !!options.excludedFunctions?.includes(nodeName) && + !!options.allowedHooks?.includes(argumentNodeName) + ); +}; + const isJestFnCall = (node: TSESTree.CallExpression): boolean => { if (isDescribeCall(node) || isTestCaseCall(node) || isHook(node)) { return true; @@ -27,12 +55,15 @@ const isNullOrUndefined = (node: TSESTree.Expression): boolean => { ); }; -const shouldBeInHook = (node: TSESTree.Node): boolean => { +const shouldBeInHook = ( + node: TSESTree.Node, + options: RequireHooksOptions, +): boolean => { switch (node.type) { case AST_NODE_TYPES.ExpressionStatement: - return shouldBeInHook(node.expression); + return shouldBeInHook(node.expression, options); case AST_NODE_TYPES.CallExpression: - return !isJestFnCall(node); + return !(isJestFnCall(node) || isExcludedFnCall(node, options)); case AST_NODE_TYPES.VariableDeclaration: { if (node.kind === 'const') { return false; @@ -48,7 +79,14 @@ const shouldBeInHook = (node: TSESTree.Node): boolean => { } }; -export default createRule({ +const defaultAllowedHooks = [ + 'beforeAll', + 'beforeEach', + 'afterAll', + 'afterEach', +]; + +export default createRule<[RequireHooksOptions], 'useHook'>({ name: __filename, meta: { docs: { @@ -60,13 +98,36 @@ export default createRule({ useHook: 'This should be done within a hook', }, type: 'suggestion', - schema: [], + schema: [ + { + type: 'object', + properties: { + excludedFunctions: { + type: 'array', + items: { type: 'string' }, + }, + allowedHooks: { + type: 'array', + items: { type: 'string' }, + default: defaultAllowedHooks, + }, + }, + additionalProperties: false, + }, + ], }, - defaultOptions: [], + defaultOptions: [ + { + excludedFunctions: [], + }, + ], create(context) { + const { allowedHooks = defaultAllowedHooks, excludedFunctions = [] } = + context.options[0] ?? {}; + const checkBlockBody = (body: TSESTree.BlockStatement['body']) => { for (const statement of body) { - if (shouldBeInHook(statement)) { + if (shouldBeInHook(statement, { allowedHooks, excludedFunctions })) { context.report({ node: statement, messageId: 'useHook',