[Rule Request] no-misused-promises

[oleg-codaio]

This is related to #3804 and of course no-floating-promises. There are a number of pitfalls when working with promises and async code.

First off:

async function printUrl(url: string) {
  console.log(await (await fetch(url)).json());
}

const Urls = ['palantir.com', 'google.com'];

async const myFunction() {
  Urls.forEach(async url => await printUrl(url));
  console.log('Done!');
}

What happens here is that Array#forEach doesn't support promises natively, and will simply fire them off and run onto the Done line before the URLs are fetched. printUrl() has a return type of Promise<void>, but non-void functions in TS are assignable to void function parameters. Generally that rule is fine, but it'd be nice to handle promises specially to avoid this issue. One place where this can be a problem is in unit tests, where using forEach with async handlers can end up not actually running the test code.

Here's another scenario:

async function checkIsUserAllowed(id: number) {
  const user = await postgres.fetchUser(id)
  return Boolean(user && user.hasAccess);
}

async const myFunction() {
  const user = 42;
  if (checkIsUserAllowed(user)) { // should be `await checkIsUserAllowed(user)`!
    console.log('Access granted');
  }
}

It would be great to be able to catch this scenario (see #3804), though one thing to be cautious about is to still allow handling working with types like Promise<any> | undefined when explicitly needed to check whether a value is there or not. This could be allowed using Boolean(promiseValue), for instance.

Thoughts on whether handling these issues can be baked into an existing rule or be worthwhile to stand on its own as part of core rules?

[JoshuaKGoldberg]

@vaskevich Very much agreed with your use cases!

However, same as #3804 - what about strict-boolean-expressions doesn't do it for you? It's intended to catch these types of things (e.g. you passed a Promise instead of a boolean to an if). Can we improve that rule so you can use it for this?

Edit: re-reading your comment there, would your "allow-all-unions-except": <symbols> be enough?

[oleg-codaio]

I think the issue with using strict-boolean-expressions everywhere (at least for us) is that it would condition folks to write if (Boolean(foo)) {} everywhere and then that would encapsulate a promise one day unintentionally. Something like "allow-all-unions-except": <symbols> would work for the second case (same as in #3804) but for the first one I think we'd need a different rule.

[JoshuaKGoldberg]

if (Boolean(foo)) {

It sounds like this request is logically equivalent to the strict-boolean-expressions having a config setting to allow or disallow certain types, so that you could start off by only checking Promises in boolean expressions. Something like:

{
    "strict-boolean-expressions": [true, {
        "object-types": "^Promise$"
    }]
}

Really what I'm getting at is that there already exist rules that catch all the mentioned issues, so the problem feels more like a lack of flexibility on strict-boolean-expression's part. If the rule could be configured to only check for this case, would that do it for you?

Edit: Sorry, re-read your previous response - yes, the first case of an Array.forEach sounds like it should have its own rule.

[eltonio450]

Hello,

After a tour of all the issues related to Promises in if statement, I am not sure of what is implemented, what is not, what is planned, and what we should use.

@JoshuaKGoldberg does this config work ? should I use a specific branch ?

Which direction on this topic has been chosen ? It seems that "strict-boolean-expression" might be the best choice, but not clear

Glad to help if needed :)

Best

[JoshuaKGoldberg]

@eltonio450 great question. I recommend enabling strict-boolean-expressions, strict-type-predicates, and no-floating-promises. Together, these will...

• ...stop you from starting Promises without handling them in some way
• ...stop you from directly using complex objects in if statements (e.g. if (createPromise()) {)
• ...stop you from comparing Promises to non-Promises (e.g. if (createPromise() === true) {)

There's no way to make strict-boolean-expressions only work for Promises, which means it can be a lot of work to turn it on for large existing projects.

[eltonio450]

Ok thanks. Correct me if I am wrong but with this config, it becomes impossible to check if an object is defined by using:

if(object) {
    console.log(object.hello)
}
else {
    console.warn("object is undefined !)
}

[JoshuaKGoldberg]

Note: per #4534, this issue will be closed in less than a month if no PR is sent to add the rule. If you really need the rule, custom rules are always an option and can be maintained outside this repo!

[guidsdo]

@JoshuaKGoldberg sure :-)