Skip to content

Commit

Permalink
Less aggressive subtype reduction in union types
Browse files Browse the repository at this point in the history
  • Loading branch information
ahejlsberg committed Oct 10, 2014
1 parent 5c661ba commit 483afea
Showing 1 changed file with 37 additions and 11 deletions.
48 changes: 37 additions & 11 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2066,7 +2066,7 @@ module ts {
}

function resolveTupleTypeMembers(type: TupleType) {
var arrayType = resolveObjectTypeMembers(createArrayType(getBestCommonType(type.elementTypes)));
var arrayType = resolveObjectTypeMembers(createArrayType(getUnionType(type.elementTypes)));
var members = createTupleTypeMemberSymbols(type.elementTypes);
addInheritedMembers(members, arrayType.properties);
setObjectTypeMembers(type, members, arrayType.callSignatures, arrayType.constructSignatures, arrayType.stringIndexType, arrayType.numberIndexType);
Expand Down Expand Up @@ -2716,13 +2716,41 @@ module ts {
}
}

function getUnionType(types: Type[]): Type {
function containsAnyType(types: Type[]) {

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Oct 11, 2014

Contributor

containsTypeAny

for (var i = 0; i < types.length; i++) {
if (types[i].flags & TypeFlags.Any) {
return true;
}
}
return false;
}

function removeAllButLast(types: Type[], typeToRemove: Type) {
var i = types.length;
while (i > 0 && types.length > 1) {
i--;
if (types[i] === typeToRemove) {
types.splice(i, 1);
}
}
}

function getUnionType(types: Type[], noSubtypeReduction?: boolean): Type {

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Oct 11, 2014

Contributor

I would make it required. Also, I would invert it to performSubtypeReduction so it's affirmative (I find it clearer)

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Oct 11, 2014

Contributor

Please add a comment explaining when this should be true and when it should be false, and why

if (types.length === 0) {
return emptyObjectType;
}
var sortedTypes: Type[] = [];
addTypesToSortedSet(sortedTypes, types);
removeSubtypes(sortedTypes);
if (noSubtypeReduction) {
if (containsAnyType(sortedTypes)) {
return anyType;
}
removeAllButLast(sortedTypes, undefinedType);
removeAllButLast(sortedTypes, nullType);
}
else {
removeSubtypes(sortedTypes);
}
if (sortedTypes.length === 1) {
return sortedTypes[0];
}
Expand All @@ -2738,7 +2766,7 @@ module ts {
function getTypeFromUnionTypeNode(node: UnionTypeNode): Type {
var links = getNodeLinks(node);
if (!links.resolvedType) {
links.resolvedType = getUnionType(map(node.types, getTypeFromTypeNode));
links.resolvedType = getUnionType(map(node.types, getTypeFromTypeNode), /*noSubtypeReduction*/ true);
}
return links.resolvedType;
}
Expand Down Expand Up @@ -2956,7 +2984,7 @@ module ts {
return createTupleType(instantiateList((<TupleType>type).elementTypes, mapper, instantiateType));
}
if (type.flags & TypeFlags.Union) {
return getUnionType(instantiateList((<UnionType>type).types, mapper, instantiateType));
return getUnionType(instantiateList((<UnionType>type).types, mapper, instantiateType), /*noSubtypeReduction*/ true);
}
}
return type;
Expand Down Expand Up @@ -3606,7 +3634,7 @@ module ts {
return forEach(types, t => isSupertypeOfEach(t, types) ? t : undefined);
}

function getBestCommonType(types: Type[], contextualType?: Type): Type {
function getBestCommonType(types: Type[], contextualType: Type): Type {
return contextualType && isSupertypeOfEach(contextualType, types) ? contextualType : getUnionType(types); }

function isTypeOfObjectLiteral(type: Type): boolean {
Expand Down Expand Up @@ -4558,7 +4586,7 @@ module ts {
return createTupleType(elementTypes);
}
var contextualElementType = contextualType && !isInferentialContext(contextualMapper) ? getIndexTypeOfType(contextualType, IndexKind.Number) : undefined;
var elementType = elements.length || contextualElementType ? getBestCommonType(deduplicate(elementTypes), contextualElementType) : undefinedType;
var elementType = elements.length || contextualElementType ? getBestCommonType(elementTypes, contextualElementType) : undefinedType;
return createArrayType(elementType);
}

Expand Down Expand Up @@ -5601,7 +5629,7 @@ module ts {
case SyntaxKind.AmpersandAmpersandToken:
return rightType;
case SyntaxKind.BarBarToken:
return getBestCommonType([leftType, rightType], isInferentialContext(contextualMapper) ? undefined : getContextualType(node));
return getUnionType([leftType, rightType]);
case SyntaxKind.EqualsToken:
checkAssignmentOperator(rightType);
return rightType;
Expand Down Expand Up @@ -5651,9 +5679,7 @@ module ts {
checkExpression(node.condition);
var type1 = checkExpression(node.whenTrue, contextualMapper);
var type2 = checkExpression(node.whenFalse, contextualMapper);
var contextualType = isInferentialContext(contextualMapper) ? undefined : getContextualType(node);
var resultType = getBestCommonType([type1, type2], contextualType);
return resultType;
return getUnionType([type1, type2]);
}

function checkExpressionWithContextualType(node: Expression, contextualType: Type, contextualMapper?: TypeMapper): Type {
Expand Down

0 comments on commit 483afea

Please sign in to comment.