microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.12k stars 12.5k forks source link

Control Flow Analysis of Aliased Conditions and Discriminants breaks on else statement if condition not strictly boolean #50637

Open greg-hornby-roam opened 2 years ago

greg-hornby-roam commented 2 years ago

Bug Report

🔎 Search Terms

Control Flow Analysis of Aliased Conditions and Discriminants

🕗 Version & Regression Information

4.4+

⏯ Playground Link

Playground link with relevant code

💻 Code

class A {}
class B {}
class C {}

declare const test: A | B | C;

const isAOrB = (test instanceof A || test instanceof B);

declare function getSomeData(input: A | B): {}
const getData = (test instanceof A || test instanceof B) && getSomeData(test);

if (isAOrB) {
  test; //expect A | B - passes
} else {
  test; //expect C - passes
}

if (getData) {
  test; //expect A | B - passes
} else {
  test; //expect C - fails
}

🙁 Actual behavior

the else statement loses it's narrowing because getData is not strictly a boolean it seems.

🙂 Expected behavior

the if/else of getData should behave the same as the if/else of isAOrB

whzx5byb commented 2 years ago

Note that if getSomeData returns a falsy value(false, 0, ""...), the else branch will be executed, even the input is an instance of A or B.

MartinJohns commented 2 years ago

What whzx5byb said. I don't see a bug here either.

greg-hornby-roam commented 2 years ago

Okay my bad let me change the return type of getSomeData that it can't allow falsey values. It's still the same issue

class A {}
class B {}
class C {}

declare const test: A | B | C;

const isAOrB = (test instanceof A || test instanceof B);

declare function getSomeData(input: A | B): {prop1: string; prop2: number;}
const getData = (test instanceof A || test instanceof B) && getSomeData(test);

if (isAOrB) {
  test; //expect A | B - passes
} else {
  test; //expect C - passes
}

if (getData) {
  test; //expect A | B - passes
} else {
  test; //expect C - fails
}

https://www.typescriptlang.org/play?ts=4.4.4#code/MYGwhgzhAECC0G8C+AoUkYCFGvVaAwjiigCYCm6ATudMAPYB2EALtC+awFxzQA+0bAIIBuEg2ZsAlhFgB5KtgC80ABQdW0KZLCNg5egDNefARuk69B45gCUYspXA1ohgK56WUptADm5FgBlegBbcgARMBYwVW0ABzcWHngBOx4EOKp6OIBGHlYqbV8RaEzsgCYeRjcQgCNyKhFcJk1-Fkjo6BV1TgtWXX0jEzNerUtBm1toADJpvwDgsI6Y83sSKWNY2QU7RBRodl6SgHpj8gAPOMo2FMFoAFpSjE4UJGhyEAhaBH3D1hOzpdroQHk8oC9UCgNmo2sspj8DuYARcrsAbvw7o84s8IK93p9vr8kdBTijgURHoYwFJPq8gA

whzx5byb commented 2 years ago

@greg-hornby-roam It seems similar to https://github.com/microsoft/TypeScript/issues/33878

andrewbranch commented 2 years ago

Hm, there are two separate things happening. One is that non-literals after the && eliminates the narrowing in the false branch, independent of aliasing the condition:

const someData = true;
if ((test instanceof A || test instanceof B) && someData) {
  test; // A | B
} else {
  test; // A | B | C
}

And the second is that aliasing does behave differently when the truthy && is inlined:

if ((test instanceof A || test instanceof B) && true) {
  test; // A | B
} else {
  test; // C
}

const aliasedCondition = (test instanceof A || test instanceof B) && true;
if (aliasedCondition) {
  test; // A | B
} else {
  test; // A | B | C
}

Pretty weird.

Zzzen commented 2 years ago

Only true/false keyword works in ordinary conditions. link

if ((test instanceof A || test instanceof B) && 11) {
  test; //expect A | B - passes
} else {
  test; //expect C - fails
}