Rule proposal: No promise inside if condition #365

joshuaavalon · 2019-03-19T02:22:52Z

This rule proposal was original created at eslint/eslint#11525. However, due to the limited information that can get from static analysis in JavaScript, this proposal is now created on TypeScript instead.

Please describe what the rule should do:

The rule should warn about putting promise in a if condition because it always resolve to true.

In most of the cases, we want to use the result of the promise instead.

What category of rule is this? (place an "X" next to just one item)

[X] Warns about a potential error (problem)
[ ] Suggests an alternate way of doing something (suggestion)
[ ] Enforces code style (layout)
[ ] Other (please specify:)

Provide 2-3 code examples that this rule will warn about:

async function isValid() {
  return false;
}

if (isValid()) { // This is most likely an error because promise always resolves to true
  // Do work
}

Why should this rule be included in ESLint (instead of a plugin)?

This is a common mistake instead of code style. I think it should be included in ESLint to avoid this mistake.

ThomasdenH · 2019-03-20T08:08:39Z

I think this would be caught by strict-boolean-expressions from tslint, but I'm not sure whether their suggestion would be correct.

golopot · 2019-06-15T18:19:11Z

Likewise function type and object type are also always truthy inside if condition. Is it better to roll all of these into one rule? This rule will be similar to the core rule no-constant-condition.

joeytwiddle · 2019-09-10T10:32:19Z

This would be very helpful. (One of the few cases TypeScript doesn't save me!)

It would be good if it would also catch boolean operations, as well as if conditions.

// mistake
const results = isValid() && doWork();

// should be
const results = (await isValid()) && doWork();

// mistake
const shouldReject = !isValid();

// should be
const shouldReject = !(await isValid());

Also, in this comment princejef reminds us that we should not perform this test on "maybe" promises, of the type Promise<any> | undefined.

bradzacher · 2019-09-10T16:09:57Z

I believe that this should be handled now by the rule no-misused-promises added in #612.

Additionally, we have strict-boolean-expressions which should block it as well.

There is also no-unnecessary-condition which should help check this #699.

I'm going to close this as it should be covered now.

joshuaavalon added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Mar 19, 2019

joshuaavalon mentioned this issue Mar 19, 2019

New rule: No promise inside if condition eslint/eslint#11525

Closed

bradzacher added enhancement: new plugin rule New rule request for eslint-plugin and removed triage Waiting for team members to take a look labels Mar 20, 2019

golopot mentioned this issue Jun 15, 2019

Rule proposal: no-misused-promises #508

Closed

bradzacher closed this as completed Sep 10, 2019

github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rule proposal: No promise inside if condition #365

Rule proposal: No promise inside if condition #365

joshuaavalon commented Mar 19, 2019

ThomasdenH commented Mar 20, 2019

golopot commented Jun 15, 2019 •

edited

Loading

joeytwiddle commented Sep 10, 2019 •

edited

Loading

bradzacher commented Sep 10, 2019

Rule proposal: No promise inside if condition #365

Rule proposal: No promise inside if condition #365

Comments

joshuaavalon commented Mar 19, 2019

ThomasdenH commented Mar 20, 2019

golopot commented Jun 15, 2019 • edited Loading

joeytwiddle commented Sep 10, 2019 • edited Loading

bradzacher commented Sep 10, 2019

golopot commented Jun 15, 2019 •

edited

Loading

joeytwiddle commented Sep 10, 2019 •

edited

Loading