From ce8cd612b7c4c16dc29934118b191d3fbe1ffc07 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 17 Oct 2021 23:18:17 +1300 Subject: [PATCH] fix(require-hook): check variables are either `const` or declarations (#959) --- docs/rules/require-hook.md | 2 +- src/rules/__tests__/require-hook.test.ts | 107 +++++++++++++++++++++++ src/rules/require-hook.ts | 17 ++++ 3 files changed, 125 insertions(+), 1 deletion(-) diff --git a/docs/rules/require-hook.md b/docs/rules/require-hook.md index 473749317..ce0116525 100644 --- a/docs/rules/require-hook.md +++ b/docs/rules/require-hook.md @@ -19,7 +19,7 @@ directly within the body of a `describe`, _except_ for the following: - `import` statements - `const` variables -- `let` _declarations_ +- `let` _declarations_, and initializations to `null` or `undefined` - Types - Calls to the standard Jest globals diff --git a/src/rules/__tests__/require-hook.test.ts b/src/rules/__tests__/require-hook.test.ts index d0dfbc702..3e1515cd8 100644 --- a/src/rules/__tests__/require-hook.test.ts +++ b/src/rules/__tests__/require-hook.test.ts @@ -76,6 +76,20 @@ ruleTester.run('require-hook', rule, { }); }); `, + dedent` + let consoleErrorSpy = null; + + beforeEach(() => { + consoleErrorSpy = jest.spyOn(console, 'error'); + }); + `, + dedent` + let consoleErrorSpy = undefined; + + beforeEach(() => { + consoleErrorSpy = jest.spyOn(console, 'error'); + }); + `, dedent` describe('some tests', () => { beforeEach(() => { @@ -145,6 +159,27 @@ ruleTester.run('require-hook', rule, { }, ], }, + { + code: dedent` + let { setup } = require('./test-utils'); + + describe('some tests', () => { + setup(); + }); + `, + errors: [ + { + messageId: 'useHook', + line: 1, + column: 1, + }, + { + messageId: 'useHook', + line: 4, + column: 3, + }, + ], + }, { code: dedent` describe('some tests', () => { @@ -176,6 +211,73 @@ ruleTester.run('require-hook', rule, { }, ], }, + { + code: dedent` + let consoleErrorSpy = jest.spyOn(console, 'error'); + + describe('when loading cities from the api', () => { + let consoleWarnSpy = jest.spyOn(console, 'warn'); + }); + `, + errors: [ + { + messageId: 'useHook', + line: 1, + column: 1, + }, + { + messageId: 'useHook', + line: 4, + column: 3, + }, + ], + }, + { + code: dedent` + let consoleErrorSpy = null; + + describe('when loading cities from the api', () => { + let consoleWarnSpy = jest.spyOn(console, 'warn'); + }); + `, + errors: [ + { + messageId: 'useHook', + line: 4, + column: 3, + }, + ], + }, + { + code: 'let value = 1', + errors: [ + { + messageId: 'useHook', + line: 1, + column: 1, + }, + ], + }, + { + code: "let consoleErrorSpy, consoleWarnSpy = jest.spyOn(console, 'error');", + errors: [ + { + messageId: 'useHook', + line: 1, + column: 1, + }, + ], + }, + { + code: "let consoleErrorSpy = jest.spyOn(console, 'error'), consoleWarnSpy;", + errors: [ + { + messageId: 'useHook', + line: 1, + column: 1, + }, + ], + }, { code: dedent` import { database, isCity } from '../database'; @@ -236,6 +338,11 @@ ruleTester.run('require-hook', rule, { line: 16, column: 1, }, + { + messageId: 'useHook', + line: 31, + column: 3, + }, { messageId: 'useHook', line: 33, diff --git a/src/rules/require-hook.ts b/src/rules/require-hook.ts index cb8dc06cf..179e554a8 100644 --- a/src/rules/require-hook.ts +++ b/src/rules/require-hook.ts @@ -8,6 +8,7 @@ import { isDescribeCall, isFunction, isHook, + isIdentifier, isTestCaseCall, } from './utils'; @@ -19,12 +20,28 @@ const isJestFnCall = (node: TSESTree.CallExpression): boolean => { return !!getNodeName(node)?.startsWith('jest.'); }; +const isNullOrUndefined = (node: TSESTree.Expression): boolean => { + return ( + (node.type === AST_NODE_TYPES.Literal && node.value === null) || + isIdentifier(node, 'undefined') + ); +}; + const shouldBeInHook = (node: TSESTree.Node): boolean => { switch (node.type) { case AST_NODE_TYPES.ExpressionStatement: return shouldBeInHook(node.expression); case AST_NODE_TYPES.CallExpression: return !isJestFnCall(node); + case AST_NODE_TYPES.VariableDeclaration: { + if (node.kind === 'const') { + return false; + } + + return node.declarations.some( + ({ init }) => init !== null && !isNullOrUndefined(init), + ); + } default: return false;