From 6a55921e2b9245102c4c373759dde4c36c3b4c08 Mon Sep 17 00:00:00 2001 From: Nikita Date: Thu, 12 Sep 2019 10:56:08 +0200 Subject: [PATCH] feat(eslint-plugin): [no-floating-promises] Add ignoreVoid option (#796) --- .../docs/rules/no-floating-promises.md | 30 ++++ .../src/rules/no-floating-promises.ts | 141 ++++++++++-------- .../tests/rules/no-floating-promises.test.ts | 8 + 3 files changed, 120 insertions(+), 59 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-floating-promises.md b/packages/eslint-plugin/docs/rules/no-floating-promises.md index 75bc49efa66..f52a510b546 100644 --- a/packages/eslint-plugin/docs/rules/no-floating-promises.md +++ b/packages/eslint-plugin/docs/rules/no-floating-promises.md @@ -36,6 +36,36 @@ returnsPromise().then(() => {}, () => {}); Promise.reject('value').catch(() => {}); ``` +## Options + +The rule accepts an options object with the following properties: + +```ts +type Options = { + // if true, checking void expresions will be skipped + ignoreVoid?: boolean; +}; + +const defaults = { + ignoreVoid: false, +}; +``` + +### ignoreVoid + +This allows to easily suppress false-positives with void operator. + +Examples of **correct** code for this rule with `{ ignoreVoid: true }`: + +```ts +async function returnsPromise() { + return 'value'; +} +void returnsPromise(); + +void Promise.reject('value'); +``` + ## When Not To Use It If you do not use Promise-like values in your codebase or want to allow them to diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index 816cc084670..32538271b94 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -3,7 +3,13 @@ import * as ts from 'typescript'; import * as util from '../util'; -export default util.createRule({ +type Options = [ + { + ignoreVoid?: boolean; + }, +]; + +export default util.createRule({ name: 'no-floating-promises', meta: { docs: { @@ -15,12 +21,24 @@ export default util.createRule({ messages: { floating: 'Promises must be handled appropriately', }, - schema: [], + schema: [ + { + type: 'object', + properties: { + ignoreVoid: { type: 'boolean' }, + }, + additionalProperties: false, + }, + ], type: 'problem', }, - defaultOptions: [], + defaultOptions: [ + { + ignoreVoid: false, + }, + ], - create(context) { + create(context, [options]) { const parserServices = util.getParserServices(context); const checker = parserServices.program.getTypeChecker(); @@ -38,64 +56,69 @@ export default util.createRule({ } }, }; + + function isUnhandledPromise( + checker: ts.TypeChecker, + node: ts.Node, + ): boolean { + // First, check expressions whose resulting types may not be promise-like + if ( + ts.isBinaryExpression(node) && + node.operatorToken.kind === ts.SyntaxKind.CommaToken + ) { + // Any child in a comma expression could return a potentially unhandled + // promise, so we check them all regardless of whether the final returned + // value is promise-like. + return ( + isUnhandledPromise(checker, node.left) || + isUnhandledPromise(checker, node.right) + ); + } + + if (ts.isVoidExpression(node) && !options.ignoreVoid) { + // Similarly, a `void` expression always returns undefined, so we need to + // see what's inside it without checking the type of the overall expression. + return isUnhandledPromise(checker, node.expression); + } + + // Check the type. At this point it can't be unhandled if it isn't a promise + if (!isPromiseLike(checker, node)) { + return false; + } + + if (ts.isCallExpression(node)) { + // If the outer expression is a call, it must be either a `.then()` or + // `.catch()` that handles the promise. + return ( + !isPromiseCatchCallWithHandler(node) && + !isPromiseThenCallWithRejectionHandler(node) + ); + } else if (ts.isConditionalExpression(node)) { + // We must be getting the promise-like value from one of the branches of the + // ternary. Check them directly. + return ( + isUnhandledPromise(checker, node.whenFalse) || + isUnhandledPromise(checker, node.whenTrue) + ); + } else if ( + ts.isPropertyAccessExpression(node) || + ts.isIdentifier(node) || + ts.isNewExpression(node) + ) { + // If it is just a property access chain or a `new` call (e.g. `foo.bar` or + // `new Promise()`), the promise is not handled because it doesn't have the + // necessary then/catch call at the end of the chain. + return true; + } + + // We conservatively return false for all other types of expressions because + // we don't want to accidentally fail if the promise is handled internally but + // we just can't tell. + return false; + } }, }); -function isUnhandledPromise(checker: ts.TypeChecker, node: ts.Node): boolean { - // First, check expressions whose resulting types may not be promise-like - if ( - ts.isBinaryExpression(node) && - node.operatorToken.kind === ts.SyntaxKind.CommaToken - ) { - // Any child in a comma expression could return a potentially unhandled - // promise, so we check them all regardless of whether the final returned - // value is promise-like. - return ( - isUnhandledPromise(checker, node.left) || - isUnhandledPromise(checker, node.right) - ); - } else if (ts.isVoidExpression(node)) { - // Similarly, a `void` expression always returns undefined, so we need to - // see what's inside it without checking the type of the overall expression. - return isUnhandledPromise(checker, node.expression); - } - - // Check the type. At this point it can't be unhandled if it isn't a promise - if (!isPromiseLike(checker, node)) { - return false; - } - - if (ts.isCallExpression(node)) { - // If the outer expression is a call, it must be either a `.then()` or - // `.catch()` that handles the promise. - return ( - !isPromiseCatchCallWithHandler(node) && - !isPromiseThenCallWithRejectionHandler(node) - ); - } else if (ts.isConditionalExpression(node)) { - // We must be getting the promise-like value from one of the branches of the - // ternary. Check them directly. - return ( - isUnhandledPromise(checker, node.whenFalse) || - isUnhandledPromise(checker, node.whenTrue) - ); - } else if ( - ts.isPropertyAccessExpression(node) || - ts.isIdentifier(node) || - ts.isNewExpression(node) - ) { - // If it is just a property access chain or a `new` call (e.g. `foo.bar` or - // `new Promise()`), the promise is not handled because it doesn't have the - // necessary then/catch call at the end of the chain. - return true; - } - - // We conservatively return false for all other types of expressions because - // we don't want to accidentally fail if the promise is handled internally but - // we just can't tell. - return false; -} - // Modified from tsutils.isThenable() to only consider thenables which can be // rejected/caught via a second parameter. Original source (MIT licensed): // diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index 63a360715d9..013edb41bc0 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -24,6 +24,14 @@ async function test() { return Promise.resolve("value"); } `, + { + options: [{ ignoreVoid: true }], + code: ` +async function test() { + void Promise.resolve("value"); +} +`, + }, ` async function test() { await Promise.reject(new Error("message"));