Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
greg-hornby-roam opened this issue Sep 5, 2022 · 6 comments
Labels
Bug A bug in TypeScript
Milestone

Comments

@greg-hornby-roam
Copy link

Bug Report

🔎 Search Terms

Control Flow Analysis of Aliased Conditions and Discriminants

🕗 Version & Regression Information

4.4+

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about control flow analysis

⏯ 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
Copy link

whzx5byb commented Sep 5, 2022

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
Copy link
Contributor

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

@greg-hornby-roam
Copy link
Author

greg-hornby-roam commented Sep 5, 2022

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
Copy link

whzx5byb commented Sep 6, 2022

@greg-hornby-roam It seems similar to #33878

@andrewbranch andrewbranch added the Bug A bug in TypeScript label Sep 9, 2022
@andrewbranch andrewbranch added this to the Backlog milestone Sep 9, 2022
@andrewbranch
Copy link
Member

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
Copy link
Contributor

Zzzen commented Sep 11, 2022

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
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants