diff --git a/packages/eslint-plugin/docs/rules/no-misused-promises.md b/packages/eslint-plugin/docs/rules/no-misused-promises.md index b3d5dcbe0cb..203aefc178e 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-promises.md +++ b/packages/eslint-plugin/docs/rules/no-misused-promises.md @@ -13,9 +13,9 @@ See [`no-floating-promises`](./no-floating-promises.md) for detecting unhandled ## Rule Details -This rule accepts a single option which is an object with `checksConditionals` -and `checksVoidReturn` properties indicating which types of misuse to flag. -Both are enabled by default. +This rule accepts a single option which is an object with `checksConditionals`, +`checksVoidReturn`, and `checksSpreads` properties indicating which types of +misuse to flag. All are enabled by default. ## Options @@ -24,6 +24,7 @@ type Options = [ { checksConditionals?: boolean; checksVoidReturn?: boolean | ChecksVoidReturnOptions; + checksSpreads?: boolean; }, ]; @@ -39,6 +40,7 @@ const defaultOptions: Options = [ { checksConditionals: true, checksVoidReturn: true, + checksSpreads: true, }, ]; ``` @@ -101,6 +103,21 @@ For example, if you don't mind that passing a `() => Promise` to a `() => } ``` +### `"checksSpreads"` + +If you don't want to check object spreads, you can add this configuration: + +```json +{ + "@typescript-eslint/no-misused-promises": [ + "error", + { + "checksSpreads": false + } + ] +} +``` + ### `checksConditionals: true` Examples of code for this rule with `checksConditionals: true`: @@ -212,6 +229,42 @@ eventEmitter.on('some-event', () => { +### `checksSpreads: true` + +Examples of code for this rule with `checksSpreads: true`: + + + +#### ❌ Incorrect + +```ts +const getData = () => someAsyncOperation({ myArg: 'foo' }); + +return { foo: 42, ...getData() }; + +const getData2 = async () => { + await someAsyncOperation({ myArg: 'foo' }); +}; + +return { foo: 42, ...getData2() }; +``` + +#### ✅ Correct + +```ts +const getData = () => someAsyncOperation({ myArg: 'foo' }); + +return { foo: 42, ...(await getData()) }; + +const getData2 = async () => { + await someAsyncOperation({ myArg: 'foo' }); +}; + +return { foo: 42, ...(await getData2()) }; +``` + + + ## When Not To Use It If you do not use Promises in your codebase or are not concerned with possible diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 03b166ec49b..cf291820642 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -8,6 +8,7 @@ type Options = [ { checksConditionals?: boolean; checksVoidReturn?: boolean | ChecksVoidReturnOptions; + checksSpreads?: boolean; }, ]; @@ -25,7 +26,8 @@ type MessageId = | 'voidReturnVariable' | 'voidReturnProperty' | 'voidReturnReturnValue' - | 'voidReturnAttribute'; + | 'voidReturnAttribute' + | 'spread'; function parseChecksVoidReturn( checksVoidReturn: boolean | ChecksVoidReturnOptions | undefined, @@ -75,6 +77,7 @@ export default util.createRule({ voidReturnAttribute: 'Promise-returning function provided to attribute where a void return was expected.', conditional: 'Expected non-Promise value in a boolean conditional.', + spread: 'Expected a non-Promise value to be spreaded in an object.', }, schema: [ { @@ -99,6 +102,9 @@ export default util.createRule({ }, ], }, + checksSpreads: { + type: 'boolean', + }, }, }, ], @@ -108,10 +114,11 @@ export default util.createRule({ { checksConditionals: true, checksVoidReturn: true, + checksSpreads: true, }, ], - create(context, [{ checksConditionals, checksVoidReturn }]) { + create(context, [{ checksConditionals, checksVoidReturn, checksSpreads }]) { const parserServices = util.getParserServices(context); const checker = parserServices.program.getTypeChecker(); @@ -153,6 +160,10 @@ export default util.createRule({ } : {}; + const spreadChecks: TSESLint.RuleListener = { + SpreadElement: checkSpread, + }; + function checkTestConditional(node: { test: TSESTree.Expression | null; }): void { @@ -376,13 +387,37 @@ export default util.createRule({ } } + function checkSpread(node: TSESTree.SpreadElement): void { + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); + + if (isSometimesThenable(checker, tsNode.expression)) { + context.report({ + messageId: 'spread', + node: node.argument, + }); + } + } + return { ...(checksConditionals ? conditionalChecks : {}), ...(checksVoidReturn ? voidReturnChecks : {}), + ...(checksSpreads ? spreadChecks : {}), }; }, }); +function isSometimesThenable(checker: ts.TypeChecker, node: ts.Node): boolean { + const type = checker.getTypeAtLocation(node); + + for (const subType of tsutils.unionTypeParts(checker.getApparentType(type))) { + if (tsutils.isThenableType(checker, node, subType)) { + return true; + } + } + + return false; +} + // Variation on the thenable check which requires all forms of the type (read: // alternates in a union) to be thenable. Otherwise, you might be trying to // check if something is defined or undefined and get caught because one of the diff --git a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts index 518b7c1d6b5..24c9d4285d6 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -316,6 +316,63 @@ const _ = {}} />; `, filename: 'react.tsx', }, + ` +console.log({ ...(await Promise.resolve({ key: 42 })) }); + `, + ` +const getData = Promise.resolve({ key: 42 }); + +console.log({ + someData: 42, + ...(await getData()), +}); + `, + ` +declare const condition: boolean; + +console.log({ ...(condition && (await Promise.resolve({ key: 42 }))) }); +console.log({ ...(condition || (await Promise.resolve({ key: 42 }))) }); +console.log({ ...(condition ? {} : await Promise.resolve({ key: 42 })) }); +console.log({ ...(condition ? await Promise.resolve({ key: 42 }) : {}) }); + `, + ` +console.log([...(await Promise.resolve(42))]); + `, + { + code: ` +console.log({ ...Promise.resolve({ key: 42 }) }); + `, + options: [{ checksSpreads: false }], + }, + { + code: ` +const getData = Promise.resolve({ key: 42 }); + +console.log({ + someData: 42, + ...getData(), +}); + `, + options: [{ checksSpreads: false }], + }, + { + code: ` +declare const condition: boolean; + +console.log({ ...(condition && Promise.resolve({ key: 42 })) }); +console.log({ ...(condition || Promise.resolve({ key: 42 })) }); +console.log({ ...(condition ? {} : Promise.resolve({ key: 42 })) }); +console.log({ ...(condition ? Promise.resolve({ key: 42 }) : {}) }); + `, + options: [{ checksSpreads: false }], + }, + { + code: ` +// This is invalid Typescript, but it shouldn't trigger this linter specifically +console.log([...Promise.resolve(42)]); + `, + options: [{ checksSpreads: false }], + }, ], invalid: [ @@ -870,5 +927,48 @@ it('', async () => {}); }, ], }, + { + code: ` +console.log({ ...Promise.resolve({ key: 42 }) }); + `, + errors: [ + { + line: 2, + messageId: 'spread', + }, + ], + }, + { + code: ` +const getData = () => Promise.resolve({ key: 42 }); + +console.log({ + someData: 42, + ...getData(), +}); + `, + errors: [ + { + line: 6, + messageId: 'spread', + }, + ], + }, + { + code: ` +declare const condition: boolean; + +console.log({ ...(condition && Promise.resolve({ key: 42 })) }); +console.log({ ...(condition || Promise.resolve({ key: 42 })) }); +console.log({ ...(condition ? {} : Promise.resolve({ key: 42 })) }); +console.log({ ...(condition ? Promise.resolve({ key: 42 }) : {}) }); + `, + errors: [ + { line: 4, messageId: 'spread' }, + { line: 5, messageId: 'spread' }, + { line: 6, messageId: 'spread' }, + { line: 7, messageId: 'spread' }, + ], + }, ], });