diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 87e8cfd72ee..ff6a11b3726 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -146,6 +146,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md) | Disallow usage of the `any` type | :heavy_check_mark: | | | | [`@typescript-eslint/no-extra-parens`](./docs/rules/no-extra-parens.md) | Disallow unnecessary parentheses | | :wrench: | | | [`@typescript-eslint/no-extraneous-class`](./docs/rules/no-extraneous-class.md) | Forbids the use of classes as namespaces | | | | +| [`@typescript-eslint/no-floating-promises`](./docs/rules/no-floating-promises.md) | Requires Promise-like values to be handled appropriately. | | | :thought_balloon: | | [`@typescript-eslint/no-for-in-array`](./docs/rules/no-for-in-array.md) | Disallow iterating over an array with a for-in loop | | | :thought_balloon: | | [`@typescript-eslint/no-inferrable-types`](./docs/rules/no-inferrable-types.md) | Disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/no-magic-numbers`](./docs/rules/no-magic-numbers.md) | Disallows magic numbers | | | | diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index b2567afe070..f0171f73b09 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -1,4 +1,4 @@ -# Roadmap +# Roadmap ✅ = done
🌟 = in ESLint core
@@ -60,7 +60,7 @@ | [`no-dynamic-delete`] | 🛑 | N/A | | [`no-empty`] | 🌟 | [`no-empty`][no-empty] | | [`no-eval`] | 🌟 | [`no-eval`][no-eval] | -| [`no-floating-promises`] | 🛑 | N/A ([relevant plugin][plugin:promise]) | +| [`no-floating-promises`] | ✅ | [`@typescript-eslint/no-floating-promises`] | | [`no-for-in-array`] | ✅ | [`@typescript-eslint/no-for-in-array`] | | [`no-implicit-dependencies`] | 🔌 | [`import/no-extraneous-dependencies`] | | [`no-inferred-empty-object-type`] | 🛑 | N/A | @@ -612,6 +612,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- [`@typescript-eslint/no-for-in-array`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-for-in-array.md [`@typescript-eslint/no-unnecessary-qualifier`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unnecessary-qualifier.md [`@typescript-eslint/semi`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/semi.md +[`@typescript-eslint/no-floating-promises`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-floating-promises.md diff --git a/packages/eslint-plugin/docs/rules/no-floating-promises.md b/packages/eslint-plugin/docs/rules/no-floating-promises.md new file mode 100644 index 00000000000..75bc49efa66 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-floating-promises.md @@ -0,0 +1,46 @@ +# Requires Promise-like values to be handled appropriately (no-floating-promises) + +This rule forbids usage of Promise-like values in statements without handling +their errors appropriately. Unhandled promises can cause several issues, such +as improperly sequenced operations, ignored Promise rejections and more. Valid +ways of handling a Promise-valued statement include `await`ing, returning, and +either calling `.then()` with two arguments or `.catch()` with one argument. + +## Rule Details + +Examples of **incorrect** code for this rule: + +```ts +const promise = new Promise((resolve, reject) => resolve('value')); +promise; + +async function returnsPromise() { + return 'value'; +} +returnsPromise().then(() => {}); + +Promise.reject('value').catch(); +``` + +Examples of **correct** code for this rule: + +```ts +const promise = new Promise((resolve, reject) => resolve('value')); +await promise; + +async function returnsPromise() { + return 'value'; +} +returnsPromise().then(() => {}, () => {}); + +Promise.reject('value').catch(() => {}); +``` + +## When Not To Use It + +If you do not use Promise-like values in your codebase or want to allow them to +remain unhandled. + +## Related to + +- Tslint: ['no-floating-promises'](https://palantir.github.io/tslint/rules/no-floating-promises/) diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index d62f58d6af3..c92f6a36d8b 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -20,6 +20,7 @@ import noEmptyInterface from './no-empty-interface'; import noExplicitAny from './no-explicit-any'; import noExtraParens from './no-extra-parens'; import noExtraneousClass from './no-extraneous-class'; +import noFloatingPromises from './no-floating-promises'; import noForInArray from './no-for-in-array'; import noInferrableTypes from './no-inferrable-types'; import noMagicNumbers from './no-magic-numbers'; @@ -76,6 +77,7 @@ export default { 'no-explicit-any': noExplicitAny, 'no-extra-parens': noExtraParens, 'no-extraneous-class': noExtraneousClass, + 'no-floating-promises': noFloatingPromises, 'no-for-in-array': noForInArray, 'no-inferrable-types': noInferrableTypes, 'no-magic-numbers': noMagicNumbers, diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts new file mode 100644 index 00000000000..1c8cf412c07 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -0,0 +1,171 @@ +import * as tsutils from 'tsutils'; +import * as ts from 'typescript'; + +import * as util from '../util'; + +export default util.createRule({ + name: 'no-floating-promises', + meta: { + docs: { + description: 'Requires Promise-like values to be handled appropriately.', + category: 'Best Practices', + recommended: false, + }, + messages: { + floating: 'Promises must be handled appropriately', + }, + schema: [], + type: 'problem', + }, + defaultOptions: [], + + create(context) { + const parserServices = util.getParserServices(context); + const checker = parserServices.program.getTypeChecker(); + + return { + ExpressionStatement(node) { + const { expression } = parserServices.esTreeNodeToTSNodeMap.get( + node, + ) as ts.ExpressionStatement; + + if (isUnhandledPromise(checker, expression)) { + context.report({ + messageId: 'floating', + node, + }); + } + }, + }; + }, +}); + +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): +// +// https://github.com/ajafff/tsutils/blob/49d0d31050b44b81e918eae4fbaf1dfe7b7286af/util/type.ts#L95-L125 +function isPromiseLike(checker: ts.TypeChecker, node: ts.Node): boolean { + const type = checker.getTypeAtLocation(node); + for (const ty of tsutils.unionTypeParts(checker.getApparentType(type))) { + const then = ty.getProperty('then'); + if (then === undefined) { + continue; + } + + const thenType = checker.getTypeOfSymbolAtLocation(then, node); + if ( + hasMatchingSignature( + thenType, + signature => + signature.parameters.length >= 2 && + isFunctionParam(checker, signature.parameters[0], node) && + isFunctionParam(checker, signature.parameters[1], node), + ) + ) { + return true; + } + } + return false; +} + +function hasMatchingSignature( + type: ts.Type, + matcher: (signature: ts.Signature) => boolean, +): boolean { + for (const t of tsutils.unionTypeParts(type)) { + if (t.getCallSignatures().some(matcher)) { + return true; + } + } + + return false; +} + +function isFunctionParam( + checker: ts.TypeChecker, + param: ts.Symbol, + node: ts.Node, +): boolean { + const type: ts.Type | undefined = checker.getApparentType( + checker.getTypeOfSymbolAtLocation(param, node), + ); + for (const t of tsutils.unionTypeParts(type)) { + if (t.getCallSignatures().length !== 0) { + return true; + } + } + return false; +} + +function isPromiseCatchCallWithHandler(expression: ts.CallExpression): boolean { + return ( + tsutils.isPropertyAccessExpression(expression.expression) && + expression.expression.name.text === 'catch' && + expression.arguments.length >= 1 + ); +} + +function isPromiseThenCallWithRejectionHandler( + expression: ts.CallExpression, +): boolean { + return ( + tsutils.isPropertyAccessExpression(expression.expression) && + expression.expression.name.text === 'then' && + expression.arguments.length >= 2 + ); +} diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts new file mode 100644 index 00000000000..63a360715d9 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -0,0 +1,547 @@ +import rule from '../../src/rules/no-floating-promises'; +import { RuleTester, getFixturesRootDir } from '../RuleTester'; + +const rootDir = getFixturesRootDir(); +const messageId = 'floating'; + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2018, + tsconfigRootDir: rootDir, + project: './tsconfig.json', + }, + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('no-floating-promises', rule, { + valid: [ + ` +async function test() { + await Promise.resolve("value"); + Promise.resolve("value").then(() => {}, () => {}); + Promise.resolve("value").then(() => {}).catch(() => {}); + Promise.resolve("value").catch(() => {}); + return Promise.resolve("value"); +} +`, + ` +async function test() { + await Promise.reject(new Error("message")); + Promise.reject(new Error("message")).then(() => {}, () => {}); + Promise.reject(new Error("message")).then(() => {}).catch(() => {}); + Promise.reject(new Error("message")).catch(() => {}); + return Promise.reject(new Error("message")); +} +`, + ` +async function test() { + await (async () => true)(); + (async () => true)().then(() => {}, () => {}); + (async () => true)().then(() => {}).catch(() => {}); + (async () => true)().catch(() => {}); + return (async () => true)(); +} +`, + ` +async function test() { + async function returnsPromise() {} + await returnsPromise(); + returnsPromise().then(() => {}, () => {}); + returnsPromise().then(() => {}).catch(() => {}); + returnsPromise().catch(() => {}); + return returnsPromise(); +} +`, + ` +async function test() { + const x = Promise.resolve(); + const y = x.then(() => {}); + y.catch(() => {}); +} +`, + ` +async function test() { + Math.random() > 0.5 ? Promise.resolve().catch(() => {}) : null; +} +`, + ` +async function test() { + Promise.resolve().catch(() => {}), 123; + 123, Promise.resolve().then(() => {}, () => {}); + 123, Promise.resolve().then(() => {}, () => {}), 123; +} +`, + ` +async function test() { + void Promise.resolve().catch(() => {}); +} +`, + ` +async function test() { + Promise.resolve().catch(() => {}) || Promise.resolve().then(() => {}, () => {}); +} +`, + ` +async function test() { + const promiseValue: Promise; + + await promiseValue; + promiseValue.then(() => {}, () => {}); + promiseValue.then(() => {}).catch(() => {}); + promiseValue.catch(() => {}); + return promiseValue; +} +`, + ` +async function test() { + const promiseUnion: Promise | number; + + await promiseUnion; + promiseUnion.then(() => {}, () => {}); + promiseUnion.then(() => {}).catch(() => {}); + promiseUnion.catch(() => {}); + return promiseUnion; +} +`, + ` +async function test() { + const promiseIntersection: Promise & number; + + await promiseIntersection; + promiseIntersection.then(() => {}, () => {}); + promiseIntersection.then(() => {}).catch(() => {}); + promiseIntersection.catch(() => {}); + return promiseIntersection; +} +`, + ` +async function test() { + class CanThen extends Promise {} + const canThen: CanThen = Foo.resolve(2); + + await canThen; + canThen.then(() => {}, () => {}); + canThen.then(() => {}).catch(() => {}); + canThen.catch(() => {}); + return canThen; +} +`, + ` +async function test() { + await (Math.random() > 0.5 ? numberPromise : 0); + await (Math.random() > 0.5 ? foo : 0); + await (Math.random() > 0.5 ? bar : 0); + + const intersectionPromise: Promise & number; + await intersectionPromise; +} +`, + ` +async function test() { + class Thenable { + then(callback: () => {}): Thenable { return new Thenable(); } + }; + const thenable = new Thenable(); + + await thenable; + thenable; + thenable.then(() => {}); + return thenable; +} +`, + ` +async function test() { + class NonFunctionParamThenable { + then(param: string, param2: number): NonFunctionParamThenable { + return new NonFunctionParamThenable(); + } + }; + const thenable = new NonFunctionParamThenable(); + + await thenable; + thenable; + thenable.then('abc', 'def'); + return thenable; +} +`, + ` +async function test() { + class NonFunctionThenable { then: number }; + const thenable = new NonFunctionThenable(); + + thenable; + thenable.then; + return thenable; +} +`, + ` +async function test() { + class CatchableThenable { + then(callback: () => {}, callback: () => {}): CatchableThenable { + return new CatchableThenable(); + } + }; + const thenable = new CatchableThenable(); + + await thenable + return thenable; +} +`, + ` +// https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/promise-polyfill/index.d.ts +// Type definitions for promise-polyfill 6.0 +// Project: https://github.com/taylorhakes/promise-polyfill +// Definitions by: Steve Jenkins +// Daniel Cassidy +// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped + +interface PromisePolyfillConstructor extends PromiseConstructor { + _immediateFn?: (handler: (() => void) | string) => void; +} + +declare const PromisePolyfill: PromisePolyfillConstructor; + +async function test() { + const promise = new PromisePolyfill(() => {}); + + await promise; + promise.then(() => {}, () => {}); + promise.then(() => {}).catch(() => {}); + promise.catch(() => {}); + return promise; +} +`, + ], + + invalid: [ + { + code: ` +async function test() { + Promise.resolve("value"); + Promise.resolve("value").then(() => {}); + Promise.resolve("value").catch(); +} +`, + errors: [ + { + line: 3, + messageId, + }, + { + line: 4, + messageId, + }, + { + line: 5, + messageId, + }, + ], + }, + { + code: ` +async function test() { + Promise.reject(new Error("message")); + Promise.reject(new Error("message")).then(() => {}); + Promise.reject(new Error("message")).catch(); +} +`, + errors: [ + { + line: 3, + messageId, + }, + { + line: 4, + messageId, + }, + { + line: 5, + messageId, + }, + ], + }, + { + code: ` +async function test() { + (async () => true)(); + (async () => true)().then(() => {}); + (async () => true)().catch(); +} +`, + errors: [ + { + line: 3, + messageId, + }, + { + line: 4, + messageId, + }, + { + line: 5, + messageId, + }, + ], + }, + { + code: ` +async function test() { + async function returnsPromise() {} + + returnsPromise(); + returnsPromise().then(() => {}); + returnsPromise().catch(); +} +`, + errors: [ + { + line: 5, + messageId, + }, + { + line: 6, + messageId, + }, + { + line: 7, + messageId, + }, + ], + }, + { + code: ` +async function test() { + Math.random() > 0.5 ? Promise.resolve() : null; + Math.random() > 0.5 ? null : Promise.resolve(); +} +`, + errors: [ + { + line: 3, + messageId, + }, + { + line: 4, + messageId, + }, + ], + }, + { + code: ` +async function test() { + Promise.resolve(), 123 + 123, Promise.resolve() + 123, Promise.resolve(), 123 +} +`, + errors: [ + { + line: 3, + messageId, + }, + { + line: 4, + messageId, + }, + { + line: 5, + messageId, + }, + ], + }, + { + code: ` +async function test() { + void Promise.resolve(); +} +`, + errors: [ + { + line: 3, + messageId, + }, + ], + }, + { + code: ` +async function test() { + const obj = { foo: Promise.resolve() }; + obj.foo; +} +`, + errors: [ + { + line: 4, + messageId, + }, + ], + }, + { + code: ` +async function test() { + new Promise(resolve => resolve()); +} +`, + errors: [ + { + line: 3, + messageId, + }, + ], + }, + { + code: ` +async function test() { + const promiseValue: Promise; + + promiseValue; + promiseValue.then(() => {}); + promiseValue.catch(); +} +`, + errors: [ + { + line: 5, + messageId, + }, + { + line: 6, + messageId, + }, + { + line: 7, + messageId, + }, + ], + }, + { + code: ` +async function test() { + const promiseUnion: Promise | number; + + promiseUnion; +} +`, + errors: [ + { + line: 5, + messageId, + }, + ], + }, + { + code: ` +async function test() { + const promiseIntersection: Promise & number; + + promiseIntersection; + promiseIntersection.then(() => {}) + promiseIntersection.catch(); +} +`, + errors: [ + { + line: 5, + messageId, + }, + { + line: 6, + messageId, + }, + { + line: 7, + messageId, + }, + ], + }, + { + code: ` +async function test() { + class CanThen extends Promise {} + const canThen: CanThen = Foo.resolve(2); + + canThen; + canThen.then(() => {}); + canThen.catch(); +} +`, + errors: [ + { + line: 6, + messageId, + }, + { + line: 7, + messageId, + }, + { + line: 8, + messageId, + }, + ], + }, + { + code: ` +async function test() { + class CatchableThenable { + then(callback: () => {}, callback: () => {}): CatchableThenable { + return new CatchableThenable(); + } + }; + const thenable = new CatchableThenable(); + + thenable; + thenable.then(() => {}); +} +`, + errors: [ + { + line: 10, + messageId, + }, + { + line: 11, + messageId, + }, + ], + }, + { + code: ` +// https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/promise-polyfill/index.d.ts +// Type definitions for promise-polyfill 6.0 +// Project: https://github.com/taylorhakes/promise-polyfill +// Definitions by: Steve Jenkins +// Daniel Cassidy +// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped + +interface PromisePolyfillConstructor extends PromiseConstructor { + _immediateFn?: (handler: (() => void) | string) => void; +} + +declare const PromisePolyfill: PromisePolyfillConstructor; + +async function test() { + const promise = new PromisePolyfill(() => {}); + + promise; + promise.then(() => {}); + promise.catch(); +} +`, + errors: [ + { + line: 18, + messageId, + }, + { + line: 19, + messageId, + }, + { + line: 20, + messageId, + }, + ], + }, + ], +});