Add no-useless-promise-resolve-reject rule

[cherryblossom000]

Fixes #1609

[fisker]

Check all comments.

[fisker]

Another edge case, if the return statement is in try/catch, we may breaking them

(async function() {
  try {
    return Promise.reject(1)
  } catch {
    return 2
  }
})() // Reject with 1

(async function() {
  try {
    throw 1
  } catch {
    return 2
  }
})() // Resolve with 2

[cherryblossom000]

In that case should we report an error or not? The use of Promise.reject isn't 'useless' here, but IMO it's better to refactor the code so it uses throw

[fisker]

Report but no fix? I'm not sure, either. We can just skip fix if these calls in try(with or without catch), it's fine to miss cases.

[fisker]

@cherryblossom000 I did a refactor, I this the fix logic is more clear, hope it's fine.

[cherryblossom000]

@fisker Looks good, thanks for the refactor.

[fisker]

One problem to fix

async () => (( Promise.reject(error) ));

[fisker]

Sorry. one more

async function * foo() {
	(( yield Promise.reject(error) ));
}

[fisker]

Looks good, thank you!

[sindresorhus]

@cherryblossom000 Thanks for contributing :)

[diegocr]

The use of Promise.reject isn't 'useless' here, but IMO it's better to refactor the code so it uses throw

What about cases where we may want to reject the async with a literal, but having no-throw-literal enabled doesn't allow us to use throw here? :)

[cherryblossom000]

@diegocr I think you should only reject a promise with an Error, for the same reasons as detailed in the no-throw-literal docs:

It is considered good practice to only throw the Error object itself or an object using the Error object as base objects for user-defined exceptions. The fundamental benefit of Error objects is that they automatically keep track of where they were built and originated.

A rejected promise should indicate an error. Rejecting a promise is the async equivalent of throwing an error.

If you’re doing something like return Promise.reject('oops an error!'), you can do throw new Error('oops an error!') instead.

[diegocr]

Hi @cherryblossom000, i actually meant a numeric literal, so that later we can do quick strict comparisons.

for example:

async foo(offset) {
  if (offset < 1) return Promise.reject(ERANGE);
  ....
}
...
foo().catch((err) => {
  if (err === ERANGE) bar();
  ....
});

Perhaps the rule could be improved through options to whitelist certain uses, meanwhile i've enabled it as a Warning only, but would like it as an Error to enforce on my team mates getting rid of Promise.resolve() there.

[fisker]

const ERANGE = new RangeError('Messge')?

[fisker]

class CustomError extends Error {
  ERANGE = true
}

throw new CustomError()


foo().catch((err) => {
  if (err?.ERANGE) bar();
  ....
})

[diegocr]

Nice idea @fisker, except ERANGE (amongst others) could also be returned from server-side indicating a failure to an specific API request, and tbh i am not a big fan of the overhead towards creating custom classes for something like this :)

[cherryblossom000]

If I were you I would disable the no-throw-literal rule.

[diegocr]

Yeah, that might be worth considering as well.

Alright, thanks guys for your time and help in any case :)