Feature proposal: no-double-resolve. Warn against resolving a promise twice, or execution after resolve.

[mrthehud]

Description

I've noticed some errors when reviewing PRs where a promise is resolved, but where execution flow can continue to a point where the promise is then rejected, or where continuing execution would cause some other error that should be avoided.

I've read through the rules available, but I don't think any of them would achieve the following:

Steps to Reproduce

For example, the following less than ideal code:

(async () => {
    const someOptionalCache = undefined;
    const value = await new Promise((resolve) => {
        if (!someOptionalCache) {
            resolve(null);
        }
        
        resolve(someOptionalCache.fetch("some-key");
    });
})()

should really cause at least a warning imo.

Expected behavior:
Open to discussion, but perhaps a warning should be issued to the effect of 'a promise should not be resolved multiple times'

Naturally, it's entirely possible to want to continue executing code in a promise after it's resolved, but I don't think resolving twice would make sense, nor rejecting after a resolve, nor resolving after rejecting.

Does anybody else have thoughts (for or against) on this?

An additional or alternative rule that feels more restrictive, and might want to be ignored in some instances but not others (though would still provide protection, and may be easier to implement) would be to disallow or warn against execution of any code after resolve or reject is called.

Again, any thoughts for or against?

Examples of better code

The above could instead be either:

(async () => {
    const someOptionalCache = undefined;
    const value = await new Promise((resolve) => {
        if (!someOptionalCache) {
            resolve(null);

            return;
        }
        
        resolve(someOptionalCache.fetch("some-key");
    });
})()

or alternatively:

(async () => {
    const someOptionalCache = undefined;
    const value = await new Promise((resolve) => {
        if (!someOptionalCache) {
            resolve(null);
        } else {
            resolve(someOptionalCache.fetch("some-key");
        }
    });
})()

[xjamundx]

That's very similar to this rule:
https://eslint.org/docs/rules/callback-return

Which I support

[mrthehud]

Thanks @xjamundx, I'll check that out, it looks promising!

[mrthehud]

I ran a test with some existing code using https://eslint.org/docs/rules/callback-return which did highlight a location where I was calling a callback without a return, but in that case this was correct behaviour.

As for my promise example, I've created an example https://github.com/mrthehud/eslint-plugin-promise-issue-222/blob/master/src/foo.ts, but this doesn't warn against my double calling of resolve unfortunately.

I think there may be scope here, I guess I'd want to start by adding some test cases to https://github.com/mysticatea/eslint-plugin-node/blob/master/tests/lib/rules/callback-return.js to reprodue.

[github-actions]

🎉 This issue has been resolved in version 6.1.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀