Narrow on else branch of multi-conditional guard

[weswigham]

Fixes #1270.

Removing the short-circuiting logic "shortcut" (which actually did an extra narrow when it doesn't strictly need to) fixes the issue.

[sandersn]

Worth noting: this doesn't fix #4874, but it changes the behaviour:

var bool: boolean;
var num: number;
var y: boolean | number;
if (typeof y === 'number' && typeof y !== 'number') {
    // was previously
    num = y;
    // is now:
    bool = y;
    // but y should not be assignable to anything, I think
}

[sandersn]

Can you update the comment in tests/cases/conformance/expressions/typeGuards/typeGuardOfFormExpr1OrExpr2.ts in the same way that you did with the one in checker.ts?

[sandersn]

From reading #1270, it sounds like the problem is that narrowType doesn't correctly subtract members of a union when they are primitive types, not that there is an extra narrow. The example I gave above shows that the behaviour still isn't right when all members should have been removed from the union. It's just different now.

Wouldn't the correct fix be to the code that removes members from the union?

[weswigham]

The subtractPrimitiveTypes function mentioned in the original issue no longer exists, so I was looking at other changes which solved the issue - though I now believe that removeTypesFromUnionType takes its use now.

The narrowing that needs to be done for the correct behavior is this:
(number|string >> string) | (number|string >> number >> {})
what happens is this:
(number|string >> string) | (number|string >> number >> number)
and what I've changed it to in this PR thus far is this:
(number|string >> string) | (number|string >> string)
rather than correcting the calls to removeTypesFromUnionType to allow for empty unions. Simply allowing for empty union results means that our behavior in this case:

// Narrowing occurs only if target type is a subtype of variable type
if (typeof strOrNum === "boolean") {
  //...
}

changes - specifically, in this scenario we decide that an empty union means the type should not be narrowed, but if we allow empty set returns from the type quality narrowing expression, then we get an empty set within this block rather than the original type.

Now, in my mind, narrowing to an empty set of types seems fine - so if that change in behavior is desirable, it certainly seems like the preferable fix.

[sandersn]

Let's hold this change until we have a chance to vet it in a design discussion. Narrowing to the empty set makes sense to me in most cases, but there may be good reasons not to.

Specifically, I think the case in #4029 should be empty set:

var x: string | number;
if(typeof x === 'string') {
}
else if(typeof x === 'number') {
}
else {
  // some kind of fallback... x should be {} not string | number
}

[weswigham]

Superseded by #5442.