Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Rule suggestion: require await of async functions #892

Closed
cevek opened this issue Dec 22, 2015 · 13 comments
Closed

Rule suggestion: require await of async functions #892

cevek opened this issue Dec 22, 2015 · 13 comments

Comments

@cevek
Copy link

cevek commented Dec 22, 2015

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
Copy link
Contributor

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.

image

@adidahiya
Copy link
Contributor

blocked on #680

@aseemk
Copy link

aseemk commented Oct 19, 2016

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 adidahiya changed the title How can I write custom lint for require await of async functions? Rule suggestion: require await of async functions Oct 19, 2016
@adidahiya
Copy link
Contributor

yep, this feature request is now unblocked, accepting PRs

@andy-hanson
Copy link
Contributor

@JoshuaKGoldberg @cevek I still suspect this is resolved by #1632. See my comment: #1632 (comment)

@JoshuaKGoldberg
Copy link
Contributor

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

@tomholub
Copy link

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
Copy link
Contributor

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

@cevek
Copy link
Author

cevek commented Aug 14, 2018

@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
Copy link

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

@bennycode
Copy link
Contributor

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
Copy link
Contributor

JoshuaKGoldberg commented Oct 9, 2018

@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 #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
Copy link

amirsaleem-kn commented May 29, 2020

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?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants