From cb523835d120d08d4279895f53bc440f7d2a393c Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 17 Oct 2021 10:38:00 +1300 Subject: [PATCH 1/3] fix(require-hook): check variables are either `const` or declarations --- src/rules/__tests__/require-hook.test.ts | 46 ++++++++++++++++++++++++ src/rules/require-hook.ts | 7 ++++ 2 files changed, 53 insertions(+) diff --git a/src/rules/__tests__/require-hook.test.ts b/src/rules/__tests__/require-hook.test.ts index d0dfbc702..329ed8b00 100644 --- a/src/rules/__tests__/require-hook.test.ts +++ b/src/rules/__tests__/require-hook.test.ts @@ -176,6 +176,47 @@ 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: "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 +277,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..8a2f7fb1f 100644 --- a/src/rules/require-hook.ts +++ b/src/rules/require-hook.ts @@ -25,6 +25,13 @@ const shouldBeInHook = (node: TSESTree.Node): boolean => { 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); + } default: return false; From 2e2ccccbd1e59be170ca474e2f6fed0f0db77e3b Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 17 Oct 2021 11:04:40 +1300 Subject: [PATCH 2/3] test(require-hook): ensure `let ... = require(...)` is disallowed --- src/rules/__tests__/require-hook.test.ts | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/rules/__tests__/require-hook.test.ts b/src/rules/__tests__/require-hook.test.ts index 329ed8b00..212d2546d 100644 --- a/src/rules/__tests__/require-hook.test.ts +++ b/src/rules/__tests__/require-hook.test.ts @@ -145,6 +145,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', () => { From a96afa06e467796a14064f9d36299246bfa1f959 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 17 Oct 2021 14:25:41 +1300 Subject: [PATCH 3/3] fix(require-hook): allow initializing to `null` or `undefined` --- docs/rules/require-hook.md | 2 +- src/rules/__tests__/require-hook.test.ts | 40 ++++++++++++++++++++++++ src/rules/require-hook.ts | 12 ++++++- 3 files changed, 52 insertions(+), 2 deletions(-) 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 212d2546d..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(() => { @@ -218,6 +232,32 @@ ruleTester.run('require-hook', rule, { }, ], }, + { + 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: [ diff --git a/src/rules/require-hook.ts b/src/rules/require-hook.ts index 8a2f7fb1f..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,6 +20,13 @@ 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: @@ -30,7 +38,9 @@ const shouldBeInHook = (node: TSESTree.Node): boolean => { return false; } - return node.declarations.some(({ init }) => init !== null); + return node.declarations.some( + ({ init }) => init !== null && !isNullOrUndefined(init), + ); } default: