Merge pull request #11198 from Microsoft/partiallyDiscriminatedUnions

ahejlsberg · web-flow · commit 98f3f68c46da · 2016-09-27T16:55:17.000-07:00
Properly handle partially discriminated unions
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -4366,15 +4366,27 @@ namespace ts {
         function getPropertiesOfUnionOrIntersectionType(type: UnionOrIntersectionType): Symbol[] {
             for (const current of type.types) {
                 for (const prop of getPropertiesOfType(current)) {
-                    getPropertyOfUnionOrIntersectionType(type, prop.name);
+                    getUnionOrIntersectionProperty(type, prop.name);
                 }
                 // The properties of a union type are those that are present in all constituent types, so
                 // we only need to check the properties of the first type
                 if (type.flags & TypeFlags.Union) {
                     break;
                 }
             }
-            return type.resolvedProperties ? symbolsToArray(type.resolvedProperties) : emptyArray;
+            const props = type.resolvedProperties;
+            if (props) {
+                const result: Symbol[] = [];
+                for (const key in props) {
+                    const prop = props[key];
+                    // We need to filter out partial properties in union types
+                    if (!(prop.flags & SymbolFlags.SyntheticProperty && (<TransientSymbol>prop).isPartial)) {
+                        result.push(prop);
+                    }
+                }
+                return result;
+            }
+            return emptyArray;
         }
 
         function getPropertiesOfType(type: Type): Symbol[] {
@@ -4427,6 +4439,7 @@ namespace ts {
             // Flags we want to propagate to the result if they exist in all source symbols
             let commonFlags = (containingType.flags & TypeFlags.Intersection) ? SymbolFlags.Optional : SymbolFlags.None;
             let isReadonly = false;
+            let isPartial = false;
             for (const current of types) {
                 const type = getApparentType(current);
                 if (type !== unknownType) {
@@ -4444,21 +4457,20 @@ namespace ts {
                         }
                     }
                     else if (containingType.flags & TypeFlags.Union) {
-                        // A union type requires the property to be present in all constituent types
-                        return undefined;
+                        isPartial = true;
                     }
                 }
             }
             if (!props) {
                 return undefined;
             }
-            if (props.length === 1) {
+            if (props.length === 1 && !isPartial) {
                 return props[0];
             }
             const propTypes: Type[] = [];
             const declarations: Declaration[] = [];
             let commonType: Type = undefined;
-            let hasCommonType = true;
+            let hasNonUniformType = false;
             for (const prop of props) {
                 if (prop.declarations) {
                     addRange(declarations, prop.declarations);
@@ -4468,25 +4480,26 @@ namespace ts {
                     commonType = type;
                 }
                 else if (type !== commonType) {
-                    hasCommonType = false;
+                    hasNonUniformType = true;
                 }
-                propTypes.push(getTypeOfSymbol(prop));
+                propTypes.push(type);
             }
-            const result = <TransientSymbol>createSymbol(
-                SymbolFlags.Property |
-                SymbolFlags.Transient |
-                SymbolFlags.SyntheticProperty |
-                commonFlags,
-                name);
+            const result = <TransientSymbol>createSymbol(SymbolFlags.Property | SymbolFlags.Transient | SymbolFlags.SyntheticProperty | commonFlags, name);
             result.containingType = containingType;
-            result.hasCommonType = hasCommonType;
+            result.hasNonUniformType = hasNonUniformType;
+            result.isPartial = isPartial;
             result.declarations = declarations;
             result.isReadonly = isReadonly;
             result.type = containingType.flags & TypeFlags.Union ? getUnionType(propTypes) : getIntersectionType(propTypes);
             return result;
         }
 
-        function getPropertyOfUnionOrIntersectionType(type: UnionOrIntersectionType, name: string): Symbol {
+        // Return the symbol for a given property in a union or intersection type, or undefined if the property
+        // does not exist in any constituent type. Note that the returned property may only be present in some
+        // constituents, in which case the isPartial flag is set when the containing type is union type. We need
+        // these partial properties when identifying discriminant properties, but otherwise they are filtered out
+        // and do not appear to be present in the union type.
+        function getUnionOrIntersectionProperty(type: UnionOrIntersectionType, name: string): Symbol {
             const properties = type.resolvedProperties || (type.resolvedProperties = createMap<Symbol>());
             let property = properties[name];
             if (!property) {
@@ -4498,6 +4511,12 @@ namespace ts {
             return property;
         }
 
+        function getPropertyOfUnionOrIntersectionType(type: UnionOrIntersectionType, name: string): Symbol {
+            const property = getUnionOrIntersectionProperty(type, name);
+            // We need to filter out partial properties in union types
+            return property && !(property.flags & SymbolFlags.SyntheticProperty && (<TransientSymbol>property).isPartial) ? property : undefined;
+        }
+
         /**
          * Return the symbol for the property with the given name in the given type. Creates synthetic union properties when
          * necessary, maps primitive types and type parameters are to their apparent types, and augments with properties from
@@ -8078,21 +8097,10 @@ namespace ts {
 
         function isDiscriminantProperty(type: Type, name: string) {
             if (type && type.flags & TypeFlags.Union) {
-                let prop = getPropertyOfType(type, name);
-                if (!prop) {
-                    // The type may be a union that includes nullable or primitive types. If filtering
-                    // those out produces a different type, get the property from that type instead.
-                    // Effectively, we're checking if this *could* be a discriminant property once nullable
-                    // and primitive types are removed by other type guards.
-                    const filteredType = getTypeWithFacts(type, TypeFacts.Discriminatable);
-                    if (filteredType !== type && filteredType.flags & TypeFlags.Union) {
-                        prop = getPropertyOfType(filteredType, name);
-                    }
-                }
+                const prop = getUnionOrIntersectionProperty(<UnionType>type, name);
                 if (prop && prop.flags & SymbolFlags.SyntheticProperty) {
                     if ((<TransientSymbol>prop).isDiscriminantProperty === undefined) {
-                        (<TransientSymbol>prop).isDiscriminantProperty = !(<TransientSymbol>prop).hasCommonType &&
-                            isLiteralType(getTypeOfSymbol(prop));
+                        (<TransientSymbol>prop).isDiscriminantProperty = (<TransientSymbol>prop).hasNonUniformType && isLiteralType(getTypeOfSymbol(prop));
                     }
                     return (<TransientSymbol>prop).isDiscriminantProperty;
                 }
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -2289,7 +2289,8 @@ namespace ts {
         mapper?: TypeMapper;                // Type mapper for instantiation alias
         referenced?: boolean;               // True if alias symbol has been referenced as a value
         containingType?: UnionOrIntersectionType; // Containing union or intersection type for synthetic property
-        hasCommonType?: boolean;            // True if constituents of synthetic property all have same type
+        hasNonUniformType?: boolean;        // True if constituents have non-uniform types
+        isPartial?: boolean;                // True if syntheric property of union type occurs in some but not all constituents
         isDiscriminantProperty?: boolean;   // True if discriminant synthetic property
         resolvedExports?: SymbolTable;      // Resolved exports of module
         exportsChecked?: boolean;           // True if exports of external module have been checked
diff --git a/tests/baselines/reference/partiallyDiscriminantedUnions.js b/tests/baselines/reference/partiallyDiscriminantedUnions.js
@@ -0,0 +1,77 @@
+//// [partiallyDiscriminantedUnions.ts]
+// Repro from #10586
+
+interface A1 {
+    type: 'a';
+    subtype: 1;
+}
+
+interface A2 {
+    type: 'a';
+    subtype: 2;
+    foo: number;
+}
+
+interface B {
+    type: 'b';
+}
+
+type AB = A1 | A2 | B;
+
+const ab: AB = <AB>{};
+
+if (ab.type === 'a') {
+    if (ab.subtype === 2) {
+        ab.foo;
+    }
+}
+
+// Repro from #11185
+
+class Square { kind: "square"; }
+class Circle { kind: "circle"; }
+
+type Shape = Circle | Square;
+type Shapes = Shape | Array<Shape>;
+
+function isShape(s : Shapes): s is Shape {
+    return !Array.isArray(s);
+}
+
+function fail(s: Shapes) {
+    if (isShape(s)) {
+        if (s.kind === "circle") {
+            let c: Circle = s;
+        }
+    }
+}
+
+//// [partiallyDiscriminantedUnions.js]
+// Repro from #10586
+var ab = {};
+if (ab.type === 'a') {
+    if (ab.subtype === 2) {
+        ab.foo;
+    }
+}
+// Repro from #11185
+var Square = (function () {
+    function Square() {
+    }
+    return Square;
+}());
+var Circle = (function () {
+    function Circle() {
+    }
+    return Circle;
+}());
+function isShape(s) {
+    return !Array.isArray(s);
+}
+function fail(s) {
+    if (isShape(s)) {
+        if (s.kind === "circle") {
+            var c = s;
+        }
+    }
+}
diff --git a/tests/baselines/reference/partiallyDiscriminantedUnions.symbols b/tests/baselines/reference/partiallyDiscriminantedUnions.symbols
@@ -0,0 +1,117 @@
+=== tests/cases/compiler/partiallyDiscriminantedUnions.ts ===
+// Repro from #10586
+
+interface A1 {
+>A1 : Symbol(A1, Decl(partiallyDiscriminantedUnions.ts, 0, 0))
+
+    type: 'a';
+>type : Symbol(A1.type, Decl(partiallyDiscriminantedUnions.ts, 2, 14))
+
+    subtype: 1;
+>subtype : Symbol(A1.subtype, Decl(partiallyDiscriminantedUnions.ts, 3, 14))
+}
+
+interface A2 {
+>A2 : Symbol(A2, Decl(partiallyDiscriminantedUnions.ts, 5, 1))
+
+    type: 'a';
+>type : Symbol(A2.type, Decl(partiallyDiscriminantedUnions.ts, 7, 14))
+
+    subtype: 2;
+>subtype : Symbol(A2.subtype, Decl(partiallyDiscriminantedUnions.ts, 8, 14))
+
+    foo: number;
+>foo : Symbol(A2.foo, Decl(partiallyDiscriminantedUnions.ts, 9, 15))
+}
+
+interface B {
+>B : Symbol(B, Decl(partiallyDiscriminantedUnions.ts, 11, 1))
+
+    type: 'b';
+>type : Symbol(B.type, Decl(partiallyDiscriminantedUnions.ts, 13, 13))
+}
+
+type AB = A1 | A2 | B;
+>AB : Symbol(AB, Decl(partiallyDiscriminantedUnions.ts, 15, 1))
+>A1 : Symbol(A1, Decl(partiallyDiscriminantedUnions.ts, 0, 0))
+>A2 : Symbol(A2, Decl(partiallyDiscriminantedUnions.ts, 5, 1))
+>B : Symbol(B, Decl(partiallyDiscriminantedUnions.ts, 11, 1))
+
+const ab: AB = <AB>{};
+>ab : Symbol(ab, Decl(partiallyDiscriminantedUnions.ts, 19, 5))
+>AB : Symbol(AB, Decl(partiallyDiscriminantedUnions.ts, 15, 1))
+>AB : Symbol(AB, Decl(partiallyDiscriminantedUnions.ts, 15, 1))
+
+if (ab.type === 'a') {
+>ab.type : Symbol(type, Decl(partiallyDiscriminantedUnions.ts, 2, 14), Decl(partiallyDiscriminantedUnions.ts, 7, 14), Decl(partiallyDiscriminantedUnions.ts, 13, 13))
+>ab : Symbol(ab, Decl(partiallyDiscriminantedUnions.ts, 19, 5))
+>type : Symbol(type, Decl(partiallyDiscriminantedUnions.ts, 2, 14), Decl(partiallyDiscriminantedUnions.ts, 7, 14), Decl(partiallyDiscriminantedUnions.ts, 13, 13))
+
+    if (ab.subtype === 2) {
+>ab.subtype : Symbol(subtype, Decl(partiallyDiscriminantedUnions.ts, 3, 14), Decl(partiallyDiscriminantedUnions.ts, 8, 14))
+>ab : Symbol(ab, Decl(partiallyDiscriminantedUnions.ts, 19, 5))
+>subtype : Symbol(subtype, Decl(partiallyDiscriminantedUnions.ts, 3, 14), Decl(partiallyDiscriminantedUnions.ts, 8, 14))
+
+        ab.foo;
+>ab.foo : Symbol(A2.foo, Decl(partiallyDiscriminantedUnions.ts, 9, 15))
+>ab : Symbol(ab, Decl(partiallyDiscriminantedUnions.ts, 19, 5))
+>foo : Symbol(A2.foo, Decl(partiallyDiscriminantedUnions.ts, 9, 15))
+    }
+}
+
+// Repro from #11185
+
+class Square { kind: "square"; }
+>Square : Symbol(Square, Decl(partiallyDiscriminantedUnions.ts, 25, 1))
+>kind : Symbol(Square.kind, Decl(partiallyDiscriminantedUnions.ts, 29, 14))
+
+class Circle { kind: "circle"; }
+>Circle : Symbol(Circle, Decl(partiallyDiscriminantedUnions.ts, 29, 32))
+>kind : Symbol(Circle.kind, Decl(partiallyDiscriminantedUnions.ts, 30, 14))
+
+type Shape = Circle | Square;
+>Shape : Symbol(Shape, Decl(partiallyDiscriminantedUnions.ts, 30, 32))
+>Circle : Symbol(Circle, Decl(partiallyDiscriminantedUnions.ts, 29, 32))
+>Square : Symbol(Square, Decl(partiallyDiscriminantedUnions.ts, 25, 1))
+
+type Shapes = Shape | Array<Shape>;
+>Shapes : Symbol(Shapes, Decl(partiallyDiscriminantedUnions.ts, 32, 29))
+>Shape : Symbol(Shape, Decl(partiallyDiscriminantedUnions.ts, 30, 32))
+>Array : Symbol(Array, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))
+>Shape : Symbol(Shape, Decl(partiallyDiscriminantedUnions.ts, 30, 32))
+
+function isShape(s : Shapes): s is Shape {
+>isShape : Symbol(isShape, Decl(partiallyDiscriminantedUnions.ts, 33, 35))
+>s : Symbol(s, Decl(partiallyDiscriminantedUnions.ts, 35, 17))
+>Shapes : Symbol(Shapes, Decl(partiallyDiscriminantedUnions.ts, 32, 29))
+>s : Symbol(s, Decl(partiallyDiscriminantedUnions.ts, 35, 17))
+>Shape : Symbol(Shape, Decl(partiallyDiscriminantedUnions.ts, 30, 32))
+
+    return !Array.isArray(s);
+>Array.isArray : Symbol(ArrayConstructor.isArray, Decl(lib.d.ts, --, --))
+>Array : Symbol(Array, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))
+>isArray : Symbol(ArrayConstructor.isArray, Decl(lib.d.ts, --, --))
+>s : Symbol(s, Decl(partiallyDiscriminantedUnions.ts, 35, 17))
+}
+
+function fail(s: Shapes) {
+>fail : Symbol(fail, Decl(partiallyDiscriminantedUnions.ts, 37, 1))
+>s : Symbol(s, Decl(partiallyDiscriminantedUnions.ts, 39, 14))
+>Shapes : Symbol(Shapes, Decl(partiallyDiscriminantedUnions.ts, 32, 29))
+
+    if (isShape(s)) {
+>isShape : Symbol(isShape, Decl(partiallyDiscriminantedUnions.ts, 33, 35))
+>s : Symbol(s, Decl(partiallyDiscriminantedUnions.ts, 39, 14))
+
+        if (s.kind === "circle") {
+>s.kind : Symbol(kind, Decl(partiallyDiscriminantedUnions.ts, 29, 14), Decl(partiallyDiscriminantedUnions.ts, 30, 14))
+>s : Symbol(s, Decl(partiallyDiscriminantedUnions.ts, 39, 14))
+>kind : Symbol(kind, Decl(partiallyDiscriminantedUnions.ts, 29, 14), Decl(partiallyDiscriminantedUnions.ts, 30, 14))
+
+            let c: Circle = s;
+>c : Symbol(c, Decl(partiallyDiscriminantedUnions.ts, 42, 15))
+>Circle : Symbol(Circle, Decl(partiallyDiscriminantedUnions.ts, 29, 32))
+>s : Symbol(s, Decl(partiallyDiscriminantedUnions.ts, 39, 14))
+        }
+    }
+}
diff --git a/tests/baselines/reference/partiallyDiscriminantedUnions.types b/tests/baselines/reference/partiallyDiscriminantedUnions.types
diff --git a/tests/cases/compiler/partiallyDiscriminantedUnions.ts b/tests/cases/compiler/partiallyDiscriminantedUnions.ts
