Skip to content
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

object possibly null in short circuit evaluation #10087

Closed
nippur72 opened this issue Aug 2, 2016 · 2 comments
Closed

object possibly null in short circuit evaluation #10087

nippur72 opened this issue Aug 2, 2016 · 2 comments
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@nippur72
Copy link

nippur72 commented Aug 2, 2016

An if with condition that contains an explicited short-circuit evaluation cause null check to fail:

   const m:string|null = "a";

   if(m===null || (m.length>50)) throw "";
   m.length;  // ok

   if(m===null || (m!==null && m.length>50)) throw "";
   m.length;  // fails with: Object is possibly 'null'
@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Aug 2, 2016
@ahejlsberg
Copy link
Member

This actually behaves as designed due to #8548. It's easiest to understand why by breaking it into equivalent separate operations:

if (m === null) throw "";
m;  // string
if (m !== null) {
    m;  // string
    if (m.length > 50) throw "";
    m;  // string
}
else {
    m;  // null (because of #8548)
}
m;  // string | null

In the else clause above, the compiler has computed the control flow type of m to be never. But, because of #8548, the if statement is treated as an assertion that m could be null, and the control flow type of m is therefore changed to null. And since the control flow type of a variable following an if statement is the union of the control flow types at the end of each of the branches, the type of m becomes string | null.

Now, while this explains what's happening, it doesn't necessarily make it right. Without the behavior in #8548, the type of m would be never in the else clause (since, according to control flow analysis, it could never execute). And since string | never is the same as string, we'd give type string to m after the if statement as expected. That certainly would make more sense here.

It may be that #8548 causes more harm than good. We've already had some feedback to that effect, so I'll think about alternatives. One issue is that #8458 also solves the problem we had with this code:

function foo() {
    let x: string | number | boolean = 0;
    x;  // number
    while (cond) {
        x;  // Initially number, then string | number
        x = typeof x === "string" ? x.slice() : "abc";
        x;  // string
    }
    x;
}

During control flow analysis, the initial type of x is number, which cases x to be given type never in the true branch of the conditional operator, which then causes x.slice() to be an error. If we were to back out #8548, we'd need a different solution to that issue.

@ahejlsberg
Copy link
Member

Fixed in #10118.

@ahejlsberg ahejlsberg added the Fixed A PR has been merged for this issue label Aug 3, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

3 participants