Using Array and Boolean in condition does not trigger a warning

[bodinsamuel]

TypeScript Version: 3.8.0-dev.20200115

Search Terms: overlap boolean vs array condition

Code

function isAllowed() {
  const myCondition = false;
  if (myCondition === false) {
    return [];
  }

  return true;
}

if (isAllowed()) {
  // boolean and array will results in a "true" condition
  console.log('allowed');
} else {
  // never reached
  console.log('not allowed');
}

Expected behavior:

• Seeing a message saying something like This condition can produce unexpected results since the types 'boolean' and 'array' have no overlap (not really good but you get the idea)

Actual behavior:
I won't say that it's a bug, but something that could be improved. I know that it's valid javascript but this a bug in the code that should/could be prevented by Typescript

• Typescript does not complain that comparing boolean and array can results in bad condition

Already fixable by

• It does complain if you write if (isAllowed() === true) but it's easy to not do it.

Playground Link: http://www.typescriptlang.org/play/?ts=3.8.0-dev.20200115&ssl=1&ssc=1&pln=15&pc=1#code/GYVwdgxgLglg9mABDAzgQQDYbgdwKYAmAFAJSIDeAUIohAilIgLYCeAwggTLAogLyIoAJxB4A3NWTBERVhzBceSPisEi8ZKjRpC8UEEKQBtALoSaAX0qTd+w4mABDDCnGUrlGNKKpM2fMQkmpJ0YChwGHgAdNgA5kQA5M7+hAkkEhaIeC54FCH0EdFxiWBwjMm4qenulEA

[bodinsamuel]

nb: it's not really related to boolean and array, but that's an easy mistake. Maybe it can be covered by a strict check that would require if condition to always be compared with boolean

[Jerska]

It took me a bit of time to wrap my head around this, but indeed, I would love if typescript could warn when a condition is both checking for:

• presence (non-null, non-undefined)
• value truthiness

Here's an example with number | null : https://www.typescriptlang.org/play/?ts=3.8.0-dev.20200115&ssl=1&ssc=1&pln=7&pc=2#code...

This has always been a thing to keep in mind with JavaScript, but having a strict flag forbidding using a condition which checks for both presence and value truthiness would be great.

The example in @bodinsamuel's first message goes one step further, because null and undefined are never even mentionned. The infered return type here is boolean | [].
The thing is boolean can be both truthy and falsy, whereas [] will always be truthy.
So overall, I feel like the warning that could be helpful would be if you're using, in a condition, a union type that contains a mix of:

• always falsy types (null, undefined)
• both truthy and falsy types (boolean, number, string)
• always truthy types

If this was considered, this would probably need to be a new strict flag.

[RyanCavanaugh]

So thinking these through (this is my gut check, please disagree):

• boolean | undefined - bad (conflates false and undefined)
• boolean | object - bad (conflates true and {})
• number | undefined - bad (conflates 0 and undefined)
• object | number - bad? (conflates 1 and {})
• boolean | number - bad? (conflates on both sides)
• object | undefined - good (falsiness is unambiguous)

The rule I can backport on to this intuition is "If there is more than one truthy value or more than one falsy value, those values should be of the same type".

What's your intuition?

[DanielRosenwasser]

Is boolean | string bad? If so, how can you model JavaScript code typed like boolean | "auto"?

[RyanCavanaugh]

What's a reasonable spelling of blah that makes this code make sense?

declare function blah(): boolean | string;
function fn() {
  if (blah()) {
    // Do one thing
  } else {
    // Do something else
  }
}

[bodinsamuel]

The rule I can backport on to this intuition is "If there is more than one truthy value or more than one falsy value, those values should be of the same type".

I like that, feels like a fair and understandable warning. At least for a beginning; it seems super useful.

object | undefined - good (falsiness is unambiguous)

This is where I would go even more strict, I would considered it bad as it results to always true on one side and always false on the other. But I just learn that {} === undefined is considered valid in Typescript. I suppose because an object is any ?

I would apply to the same rule with boolean | string or boolean | array, if there one side of the type is always truthy or falsy, it should trigger a warning (unless there is an actual comparison in the if)

[bodinsamuel]

Adding salt to my own issue, while searching for more use case I discovered there is an eslint rule that basically do that (or almost).
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md

It can interpreted in both way:

• eslint is maybe the tool for that
• typescript is maybe not strict enough

[RyanCavanaugh]

This is where I would go even more strict, I would considered it bad as it results to always true on one side and always false on the other.

I don't understand the point of that. Code like this is extremely idiomatic and poses no risk of confusion:

declare function getLoggedInUser(): User | undefined;

const user = getLoggedInUser();
if (user) {
  console.log(`Hello, ${user.name}`);
} else {
  console.log(`Please log in`);
}

[DanielRosenwasser]

@RyanCavanaugh I think boolean | "string-literal" is the interesting case.

declare function blah(): boolean | "auto";
function fn() {
  let x = blah();
  if (x) {
    return x === "auto" ? detectCondition() : true;
  } else {
    return false;
  }
}

Could it have checked for !x instead to avoid some extra nesting? Yes, but it's feasible code.

[mwgamble]

In my experience, anyone attempting to do something like this:

const myArray = functionThatReturnsVariableLengthArray();
if (myArray) {
    // do stuff
}

are always making a mistake. The following is their intention:

if (myArray.length > 0) {

I don't think it ever makes sense to evaluate an array (or any object) directly as a boolean.

[Jack-Works]

Mistake falsy:

number(0 / NaN)
string("")
bigint(0n)

If the "mistake falsy" type appears in the if the condition with boolean undefined or null, raise a type error.

Therefore my conclusion:

• boolean | undefined - good
• boolean | object - good
• number | undefined - bad (the number is mistake falsy type)
• number | string - good
• number | string | null - bad
• object | number - good
• boolean | number - bad
• object | undefined - good

if (x as string | undefined)
// Error: The speical value "" in type "string" can also be falsy in this context.
if (x as number)
// Ok
if (x as number | undefined)
// Error: The speical value 0 or NaN in type "number" can also be falsy in this context.
if (x as bigint | undefined)
// Error: The speical value 0n in type "bigint" can also be falsy in this context.

Mistake truthy type:
Any type excludes mistake falsy type and null, boolean and undefined

If mistake truthy type appears in the if condition, treat it as true.

if (x as any[])
// Error: This condition will always be true since type any[] has no overlap with false.
if (x as object)
// Error: This condition will always be true since type object has no overlap with false.
if (x as object | undefined)
// OK.