diff --git a/README.md b/README.md index f4ec952a4..fd8153170 100644 --- a/README.md +++ b/README.md @@ -232,6 +232,7 @@ installations requiring long-term consistency. | [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 | | ![fixable][] | +| [prefer-mock-promise-shorthand](docs/rules/prefer-mock-promise-shorthand.md) | Prefer mock resolved/rejected shorthands for promises | | ![fixable][] | | [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()` | | ![fixable][] | | [prefer-strict-equal](docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | ![suggest][] | diff --git a/docs/rules/prefer-mock-promise-shorthand.md b/docs/rules/prefer-mock-promise-shorthand.md new file mode 100644 index 000000000..f7dda79a3 --- /dev/null +++ b/docs/rules/prefer-mock-promise-shorthand.md @@ -0,0 +1,34 @@ +# Prefer mock resolved/rejected shorthands for promises (`prefer-mock-promise-shorthand`) + +When working with mocks of functions that return promises, Jest provides some +API sugar functions to reduce the amount of boilerplate you have to write. + +These methods should be preferred when possible. + +## Rule Details + +The following patterns are warnings: + +```js +jest.fn().mockImplementation(() => Promise.resolve(123)); +jest + .spyOn(fs.promises, 'readFile') + .mockReturnValue(Promise.reject(new Error('oh noes!'))); + +myFunction + .mockReturnValueOnce(Promise.resolve(42)) + .mockImplementationOnce(() => Promise.resolve(42)) + .mockReturnValue(Promise.reject(new Error('too many calls!'))); +``` + +The following patterns are not warnings: + +```js +jest.fn().mockResolvedValue(123); +jest.spyOn(fs.promises, 'readFile').mockRejectedValue(new Error('oh noes!')); + +myFunction + .mockResolvedValueOnce(42) + .mockResolvedValueOnce(42) + .mockRejectedValue(new Error('too many calls!')); +``` diff --git a/src/__tests__/__snapshots__/rules.test.ts.snap b/src/__tests__/__snapshots__/rules.test.ts.snap index 4d9cbd30b..ab02a0366 100644 --- a/src/__tests__/__snapshots__/rules.test.ts.snap +++ b/src/__tests__/__snapshots__/rules.test.ts.snap @@ -43,6 +43,7 @@ Object { "jest/prefer-hooks-in-order": "error", "jest/prefer-hooks-on-top": "error", "jest/prefer-lowercase-title": "error", + "jest/prefer-mock-promise-shorthand": "error", "jest/prefer-snapshot-hint": "error", "jest/prefer-spy-on": "error", "jest/prefer-strict-equal": "error", diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index 826bfb533..a8c66cc18 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 = 49; +const numberOfRules = 50; const ruleNames = Object.keys(plugin.rules); const deprecatedRules = Object.entries(plugin.rules) .filter(([, rule]) => rule.meta.deprecated) diff --git a/src/rules/__tests__/prefer-mock-promise-shorthand.test.ts b/src/rules/__tests__/prefer-mock-promise-shorthand.test.ts new file mode 100644 index 000000000..9ae057df7 --- /dev/null +++ b/src/rules/__tests__/prefer-mock-promise-shorthand.test.ts @@ -0,0 +1,398 @@ +import { TSESLint } from '@typescript-eslint/utils'; +import dedent from 'dedent'; +import rule from '../prefer-mock-promise-shorthand'; +import { espreeParser } from './test-utils'; + +const ruleTester = new TSESLint.RuleTester({ + parser: espreeParser, + parserOptions: { + ecmaVersion: 6, + }, +}); + +ruleTester.run('prefer-mock-shorthand', rule, { + valid: [ + 'describe()', + 'it()', + 'describe.skip()', + 'it.skip()', + 'test()', + 'test.skip()', + 'var appliedOnly = describe.only; appliedOnly.apply(describe)', + 'var calledOnly = it.only; calledOnly.call(it)', + 'it.each()()', + 'it.each`table`()', + 'test.each()()', + 'test.each`table`()', + 'test.concurrent()', + 'jest.fn().mockResolvedValue(42)', + 'jest.fn(() => Promise.resolve(42))', + 'jest.fn(() => Promise.reject(42))', + 'aVariable.mockImplementation', + 'aVariable.mockImplementation()', + 'aVariable.mockImplementation([])', + 'aVariable.mockImplementation(() => {})', + 'aVariable.mockImplementation(() => [])', + 'aVariable.mockReturnValue(() => Promise.resolve(1))', + 'aVariable.mockReturnValue(Promise.resolve(1).then(() => 1))', + 'aVariable.mockReturnValue(Promise.reject(1).then(() => 1))', + 'aVariable.mockReturnValue(Promise.reject().then(() => 1))', + 'aVariable.mockReturnValue(new Promise(resolve => resolve(1)))', + 'aVariable.mockReturnValue(new Promise((_, reject) => reject(1)))', + dedent` + aVariable.mockImplementation(() => { + const value = new Date(); + + return Promise.resolve(value); + }); + `, + dedent` + aVariable.mockImplementation(() => { + return Promise.resolve(value) + .then(value => value + 1); + }); + `, + dedent` + aVariable.mockImplementation(() => { + return Promise.all([1, 2, 3]); + }); + `, + 'aVariable.mockImplementation(() => Promise.all([1, 2, 3]));', + 'aVariable.mockReturnValue(Promise.all([1, 2, 3]));', + ], + + invalid: [ + { + code: 'jest.fn().mockImplementation(() => Promise.resolve(42))', + output: 'jest.fn().mockResolvedValue(42)', + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockResolvedValue' }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'jest.fn().mockImplementation(() => Promise.reject(42))', + output: 'jest.fn().mockRejectedValue(42)', + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockRejectedValue' }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'aVariable.mockImplementation(() => Promise.resolve(42))', + output: 'aVariable.mockResolvedValue(42)', + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockResolvedValue' }, + column: 11, + line: 1, + }, + ], + }, + { + code: dedent` + aVariable.mockImplementation(() => { + return Promise.resolve(42) + }) + `, + output: 'aVariable.mockResolvedValue(42)', + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockResolvedValue' }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'aVariable.mockImplementation(() => Promise.reject(42))', + output: 'aVariable.mockRejectedValue(42)', + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockRejectedValue' }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'aVariable.mockImplementationOnce(() => Promise.resolve(42))', + output: 'aVariable.mockResolvedValueOnce(42)', + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockResolvedValueOnce' }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'aVariable.mockImplementationOnce(() => Promise.reject(42))', + output: 'aVariable.mockRejectedValueOnce(42)', + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockRejectedValueOnce' }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'jest.fn().mockReturnValue(Promise.resolve(42))', + output: 'jest.fn().mockResolvedValue(42)', + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockResolvedValue' }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'jest.fn().mockReturnValue(Promise.reject(42))', + output: 'jest.fn().mockRejectedValue(42)', + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockRejectedValue' }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'aVariable.mockReturnValue(Promise.resolve(42))', + output: 'aVariable.mockResolvedValue(42)', + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockResolvedValue' }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'aVariable.mockReturnValue(Promise.reject(42))', + output: 'aVariable.mockRejectedValue(42)', + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockRejectedValue' }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'aVariable.mockReturnValueOnce(Promise.resolve(42))', + output: 'aVariable.mockResolvedValueOnce(42)', + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockResolvedValueOnce' }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'aVariable.mockReturnValueOnce(Promise.reject(42))', + output: 'aVariable.mockRejectedValueOnce(42)', + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockRejectedValueOnce' }, + column: 11, + line: 1, + }, + ], + }, + { + code: dedent` + aVariable.mockReturnValue(Promise.resolve({ + target: 'world', + message: 'hello' + })) + `, + output: dedent` + aVariable.mockResolvedValue({ + target: 'world', + message: 'hello' + }) + `, + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockResolvedValue' }, + column: 11, + line: 1, + }, + ], + }, + { + code: dedent` + aVariable + .mockImplementation(() => Promise.reject(42)) + .mockImplementation(() => Promise.resolve(42)) + .mockReturnValue(Promise.reject(42)) + `, + output: dedent` + aVariable + .mockRejectedValue(42) + .mockResolvedValue(42) + .mockRejectedValue(42) + `, + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockRejectedValue' }, + column: 4, + line: 2, + }, + { + messageId: 'useMockShorthand', + data: { replacement: 'mockResolvedValue' }, + column: 4, + line: 3, + }, + { + messageId: 'useMockShorthand', + data: { replacement: 'mockRejectedValue' }, + column: 4, + line: 4, + }, + ], + }, + { + code: dedent` + aVariable + .mockReturnValueOnce(Promise.reject(42)) + .mockImplementation(() => Promise.resolve(42)) + .mockReturnValueOnce(Promise.reject(42)) + `, + output: dedent` + aVariable + .mockRejectedValueOnce(42) + .mockResolvedValue(42) + .mockRejectedValueOnce(42) + `, + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockRejectedValueOnce' }, + column: 4, + line: 2, + }, + { + messageId: 'useMockShorthand', + data: { replacement: 'mockResolvedValue' }, + column: 4, + line: 3, + }, + { + messageId: 'useMockShorthand', + data: { replacement: 'mockRejectedValueOnce' }, + column: 4, + line: 4, + }, + ], + }, + { + code: dedent` + aVariable.mockReturnValueOnce( + Promise.reject( + new Error('oh noes!') + ) + ) + `, + output: dedent` + aVariable.mockRejectedValueOnce( + new Error('oh noes!') + ) + `, + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockRejectedValueOnce' }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'jest.fn().mockReturnValue(Promise.resolve(42), xyz)', + output: 'jest.fn().mockResolvedValue(42, xyz)', + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockResolvedValue' }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'jest.fn().mockImplementation(() => Promise.reject(42), xyz)', + output: 'jest.fn().mockRejectedValue(42, xyz)', + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockRejectedValue' }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'aVariable.mockReturnValueOnce(Promise.resolve(42, xyz))', + output: null, + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockResolvedValueOnce' }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'aVariable.mockReturnValueOnce(Promise.resolve())', + output: 'aVariable.mockResolvedValueOnce(undefined)', + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockResolvedValueOnce' }, + column: 11, + line: 1, + }, + ], + }, + { + code: 'jest.spyOn(fs, "readFile").mockReturnValue(Promise.reject(new Error("oh noes!")))', + output: `jest.spyOn(fs, "readFile").mockRejectedValue(new Error("oh noes!"))`, + errors: [ + { + messageId: 'useMockShorthand', + data: { replacement: 'mockRejectedValue' }, + column: 28, + line: 1, + }, + ], + }, + ], +}); diff --git a/src/rules/prefer-mock-promise-shorthand.ts b/src/rules/prefer-mock-promise-shorthand.ts new file mode 100644 index 000000000..47cfde8b2 --- /dev/null +++ b/src/rules/prefer-mock-promise-shorthand.ts @@ -0,0 +1,129 @@ +import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'; +import { + AccessorNode, + FunctionExpression, + createRule, + getAccessorValue, + getNodeName, + isFunction, + isSupportedAccessor, +} from './utils'; + +const withOnce = (name: string, addOnce: boolean): string => { + return `${name}${addOnce ? 'Once' : ''}`; +}; + +const findSingleReturnArgumentNode = ( + fnNode: FunctionExpression, +): TSESTree.Expression | null => { + if (fnNode.body.type !== AST_NODE_TYPES.BlockStatement) { + return fnNode.body; + } + + if (fnNode.body.body[0]?.type === AST_NODE_TYPES.ReturnStatement) { + return fnNode.body.body[0].argument; + } + + return null; +}; + +export default createRule({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Prefer mock resolved/rejected shorthands for promises', + recommended: false, + }, + messages: { + useMockShorthand: 'Prefer {{ replacement }}', + }, + schema: [], + type: 'suggestion', + fixable: 'code', + }, + defaultOptions: [], + create(context) { + const report = ( + property: AccessorNode, + isOnce: boolean, + outerArgNode: TSESTree.Node, + innerArgNode: TSESTree.Node | null = outerArgNode, + ) => { + if (innerArgNode?.type !== AST_NODE_TYPES.CallExpression) { + return; + } + + const argName = getNodeName(innerArgNode); + + if (argName !== 'Promise.resolve' && argName !== 'Promise.reject') { + return; + } + + const replacement = withOnce( + argName.endsWith('reject') ? 'mockRejectedValue' : 'mockResolvedValue', + isOnce, + ); + + context.report({ + node: property, + messageId: 'useMockShorthand', + data: { replacement }, + fix(fixer) { + const sourceCode = context.getSourceCode(); + + // there shouldn't be more than one argument, but if there is don't try + // fixing since we have no idea what to do with the extra arguments + if (innerArgNode.arguments.length > 1) { + return null; + } + + return [ + fixer.replaceText(property, replacement), + fixer.replaceText( + outerArgNode, + // the value argument for both Promise methods is optional, + // whereas for Jest they're required so use an explicit undefined + // if no argument is being passed to the call we're replacing + innerArgNode.arguments.length === 1 + ? sourceCode.getText(innerArgNode.arguments[0]) + : 'undefined', + ), + ]; + }, + }); + }; + + return { + CallExpression(node) { + if ( + node.callee.type !== AST_NODE_TYPES.MemberExpression || + !isSupportedAccessor(node.callee.property) || + node.arguments.length === 0 + ) { + return; + } + + const mockFnName = getAccessorValue(node.callee.property); + const isOnce = mockFnName.endsWith('Once'); + + if (mockFnName === withOnce('mockReturnValue', isOnce)) { + report(node.callee.property, isOnce, node.arguments[0]); + } else if (mockFnName === withOnce('mockImplementation', isOnce)) { + const [arg] = node.arguments; + + if (!isFunction(arg)) { + return; + } + + report( + node.callee.property, + isOnce, + arg, + findSingleReturnArgumentNode(arg), + ); + } + }, + }; + }, +});