Merge pull request #10118 from Microsoft/limitTypeGuardAssertions

Limit "type guards as assertions" behavior

Author: ahejlsberg

src/compiler/checker.ts

@@ -215,7 +215,7 @@ namespace ts {
         const flowLoopKeys: string[] = [];
         const flowLoopTypes: Type[][] = [];
         const visitedFlowNodes: FlowNode[] = [];
-        const visitedFlowTypes: Type[] = [];
+        const visitedFlowTypes: FlowType[] = [];
         const potentialThisCollisions: Node[] = [];
         const awaitedTypeStack: number[] = [];
 
@@ -8090,21 +8090,33 @@ namespace ts {
                 f(type) ? type : neverType;
         }
 
+        function isIncomplete(flowType: FlowType) {
+            return flowType.flags === 0;
+        }
+
+        function getTypeFromFlowType(flowType: FlowType) {
+            return flowType.flags === 0 ? (<IncompleteType>flowType).type : <Type>flowType;
+        }
+
+        function createFlowType(type: Type, incomplete: boolean): FlowType {
+            return incomplete ? { flags: 0, type } : type;
+        }
+
         function getFlowTypeOfReference(reference: Node, declaredType: Type, assumeInitialized: boolean, includeOuterFunctions: boolean) {
             let key: string;
             if (!reference.flowNode || assumeInitialized && !(declaredType.flags & TypeFlags.Narrowable)) {
                 return declaredType;
             }
             const initialType = assumeInitialized ? declaredType : includeFalsyTypes(declaredType, TypeFlags.Undefined);
             const visitedFlowStart = visitedFlowCount;
-            const result = getTypeAtFlowNode(reference.flowNode);
+            const result = getTypeFromFlowType(getTypeAtFlowNode(reference.flowNode));
             visitedFlowCount = visitedFlowStart;
             if (reference.parent.kind === SyntaxKind.NonNullExpression && getTypeWithFacts(result, TypeFacts.NEUndefinedOrNull) === neverType) {
                 return declaredType;
             }
             return result;
 
-            function getTypeAtFlowNode(flow: FlowNode): Type {
+            function getTypeAtFlowNode(flow: FlowNode): FlowType {
                 while (true) {
                     if (flow.flags & FlowFlags.Shared) {
                         // We cache results of flow type resolution for shared nodes that were previously visited in
@@ -8116,7 +8128,7 @@ namespace ts {
                             }
                         }
                     }
-                    let type: Type;
+                    let type: FlowType;
                     if (flow.flags & FlowFlags.Assignment) {
                         type = getTypeAtFlowAssignment(<FlowAssignment>flow);
                         if (!type) {
@@ -8184,41 +8196,44 @@ namespace ts {
                 return undefined;
             }
 
-            function getTypeAtFlowCondition(flow: FlowCondition) {
-                let type = getTypeAtFlowNode(flow.antecedent);
+            function getTypeAtFlowCondition(flow: FlowCondition): FlowType {
+                const flowType = getTypeAtFlowNode(flow.antecedent);
+                let type = getTypeFromFlowType(flowType);
                 if (type !== neverType) {
                     // If we have an antecedent type (meaning we're reachable in some way), we first
-                    // attempt to narrow the antecedent type. If that produces the nothing type, then
-                    // we take the type guard as an indication that control could reach here in a
-                    // manner not understood by the control flow analyzer (e.g. a function argument
-                    // has an invalid type, or a nested function has possibly made an assignment to a
-                    // captured variable). We proceed by reverting to the declared type and then
+                    // attempt to narrow the antecedent type. If that produces the never type, and if
+                    // the antecedent type is incomplete (i.e. a transient type in a loop), then we
+                    // take the type guard as an indication that control *could* reach here once we
+                    // have the complete type. We proceed by reverting to the declared type and then
                     // narrow that.
                     const assumeTrue = (flow.flags & FlowFlags.TrueCondition) !== 0;
                     type = narrowType(type, flow.expression, assumeTrue);
-                    if (type === neverType) {
+                    if (type === neverType && isIncomplete(flowType)) {
                         type = narrowType(declaredType, flow.expression, assumeTrue);
                     }
                 }
-                return type;
+                return createFlowType(type, isIncomplete(flowType));
             }
 
-            function getTypeAtSwitchClause(flow: FlowSwitchClause) {
-                const type = getTypeAtFlowNode(flow.antecedent);
+            function getTypeAtSwitchClause(flow: FlowSwitchClause): FlowType {
+                const flowType = getTypeAtFlowNode(flow.antecedent);
+                let type = getTypeFromFlowType(flowType);
                 const expr = flow.switchStatement.expression;
                 if (isMatchingReference(reference, expr)) {
-                    return narrowTypeBySwitchOnDiscriminant(type, flow.switchStatement, flow.clauseStart, flow.clauseEnd);
+                    type = narrowTypeBySwitchOnDiscriminant(type, flow.switchStatement, flow.clauseStart, flow.clauseEnd);
                 }
-                if (isMatchingPropertyAccess(expr)) {
-                    return narrowTypeByDiscriminant(type, <PropertyAccessExpression>expr, t => narrowTypeBySwitchOnDiscriminant(t, flow.switchStatement, flow.clauseStart, flow.clauseEnd));
+                else if (isMatchingPropertyAccess(expr)) {
+                    type = narrowTypeByDiscriminant(type, <PropertyAccessExpression>expr, t => narrowTypeBySwitchOnDiscriminant(t, flow.switchStatement, flow.clauseStart, flow.clauseEnd));
                 }
-                return type;
+                return createFlowType(type, isIncomplete(flowType));
             }
 
-            function getTypeAtFlowBranchLabel(flow: FlowLabel) {
+            function getTypeAtFlowBranchLabel(flow: FlowLabel): FlowType {
                 const antecedentTypes: Type[] = [];
+                let seenIncomplete = false;
                 for (const antecedent of flow.antecedents) {
-                    const type = getTypeAtFlowNode(antecedent);
+                    const flowType = getTypeAtFlowNode(antecedent);
+                    const type = getTypeFromFlowType(flowType);
                     // If the type at a particular antecedent path is the declared type and the
                     // reference is known to always be assigned (i.e. when declared and initial types
                     // are the same), there is no reason to process more antecedents since the only
@@ -8229,11 +8244,14 @@ namespace ts {
                     if (!contains(antecedentTypes, type)) {
                         antecedentTypes.push(type);
                     }
+                    if (isIncomplete(flowType)) {
+                        seenIncomplete = true;
+                    }
                 }
-                return getUnionType(antecedentTypes);
+                return createFlowType(getUnionType(antecedentTypes), seenIncomplete);
             }
 
-            function getTypeAtFlowLoopLabel(flow: FlowLabel) {
+            function getTypeAtFlowLoopLabel(flow: FlowLabel): FlowType {
                 // If we have previously computed the control flow type for the reference at
                 // this flow loop junction, return the cached type.
                 const id = getFlowNodeId(flow);
@@ -8245,12 +8263,12 @@ namespace ts {
                     return cache[key];
                 }
                 // If this flow loop junction and reference are already being processed, return
-                // the union of the types computed for each branch so far. We should never see
-                // an empty array here because the first antecedent of a loop junction is always
-                // the non-looping control flow path that leads to the top.
+                // the union of the types computed for each branch so far, marked as incomplete.
+                // We should never see an empty array here because the first antecedent of a loop
+                // junction is always the non-looping control flow path that leads to the top.
                 for (let i = flowLoopStart; i < flowLoopCount; i++) {
                     if (flowLoopNodes[i] === flow && flowLoopKeys[i] === key) {
-                        return getUnionType(flowLoopTypes[i]);
+                        return createFlowType(getUnionType(flowLoopTypes[i]), /*incomplete*/ true);
                     }
                 }
                 // Add the flow loop junction and reference to the in-process stack and analyze
@@ -8261,7 +8279,7 @@ namespace ts {
                 flowLoopTypes[flowLoopCount] = antecedentTypes;
                 for (const antecedent of flow.antecedents) {
                     flowLoopCount++;
-                    const type = getTypeAtFlowNode(antecedent);
+                    const type = getTypeFromFlowType(getTypeAtFlowNode(antecedent));
                     flowLoopCount--;
                     // If we see a value appear in the cache it is a sign that control flow  analysis
                     // was restarted and completed by checkExpressionCached. We can simply pick up

src/compiler/types.ts

@@ -1606,6 +1606,16 @@ namespace ts {
         antecedent: FlowNode;
     }
 
+    export type FlowType = Type | IncompleteType;
+
+    // Incomplete types occur during control flow analysis of loops. An IncompleteType
+    // is distinguished from a regular type by a flags value of zero. Incomplete type
+    // objects are internal to the getFlowTypeOfRefecence function and never escape it.
+    export interface IncompleteType {
+        flags: TypeFlags;  // No flags set
+        type: Type;        // The type marked incomplete
+    }
+
     export interface AmdDependency {
         path: string;
         name: string;

tests/baselines/reference/stringLiteralTypesAndTuples01.types

@@ -55,5 +55,5 @@ function rawr(dino: RexOrRaptor) {
     throw "Unexpected " + dino;
 >"Unexpected " + dino : string
 >"Unexpected " : string
->dino : "t-rex"
+>dino : never
 }

tests/baselines/reference/typeGuardTautologicalConsistiency.types

@@ -15,7 +15,7 @@ if (typeof stringOrNumber === "number") {
 >"number" : "number"
 
         stringOrNumber;
->stringOrNumber : string
+>stringOrNumber : never
     }
 }
 
@@ -31,6 +31,6 @@ if (typeof stringOrNumber === "number" && typeof stringOrNumber !== "number") {
 >"number" : "number"
 
     stringOrNumber;
->stringOrNumber : string
+>stringOrNumber : never
 }
 

tests/baselines/reference/typeGuardTypeOfUndefined.types

@@ -47,7 +47,7 @@ function test2(a: any) {
 >"boolean" : "boolean"
 
             a;
->a : boolean
+>a : never
         }
         else {
             a;
@@ -129,7 +129,7 @@ function test5(a: boolean | void) {
         }
         else {
             a;
->a : void
+>a : never
         }
     }
     else {
@@ -188,7 +188,7 @@ function test7(a: boolean | void) {
     }
     else {
         a;
->a : void
+>a : never
     }
 }
 

tests/baselines/reference/typeGuardsAsAssertions.types

@@ -193,10 +193,10 @@ function f1() {
 >x : undefined
 
         x;  // string | number (guard as assertion)
->x : string | number
+>x : never
     }
     x;  // string | number | undefined
->x : string | number | undefined
+>x : undefined
 }
 
 function f2() {
@@ -216,10 +216,10 @@ function f2() {
 >"string" : "string"
 
         x;  // string (guard as assertion)
->x : string
+>x : never
     }
     x;  // string | undefined
->x : string | undefined
+>x : undefined
 }
 
 function f3() {
@@ -239,7 +239,7 @@ function f3() {
         return;
     }
     x;  // string | number (guard as assertion)
->x : string | number
+>x : never
 }
 
 function f4() {
@@ -281,7 +281,7 @@ function f5(x: string | number) {
 >"number" : "number"
 
         x;  // number (guard as assertion)
->x : number
+>x : never
     }
     else {
         x;  // string | number

tests/baselines/reference/typeGuardsInIfStatement.errors.txt

@@ -1,9 +1,10 @@
 tests/cases/conformance/expressions/typeGuards/typeGuardsInIfStatement.ts(22,10): error TS2354: No best common type exists among return expressions.
 tests/cases/conformance/expressions/typeGuards/typeGuardsInIfStatement.ts(31,10): error TS2354: No best common type exists among return expressions.
 tests/cases/conformance/expressions/typeGuards/typeGuardsInIfStatement.ts(49,10): error TS2354: No best common type exists among return expressions.
+tests/cases/conformance/expressions/typeGuards/typeGuardsInIfStatement.ts(139,17): error TS2339: Property 'toString' does not exist on type 'never'.
 
 
-==== tests/cases/conformance/expressions/typeGuards/typeGuardsInIfStatement.ts (3 errors) ====
+==== tests/cases/conformance/expressions/typeGuards/typeGuardsInIfStatement.ts (4 errors) ====
     // In the true branch statement of an 'if' statement, 
     // the type of a variable or parameter is narrowed by any type guard in the 'if' condition when true.
     // In the false branch statement of an 'if' statement, 
@@ -149,5 +150,7 @@ tests/cases/conformance/expressions/typeGuards/typeGuardsInIfStatement.ts(49,10)
             return typeof x === "number"
                 ? x.toString() // number
                 : x.toString(); // boolean | string
+                    ~~~~~~~~
+!!! error TS2339: Property 'toString' does not exist on type 'never'.
         }
     }