feat(eslint-plugin): add new rule no-misused-promises

packages/eslint-plugin/README.md

@@ -153,6 +153,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e
 | [`@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                                                                                                                             |                    |          |                   |
 | [`@typescript-eslint/no-misused-new`](./docs/rules/no-misused-new.md)                                     | Enforce valid definition of `new` and `constructor`                                                                                                 | :heavy_check_mark: |          |                   |
+| [`@typescript-eslint/no-misused-promises`](./docs/rules/no-misused-promises.md)                           | Avoid using promises in places not designed to handle them                                                                                          |                    |          | :thought_balloon: |
 | [`@typescript-eslint/no-namespace`](./docs/rules/no-namespace.md)                                         | Disallow the use of custom TypeScript modules and namespaces                                                                                        | :heavy_check_mark: |          |                   |
 | [`@typescript-eslint/no-non-null-assertion`](./docs/rules/no-non-null-assertion.md)                       | Disallows non-null assertions using the `!` postfix operator                                                                                        | :heavy_check_mark: |          |                   |
 | [`@typescript-eslint/no-object-literal-type-assertion`](./docs/rules/no-object-literal-type-assertion.md) | Forbids an object literal to appear in a type assertion expression                                                                                  | :heavy_check_mark: |          |                   |

packages/eslint-plugin/docs/rules/no-misused-promises.md

@@ -0,0 +1,119 @@
+# Avoid using promises in places not designed to handle them (no-misused-promises)
+
+This rule forbids using promises in places where the Typescript compiler
+allows them but they are not handled properly. These situations can often arise
+due to a missing `await` keyword or just a misunderstanding of the way async
+functions are handled/awaited.
+
+## Rule Details
+
+Examples of **incorrect** code for this rule with `checksConditionals: true`:
+
+```ts
+const promise = Promise.resolve('value');
+
+if (promise) {
+  // Do something
+}
+
+const val = promise ? 123 : 456;
+
+while (promise) {
+  // Do something
+}
+```
+
+Examples of **incorrect** code for this rule with `checksVoidReturn: true`:
+
+```ts
+[1, 2, 3].forEach(async value => {
+  await doSomething(value);
+});
+
+new Promise(async (resolve, reject) => {
+  await doSomething();
+  resolve();
+});
+
+const eventEmitter = new EventEmitter();
+eventEmitter.on('some-event', async () => {
+  await doSomething();
+});
+```
+
+Examples of **correct** code for this rule:
+
+```ts
+const promise = Promise.resolve('value');
+
+if (await promise) {
+  // Do something
+}
+
+const val = (await promise) ? 123 : 456;
+
+while (await promise) {
+  // Do something
+}
+
+for (const value of [1, 2, 3]) {
+  await doSomething(value);
+}
+
+new Promise((resolve, reject) => {
+  // Do something
+  resolve();
+});
+
+const eventEmitter = new EventEmitter();
+eventEmitter.on('some-event', () => {
+  doSomething();
+});
+```
+
+## Options
+
+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
+
+If you don't want functions that return promises where a void return is
+expected to be checked, your configuration will look like this:
+
+```json
+{
+  "@typescript-eslint/no-misused-promises": [
+    "error",
+    {
+      "checksVoidReturn": false
+    }
+  ]
+}
+```
+
+Likewise, if you don't want to check conditionals, you can configure the rule
+like this:
+
+```json
+{
+  "@typescript-eslint/no-misused-promises": [
+    "error",
+    {
+      "checksConditionals": false
+    }
+  ]
+}
+```
+
+## When Not To Use It
+
+If you do not use Promises in your codebase or are not concerned with possible
+misuses of them outside of what the Typescript compiler will check.
+
+## Related to
+
+- [`no-floating-promises`]('./no-floating-promises.md')
+
+## Further Reading
+
+- [Typescript void function assignability](https://github.com/Microsoft/TypeScript/wiki/FAQ#why-are-functions-returning-non-void-assignable-to-function-returning-void)

packages/eslint-plugin/src/rules/index.ts

@@ -27,6 +27,7 @@ import noForInArray from './no-for-in-array';
 import noInferrableTypes from './no-inferrable-types';
 import noMagicNumbers from './no-magic-numbers';
 import noMisusedNew from './no-misused-new';
+import noMisusedPromises from './no-misused-promises';
 import noNamespace from './no-namespace';
 import noNonNullAssertion from './no-non-null-assertion';
 import noObjectLiteralTypeAssertion from './no-object-literal-type-assertion';
@@ -89,6 +90,7 @@ export default {
   'no-inferrable-types': noInferrableTypes,
   'no-magic-numbers': noMagicNumbers,
   'no-misused-new': noMisusedNew,
+  'no-misused-promises': noMisusedPromises,
   'no-namespace': noNamespace,
   'no-non-null-assertion': noNonNullAssertion,
   'no-object-literal-type-assertion': noObjectLiteralTypeAssertion,

packages/eslint-plugin/src/rules/no-misused-promises.ts

@@ -0,0 +1,253 @@
+import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
+import * as tsutils from 'tsutils';
+import ts from 'typescript';
+
+import * as util from '../util';
+
+type Options = [
+  {
+    checksConditionals?: boolean;
+    checksVoidReturn?: boolean;
+  }
+];
+
+export default util.createRule<Options, 'conditional' | 'voidReturn'>({
+  name: 'no-misused-promises',
+  meta: {
+    docs: {
+      description: 'Avoid using promises in places not designed to handle them',
+      category: 'Best Practices',
+      recommended: false,
+    },
+    messages: {
+      voidReturn:
+        'Promise returned in function argument where a void return was expected.',
+      conditional: 'Expected non-Promise value in a boolean conditional.',
+    },
+    schema: [
+      {
+        type: 'object',
+        properties: {
+          checksConditionals: {
+            type: 'boolean',
+          },
+          checksVoidReturn: {
+            type: 'boolean',
+          },
+        },
+      },
+    ],
+    type: 'problem',
+  },
+  defaultOptions: [
+    {
+      checksConditionals: true,
+      checksVoidReturn: true,
+    },
+  ],
+
+  create(context, [{ checksConditionals, checksVoidReturn }]) {
+    const parserServices = util.getParserServices(context);
+    const checker = parserServices.program.getTypeChecker();
+
+    const conditionalChecks: TSESLint.RuleListener = {
+      ConditionalExpression: checkTestConditional,
+      DoWhileStatement: checkTestConditional,
+      ForStatement: checkTestConditional,
+      IfStatement: checkTestConditional,
+      LogicalExpression(node) {
+        // We only check the lhs of a logical expression because the rhs might
+        // be the return value of a short circuit expression.
+        checkConditional(node.left);
+      },
+      UnaryExpression(node) {
+        if (node.operator === '!') {
+          checkConditional(node.argument);
+        }
+      },
+      WhileStatement: checkTestConditional,
+    };
+
+    const voidReturnChecks: TSESLint.RuleListener = {
+      CallExpression: checkArguments,
+      NewExpression: checkArguments,
+    };
+
+    function checkTestConditional(node: { test: TSESTree.Expression | null }) {
+      if (node.test) {
+        checkConditional(node.test);
+      }
+    }
+
+    function checkConditional(node: TSESTree.Expression) {
+      const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node);
+      if (isAlwaysThenable(checker, tsNode)) {
+        context.report({
+          messageId: 'conditional',
+          node,
+        });
+      }
+    }
+
+    function checkArguments(
+      node: TSESTree.CallExpression | TSESTree.NewExpression,
+    ) {
+      const tsNode = parserServices.esTreeNodeToTSNodeMap.get<
+        ts.CallExpression | ts.NewExpression
+      >(node);
+      const voidParams = voidFunctionParams(checker, tsNode);
+      if (voidParams.size === 0) {
+        return;
+      }
+
+      for (const [index, argument] of node.arguments.entries()) {
+        if (!voidParams.has(index)) {
+          continue;
+        }
+
+        const tsNode = parserServices.esTreeNodeToTSNodeMap.get(argument);
+        if (returnsThenable(checker, tsNode as ts.Expression)) {
+          context.report({
+            messageId: 'voidReturn',
+            node: argument,
+          });
+        }
+      }
+    }
+
+    return {
+      ...(checksConditionals ? conditionalChecks : {}),
+      ...(checksVoidReturn ? voidReturnChecks : {}),
+    };
+  },
+});
+
+// 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
+// branches is thenable.
+function isAlwaysThenable(checker: ts.TypeChecker, node: ts.Node) {
+  const type = checker.getTypeAtLocation(node);
+
+  for (const subType of tsutils.unionTypeParts(checker.getApparentType(type))) {
+    const thenProp = subType.getProperty('then');
+
+    // If one of the alternates has no then property, it is not thenable in all
+    // cases.
+    if (thenProp === undefined) {
+      return false;
+    }
+
+    // We walk through each variation of the then property. Since we know it
+    // exists at this point, we just need at least one of the alternates to
+    // be of the right form to consider it thenable.
+    const thenType = checker.getTypeOfSymbolAtLocation(thenProp, node);
+    let hasThenableSignature = false;
+    for (const subType of tsutils.unionTypeParts(thenType)) {
+      for (const signature of subType.getCallSignatures()) {
+        if (
+          signature.parameters.length !== 0 &&
+          isFunctionParam(checker, signature.parameters[0], node)
+        ) {
+          hasThenableSignature = true;
+          break;
+        }
+      }
+
+      // We only need to find one variant of the then property that has a
+      // function signature for it to be thenable.
+      if (hasThenableSignature) {
+        break;
+      }
+    }
+
+    // If no flavors of the then property are thenable, we don't consider the
+    // overall type to be thenable
+    if (!hasThenableSignature) {
+      return false;
+    }
+  }
+
+  // If all variants are considered thenable (i.e. haven't returned false), we
+  // consider the overall type thenable
+  return true;
+}
+
+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 subType of tsutils.unionTypeParts(type)) {
+    if (subType.getCallSignatures().length !== 0) {
+      return true;
+    }
+  }
+  return false;
+}
+
+// Get the positions of parameters which are void functions (and not also
+// thenable functions). These are the candidates for the void-return check at
+// the current call site.
+function voidFunctionParams(
+  checker: ts.TypeChecker,
+  node: ts.CallExpression | ts.NewExpression,
+) {
+  const voidReturnIndices = new Set<number>();
+  const thenableReturnIndices = new Set<number>();
+  const type = checker.getTypeAtLocation(node.expression);
+
+  for (const subType of tsutils.unionTypeParts(type)) {
+    // Standard function calls and `new` have two different types of signatures
+    const signatures = ts.isCallExpression(node)
+      ? subType.getCallSignatures()
+      : subType.getConstructSignatures();
+    for (const signature of signatures) {
+      for (const [index, parameter] of signature.parameters.entries()) {
+        const type = checker.getTypeOfSymbolAtLocation(
+          parameter,
+          node.expression,
+        );
+        for (const subType of tsutils.unionTypeParts(type)) {
+          for (const signature of subType.getCallSignatures()) {
+            const returnType = signature.getReturnType();
+            if (tsutils.isTypeFlagSet(returnType, ts.TypeFlags.Void)) {
+              voidReturnIndices.add(index);
+            } else if (
+              tsutils.isThenableType(checker, node.expression, returnType)
+            ) {
+              thenableReturnIndices.add(index);
+            }
+          }
+        }
+      }
+    }
+  }
+
+  // If a certain positional argument accepts both thenable and void returns,
+  // a promise-returning function is valid
+  for (const thenable of thenableReturnIndices) {
+    voidReturnIndices.delete(thenable);
+  }
+
+  return voidReturnIndices;
+}
+
+// Returns true if the expression is a function that returns a thenable
+function returnsThenable(checker: ts.TypeChecker, node: ts.Expression) {
+  const type = checker.getApparentType(checker.getTypeAtLocation(node));
+
+  for (const subType of tsutils.unionTypeParts(type)) {
+    for (const signature of subType.getCallSignatures()) {
+      const returnType = signature.getReturnType();
+      if (tsutils.isThenableType(checker, node, returnType)) {
+        return true;
+      }
+    }
+  }
+
+  return false;
+}