Rule proposal: no-misused-promises

[princjef]

While no-floating-promises can be used to catch cases where a promise stands by itself in a statement, it does not help for situations where the promise is passed to code that will not handle it. Example:

[1, 2, 3].forEach(num => Promise.reject(num));

This example will compile because Typescript explicitly allows returning any value to a function expecting a void return. While this is alright for most values, if the returned value is a Promise that rejects, it will almost certainly be unhandled.

This rule would also handle a situation that I encounter distressingly often with people learning async/await and promises:

new Promise(async () => {
    await someAsyncOperation();
});

There was previously a discussion about this kind of situation in palantir/tslint#3983, which seemed to garner support, but never made it into an actual rule. I have borrowed the rule name from that issue for now, but I actually think it would be better to name it something more specific as I think it makes sense to implement the parameter return type portion of that proposal as its own rule.

[bradzacher]

I would call the first example "invalid" because that code will clearly fail when actually used, because the types will not match up.

Though if they're using async/await very very wrong, then I can understand it:

async const myFunction(...args: string[]) {
  args.forEach(async arg => await doSomething(arg));
  console.log('Done!');
}

Another use I can understand is:

async function something() {}
if (something()) {
  console.log("I forgot to await");
}

This looks like it'd be very specific to promises, but I'm open to it.

One thing to note is that latter example requires type information, but the former doesn't.
Maybe there's some parts of this proposal that belong in something like eslint-plugin-promises and some parts that belong here?

[princjef]

Yeah sorry I was mostly trying to demonstrate the type aspect of it. The example you provided with an async function is more akin to what I generally see people do by mistake. They are definitely misuses of async/await, but I actually see a decent number of these issues for new JS/TS developers.

The boolean check of a promise is actually another example discussed in the linked tslint issue. It was proposed there that it could be addressed by adding an option to strict-boolean-expressions to support it. The use cases are a little different, but I could see going that route.

One thing to note is that latter example requires type information, but the former doesn't.

While there are cases that can be caught without type information, I think a rule for the function return case becomes much more powerful with it. From a type perspective, any function returning a thenable value which is provided as an argument for a function parameter with return type void should probably be flagged. Without type information, you have to pick specific problematic functions (e.g. Array.forEach) and can only reliably detect an issue if an async function is passed to it.

[princjef]

Coming back around to this, having a single rule to represent both of the cases described in the conversation above is feeling more reasonable to me. Either rule by itself would be very specific to a single use case and the "returning a promise in a parameter with void return type" one is really hard to succintly describe in a rule. The broader intent of avoiding promises being accidentally provided in places where they probably don't belong seems like a better umbrella goal.

With that, here's my thinking. We add a no-misused-promises rule with two initial checks (potentially more over time as needed):

1. No promise conditionals - flags for things like if (somePromise) { because you probably wanted it to be if (await somePromise) {. Notably, this check should use a variant of the thenable check which only matches if all alternates in a union type are thenable. Otherwise, a type like Promise<any> | undefined would get flagged, even though it would be a valid thing to check in a conditional.

2. No promise returns in void functions - flags for the array.forEach(async () => { await doSomething(); }); case and anywhere else a function with a promise return is passed as an argument where the parameter expects a void return type. This prevents people from accidentally thinking the promise will be awaited/handled for them by whatever invokes the function. This one can use the normal thenable check (one or more possible return types are thenable).

Since there will be cases where people want to have only a subset of these, the rule can be configured as such (default shown, tweak by setting the checks array):

{
  "@typescript-eslint/no-misused-promises": [
    "error",
    {
      "checks": ["conditional", "void-return"]
    }
  ]
}

If this sounds reasonable, I can work to put together a PR in the next few days.

[golopot]

The "No promise conditionals" part the previous comment was already proposed in #365.