From 94a0daf2ea9261e22b8def1511034f14ca332a49 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Tue, 27 Sep 2016 16:03:12 -0700 Subject: [PATCH 1/2] Properly handle partially discriminated unions --- src/compiler/checker.ts | 66 +++++++++++++++++++++++------------------ src/compiler/types.ts | 3 +- 2 files changed, 39 insertions(+), 30 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 72d2ca92af0..638ff533e3e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4366,7 +4366,7 @@ 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 @@ -4374,7 +4374,19 @@ namespace ts { 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 && (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 = createSymbol( - SymbolFlags.Property | - SymbolFlags.Transient | - SymbolFlags.SyntheticProperty | - commonFlags, - name); + const result = 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()); 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 && (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(type, name); if (prop && prop.flags & SymbolFlags.SyntheticProperty) { if ((prop).isDiscriminantProperty === undefined) { - (prop).isDiscriminantProperty = !(prop).hasCommonType && - isLiteralType(getTypeOfSymbol(prop)); + (prop).isDiscriminantProperty = (prop).hasNonUniformType && isLiteralType(getTypeOfSymbol(prop)); } return (prop).isDiscriminantProperty; } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index af71846145f..ab48ae543c2 100644 --- 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 From 8b26cedf317008099483060fcf4c336bc744cd3e Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Tue, 27 Sep 2016 16:03:57 -0700 Subject: [PATCH 2/2] Add regression tests --- .../partiallyDiscriminantedUnions.js | 77 +++++++++++ .../partiallyDiscriminantedUnions.symbols | 117 ++++++++++++++++ .../partiallyDiscriminantedUnions.types | 128 ++++++++++++++++++ .../compiler/partiallyDiscriminantedUnions.ts | 46 +++++++ 4 files changed, 368 insertions(+) create mode 100644 tests/baselines/reference/partiallyDiscriminantedUnions.js create mode 100644 tests/baselines/reference/partiallyDiscriminantedUnions.symbols create mode 100644 tests/baselines/reference/partiallyDiscriminantedUnions.types create mode 100644 tests/cases/compiler/partiallyDiscriminantedUnions.ts diff --git a/tests/baselines/reference/partiallyDiscriminantedUnions.js b/tests/baselines/reference/partiallyDiscriminantedUnions.js new file mode 100644 index 00000000000..4c1a0a77f53 --- /dev/null +++ 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 = {}; + +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; + +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 new file mode 100644 index 00000000000..01d6143b9b2 --- /dev/null +++ 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 : 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; +>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 new file mode 100644 index 00000000000..911c949373b --- /dev/null +++ b/tests/baselines/reference/partiallyDiscriminantedUnions.types @@ -0,0 +1,128 @@ +=== tests/cases/compiler/partiallyDiscriminantedUnions.ts === +// Repro from #10586 + +interface A1 { +>A1 : A1 + + type: 'a'; +>type : "a" + + subtype: 1; +>subtype : 1 +} + +interface A2 { +>A2 : A2 + + type: 'a'; +>type : "a" + + subtype: 2; +>subtype : 2 + + foo: number; +>foo : number +} + +interface B { +>B : B + + type: 'b'; +>type : "b" +} + +type AB = A1 | A2 | B; +>AB : AB +>A1 : A1 +>A2 : A2 +>B : B + +const ab: AB = {}; +>ab : AB +>AB : AB +>{} : AB +>AB : AB +>{} : {} + +if (ab.type === 'a') { +>ab.type === 'a' : boolean +>ab.type : "a" | "b" +>ab : AB +>type : "a" | "b" +>'a' : "a" + + if (ab.subtype === 2) { +>ab.subtype === 2 : boolean +>ab.subtype : 1 | 2 +>ab : A1 | A2 +>subtype : 1 | 2 +>2 : 2 + + ab.foo; +>ab.foo : number +>ab : A2 +>foo : number + } +} + +// Repro from #11185 + +class Square { kind: "square"; } +>Square : Square +>kind : "square" + +class Circle { kind: "circle"; } +>Circle : Circle +>kind : "circle" + +type Shape = Circle | Square; +>Shape : Shape +>Circle : Circle +>Square : Square + +type Shapes = Shape | Array; +>Shapes : Shapes +>Shape : Shape +>Array : T[] +>Shape : Shape + +function isShape(s : Shapes): s is Shape { +>isShape : (s: Shapes) => s is Shape +>s : Shapes +>Shapes : Shapes +>s : any +>Shape : Shape + + return !Array.isArray(s); +>!Array.isArray(s) : boolean +>Array.isArray(s) : boolean +>Array.isArray : (arg: any) => arg is any[] +>Array : ArrayConstructor +>isArray : (arg: any) => arg is any[] +>s : Shapes +} + +function fail(s: Shapes) { +>fail : (s: Shapes) => void +>s : Shapes +>Shapes : Shapes + + if (isShape(s)) { +>isShape(s) : boolean +>isShape : (s: Shapes) => s is Shape +>s : Shapes + + if (s.kind === "circle") { +>s.kind === "circle" : boolean +>s.kind : "square" | "circle" +>s : Shape +>kind : "square" | "circle" +>"circle" : "circle" + + let c: Circle = s; +>c : Circle +>Circle : Circle +>s : Circle + } + } +} diff --git a/tests/cases/compiler/partiallyDiscriminantedUnions.ts b/tests/cases/compiler/partiallyDiscriminantedUnions.ts new file mode 100644 index 00000000000..b4f89951f4c --- /dev/null +++ b/tests/cases/compiler/partiallyDiscriminantedUnions.ts @@ -0,0 +1,46 @@ +// 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 = {}; + +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; + +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; + } + } +} \ No newline at end of file