Skip to content

Commit

Permalink
feat(eslint-plugin): add new rule no-floating-promises (#495)
Browse files Browse the repository at this point in the history
  • Loading branch information
princjef authored and bradzacher committed Jun 10, 2019
1 parent 2c557f1 commit 61e6385
Show file tree
Hide file tree
Showing 6 changed files with 770 additions and 2 deletions.
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -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 | | | |
Expand Down
5 changes: 3 additions & 2 deletions packages/eslint-plugin/ROADMAP.md
@@ -1,4 +1,4 @@
# Roadmap
# Roadmap

✅ = done<br>
🌟 = in ESLint core<br>
Expand Down Expand Up @@ -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 |
Expand Down Expand Up @@ -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

<!-- eslint-plugin-import -->

Expand Down
46 changes: 46 additions & 0 deletions 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/)
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
171 changes: 171 additions & 0 deletions 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
);
}

0 comments on commit 61e6385

Please sign in to comment.