diff --git a/README.md b/README.md index cdec41a6a..68e8855e5 100644 --- a/README.md +++ b/README.md @@ -210,58 +210,59 @@ set to warn in.\ πŸ’‘ Manually fixable by [editor suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).\ ❌ Deprecated. -| NameΒ Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β  | Description | πŸ’Ό | ⚠️ | πŸ”§ | πŸ’‘ | ❌ | -| :--------------------------------------------------------------------------- | :------------------------------------------------------------------ | :-- | :-- | :-- | :-- | :-- | -| [consistent-test-it](docs/rules/consistent-test-it.md) | Enforce `test` and `it` usage conventions | | | πŸ”§ | | | -| [expect-expect](docs/rules/expect-expect.md) | Enforce assertion to be made in a test body | | βœ… | | | | -| [max-expects](docs/rules/max-expects.md) | Enforces a maximum number assertion calls in a test body | | | | | | -| [max-nested-describe](docs/rules/max-nested-describe.md) | Enforces a maximum depth to nested describe calls | | | | | | -| [no-alias-methods](docs/rules/no-alias-methods.md) | Disallow alias methods | βœ… | 🎨 | πŸ”§ | | | -| [no-commented-out-tests](docs/rules/no-commented-out-tests.md) | Disallow commented out tests | | βœ… | | | | -| [no-conditional-expect](docs/rules/no-conditional-expect.md) | Disallow calling `expect` conditionally | βœ… | | | | | -| [no-conditional-in-test](docs/rules/no-conditional-in-test.md) | Disallow conditional logic in tests | | | | | | -| [no-deprecated-functions](docs/rules/no-deprecated-functions.md) | Disallow use of deprecated functions | βœ… | | πŸ”§ | | | -| [no-disabled-tests](docs/rules/no-disabled-tests.md) | Disallow disabled tests | | βœ… | | | | -| [no-done-callback](docs/rules/no-done-callback.md) | Disallow using a callback in asynchronous tests and hooks | βœ… | | | πŸ’‘ | | -| [no-duplicate-hooks](docs/rules/no-duplicate-hooks.md) | Disallow duplicate setup and teardown hooks | | | | | | -| [no-export](docs/rules/no-export.md) | Disallow using `exports` in files containing tests | βœ… | | | | | -| [no-focused-tests](docs/rules/no-focused-tests.md) | Disallow focused tests | βœ… | | | πŸ’‘ | | -| [no-hooks](docs/rules/no-hooks.md) | Disallow setup and teardown hooks | | | | | | -| [no-identical-title](docs/rules/no-identical-title.md) | Disallow identical titles | βœ… | | | | | -| [no-if](docs/rules/no-if.md) | Disallow conditional logic | | | | | ❌ | -| [no-interpolation-in-snapshots](docs/rules/no-interpolation-in-snapshots.md) | Disallow string interpolation inside snapshots | βœ… | | | | | -| [no-jasmine-globals](docs/rules/no-jasmine-globals.md) | Disallow Jasmine globals | βœ… | | πŸ”§ | | | -| [no-large-snapshots](docs/rules/no-large-snapshots.md) | Disallow large snapshots | | | | | | -| [no-mocks-import](docs/rules/no-mocks-import.md) | Disallow manually importing from `__mocks__` | βœ… | | | | | -| [no-restricted-jest-methods](docs/rules/no-restricted-jest-methods.md) | Disallow specific `jest.` methods | | | | | | -| [no-restricted-matchers](docs/rules/no-restricted-matchers.md) | Disallow specific matchers & modifiers | | | | | | -| [no-standalone-expect](docs/rules/no-standalone-expect.md) | Disallow using `expect` outside of `it` or `test` blocks | βœ… | | | | | -| [no-test-prefixes](docs/rules/no-test-prefixes.md) | Require using `.only` and `.skip` over `f` and `x` | βœ… | | πŸ”§ | | | -| [no-test-return-statement](docs/rules/no-test-return-statement.md) | Disallow explicitly returning from tests | | | | | | -| [prefer-called-with](docs/rules/prefer-called-with.md) | Suggest using `toBeCalledWith()` or `toHaveBeenCalledWith()` | | | | | | -| [prefer-comparison-matcher](docs/rules/prefer-comparison-matcher.md) | Suggest using the built-in comparison matchers | | | πŸ”§ | | | -| [prefer-each](docs/rules/prefer-each.md) | Prefer using `.each` rather than manual loops | | | | | | -| [prefer-equality-matcher](docs/rules/prefer-equality-matcher.md) | Suggest using the built-in equality matchers | | | | πŸ’‘ | | -| [prefer-expect-assertions](docs/rules/prefer-expect-assertions.md) | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | | | πŸ’‘ | | -| [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-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 | | | | | | -| [prefer-spy-on](docs/rules/prefer-spy-on.md) | Suggest using `jest.spyOn()` | | | πŸ”§ | | | -| [prefer-strict-equal](docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | | | πŸ’‘ | | -| [prefer-to-be](docs/rules/prefer-to-be.md) | Suggest using `toBe()` for primitive literals | 🎨 | | πŸ”§ | | | -| [prefer-to-contain](docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | 🎨 | | πŸ”§ | | | -| [prefer-to-have-length](docs/rules/prefer-to-have-length.md) | Suggest using `toHaveLength()` | 🎨 | | πŸ”§ | | | -| [prefer-todo](docs/rules/prefer-todo.md) | Suggest using `test.todo` | | | πŸ”§ | | | -| [require-hook](docs/rules/require-hook.md) | Require setup and teardown code to be within a hook | | | | | | -| [require-to-throw-message](docs/rules/require-to-throw-message.md) | Require a message for `toThrow()` | | | | | | -| [require-top-level-describe](docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `describe` block | | | | | | -| [valid-describe-callback](docs/rules/valid-describe-callback.md) | Enforce valid `describe()` callback | βœ… | | | | | -| [valid-expect](docs/rules/valid-expect.md) | Enforce valid `expect()` usage | βœ… | | | | | -| [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md) | Require promises that have expectations in their chain to be valid | βœ… | | | | | -| [valid-title](docs/rules/valid-title.md) | Enforce valid titles | βœ… | | πŸ”§ | | | +| NameΒ Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β Β  | Description | πŸ’Ό | ⚠️ | πŸ”§ | πŸ’‘ | ❌ | +| :--------------------------------------------------------------------------- | :------------------------------------------------------------------------ | :-- | :-- | :-- | :-- | :-- | +| [consistent-test-it](docs/rules/consistent-test-it.md) | Enforce `test` and `it` usage conventions | | | πŸ”§ | | | +| [expect-expect](docs/rules/expect-expect.md) | Enforce assertion to be made in a test body | | βœ… | | | | +| [max-expects](docs/rules/max-expects.md) | Enforces a maximum number assertion calls in a test body | | | | | | +| [max-nested-describe](docs/rules/max-nested-describe.md) | Enforces a maximum depth to nested describe calls | | | | | | +| [no-alias-methods](docs/rules/no-alias-methods.md) | Disallow alias methods | βœ… | 🎨 | πŸ”§ | | | +| [no-commented-out-tests](docs/rules/no-commented-out-tests.md) | Disallow commented out tests | | βœ… | | | | +| [no-conditional-expect](docs/rules/no-conditional-expect.md) | Disallow calling `expect` conditionally | βœ… | | | | | +| [no-conditional-in-test](docs/rules/no-conditional-in-test.md) | Disallow conditional logic in tests | | | | | | +| [no-deprecated-functions](docs/rules/no-deprecated-functions.md) | Disallow use of deprecated functions | βœ… | | πŸ”§ | | | +| [no-disabled-tests](docs/rules/no-disabled-tests.md) | Disallow disabled tests | | βœ… | | | | +| [no-done-callback](docs/rules/no-done-callback.md) | Disallow using a callback in asynchronous tests and hooks | βœ… | | | πŸ’‘ | | +| [no-duplicate-hooks](docs/rules/no-duplicate-hooks.md) | Disallow duplicate setup and teardown hooks | | | | | | +| [no-export](docs/rules/no-export.md) | Disallow using `exports` in files containing tests | βœ… | | | | | +| [no-focused-tests](docs/rules/no-focused-tests.md) | Disallow focused tests | βœ… | | | πŸ’‘ | | +| [no-hooks](docs/rules/no-hooks.md) | Disallow setup and teardown hooks | | | | | | +| [no-identical-title](docs/rules/no-identical-title.md) | Disallow identical titles | βœ… | | | | | +| [no-if](docs/rules/no-if.md) | Disallow conditional logic | | | | | ❌ | +| [no-interpolation-in-snapshots](docs/rules/no-interpolation-in-snapshots.md) | Disallow string interpolation inside snapshots | βœ… | | | | | +| [no-jasmine-globals](docs/rules/no-jasmine-globals.md) | Disallow Jasmine globals | βœ… | | πŸ”§ | | | +| [no-large-snapshots](docs/rules/no-large-snapshots.md) | Disallow large snapshots | | | | | | +| [no-mocks-import](docs/rules/no-mocks-import.md) | Disallow manually importing from `__mocks__` | βœ… | | | | | +| [no-restricted-jest-methods](docs/rules/no-restricted-jest-methods.md) | Disallow specific `jest.` methods | | | | | | +| [no-restricted-matchers](docs/rules/no-restricted-matchers.md) | Disallow specific matchers & modifiers | | | | | | +| [no-standalone-expect](docs/rules/no-standalone-expect.md) | Disallow using `expect` outside of `it` or `test` blocks | βœ… | | | | | +| [no-test-prefixes](docs/rules/no-test-prefixes.md) | Require using `.only` and `.skip` over `f` and `x` | βœ… | | πŸ”§ | | | +| [no-test-return-statement](docs/rules/no-test-return-statement.md) | Disallow explicitly returning from tests | | | | | | +| [no-untyped-mock-factory](docs/rules/no-untyped-mock-factory.md) | Disallow using `jest.mock()` factories without an explicit type parameter | | | πŸ”§ | | | +| [prefer-called-with](docs/rules/prefer-called-with.md) | Suggest using `toBeCalledWith()` or `toHaveBeenCalledWith()` | | | | | | +| [prefer-comparison-matcher](docs/rules/prefer-comparison-matcher.md) | Suggest using the built-in comparison matchers | | | πŸ”§ | | | +| [prefer-each](docs/rules/prefer-each.md) | Prefer using `.each` rather than manual loops | | | | | | +| [prefer-equality-matcher](docs/rules/prefer-equality-matcher.md) | Suggest using the built-in equality matchers | | | | πŸ’‘ | | +| [prefer-expect-assertions](docs/rules/prefer-expect-assertions.md) | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | | | πŸ’‘ | | +| [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-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 | | | | | | +| [prefer-spy-on](docs/rules/prefer-spy-on.md) | Suggest using `jest.spyOn()` | | | πŸ”§ | | | +| [prefer-strict-equal](docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | | | πŸ’‘ | | +| [prefer-to-be](docs/rules/prefer-to-be.md) | Suggest using `toBe()` for primitive literals | 🎨 | | πŸ”§ | | | +| [prefer-to-contain](docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | 🎨 | | πŸ”§ | | | +| [prefer-to-have-length](docs/rules/prefer-to-have-length.md) | Suggest using `toHaveLength()` | 🎨 | | πŸ”§ | | | +| [prefer-todo](docs/rules/prefer-todo.md) | Suggest using `test.todo` | | | πŸ”§ | | | +| [require-hook](docs/rules/require-hook.md) | Require setup and teardown code to be within a hook | | | | | | +| [require-to-throw-message](docs/rules/require-to-throw-message.md) | Require a message for `toThrow()` | | | | | | +| [require-top-level-describe](docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `describe` block | | | | | | +| [valid-describe-callback](docs/rules/valid-describe-callback.md) | Enforce valid `describe()` callback | βœ… | | | | | +| [valid-expect](docs/rules/valid-expect.md) | Enforce valid `expect()` usage | βœ… | | | | | +| [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md) | Require promises that have expectations in their chain to be valid | βœ… | | | | | +| [valid-title](docs/rules/valid-title.md) | Enforce valid titles | βœ… | | πŸ”§ | | | ### Requires Type Checking diff --git a/docs/rules/no-untyped-mock-factory.md b/docs/rules/no-untyped-mock-factory.md new file mode 100644 index 000000000..ffd859630 --- /dev/null +++ b/docs/rules/no-untyped-mock-factory.md @@ -0,0 +1,72 @@ +# Disallow using `jest.mock()` factories without an explicit type parameter (`no-untyped-mock-factory`) + +πŸ”§ This rule is automatically fixable by the +[`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix). + + + +By default, `jest.mock` and `jest.doMock` allow any type to be returned by a +mock factory. A generic type parameter can be used to enforce that the factory +returns an object with the same shape as the original module, or some other +strict type. Requiring a type makes it easier to use TypeScript to catch changes +needed in test mocks when the source module changes. + +> **Warning** +> +> This rule expects to be run on TypeScript files **only**. If you are using a +> codebase that has a mix of JavaScript and TypeScript tests, you can use +> [overrides](https://eslint.org/docs/latest/user-guide/configuring/configuration-files#how-do-overrides-work) +> to apply this rule to just your TypeScript test files. + +## Rule details + +This rule triggers a warning if `mock()` or `doMock()` is used without a generic +type parameter or return type. + +The following patterns are considered errors: + +```ts +jest.mock('../moduleName', () => { + return jest.fn(() => 42); +}); + +jest.mock('./module', () => ({ + ...jest.requireActual('./module'), + foo: jest.fn(), +})); + +jest.mock('random-num', () => { + return jest.fn(() => 42); +}); +``` + +The following patterns are not considered errors: + +```ts +// Uses typeof import() +jest.mock('../moduleName', () => { + return jest.fn(() => 42); +}); + +jest.mock('./module', () => ({ + ...jest.requireActual('./module'), + foo: jest.fn(), +})); + +// Uses custom type +jest.mock<() => number>('random-num', () => { + return jest.fn(() => 42); +}); + +// No factory +jest.mock('random-num'); + +// Virtual mock +jest.mock( + '../moduleName', + () => { + return jest.fn(() => 42); + }, + { virtual: true }, +); +``` diff --git a/src/__tests__/__snapshots__/rules.test.ts.snap b/src/__tests__/__snapshots__/rules.test.ts.snap index bb7d785e7..2e5f25df7 100644 --- a/src/__tests__/__snapshots__/rules.test.ts.snap +++ b/src/__tests__/__snapshots__/rules.test.ts.snap @@ -35,6 +35,7 @@ exports[`rules should export configs that refer to actual rules 1`] = ` "jest/no-standalone-expect": "error", "jest/no-test-prefixes": "error", "jest/no-test-return-statement": "error", + "jest/no-untyped-mock-factory": "error", "jest/prefer-called-with": "error", "jest/prefer-comparison-matcher": "error", "jest/prefer-each": "error", diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index 86fdf0a38..fa080af37 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 = 51; +const numberOfRules = 52; const ruleNames = Object.keys(plugin.rules); const deprecatedRules = Object.entries(plugin.rules) .filter(([, rule]) => rule.meta.deprecated) diff --git a/src/rules/__tests__/no-untyped-mock-factory.test.ts b/src/rules/__tests__/no-untyped-mock-factory.test.ts new file mode 100644 index 000000000..171497699 --- /dev/null +++ b/src/rules/__tests__/no-untyped-mock-factory.test.ts @@ -0,0 +1,155 @@ +import { TSESLint } from '@typescript-eslint/utils'; +import dedent from 'dedent'; +import rule from '../no-untyped-mock-factory'; + +const ruleTester = new TSESLint.RuleTester({ + parser: require.resolve('@typescript-eslint/parser'), +}); + +ruleTester.run('no-untyped-mock-factory', rule, { + valid: [ + "jest.mock('random-number');", + dedent` + jest.mock('../moduleName', () => { + return jest.fn(() => 42); + }); + `, + dedent` + jest.mock('./module', () => ({ + ...jest.requireActual('./module'), + foo: jest.fn() + })); + `, + dedent` + jest.mock('bar', () => ({ + ...jest.requireActual('bar'), + foo: jest.fn() + })); + `, + dedent` + jest.doMock('./module', (): typeof import('./module') => ({ + ...jest.requireActual('./module'), + foo: jest.fn() + })); + `, + dedent` + jest.mock('../moduleName', function (): typeof import('../moduleName') { + return jest.fn(() => 42); + }); + `, + dedent` + jest.mock<() => number>('random-num', () => { + return jest.fn(() => 42); + }); + `, + dedent` + jest['doMock']<() => number>('random-num', () => { + return jest.fn(() => 42); + }); + `, + dedent` + jest.mock('random-num', () => { + return jest.fn(() => 42); + }); + `, + dedent` + jest.mock( + '../moduleName', + () => { + return jest.fn(() => 42) + }, + {virtual: true}, + ); + `, + dedent` + jest.mock('../moduleName', function (): (() => number) { + return jest.fn(() => 42); + }); + `, + // Should not match + dedent` + mockito<() => number>('foo', () => { + return jest.fn(() => 42); + }); + `, + ], + invalid: [ + { + code: dedent` + jest.mock('../moduleName', () => { + return jest.fn(() => 42); + }); + `, + output: dedent` + jest.mock('../moduleName', () => { + return jest.fn(() => 42); + }); + `, + errors: [{ messageId: 'addTypeParameterToModuleMock' }], + }, + { + code: dedent` + jest.mock("./module", () => ({ + ...jest.requireActual('./module'), + foo: jest.fn() + })); + `, + output: dedent` + jest.mock("./module", () => ({ + ...jest.requireActual('./module'), + foo: jest.fn() + })); + `, + errors: [{ messageId: 'addTypeParameterToModuleMock' }], + }, + { + code: dedent` + jest.mock('random-num', () => { + return jest.fn(() => 42); + }); + `, + output: dedent` + jest.mock('random-num', () => { + return jest.fn(() => 42); + }); + `, + errors: [{ messageId: 'addTypeParameterToModuleMock' }], + }, + { + code: dedent` + jest.doMock('random-num', () => { + return jest.fn(() => 42); + }); + `, + output: dedent` + jest.doMock('random-num', () => { + return jest.fn(() => 42); + }); + `, + errors: [{ messageId: 'addTypeParameterToModuleMock' }], + }, + { + code: dedent` + jest['mock']('random-num', () => { + return jest.fn(() => 42); + }); + `, + output: dedent` + jest['mock']('random-num', () => { + return jest.fn(() => 42); + }); + `, + errors: [{ messageId: 'addTypeParameterToModuleMock' }], + }, + { + code: dedent` + const moduleToMock = 'random-num'; + jest.mock(moduleToMock, () => { + return jest.fn(() => 42); + }); + `, + output: null, + errors: [{ messageId: 'addTypeParameterToModuleMock' }], + }, + ], +}); diff --git a/src/rules/no-untyped-mock-factory.ts b/src/rules/no-untyped-mock-factory.ts new file mode 100644 index 000000000..076ae224e --- /dev/null +++ b/src/rules/no-untyped-mock-factory.ts @@ -0,0 +1,87 @@ +import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'; +import { + createRule, + getAccessorValue, + isFunction, + isSupportedAccessor, + isTypeOfJestFnCall, +} from './utils'; + +const findModuleName = ( + node: TSESTree.Literal | TSESTree.Node, +): TSESTree.StringLiteral | null => { + if (node.type === AST_NODE_TYPES.Literal && typeof node.value === 'string') { + return node; + } + + return null; +}; + +export default createRule({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: + 'Disallow using `jest.mock()` factories without an explicit type parameter', + recommended: false, + }, + messages: { + addTypeParameterToModuleMock: + 'Add a type parameter to the mock factory such as `typeof import({{ moduleName }})`', + }, + fixable: 'code', + schema: [], + type: 'suggestion', + }, + defaultOptions: [], + create(context) { + return { + CallExpression(node: TSESTree.CallExpression): void { + const { callee, typeParameters } = node; + + if (callee.type !== AST_NODE_TYPES.MemberExpression) { + return; + } + + const { property } = callee; + + if ( + node.arguments.length === 2 && + isTypeOfJestFnCall(node, context, ['jest']) && + isSupportedAccessor(property) && + ['mock', 'doMock'].includes(getAccessorValue(property)) + ) { + const [nameNode, factoryNode] = node.arguments; + + const hasTypeParameter = + typeParameters !== undefined && typeParameters.params.length > 0; + const hasReturnType = + isFunction(factoryNode) && factoryNode.returnType !== undefined; + + if (hasTypeParameter || hasReturnType) { + return; + } + + const moduleName = findModuleName(nameNode); + + context.report({ + messageId: 'addTypeParameterToModuleMock', + data: { moduleName: moduleName?.raw ?? './module-name' }, + node, + fix(fixer) { + if (!moduleName) return []; + + return [ + fixer.insertTextAfter( + callee, + ``, + ), + ]; + }, + }); + } + }, + }; + }, +});