eslint-community / eslint-plugin-promise

Enforce best practices for JavaScript promises
ISC License
942 stars 91 forks source link

Unable to report unhandled promises #203

Open jazzfog opened 4 years ago

jazzfog commented 4 years ago

Description

This, I guess, is more of a feature request than an issue: It would be great to be able to report all the un-handled promises. Meaning: for every async-function call require an await before or .then() after.

Use case:

Consider an async function validating a username and returning Promise<boolean>.

const isUsernameValid = async (name: string) => {
    return (await findUserByName(name)).length === 0;
};

Using this function it is easy to forget to use await before the it

// Error: this will always result in "Register user" because the promise
// object is getting implicitly converted to `true`
if (isUsernameValid(name)) {
    // Register user
} else {
    // Show error
}

Expected behavior: Report un-handled promises and require either await before

if (await isUsernameValid(name)) { ... }

or .then() - now the promise is not un-handled, this will indicate that the awaiting is not desired

function createUser() {
    await createDbRecord();

    // Not waiting for the async function to resolve, at the same time 
    // indication that we are aware that we are making an async call.
    logRegistration().then();
}

JetBrains IDEs have this check but it would be great to catch this in CI

prom-1 prom-2

svachalek commented 4 years ago

Came here looking to find the history on just this issue. This is by far the most common bug I have seen with promises, particularly the Promise<void> case.

stefanpl commented 3 years ago

@jazzfog I think the last case you outlined (just calling a promise-returning function and doing nothing with the promise) is well caught by @typescript-eslint/no-floating-promises.

The other case is what brought me here, indeed a very frequent source of trouble: a promise (directly after being created, or via being assigned to a variable first) is handed to a boolean check of some sorts. I cannot imagine a case where this would actually be desirable, since it will always be truthy.

Some variations, assuming the async isUsernameValid from above:


// Ternary expression … always true
return isUsernameValid(name) ? userByName(name) : null;

// Save to a variable first … same problem
const hasValidUser = isUsernameValid(name);
if (hasValidUser) { … }

// Return boolean value … ditto
return isUsernameValid(name) && isPasswordValid(password);

I'm not sure what JetBrains exactly means by where a value is expected. I've only had this problem so far with cases where a boolean is expected. Any hints to other cases highly appreciated.

@xjamundx @macklinu this has been open for quite some time. I'd be happy to draft a PR, if I can rely on some support from your side (would be my first eslint rule to implement …).

brettz9 commented 3 months ago

Just an FYI that some async functions may not be in the same file, and ESLint does not track those.