From 37478d860eb15841f2ab73bb3fb6d94f51841638 Mon Sep 17 00:00:00 2001 From: Maxime Gerbe Date: Sat, 6 Apr 2024 23:32:00 +0200 Subject: [PATCH] feat: add `prefer-importing-jest-globals` rule (#1490) * feat: prefer importing jest globals [new rule] - Fix #1101 Issue: https://github.com/jest-community/eslint-plugin-jest/issues/1101 * test(prefer-importing-jest-globals): use `FlatCompatRuleTester` * test(prefer-importing-jest-globals): clean up tests * test(prefer-importing-jest-globals): explicitly set the source type to `script` * fix(prefer-importing-jest-globals): remove unneeded properties from rule config * fix(prefer-importing-jest-globals): support template literals * refactor(prefer-importing-jest-globals): use accessor utilities and optional chaining * refactor(prefer-importing-jest-globals): remove unneeded code * refactor(prefer-importing-jest-globals): use a Set --------- Co-authored-by: Gareth Jones --- README.md | 1 + docs/rules/prefer-importing-jest-globals.md | 47 ++ .../__snapshots__/rules.test.ts.snap | 2 + src/__tests__/rules.test.ts | 2 +- .../prefer-importing-jest-globals.test.ts | 504 ++++++++++++++++++ src/rules/prefer-importing-jest-globals.ts | 163 ++++++ 6 files changed, 718 insertions(+), 1 deletion(-) create mode 100644 docs/rules/prefer-importing-jest-globals.md create mode 100644 src/rules/__tests__/prefer-importing-jest-globals.test.ts create mode 100644 src/rules/prefer-importing-jest-globals.ts diff --git a/README.md b/README.md index 825ed26a8..0fbc1ca73 100644 --- a/README.md +++ b/README.md @@ -328,6 +328,7 @@ set to warn in.\ | [prefer-expect-resolves](docs/rules/prefer-expect-resolves.md) | Prefer `await expect(...).resolves` over `expect(await ...)` syntax | | | 🔧 | | | [prefer-hooks-in-order](docs/rules/prefer-hooks-in-order.md) | Prefer having hooks in a consistent order | | | | | | [prefer-hooks-on-top](docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | | | | +| [prefer-importing-jest-globals](docs/rules/prefer-importing-jest-globals.md) | Prefer importing Jest globals | | | 🔧 | | | [prefer-lowercase-title](docs/rules/prefer-lowercase-title.md) | Enforce lowercase test names | | | 🔧 | | | [prefer-mock-promise-shorthand](docs/rules/prefer-mock-promise-shorthand.md) | Prefer mock resolved/rejected shorthands for promises | | | 🔧 | | | [prefer-snapshot-hint](docs/rules/prefer-snapshot-hint.md) | Prefer including a hint with external snapshots | | | | | diff --git a/docs/rules/prefer-importing-jest-globals.md b/docs/rules/prefer-importing-jest-globals.md new file mode 100644 index 000000000..b7020a40f --- /dev/null +++ b/docs/rules/prefer-importing-jest-globals.md @@ -0,0 +1,47 @@ +# Prefer importing Jest globals (`prefer-importing-jest-globals`) + +🔧 This rule is automatically fixable by the +[`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix). + + + +This rule aims to enforce explicit imports from `@jest/globals`. + +1. This is useful for ensuring that the Jest APIs are imported the same way in + the codebase. +2. When you can't modify Jest's + [`injectGlobals`](https://jestjs.io/docs/configuration#injectglobals-boolean) + configuration property, this rule can help to ensure that the Jest globals + are imported explicitly and facilitate a migration to `@jest/globals`. + +## Rule details + +Examples of **incorrect** code for this rule + +```js +/* eslint jest/prefer-importing-jest-globals: "error" */ + +describe('foo', () => { + it('accepts this input', () => { + // ... + }); +}); +``` + +Examples of **correct** code for this rule + +```js +/* eslint jest/prefer-importing-jest-globals: "error" */ + +import { describe, it } from '@jest/globals'; + +describe('foo', () => { + it('accepts this input', () => { + // ... + }); +}); +``` + +## Further Reading + +- [Documentation](https://jestjs.io/docs/api) diff --git a/src/__tests__/__snapshots__/rules.test.ts.snap b/src/__tests__/__snapshots__/rules.test.ts.snap index b0d635013..9112e08ee 100644 --- a/src/__tests__/__snapshots__/rules.test.ts.snap +++ b/src/__tests__/__snapshots__/rules.test.ts.snap @@ -45,6 +45,7 @@ exports[`rules should export configs that refer to actual rules 1`] = ` "jest/prefer-expect-resolves": "error", "jest/prefer-hooks-in-order": "error", "jest/prefer-hooks-on-top": "error", + "jest/prefer-importing-jest-globals": "error", "jest/prefer-lowercase-title": "error", "jest/prefer-mock-promise-shorthand": "error", "jest/prefer-snapshot-hint": "error", @@ -126,6 +127,7 @@ exports[`rules should export configs that refer to actual rules 1`] = ` "jest/prefer-expect-resolves": "error", "jest/prefer-hooks-in-order": "error", "jest/prefer-hooks-on-top": "error", + "jest/prefer-importing-jest-globals": "error", "jest/prefer-lowercase-title": "error", "jest/prefer-mock-promise-shorthand": "error", "jest/prefer-snapshot-hint": "error", diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index db8eb4635..b70ba93ac 100644 --- a/src/__tests__/rules.test.ts +++ b/src/__tests__/rules.test.ts @@ -2,7 +2,7 @@ import { existsSync } from 'fs'; import { resolve } from 'path'; import plugin from '../'; -const numberOfRules = 52; +const numberOfRules = 53; const ruleNames = Object.keys(plugin.rules); const deprecatedRules = Object.entries(plugin.rules) .filter(([, rule]) => rule.meta.deprecated) diff --git a/src/rules/__tests__/prefer-importing-jest-globals.test.ts b/src/rules/__tests__/prefer-importing-jest-globals.test.ts new file mode 100644 index 000000000..00add8d90 --- /dev/null +++ b/src/rules/__tests__/prefer-importing-jest-globals.test.ts @@ -0,0 +1,504 @@ +import dedent from 'dedent'; +import rule from '../prefer-importing-jest-globals'; +import { FlatCompatRuleTester, espreeParser } from './test-utils'; + +const ruleTester = new FlatCompatRuleTester({ + parser: espreeParser, + parserOptions: { + ecmaVersion: 2015, + sourceType: 'script', + }, +}); + +ruleTester.run('prefer-importing-jest-globals', rule, { + valid: [ + { + code: dedent` + // with import + import { test, expect } from '@jest/globals'; + test('should pass', () => { + expect(true).toBeDefined(); + }); + `, + parserOptions: { sourceType: 'module' }, + }, + { + code: dedent` + // with require + const { test, expect } = require('@jest/globals'); + test('should pass', () => { + expect(true).toBeDefined(); + }); + `, + }, + { + code: dedent` + const { test, expect } = require(\`@jest/globals\`); + test('should pass', () => { + expect(true).toBeDefined(); + }); + `, + }, + { + code: dedent` + import { it as itChecks } from '@jest/globals'; + itChecks("foo"); + `, + parserOptions: { sourceType: 'module' }, + }, + { + code: dedent` + const { test } = require('@jest/globals'); + test("foo"); + `, + }, + { + code: dedent` + const { test } = require('my-test-library'); + test("foo"); + `, + }, + ], + invalid: [ + { + code: dedent` + import describe from '@jest/globals'; + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + output: dedent` + import { describe, expect, test } from '@jest/globals'; + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + parserOptions: { sourceType: 'module' }, + errors: [ + { + endColumn: 7, + column: 3, + line: 3, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + { + code: dedent` + import React from 'react'; + import { yourFunction } from './yourFile'; + import something from "something"; + import { test } from '@jest/globals'; + import { xit } from '@jest/globals'; + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + output: dedent` + import React from 'react'; + import { yourFunction } from './yourFile'; + import something from "something"; + import { describe, expect, test } from '@jest/globals'; + import { xit } from '@jest/globals'; + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + parserOptions: { sourceType: 'module' }, + errors: [ + { + endColumn: 9, + column: 1, + line: 6, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + { + code: dedent` + console.log('hello'); + import * as fs from 'fs'; + const { test, 'describe': describe } = require('@jest/globals'); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + output: dedent` + console.log('hello'); + import * as fs from 'fs'; + import { describe, expect, test } from '@jest/globals'; + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + parserOptions: { sourceType: 'module' }, + errors: [ + { + endColumn: 9, + column: 3, + line: 6, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + { + code: dedent` + console.log('hello'); + import jestGlobals from '@jest/globals'; + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + output: dedent` + console.log('hello'); + import { describe, expect, jestGlobals, test } from '@jest/globals'; + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + parserOptions: { sourceType: 'module' }, + errors: [ + { + endColumn: 9, + column: 1, + line: 3, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + { + code: dedent` + import { pending } from 'actions'; + describe('foo', () => { + test.each(['hello', 'world'])("%s", (a) => {}); + }); + `, + output: dedent` + import { pending } from 'actions'; + import { describe, test } from '@jest/globals'; + describe('foo', () => { + test.each(['hello', 'world'])("%s", (a) => {}); + }); + `, + parserOptions: { sourceType: 'module' }, + errors: [ + { + endColumn: 9, + column: 1, + line: 2, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + { + code: dedent` + const {describe} = require('@jest/globals'); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + output: dedent` + const { describe, expect, test } = require('@jest/globals'); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + errors: [ + { + endColumn: 7, + column: 3, + line: 3, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + { + code: dedent` + const {describe} = require(\`@jest/globals\`); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + // todo: we should really maintain the template literals + output: dedent` + const { describe, expect, test } = require('@jest/globals'); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + errors: [ + { + endColumn: 7, + column: 3, + line: 3, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + { + code: dedent` + const source = 'globals'; + const {describe} = require(\`@jest/\${source}\`); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + // todo: this shouldn't be indenting the "test" + output: dedent` + const source = 'globals'; + const {describe} = require(\`@jest/\${source}\`); + describe("suite", () => { + const { expect, test } = require('@jest/globals'); + test("foo"); + expect(true).toBeDefined(); + }) + `, + errors: [ + { + endColumn: 7, + column: 3, + line: 4, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + { + code: dedent` + const { [() => {}]: it } = require('@jest/globals'); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + output: dedent` + const { describe, expect, test } = require('@jest/globals'); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + errors: [ + { + endColumn: 9, + column: 1, + line: 2, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + { + code: dedent` + console.log('hello'); + const fs = require('fs'); + const { test, 'describe': describe } = require('@jest/globals'); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + output: dedent` + console.log('hello'); + const fs = require('fs'); + const { describe, expect, test } = require('@jest/globals'); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + errors: [ + { + endColumn: 9, + column: 3, + line: 6, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + { + code: dedent` + console.log('hello'); + const jestGlobals = require('@jest/globals'); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + output: dedent` + console.log('hello'); + const { describe, expect, test } = require('@jest/globals'); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + errors: [ + { + endColumn: 9, + column: 1, + line: 3, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + { + code: dedent` + const { pending } = require('actions'); + describe('foo', () => { + test.each(['hello', 'world'])("%s", (a) => {}); + }); + `, + output: dedent` + const { pending } = require('actions'); + const { describe, test } = require('@jest/globals'); + describe('foo', () => { + test.each(['hello', 'world'])("%s", (a) => {}); + }); + `, + errors: [ + { + endColumn: 9, + column: 1, + line: 2, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + { + code: dedent` + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + output: dedent` + const { describe, expect, test } = require('@jest/globals'); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + errors: [ + { + endColumn: 9, + column: 1, + line: 1, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + { + code: dedent` + #!/usr/bin/env node + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + output: dedent` + #!/usr/bin/env node + const { describe, expect, test } = require('@jest/globals'); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + parserOptions: { sourceType: 'script' }, + errors: [ + { + endColumn: 9, + column: 1, + line: 2, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + { + code: dedent` + // with comment above + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + output: dedent` + // with comment above + const { describe, expect, test } = require('@jest/globals'); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + errors: [ + { + endColumn: 9, + column: 1, + line: 2, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + { + code: dedent` + 'use strict'; + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + output: dedent` + 'use strict'; + const { describe, expect, test } = require('@jest/globals'); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + errors: [ + { + endColumn: 9, + column: 1, + line: 2, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + { + code: dedent` + \`use strict\`; + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + output: dedent` + \`use strict\`; + const { describe, expect, test } = require('@jest/globals'); + describe("suite", () => { + test("foo"); + expect(true).toBeDefined(); + }) + `, + errors: [ + { + endColumn: 9, + column: 1, + line: 2, + messageId: 'preferImportingJestGlobal', + }, + ], + }, + ], +}); diff --git a/src/rules/prefer-importing-jest-globals.ts b/src/rules/prefer-importing-jest-globals.ts new file mode 100644 index 000000000..49f2b794f --- /dev/null +++ b/src/rules/prefer-importing-jest-globals.ts @@ -0,0 +1,163 @@ +import { AST_NODE_TYPES, type TSESTree } from '@typescript-eslint/utils'; +import { + createRule, + getAccessorValue, + getSourceCode, + isIdentifier, + isStringNode, + isSupportedAccessor, + parseJestFnCall, +} from './utils'; + +const createFixerImports = ( + isModule: boolean, + functionsToImport: Set, +) => { + const allImportsFormatted = Array.from(functionsToImport).sort().join(', '); + + return isModule + ? `import { ${allImportsFormatted} } from '@jest/globals';` + : `const { ${allImportsFormatted} } = require('@jest/globals');`; +}; + +export default createRule({ + name: __filename, + meta: { + docs: { + description: 'Prefer importing Jest globals', + }, + messages: { + preferImportingJestGlobal: `Import the following Jest functions from '@jest/globals': {{ jestFunctions }}`, + }, + fixable: 'code', + type: 'problem', + schema: [], + }, + defaultOptions: [], + create(context) { + const importedFunctionsWithSource: Record = {}; + const functionsToImport = new Set(); + let reportingNode: TSESTree.Node; + + return { + ImportDeclaration(node: TSESTree.ImportDeclaration) { + node.specifiers.forEach(specifier => { + if (specifier.type === 'ImportSpecifier') { + importedFunctionsWithSource[specifier.local.name] = + node.source.value; + } + }); + }, + CallExpression(node: TSESTree.CallExpression) { + const jestFnCall = parseJestFnCall(node, context); + + if (!jestFnCall) { + return; + } + + if (jestFnCall.head.type !== 'import') { + functionsToImport.add(jestFnCall.name); + reportingNode ||= jestFnCall.head.node; + } + }, + 'Program:exit'() { + // this means we found at least one function to import + if (!reportingNode) { + return; + } + + const isModule = context.parserOptions.sourceType === 'module'; + + context.report({ + node: reportingNode, + messageId: 'preferImportingJestGlobal', + data: { jestFunctions: Array.from(functionsToImport).join(', ') }, + fix(fixer) { + const sourceCode = getSourceCode(context); + const [firstNode] = sourceCode.ast.body; + + // check if "use strict" directive exists + if ( + firstNode.type === AST_NODE_TYPES.ExpressionStatement && + isStringNode(firstNode.expression, 'use strict') + ) { + return fixer.insertTextAfter( + firstNode, + `\n${createFixerImports(isModule, functionsToImport)}`, + ); + } + + const importNode = sourceCode.ast.body.find( + node => + node.type === AST_NODE_TYPES.ImportDeclaration && + node.source.value === '@jest/globals', + ); + + if (importNode?.type === AST_NODE_TYPES.ImportDeclaration) { + for (const specifier of importNode.specifiers) { + if ( + specifier.type === AST_NODE_TYPES.ImportSpecifier && + specifier.imported?.name + ) { + functionsToImport.add(specifier.imported.name); + } + + if (specifier.type === AST_NODE_TYPES.ImportDefaultSpecifier) { + functionsToImport.add(specifier.local.name); + } + } + + return fixer.replaceText( + importNode, + createFixerImports(isModule, functionsToImport), + ); + } + + const requireNode = sourceCode.ast.body.find( + node => + node.type === AST_NODE_TYPES.VariableDeclaration && + node.declarations.some( + declaration => + declaration.init?.type === AST_NODE_TYPES.CallExpression && + isIdentifier(declaration.init.callee, 'require') && + isStringNode( + declaration.init.arguments[0], + '@jest/globals', + ) && + (declaration.id.type === AST_NODE_TYPES.Identifier || + declaration.id.type === AST_NODE_TYPES.ObjectPattern), + ), + ); + + if (requireNode?.type !== AST_NODE_TYPES.VariableDeclaration) { + return fixer.insertTextBefore( + reportingNode, + `${createFixerImports(isModule, functionsToImport)}\n`, + ); + } + + if ( + requireNode.declarations[0]?.id.type === + AST_NODE_TYPES.ObjectPattern + ) { + for (const property of requireNode.declarations[0].id + .properties) { + if ( + property.type === AST_NODE_TYPES.Property && + isSupportedAccessor(property.key) + ) { + functionsToImport.add(getAccessorValue(property.key)); + } + } + } + + return fixer.replaceText( + requireNode, + `${createFixerImports(isModule, functionsToImport)}`, + ); + }, + }); + }, + }; + }, +});