Improve type guard consistiency

src/compiler/checker.ts

@@ -6177,27 +6177,6 @@ namespace ts {
             Debug.fail("should not get here");
         }
 
-        // For a union type, remove all constituent types that are of the given type kind (when isOfTypeKind is true)
-        // or not of the given type kind (when isOfTypeKind is false)
-        function removeTypesFromUnionType(type: Type, typeKind: TypeFlags, isOfTypeKind: boolean, allowEmptyUnionResult: boolean): Type {
-            if (type.flags & TypeFlags.Union) {
-                let types = (<UnionType>type).types;
-                if (forEach(types, t => !!(t.flags & typeKind) === isOfTypeKind)) {
-                    // Above we checked if we have anything to remove, now use the opposite test to do the removal
-                    let narrowedType = getUnionType(filter(types, t => !(t.flags & typeKind) === isOfTypeKind));
-                    if (allowEmptyUnionResult || narrowedType !== emptyObjectType) {
-                        return narrowedType;
-                    }
-                }
-            }
-            else if (allowEmptyUnionResult && !!(type.flags & typeKind) === isOfTypeKind) {
-                // Use getUnionType(emptyArray) instead of emptyObjectType in case the way empty union types
-                // are represented ever changes.
-                return getUnionType(emptyArray);
-            }
-            return type;
-        }
-
         function hasInitializer(node: VariableLikeDeclaration): boolean {
             return !!(node.initializer || isBindingPattern(node.parent) && hasInitializer(<VariableLikeDeclaration>node.parent.parent));
         }
@@ -6299,53 +6278,71 @@ namespace ts {
             // Only narrow when symbol is variable of type any or an object, union, or type parameter type
             if (node && symbol.flags & SymbolFlags.Variable) {
                 if (isTypeAny(type) || type.flags & (TypeFlags.ObjectType | TypeFlags.Union | TypeFlags.TypeParameter)) {
+                    let originalType = type;
+                    let nodeStack: {node: Node, child: Node}[] = [];
                     loop: while (node.parent) {
                         let child = node;
                         node = node.parent;
-                        let narrowedType = type;
+                        switch (node.kind) {
+                            case SyntaxKind.IfStatement:
+                            case SyntaxKind.ConditionalExpression:
+                            case SyntaxKind.BinaryExpression:
+                                nodeStack.push({node, child});
+                                break;
+                            case SyntaxKind.SourceFile:
+                            case SyntaxKind.ModuleDeclaration:
+                            case SyntaxKind.FunctionDeclaration:
+                            case SyntaxKind.MethodDeclaration:
+                            case SyntaxKind.MethodSignature:
+                            case SyntaxKind.GetAccessor:
+                            case SyntaxKind.SetAccessor:
+                            case SyntaxKind.Constructor:
+                                // Stop at the first containing function or module declaration
+                                break loop;
+                        }
+                    }
+
+                    let nodes: {node: Node, child: Node};
+                    while (nodes = nodeStack.pop()) {
+                        let {node, child} = nodes;
                         switch (node.kind) {
                             case SyntaxKind.IfStatement:
                                 // In a branch of an if statement, narrow based on controlling expression
                                 if (child !== (<IfStatement>node).expression) {
-                                    narrowedType = narrowType(type, (<IfStatement>node).expression, /*assumeTrue*/ child === (<IfStatement>node).thenStatement);
+                                    type = narrowType(type, (<IfStatement>node).expression, /*assumeTrue*/ child === (<IfStatement>node).thenStatement);
                                 }
                                 break;
                             case SyntaxKind.ConditionalExpression:
                                 // In a branch of a conditional expression, narrow based on controlling condition
                                 if (child !== (<ConditionalExpression>node).condition) {
-                                    narrowedType = narrowType(type, (<ConditionalExpression>node).condition, /*assumeTrue*/ child === (<ConditionalExpression>node).whenTrue);
+                                    type = narrowType(type, (<ConditionalExpression>node).condition, /*assumeTrue*/ child === (<ConditionalExpression>node).whenTrue);
                                 }
                                 break;
                             case SyntaxKind.BinaryExpression:
                                 // In the right operand of an && or ||, narrow based on left operand
                                 if (child === (<BinaryExpression>node).right) {
                                     if ((<BinaryExpression>node).operatorToken.kind === SyntaxKind.AmpersandAmpersandToken) {
-                                        narrowedType = narrowType(type, (<BinaryExpression>node).left, /*assumeTrue*/ true);
+                                        type = narrowType(type, (<BinaryExpression>node).left, /*assumeTrue*/ true);
                                     }
                                     else if ((<BinaryExpression>node).operatorToken.kind === SyntaxKind.BarBarToken) {
-                                        narrowedType = narrowType(type, (<BinaryExpression>node).left, /*assumeTrue*/ false);
+                                        type = narrowType(type, (<BinaryExpression>node).left, /*assumeTrue*/ false);
                                     }
                                 }
                                 break;
-                            case SyntaxKind.SourceFile:
-                            case SyntaxKind.ModuleDeclaration:
-                            case SyntaxKind.FunctionDeclaration:
-                            case SyntaxKind.MethodDeclaration:
-                            case SyntaxKind.MethodSignature:
-                            case SyntaxKind.GetAccessor:
-                            case SyntaxKind.SetAccessor:
-                            case SyntaxKind.Constructor:
-                                // Stop at the first containing function or module declaration
-                                break loop;
+                            default:
+                                Debug.fail("Unreachable!");
                         }
-                        // Use narrowed type if construct contains no assignments to variable
-                        if (narrowedType !== type) {
-                            if (isVariableAssignedWithin(symbol, node)) {
-                                break;
-                            }
-                            type = narrowedType;
+
+                        // Use original type if construct contains assignments to variable
+                        if (type != originalType && isVariableAssignedWithin(symbol, node)) {
+                            type = originalType;
                         }
                     }
+
+                    // Preserve old top-level behavior - if the branch is really an empty set, revert to prior type
+                    if (type === getUnionType(emptyArray)) {
+                        type = originalType;
+                    }
                 }
             }
 
@@ -6361,31 +6358,31 @@ namespace ts {
                 if (left.expression.kind !== SyntaxKind.Identifier || getResolvedSymbol(<Identifier>left.expression) !== symbol) {
                     return type;
                 }
-                let typeInfo = primitiveTypeInfo[right.text];
                 if (expr.operatorToken.kind === SyntaxKind.ExclamationEqualsEqualsToken) {
                     assumeTrue = !assumeTrue;
                 }
-                if (assumeTrue) {
-                    // Assumed result is true. If check was not for a primitive type, remove all primitive types
-                    if (!typeInfo) {
-                        return removeTypesFromUnionType(type, /*typeKind*/ TypeFlags.StringLike | TypeFlags.NumberLike | TypeFlags.Boolean | TypeFlags.ESSymbol,
-                            /*isOfTypeKind*/ true, /*allowEmptyUnionResult*/ false);
-                    }
-                    // Check was for a primitive type, return that primitive type if it is a subtype
-                    if (isTypeSubtypeOf(typeInfo.type, type)) {
-                        return typeInfo.type;
-                    }
-                    // Otherwise, remove all types that aren't of the primitive type kind. This can happen when the type is
-                    // union of enum types and other types.
-                    return removeTypesFromUnionType(type, /*typeKind*/ typeInfo.flags, /*isOfTypeKind*/ false, /*allowEmptyUnionResult*/ false);
+                let typeInfo = primitiveTypeInfo[right.text];
+                // If the type to be narrowed is any and we're checking a primitive with assumeTrue=true, return the primitive
+                if (!!(type.flags & TypeFlags.Any) && typeInfo && assumeTrue) {
+                    return typeInfo.type;
+                }
+                let flags: TypeFlags;
+                if (typeInfo) {
+                    flags =  typeInfo.flags;
                 }
                 else {
-                    // Assumed result is false. If check was for a primitive type, remove that primitive type
-                    if (typeInfo) {
-                        return removeTypesFromUnionType(type, /*typeKind*/ typeInfo.flags, /*isOfTypeKind*/ true, /*allowEmptyUnionResult*/ false);
-                    }
-                    // Otherwise we don't have enough information to do anything.
-                    return type;
+                    assumeTrue = !assumeTrue;
+                    flags = TypeFlags.NumberLike | TypeFlags.StringLike | TypeFlags.ESSymbol | TypeFlags.Boolean;
+                }
+                // At this point we can bail if it's not a union
+                if (!(type.flags & TypeFlags.Union)) {
+                    // If the active non-union type would be removed from a union by this type guard, return an empty union
+                    return filterUnion(type) ? type : getUnionType(emptyArray);
+                }
+                return getUnionType(filter((type as UnionType).types, filterUnion), /*noSubtypeReduction*/ true);
+
+                function filterUnion(t: Type) {
+                    return assumeTrue === !!(t.flags & flags);
                 }
             }
 
@@ -6399,7 +6396,7 @@ namespace ts {
                     // and the second operand was false. We narrow with those assumptions and union the two resulting types.
                     return getUnionType([
                         narrowType(type, expr.left, /*assumeTrue*/ false),
-                        narrowType(narrowType(type, expr.left, /*assumeTrue*/ true), expr.right, /*assumeTrue*/ false)
+                        narrowType(type, expr.right, /*assumeTrue*/ false)
                     ]);
                 }
             }
@@ -6410,7 +6407,7 @@ namespace ts {
                     // and the second operand was true. We narrow with those assumptions and union the two resulting types.
                     return getUnionType([
                         narrowType(type, expr.left, /*assumeTrue*/ true),
-                        narrowType(narrowType(type, expr.left, /*assumeTrue*/ false), expr.right, /*assumeTrue*/ true)
+                        narrowType(type, expr.right, /*assumeTrue*/ true)
                     ]);
                 }
                 else {
@@ -12556,7 +12553,13 @@ namespace ts {
 
             // After we remove all types that are StringLike, we will know if there was a string constituent
             // based on whether the remaining type is the same as the initial type.
-            let arrayType = removeTypesFromUnionType(arrayOrStringType, TypeFlags.StringLike, /*isTypeOfKind*/ true, /*allowEmptyUnionResult*/ true);
+            let arrayType = arrayOrStringType;
+            if (arrayOrStringType.flags & TypeFlags.Union) {
+                arrayType = getUnionType(filter((arrayOrStringType as UnionType).types, t => !(t.flags & TypeFlags.StringLike)));
+            }
+            else if (arrayOrStringType.flags & TypeFlags.StringLike) {
+                arrayType = getUnionType(emptyArray);
+            }
             let hasStringConstituent = arrayOrStringType !== arrayType;
 
             let reportedError = false;

tests/baselines/reference/symbolType18.types

@@ -21,5 +21,5 @@ if (typeof x === "object") {
 }
 else {
     x;
->x : symbol | Foo
+>x : symbol
 }

tests/baselines/reference/typeGuardEnums.js

@@ -0,0 +1,41 @@
+//// [typeGuardEnums.ts]
+enum E {}
+enum V {}
+
+let x: number|string|E|V;
+
+if (typeof x === "number") {
+    x; // number|E|V
+}
+else {
+    x; // string
+}
+
+if (typeof x !== "number") {
+    x; // string
+}
+else {
+    x; // number|E|V
+}
+
+
+//// [typeGuardEnums.js]
+var E;
+(function (E) {
+})(E || (E = {}));
+var V;
+(function (V) {
+})(V || (V = {}));
+var x;
+if (typeof x === "number") {
+    x; // number|E|V
+}
+else {
+    x; // string
+}
+if (typeof x !== "number") {
+    x; // string
+}
+else {
+    x; // number|E|V
+}

tests/baselines/reference/typeGuardEnums.symbols

@@ -0,0 +1,34 @@
+=== tests/cases/conformance/expressions/typeGuards/typeGuardEnums.ts ===
+enum E {}
+>E : Symbol(E, Decl(typeGuardEnums.ts, 0, 0))
+
+enum V {}
+>V : Symbol(V, Decl(typeGuardEnums.ts, 0, 9))
+
+let x: number|string|E|V;
+>x : Symbol(x, Decl(typeGuardEnums.ts, 3, 3))
+>E : Symbol(E, Decl(typeGuardEnums.ts, 0, 0))
+>V : Symbol(V, Decl(typeGuardEnums.ts, 0, 9))
+
+if (typeof x === "number") {
+>x : Symbol(x, Decl(typeGuardEnums.ts, 3, 3))
+
+    x; // number|E|V
+>x : Symbol(x, Decl(typeGuardEnums.ts, 3, 3))
+}
+else {
+    x; // string
+>x : Symbol(x, Decl(typeGuardEnums.ts, 3, 3))
+}
+
+if (typeof x !== "number") {
+>x : Symbol(x, Decl(typeGuardEnums.ts, 3, 3))
+
+    x; // string
+>x : Symbol(x, Decl(typeGuardEnums.ts, 3, 3))
+}
+else {
+    x; // number|E|V
+>x : Symbol(x, Decl(typeGuardEnums.ts, 3, 3))
+}
+

tests/baselines/reference/typeGuardEnums.types

@@ -0,0 +1,40 @@
+=== tests/cases/conformance/expressions/typeGuards/typeGuardEnums.ts ===
+enum E {}
+>E : E
+
+enum V {}
+>V : V
+
+let x: number|string|E|V;
+>x : number | string | E | V
+>E : E
+>V : V
+
+if (typeof x === "number") {
+>typeof x === "number" : boolean
+>typeof x : string
+>x : number | string | E | V
+>"number" : string
+
+    x; // number|E|V
+>x : number | E | V
+}
+else {
+    x; // string
+>x : string
+}
+
+if (typeof x !== "number") {
+>typeof x !== "number" : boolean
+>typeof x : string
+>x : number | string | E | V
+>"number" : string
+
+    x; // string
+>x : string
+}
+else {
+    x; // number|E|V
+>x : number | E | V
+}
+

tests/baselines/reference/typeGuardNesting.js

@@ -0,0 +1,31 @@
+//// [typeGuardNesting.ts]
+let strOrBool: string|boolean;
+if ((typeof strOrBool === 'boolean' && !strOrBool) || typeof strOrBool === 'string') {
+	let label: string = (typeof strOrBool === 'string') ? strOrBool : "string";
+	let bool: boolean = (typeof strOrBool === 'boolean') ? strOrBool : false;
+	let label2: string = (typeof strOrBool !== 'boolean') ? strOrBool : "string";
+	let bool2: boolean = (typeof strOrBool !== 'string') ? strOrBool : false;
+}
+
+if ((typeof strOrBool !== 'string' && !strOrBool) || typeof strOrBool !== 'boolean') {
+	let label: string = (typeof strOrBool === 'string') ? strOrBool : "string";
+	let bool: boolean = (typeof strOrBool === 'boolean') ? strOrBool : false;
+	let label2: string = (typeof strOrBool !== 'boolean') ? strOrBool : "string";
+	let bool2: boolean = (typeof strOrBool !== 'string') ? strOrBool : false;
+}
+
+
+//// [typeGuardNesting.js]
+var strOrBool;
+if ((typeof strOrBool === 'boolean' && !strOrBool) || typeof strOrBool === 'string') {
+    var label = (typeof strOrBool === 'string') ? strOrBool : "string";
+    var bool = (typeof strOrBool === 'boolean') ? strOrBool : false;
+    var label2 = (typeof strOrBool !== 'boolean') ? strOrBool : "string";
+    var bool2 = (typeof strOrBool !== 'string') ? strOrBool : false;
+}
+if ((typeof strOrBool !== 'string' && !strOrBool) || typeof strOrBool !== 'boolean') {
+    var label = (typeof strOrBool === 'string') ? strOrBool : "string";
+    var bool = (typeof strOrBool === 'boolean') ? strOrBool : false;
+    var label2 = (typeof strOrBool !== 'boolean') ? strOrBool : "string";
+    var bool2 = (typeof strOrBool !== 'string') ? strOrBool : false;
+}