Tslint: Rule suggestion: require `await` of async functions

[cevek]

For example:

async function foo(){
}
async function bar(){
    foo(); // I need absent await warning 
    return 234;
}

I need just simple example how to start, find method declaration and check that is async function

[jkillian]

I'm not sure off the top of my head what exactly you'll need to do, but atom-typescript has a cool AST viewer which helps a lot when trying to write lint rules (see image below).

However, I'm not sure this is possible at the moment. TSLint lints files on a file by file basis currently, so it your function comes from another file, it has no way of knowing that it's an async function.

If you're only dealing with function declarations and calls from the same file, I imagine you could do something like this:

Override visitCallExpression and find all references to that function in the file, similarly to here. If you can find a reference that's a function declaration / function expression, see if it contains the async flag. If it does, and if the call expression isn't part of an async statement, generate a lint failure. This is a very rough sketch though, I'm sure there'd be more complications / there might be better language services features you can use.

[adidahiya]

blocked on #680

[aseemk]

We just hit a bug where we weren't properly awaiting an async void function. This lint rule would be great!

One addition to the rule: it'd be nice if the rule automatically saw if you were legitimately wanted to run something in parallel, not await it — but you should still be thening or catching errors.

async function foo() { ... }

async function bar() {
    // This should fail:
    foo();

    // But these should pass:
    await foo();

    foo().then(
        () => { ... },
        (err) => { ... }
    )

    foo()
        .catch((err) => { ... });

    // Maybe ideally this should fail too, since it could lead to silent errors:
    foo().then(() => { ... });
}

[adidahiya]

yep, this feature request is now unblocked, accepting PRs

[andy-hanson]

@JoshuaKGoldberg @cevek I still suspect this is resolved by #1632. See my comment: https://github.com/palantir/tslint/pull/1632#issuecomment-273357419

[JoshuaKGoldberg]

Agreed. I couldn't think of any situations in which #1632's didn't catch this behavior. Thanks for the bump!

[tomholub]

Here's one situation that's not covered. I've just hit a bug as follows:

let value = some_async_function(); // forgot to await!
if (value === null) {
  // will never happen
} else {
  // will always happen
}

This is not the first time, and not the last. no-floating-promises does not cover this, and TypeScript doesn't tell me that I'm comparing Promise<x> to null, which is probably not what I intended.

I propose a consume-promises rule, that will require each promise to either have a p.then(success, error), p.then(success).catch(error) or await p

[JoshuaKGoldberg]

@tomholub have you tried enabling strict-boolean-expressions? It should complain on the value === null comparison.

[cevek]

@tomholub I think more better to disable primitive operations, to boolean checks and check against null and undefined

It is cover all buggy cases

async foo() {
    var b = bar(); // Promise.resolve();
    if (b) some(); 
    b ? some() : some();
    x(!b);
    x(+b);
    x(~b);
    x(b + 'foo');
    if (b == null) x();
    if (b === null) x();
    if (b === undefined) x();

    Promise.all([b]); // ok
    someWithPromiseArg(b); // ok
}

[tomholub]

@JoshuaKGoldberg strict-boolean-expressions will complain about most of my codebase, except the lines above - unfortunately

[bennycode]

I wonder why the "no-floating-promises" rule isn't catching this case? 😢

Here is an example to play with:

function awaitMe(): Promise<boolean> {
  return new Promise(resolve => {
    setTimeout(() => {
      resolve(false);
    }, 2500);
  });
}

function isItTrue(): boolean {
  return awaitMe() ? true : false;
}

console.log('isItTrue', isItTrue()); // It's always true because there is no await on "awaitMe"

[JoshuaKGoldberg]

@bennyn This section is technically not a floating promise, since you handle it:

awaitMe() ? true : false

strict-boolean-expressions should complain, since you're comparing a Promise in the ternary instead of the expected boolean. See discussion in https://github.com/palantir/tslint/issues/4117.

This is a common concern brought up - a little more +1 to making strict-boolean-expressions more configurable so more folks can use it :)

[amirsaleem-kn]

no-floating-promises doesn't work with the situation like this

   function awaitMe() {
       return new Promise((resolve, reject) => {
             resolve();
       });
   }

  awaitMe(); // gets warning from no-floating-promise
  const value = awaitMe(); // doesn't get any warning

Any solution to handle this?