From 95a314a754f70271682e06b2ae82f21a54130e07 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sat, 24 Jan 2015 17:21:49 -0800 Subject: [PATCH 1/6] Correct support for type guards with unions containing enums --- src/compiler/checker.ts | 53 +++++++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index a606a3f00fd..f5592560ed4 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -107,6 +107,21 @@ module ts { var diagnostics: Diagnostic[] = []; var diagnosticsModified: boolean = false; + var primitiveTypeInfo: Map<{ type: Type; flags: TypeFlags }> = { + "string": { + type: stringType, + flags: TypeFlags.StringLike + }, + "number": { + type: numberType, + flags: TypeFlags.NumberLike + }, + "boolean": { + type: booleanType, + flags: TypeFlags.Boolean + } + }; + function addDiagnostic(diagnostic: Diagnostic) { diagnostics.push(diagnostic); diagnosticsModified = true; @@ -4454,12 +4469,15 @@ module ts { Debug.fail("should not get here"); } - // Remove one or more primitive types from a union type - function subtractPrimitiveTypes(type: Type, subtractMask: TypeFlags): Type { + // For a union type, remove all constituent types for which the given flags have the given state + function removeTypesFromUnionType(type: Type, maskFlags: TypeFlags, maskState: boolean): Type { if (type.flags & TypeFlags.Union) { var types = (type).types; - if (forEach(types, t => t.flags & subtractMask)) { - return getUnionType(filter(types, t => !(t.flags & subtractMask))); + if (forEach(types, t => !(t.flags & maskFlags) !== maskState)) { + var reducedType = getUnionType(filter(types, t => !(t.flags & maskFlags) === maskState)); + if (reducedType !== emptyObjectType) { + return reducedType; + } } } return type; @@ -4635,8 +4653,8 @@ module ts { // Stop at the first containing function or module declaration break loop; } - // Use narrowed type if it is a subtype and construct contains no assignments to variable - if (narrowedType !== type && isTypeSubtypeOf(narrowedType, type)) { + // Use narrowed type if construct contains no assignments to variable + if (narrowedType !== type) { if (isVariableAssignedWithin(symbol, node)) { break; } @@ -4656,20 +4674,29 @@ module ts { if (left.expression.kind !== SyntaxKind.Identifier || getResolvedSymbol(left.expression) !== symbol) { return type; } - var t = right.text; - var checkType: Type = t === "string" ? stringType : t === "number" ? numberType : t === "boolean" ? booleanType : emptyObjectType; + var typeInfo = primitiveTypeInfo[right.text]; if (expr.operator === SyntaxKind.ExclamationEqualsEqualsToken) { assumeTrue = !assumeTrue; } if (assumeTrue) { - // The assumed result is true. If check was for a primitive type, that type is the narrowed type. Otherwise we can - // remove the primitive types from the narrowed type. - return checkType === emptyObjectType ? subtractPrimitiveTypes(type, TypeFlags.String | TypeFlags.Number | TypeFlags.Boolean) : checkType; + // Assumed result is true. If check was not for a primitive type, remove all primitive types + if (!typeInfo) { + return removeTypesFromUnionType(type, TypeFlags.StringLike | TypeFlags.NumberLike | TypeFlags.Boolean, true); + } + // 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 + return removeTypesFromUnionType(type, typeInfo.flags, false); } else { - // The assumed result is false. If check was for a primitive type we can remove that type from the narrowed type. + // Assumed result is false. If check was for a primitive type, remove that primitive type + if (typeInfo) { + return removeTypesFromUnionType(type, typeInfo.flags, true); + } // Otherwise we don't have enough information to do anything. - return checkType === emptyObjectType ? type : subtractPrimitiveTypes(type, checkType.flags); + return type; } } From 7f63180b64ed83877416ac42165c98180a7907e9 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sat, 24 Jan 2015 17:23:22 -0800 Subject: [PATCH 2/6] Adding tests --- .../reference/TypeGuardWithEnumUnion.js | 77 +++++++++++++++ .../reference/TypeGuardWithEnumUnion.types | 96 +++++++++++++++++++ .../typeGuards/TypeGuardWithEnumUnion.ts | 35 +++++++ 3 files changed, 208 insertions(+) create mode 100644 tests/baselines/reference/TypeGuardWithEnumUnion.js create mode 100644 tests/baselines/reference/TypeGuardWithEnumUnion.types create mode 100644 tests/cases/conformance/expressions/typeGuards/TypeGuardWithEnumUnion.ts diff --git a/tests/baselines/reference/TypeGuardWithEnumUnion.js b/tests/baselines/reference/TypeGuardWithEnumUnion.js new file mode 100644 index 00000000000..f582c70857a --- /dev/null +++ b/tests/baselines/reference/TypeGuardWithEnumUnion.js @@ -0,0 +1,77 @@ +//// [TypeGuardWithEnumUnion.ts] +enum Color { R, G, B } + +function f1(x: Color | string) { + if (typeof x === "number") { + var y = x; + var y: Color; + } + else { + var z = x; + var z: string; + } +} + +function f2(x: Color | string | string[]) { + if (typeof x === "object") { + var y = x; + var y: string[]; + } + if (typeof x === "number") { + var z = x; + var z: Color; + } + else { + var w = x; + var w: string | string[]; + } + if (typeof x === "string") { + var a = x; + var a: string; + } + else { + var b = x; + var b: Color | string[]; + } +} + + +//// [TypeGuardWithEnumUnion.js] +var Color; +(function (Color) { + Color[Color["R"] = 0] = "R"; + Color[Color["G"] = 1] = "G"; + Color[Color["B"] = 2] = "B"; +})(Color || (Color = {})); +function f1(x) { + if (typeof x === "number") { + var y = x; + var y; + } + else { + var z = x; + var z; + } +} +function f2(x) { + if (typeof x === "object") { + var y = x; + var y; + } + if (typeof x === "number") { + var z = x; + var z; + } + else { + var w = x; + var w; + } + if (typeof x === "string") { + var a = x; + var a; + } + else { + var b = x; + var b; + } +} diff --git a/tests/baselines/reference/TypeGuardWithEnumUnion.types b/tests/baselines/reference/TypeGuardWithEnumUnion.types new file mode 100644 index 00000000000..9296e0ad04e --- /dev/null +++ b/tests/baselines/reference/TypeGuardWithEnumUnion.types @@ -0,0 +1,96 @@ +=== tests/cases/conformance/expressions/typeGuards/TypeGuardWithEnumUnion.ts === +enum Color { R, G, B } +>Color : Color +>R : Color +>G : Color +>B : Color + +function f1(x: Color | string) { +>f1 : (x: string | Color) => void +>x : string | Color +>Color : Color + + if (typeof x === "number") { +>typeof x === "number" : boolean +>typeof x : string +>x : string | Color + + var y = x; +>y : Color +>x : Color + + var y: Color; +>y : Color +>Color : Color + } + else { + var z = x; +>z : string +>x : string + + var z: string; +>z : string + } +} + +function f2(x: Color | string | string[]) { +>f2 : (x: string | string[] | Color) => void +>x : string | string[] | Color +>Color : Color + + if (typeof x === "object") { +>typeof x === "object" : boolean +>typeof x : string +>x : string | string[] | Color + + var y = x; +>y : string[] +>x : string[] + + var y: string[]; +>y : string[] + } + if (typeof x === "number") { +>typeof x === "number" : boolean +>typeof x : string +>x : string | string[] | Color + + var z = x; +>z : Color +>x : Color + + var z: Color; +>z : Color +>Color : Color + } + else { + var w = x; +>w : string | string[] +>x : string | string[] + + var w: string | string[]; +>w : string | string[] + } + if (typeof x === "string") { +>typeof x === "string" : boolean +>typeof x : string +>x : string | string[] | Color + + var a = x; +>a : string +>x : string + + var a: string; +>a : string + } + else { + var b = x; +>b : string[] | Color +>x : string[] | Color + + var b: Color | string[]; +>b : string[] | Color +>Color : Color + } +} + diff --git a/tests/cases/conformance/expressions/typeGuards/TypeGuardWithEnumUnion.ts b/tests/cases/conformance/expressions/typeGuards/TypeGuardWithEnumUnion.ts new file mode 100644 index 00000000000..3d105e91502 --- /dev/null +++ b/tests/cases/conformance/expressions/typeGuards/TypeGuardWithEnumUnion.ts @@ -0,0 +1,35 @@ +enum Color { R, G, B } + +function f1(x: Color | string) { + if (typeof x === "number") { + var y = x; + var y: Color; + } + else { + var z = x; + var z: string; + } +} + +function f2(x: Color | string | string[]) { + if (typeof x === "object") { + var y = x; + var y: string[]; + } + if (typeof x === "number") { + var z = x; + var z: Color; + } + else { + var w = x; + var w: string | string[]; + } + if (typeof x === "string") { + var a = x; + var a: string; + } + else { + var b = x; + var b: Color | string[]; + } +} From e5e06d8ca52cc1c6361436565e62fc97f206dada Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sun, 25 Jan 2015 16:14:20 -0800 Subject: [PATCH 3/6] Allow union types containing primtives as first argument in instanceof --- src/compiler/checker.ts | 2 +- src/compiler/types.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index f5592560ed4..d3e119700b5 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -6776,7 +6776,7 @@ module ts { // and the right operand to be of type Any or a subtype of the 'Function' interface type. // The result is always of the Boolean primitive type. // NOTE: do not raise error if leftType is unknown as related error was already reported - if (!isTypeOfKind(leftType, TypeFlags.Any | TypeFlags.ObjectType | TypeFlags.TypeParameter)) { + if (isTypeOfKind(leftType, TypeFlags.Primitive)) { error(node.left, Diagnostics.The_left_hand_side_of_an_instanceof_expression_must_be_of_type_any_an_object_type_or_a_type_parameter); } // NOTE: do not raise error if right is unknown as related error was already reported diff --git a/src/compiler/types.ts b/src/compiler/types.ts index b8c842641d7..207006cdaf8 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -1286,6 +1286,7 @@ module ts { Unwidened = 0x00020000, // Unwidened type (is or contains Undefined or Null type) Intrinsic = Any | String | Number | Boolean | Void | Undefined | Null, + Primitive = String | Number | Boolean | Void | Undefined | Null | StringLiteral | Enum, StringLike = String | StringLiteral, NumberLike = Number | Enum, ObjectType = Class | Interface | Reference | Tuple | Anonymous, From 0410394de04ad02c06054aee5a554cb7655d6804 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sun, 25 Jan 2015 16:25:38 -0800 Subject: [PATCH 4/6] Modifying test --- .../reference/instanceofOperatorWithLHSIsObject.js | 7 ++++++- .../reference/instanceofOperatorWithLHSIsObject.types | 10 ++++++++++ .../instanceofOperatorWithLHSIsObject.ts | 4 +++- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/baselines/reference/instanceofOperatorWithLHSIsObject.js b/tests/baselines/reference/instanceofOperatorWithLHSIsObject.js index 5ccf195cd88..c687027ae9b 100644 --- a/tests/baselines/reference/instanceofOperatorWithLHSIsObject.js +++ b/tests/baselines/reference/instanceofOperatorWithLHSIsObject.js @@ -7,10 +7,13 @@ var x2: Function; var a: {}; var b: Object; var c: C; +var d: string | C; var r1 = a instanceof x1; var r2 = b instanceof x2; -var r3 = c instanceof x1; +var r3 = c instanceof x1; +var r4 = d instanceof x1; + //// [instanceofOperatorWithLHSIsObject.js] var C = (function () { @@ -23,6 +26,8 @@ var x2; var a; var b; var c; +var d; var r1 = a instanceof x1; var r2 = b instanceof x2; var r3 = c instanceof x1; +var r4 = d instanceof x1; diff --git a/tests/baselines/reference/instanceofOperatorWithLHSIsObject.types b/tests/baselines/reference/instanceofOperatorWithLHSIsObject.types index 26aea98028e..9962a23dced 100644 --- a/tests/baselines/reference/instanceofOperatorWithLHSIsObject.types +++ b/tests/baselines/reference/instanceofOperatorWithLHSIsObject.types @@ -20,6 +20,10 @@ var c: C; >c : C >C : C +var d: string | C; +>d : string | C +>C : C + var r1 = a instanceof x1; >r1 : boolean >a instanceof x1 : boolean @@ -38,3 +42,9 @@ var r3 = c instanceof x1; >c : C >x1 : any +var r4 = d instanceof x1; +>r4 : boolean +>d instanceof x1 : boolean +>d : string | C +>x1 : any + diff --git a/tests/cases/conformance/expressions/binaryOperators/instanceofOperator/instanceofOperatorWithLHSIsObject.ts b/tests/cases/conformance/expressions/binaryOperators/instanceofOperator/instanceofOperatorWithLHSIsObject.ts index 285335e40e7..6b88cd39f2a 100644 --- a/tests/cases/conformance/expressions/binaryOperators/instanceofOperator/instanceofOperatorWithLHSIsObject.ts +++ b/tests/cases/conformance/expressions/binaryOperators/instanceofOperator/instanceofOperatorWithLHSIsObject.ts @@ -6,7 +6,9 @@ var x2: Function; var a: {}; var b: Object; var c: C; +var d: string | C; var r1 = a instanceof x1; var r2 = b instanceof x2; -var r3 = c instanceof x1; \ No newline at end of file +var r3 = c instanceof x1; +var r4 = d instanceof x1; From 75404a7e00cc22a8873ec6eb9c7d92d6db7420c2 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Mon, 26 Jan 2015 13:09:23 -0800 Subject: [PATCH 5/6] Addressing CR feedback --- src/compiler/checker.ts | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index d3e119700b5..336bfab5ccb 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4469,14 +4469,16 @@ module ts { Debug.fail("should not get here"); } - // For a union type, remove all constituent types for which the given flags have the given state - function removeTypesFromUnionType(type: Type, maskFlags: TypeFlags, maskState: boolean): Type { + // For a union type, remove all constituent types for 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): Type { if (type.flags & TypeFlags.Union) { var types = (type).types; - if (forEach(types, t => !(t.flags & maskFlags) !== maskState)) { - var reducedType = getUnionType(filter(types, t => !(t.flags & maskFlags) === maskState)); - if (reducedType !== emptyObjectType) { - return reducedType; + 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 + var narrowedType = getUnionType(filter(types, t => !(t.flags & typeKind) === isOfTypeKind)); + if (narrowedType !== emptyObjectType) { + return narrowedType; } } } @@ -4687,7 +4689,8 @@ module ts { if (isTypeSubtypeOf(typeInfo.type, type)) { return typeInfo.type; } - // Otherwise, remove all types that aren't of the primitive type kind + // 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, typeInfo.flags, false); } else { @@ -4757,7 +4760,8 @@ module ts { return type; } - // Narrow the given type based on the given expression having the assumed boolean value + // Narrow the given type based on the given expression having the assumed boolean value. The returned type + // will be a subtype or the same type as the argument. function narrowType(type: Type, expr: Expression, assumeTrue: boolean): Type { switch (expr.kind) { case SyntaxKind.ParenthesizedExpression: From 35b2038af75bd80e4569c4896e50776e1ff0350a Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Mon, 26 Jan 2015 16:46:52 -0800 Subject: [PATCH 6/6] Addressing CR feedback --- src/compiler/checker.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 336bfab5ccb..0c4d6c2da73 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4469,7 +4469,7 @@ module ts { Debug.fail("should not get here"); } - // For a union type, remove all constituent types for that are of the given type kind (when isOfTypeKind is true) + // 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): Type { if (type.flags & TypeFlags.Union) { @@ -4683,7 +4683,7 @@ module ts { if (assumeTrue) { // Assumed result is true. If check was not for a primitive type, remove all primitive types if (!typeInfo) { - return removeTypesFromUnionType(type, TypeFlags.StringLike | TypeFlags.NumberLike | TypeFlags.Boolean, true); + return removeTypesFromUnionType(type, /*typeKind*/ TypeFlags.StringLike | TypeFlags.NumberLike | TypeFlags.Boolean, /*isOfTypeKind*/ true); } // Check was for a primitive type, return that primitive type if it is a subtype if (isTypeSubtypeOf(typeInfo.type, type)) { @@ -4691,12 +4691,12 @@ module ts { } // 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, typeInfo.flags, false); + return removeTypesFromUnionType(type, /*typeKind*/ typeInfo.flags, /*isOfTypeKind*/ false); } else { // Assumed result is false. If check was for a primitive type, remove that primitive type if (typeInfo) { - return removeTypesFromUnionType(type, typeInfo.flags, true); + return removeTypesFromUnionType(type, /*typeKind*/ typeInfo.flags, /*isOfTypeKind*/ true); } // Otherwise we don't have enough information to do anything. return type;