Skip to content

Commit

Permalink
Adding support for tuple types (e.g. [number, string])
Browse files Browse the repository at this point in the history
  • Loading branch information
@ahejlsberg
ahejlsberg committed Aug 11, 2014
1 parent c71e596 commit 5b25524
Show file tree
Hide file tree
Showing 4 changed files with 189 additions and 30 deletions.
129 changes: 102 additions & 27 deletions src/compiler/checker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ module ts {
var globalBooleanType: ObjectType;
var globalRegExpType: ObjectType;

var tupleTypes: Map<TupleType> = {};
var stringLiteralTypes: Map<StringLiteralType> = {};

var fullTypeCheck = false;
Expand Down Expand Up @@ -619,15 +620,13 @@ module ts {
}

function isOptionalProperty(propertySymbol: Symbol): boolean {
if (propertySymbol.flags & SymbolFlags.Prototype) {
return false;
}
// class C {
// constructor(public x?) { }
// }
//
// x is an optional parameter, but it is a required property.
return (propertySymbol.valueDeclaration.flags & NodeFlags.QuestionMark) && propertySymbol.valueDeclaration.kind !== SyntaxKind.Parameter;
return propertySymbol.valueDeclaration && propertySymbol.valueDeclaration.flags & NodeFlags.QuestionMark &&

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Aug 12, 2014

Contributor

I kind of like the original way better. It meant we knew the one precise case that .valueDeclaration would be undefined.
propertySymbol.valueDeclaration.kind !== SyntaxKind.Parameter;
}

function forEachSymbolTableInScope<T>(enclosingDeclaration: Node, callback: (symbolTable: SymbolTable) => T): T {
Expand Down Expand Up @@ -843,6 +842,9 @@ module ts {
else if (type.flags & (TypeFlags.Class | TypeFlags.Interface | TypeFlags.Enum | TypeFlags.TypeParameter)) {
writer.writeSymbol(type.symbol, enclosingDeclaration, SymbolFlags.Type);
}
else if (type.flags & TypeFlags.Tuple) {
writeTupleType(<TupleType>type);
}
else if (type.flags & TypeFlags.Anonymous) {
writeAnonymousType(<ObjectType>type, allowFunctionOrConstructorTypeLiteral);
}
Expand All @@ -855,6 +857,15 @@ module ts {
}
}

function writeTypeList(types: Type[]) {
for (var i = 0; i < types.length; i++) {
if (i > 0) {
writer.write(", ");
}
writeType(types[i], /*allowFunctionOrConstructorTypeLiteral*/ true);
}
}

function writeTypeReference(type: TypeReference) {
if (type.target === globalArrayType && !(flags & TypeFormatFlags.WriteArrayAsGenericType)) {
// If we are writing array element type the arrow style signatures are not allowed as
Expand All @@ -865,16 +876,17 @@ module ts {
else {
writer.writeSymbol(type.target.symbol, enclosingDeclaration, SymbolFlags.Type);
writer.write("<");
for (var i = 0; i < type.typeArguments.length; i++) {
if (i > 0) {
writer.write(", ");
}
writeType(type.typeArguments[i], /*allowFunctionOrConstructorTypeLiteral*/ true);
}
writeTypeList(type.typeArguments);
writer.write(">");
}
}

function writeTupleType(type: TupleType) {
writer.write("[");
writeTypeList(type.elementTypes);
writer.write("]");
}

function writeAnonymousType(type: ObjectType, allowFunctionOrConstructorTypeLiteral: boolean) {
// Always use 'typeof T' for type of class, enum, and module objects
if (type.symbol && type.symbol.flags & (SymbolFlags.Class | SymbolFlags.Enum | SymbolFlags.ValueModule)) {
Expand Down Expand Up @@ -1649,6 +1661,23 @@ module ts {
return [createSignature(undefined, classType.typeParameters, emptyArray, classType, 0, false, false)];
}

function createTupleTypeMemberSymbols(memberTypes: Type[]): SymbolTable {
var members: SymbolTable = {};
for (var i = 0; i < memberTypes.length; i++) {
var symbol = <TransientSymbol>createSymbol(SymbolFlags.Property | SymbolFlags.Transient, "" + i);
symbol.type = memberTypes[i];
members[i] = symbol;
}
return members;

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Aug 12, 2014

Contributor

Will we handle the Iterable case too?

This comment has been minimized.

Sign in to view

Copy link
@ahejlsberg

ahejlsberg Aug 12, 2014

Author Member

Not sure I understand what you mean. We will support destructuring of the form [x, y, z] = getSomeIterable() in the same way as an array, i.e. by inferring the element type of the iterable for x, y, and z. Is that what you're asking?

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Aug 12, 2014

Contributor

Yes, exactly. I guess that is more about destructuring, and is separate from tuple types.
}

function resolveTupleTypeMembers(type: TupleType) {
var arrayType = resolveObjectTypeMembers(createArrayType(getBestCommonType(type.elementTypes)));
var members = createTupleTypeMemberSymbols(type.elementTypes);
addInheritedMembers(members, arrayType.properties);
setObjectTypeMembers(type, members, arrayType.callSignatures, arrayType.constructSignatures, arrayType.stringIndexType, arrayType.numberIndexType);
}

function resolveAnonymousTypeMembers(type: ObjectType) {
var symbol = type.symbol;
var members = emptySymbols;
Expand Down Expand Up @@ -1682,6 +1711,9 @@ module ts {
else if (type.flags & TypeFlags.Anonymous) {
resolveAnonymousTypeMembers(<ObjectType>type);
}
else if (type.flags & TypeFlags.Tuple) {
resolveTupleTypeMembers(<TupleType>type);
}
else {
resolveTypeReferenceMembers(<TypeReference>type);
}
Expand Down Expand Up @@ -2123,6 +2155,24 @@ module ts {
return links.resolvedType;
}

function createTupleType(elementTypes: Type[]) {

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Aug 12, 2014

Contributor

getOrCreateTupleType

This comment has been minimized.

Sign in to view

Copy link
@ahejlsberg

ahejlsberg Aug 13, 2014

Author Member

I definitely don't like 'getOrCreateXXX'. Since we already use 'getXXX' everywhere for lazily created objects this should just be 'getTupleType'. And I suppose we should use 'createXXX' only when the function always creates a new object.

This comment has been minimized.

Sign in to view

Copy link
@ahejlsberg

ahejlsberg Aug 13, 2014

Author Member

Actually, on second thought I prefer sticking with 'createTupleType'. It is similar to 'createArrayType' and 'createTypeReference'. Common to all of them is that the returned object is immutable any may have come from a cache. I think it is perfectly fine to use 'createXXX' for such methods.
var id = getTypeListId(elementTypes);
var type = tupleTypes[id];
if (!type) {
type = tupleTypes[id] = <TupleType>createObjectType(TypeFlags.Tuple);
type.elementTypes = elementTypes;
}
return type;
}

function getTypeFromTupleTypeNode(node: TupleTypeNode): Type {
var links = getNodeLinks(node);
if (!links.resolvedType) {
links.resolvedType = createTupleType(map(node.elementTypes, t => getTypeFromTypeNode(t)));
}
return links.resolvedType;
}

function getTypeFromTypeLiteralNode(node: TypeLiteralNode): Type {
var links = getNodeLinks(node);
if (!links.resolvedType) {
Expand Down Expand Up @@ -2172,6 +2222,8 @@ module ts {
return getTypeFromTypeQueryNode(<TypeQueryNode>node);
case SyntaxKind.ArrayType:
return getTypeFromArrayTypeNode(<ArrayTypeNode>node);
case SyntaxKind.TupleType:
return getTypeFromTupleTypeNode(<TupleTypeNode>node);
case SyntaxKind.TypeLiteral:
return getTypeFromTypeLiteralNode(<TypeLiteralNode>node);
default:
Expand Down Expand Up @@ -2327,6 +2379,9 @@ module ts {
if (type.flags & TypeFlags.Reference) {
return createTypeReference((<TypeReference>type).target, instantiateList((<TypeReference>type).typeArguments, mapper, instantiateType));
}
if (type.flags & TypeFlags.Tuple) {
return createTupleType(instantiateList((<TupleType>type).elementTypes, mapper, instantiateType));
}
}
return type;
}
Expand Down Expand Up @@ -3015,20 +3070,16 @@ module ts {
while (isArrayType(type)) {
type = (<GenericType>type).typeArguments[0];
}

return type;
}

function getWidenedTypeOfArrayLiteral(type: Type): Type {
var elementType = (<TypeReference>type).typeArguments[0];
var widenedType = getWidenedType(elementType);

type = elementType !== widenedType ? createArrayType(widenedType) : type;

return type;
}

/* If we are widening on a literal, then we may need to the 'node' parameter for reporting purposes */
function getWidenedType(type: Type): Type {
if (type.flags & (TypeFlags.Undefined | TypeFlags.Null)) {
return anyType;
Expand Down Expand Up @@ -3125,9 +3176,9 @@ module ts {
inferFromTypes(sourceTypes[i], targetTypes[i]);
}
}
else if (source.flags & TypeFlags.ObjectType && (target.flags & TypeFlags.Reference || (target.flags & TypeFlags.Anonymous) &&
target.symbol && target.symbol.flags & (SymbolFlags.Method | SymbolFlags.TypeLiteral))) {
// If source is an object type, and target is a type reference, the type of a method, or a type literal, infer from members
else if (source.flags & TypeFlags.ObjectType && (target.flags & (TypeFlags.Reference | TypeFlags.Tuple) ||
(target.flags & TypeFlags.Anonymous) && target.symbol && target.symbol.flags & (SymbolFlags.Method | SymbolFlags.TypeLiteral))) {
// If source is an object type, and target is a type reference, a tuple type, the type of a method, or a type literal, infer from members

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Aug 12, 2014

Contributor

This needs to be broken up more. The comment just says the same thing as the code, but it's difficult to read

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Aug 12, 2014

Contributor

Specifically, I would extract it into a variable whose name describes the conditions you are checking for. Then put a comment describing the conditions you are excluding.

This comment has been minimized.

Sign in to view

Copy link
@ahejlsberg

ahejlsberg Aug 12, 2014

Author Member

I honestly don't see the problem here. The boolean expression isn't that complicated and there is a comment below that says what it does. Why would assigning the expression to a variable make it any easier to understand? Should there be variables for the preceding tests? If not, when is an expression simple enough to not warrant a variable?

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Aug 12, 2014

Contributor

3 reasons why I think it is difficult to read:

  1. I had to keep looking back and forth within the first line to correctly match parentheses with their partners. It took me several tries to correctly parse the precedence of the logical operators.
  2. It does not say which flags we deliberately excluded.
  3. It does not say the principle behind why we chose the flags we did.

The first one can be solved with a helper function or variable. The second and third can be solved with comments.
if (!isInProcess(source, target) && isWithinDepthLimit(source, sourceStack) && isWithinDepthLimit(target, targetStack)) {
if (depth === 0) {
sourceStack = [];
Expand Down Expand Up @@ -3574,7 +3625,19 @@ module ts {
function getContextualTypeForElementExpression(node: Expression): Type {
var arrayLiteral = <ArrayLiteral>node.parent;
var type = getContextualType(arrayLiteral);
return type ? getIndexTypeOfType(type, IndexKind.Number) : undefined;
if (type) {
if (type.flags & TypeFlags.Tuple) {
var index = indexOf(arrayLiteral.elements, node);
if (index >= 0) {

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Aug 12, 2014

Contributor

This should probably be an assert, not a check

This comment has been minimized.

Sign in to view

Copy link
@ahejlsberg

ahejlsberg Aug 13, 2014

Author Member

No need for a test or even an assert. I will just get rid of it.
var prop = getPropertyOfType(type, "" + index);
if (prop) {
return getTypeOfSymbol(prop);
}
}
}
return getIndexTypeOfType(type, IndexKind.Number);
}
return undefined;
}

function getContextualTypeForConditionalOperand(node: Expression): Type {
Expand Down Expand Up @@ -3633,17 +3696,23 @@ module ts {
}

function checkArrayLiteral(node: ArrayLiteral, contextualMapper?: TypeMapper): Type {
var contextualType = getContextualType(node);
var isTupleLiteral = contextualType && (contextualType.flags & TypeFlags.Tuple) !== 0;
var elementTypes: Type[] = [];
forEach(node.elements, element => {
if (element.kind !== SyntaxKind.OmittedExpression) {
var type = checkExpression(element, contextualMapper);
if (!contains(elementTypes, type)) elementTypes.push(type);
var type = element.kind !== SyntaxKind.OmittedExpression ? checkExpression(element, contextualMapper) : undefinedType;
if (isTupleLiteral || !contains(elementTypes, type)) {
elementTypes.push(type);
}
});
var contextualType = isInferentialContext(contextualMapper) ? undefined : getContextualType(node);
var contextualElementType = contextualType && getIndexTypeOfType(contextualType, IndexKind.Number);
if (isTupleLiteral) {
return createTupleType(elementTypes);
}
var contextualElementType = contextualType && !isInferentialContext(contextualMapper) ? getIndexTypeOfType(contextualType, IndexKind.Number) : undefined;
var elementType = getBestCommonType(elementTypes, contextualElementType, true);
if (!elementType) elementType = elementTypes.length ? emptyObjectType : undefinedType;
if (!elementType) {
elementType = elementTypes.length ? emptyObjectType : undefinedType;
}
return createArrayType(elementType);
}

Expand Down Expand Up @@ -3711,11 +3780,11 @@ module ts {
}

function getDeclarationKindFromSymbol(s: Symbol) {
return s.flags & SymbolFlags.Prototype ? SyntaxKind.Property : s.valueDeclaration.kind;
return s.valueDeclaration ? s.valueDeclaration.kind : SyntaxKind.Property;
}

function getDeclarationFlagsFromSymbol(s: Symbol) {
return s.flags & SymbolFlags.Prototype ? NodeFlags.Public | NodeFlags.Static : s.valueDeclaration.flags;
return s.valueDeclaration ? s.valueDeclaration.flags : s.flags & SymbolFlags.Prototype ? NodeFlags.Public | NodeFlags.Static : 0;
}

function checkPropertyAccess(node: PropertyAccess) {
Expand Down Expand Up @@ -4991,7 +5060,11 @@ module ts {
}

function checkArrayType(node: ArrayTypeNode) {
getTypeFromArrayTypeNode(node);
checkSourceElement(node.elementType);
}

function checkTupleType(node: TupleTypeNode) {
forEach(node.elementTypes, checkSourceElement);
}

function isPrivateWithinAmbient(node: Node): boolean {
Expand Down Expand Up @@ -6197,6 +6270,8 @@ module ts {
return checkTypeLiteral(<TypeLiteralNode>node);
case SyntaxKind.ArrayType:
return checkArrayType(<ArrayTypeNode>node);
case SyntaxKind.TupleType:
return checkTupleType(<TupleTypeNode>node);
case SyntaxKind.FunctionDeclaration:
return checkFunctionDeclaration(<FunctionDeclaration>node);
case SyntaxKind.Block:
Expand Down
20 changes: 20 additions & 0 deletions src/compiler/parser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,8 @@ module ts {
return children((<TypeLiteralNode>node).members);
case SyntaxKind.ArrayType:
return child((<ArrayTypeNode>node).elementType);
case SyntaxKind.TupleType:
return children((<TupleTypeNode>node).elementTypes);
case SyntaxKind.ArrayLiteral:
return children((<ArrayLiteral>node).elements);
case SyntaxKind.ObjectLiteral:
Expand Down Expand Up @@ -352,6 +354,7 @@ module ts {
Parameters, // Parameters in parameter list
TypeParameters, // Type parameters in type parameter list
TypeArguments, // Type arguments in type argument list
TupleElementTypes, // Element types in tuple element type list
Count // Number of parsing contexts
}

Expand Down Expand Up @@ -379,6 +382,7 @@ module ts {
case ParsingContext.Parameters: return Diagnostics.Parameter_declaration_expected;
case ParsingContext.TypeParameters: return Diagnostics.Type_parameter_declaration_expected;
case ParsingContext.TypeArguments: return Diagnostics.Type_argument_expected;
case ParsingContext.TupleElementTypes: return Diagnostics.Type_expected;
}
};

Expand Down Expand Up @@ -837,6 +841,7 @@ module ts {
case ParsingContext.Parameters:
return isParameter();
case ParsingContext.TypeArguments:
case ParsingContext.TupleElementTypes:
return isType();
}

Expand Down Expand Up @@ -872,6 +877,7 @@ module ts {
// Tokens other than ')' are here for better error recovery
return token === SyntaxKind.CloseParenToken || token === SyntaxKind.SemicolonToken;
case ParsingContext.ArrayLiteralMembers:
case ParsingContext.TupleElementTypes:
return token === SyntaxKind.CloseBracketToken;
case ParsingContext.Parameters:
// Tokens other than ')' and ']' (the latter for index signatures) are here for better error recovery
Expand Down Expand Up @@ -1390,6 +1396,17 @@ module ts {
return finishNode(node);
}

function parseTupleType(): TupleTypeNode {
var node = <TupleTypeNode>createNode(SyntaxKind.TupleType);
var startTokenPos = scanner.getTokenPos();
var startErrorCount = file.syntacticErrors.length;
node.elementTypes = parseBracketedList(ParsingContext.TupleElementTypes, parseType, SyntaxKind.OpenBracketToken, SyntaxKind.CloseBracketToken);
if (!node.elementTypes.length && file.syntacticErrors.length === startErrorCount) {
grammarErrorAtPos(startTokenPos, scanner.getStartPos() - startTokenPos, Diagnostics.Type_argument_list_cannot_be_empty);

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Aug 12, 2014

Contributor

This says "Type argument"!

This comment has been minimized.

Sign in to view

Copy link
@ahejlsberg

ahejlsberg Aug 13, 2014

Author Member

Didn't think it justified a new error message.

This comment has been minimized.

Sign in to view

Copy link
@danquirk

danquirk Aug 13, 2014

Member

The cost to add a new error message is very low and we should prioritize giving the best user experience by using the most specific error message we can in a given situation even if that means creating many unique errors

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Aug 13, 2014

Contributor

And it is certainly not a type argument.

This comment has been minimized.

Sign in to view

Copy link
@ahejlsberg

ahejlsberg Aug 13, 2014

Author Member

Just added a new error message.

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Aug 13, 2014

Contributor

Thanks
}
return finishNode(node);
}

function parseFunctionType(signatureKind: SyntaxKind): TypeLiteralNode {
var node = <TypeLiteralNode>createNode(SyntaxKind.TypeLiteral);
var member = <SignatureDeclaration>createNode(signatureKind);
Expand Down Expand Up @@ -1420,6 +1437,8 @@ module ts {
return parseTypeQuery();
case SyntaxKind.OpenBraceToken:
return parseTypeLiteral();
case SyntaxKind.OpenBracketToken:
return parseTupleType();
case SyntaxKind.OpenParenToken:
case SyntaxKind.LessThanToken:
return parseFunctionType(SyntaxKind.CallSignature);
Expand All @@ -1443,6 +1462,7 @@ module ts {
case SyntaxKind.VoidKeyword:
case SyntaxKind.TypeOfKeyword:
case SyntaxKind.OpenBraceToken:
case SyntaxKind.OpenBracketToken:
case SyntaxKind.LessThanToken:
case SyntaxKind.NewKeyword:
return true;
Expand Down
17 changes: 14 additions & 3 deletions src/compiler/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ module ts {
TypeQuery,
TypeLiteral,
ArrayType,
TupleType,
// Expression
ArrayLiteral,
ObjectLiteral,
Expand Down Expand Up @@ -316,6 +317,10 @@ module ts {
elementType: TypeNode;
}

export interface TupleTypeNode extends TypeNode {
elementTypes: NodeArray<TypeNode>;
}

export interface StringLiteralTypeNode extends TypeNode {
text: string;
}
Expand Down Expand Up @@ -791,13 +796,14 @@ module ts {
Class = 0x00000400, // Class
Interface = 0x00000800, // Interface
Reference = 0x00001000, // Generic type reference
Anonymous = 0x00002000, // Anonymous
FromSignature = 0x00004000, // Created for signature assignment check
Tuple = 0x00002000, // Tuple
Anonymous = 0x00004000, // Anonymous
FromSignature = 0x00008000, // Created for signature assignment check

Intrinsic = Any | String | Number | Boolean | Void | Undefined | Null,
StringLike = String | StringLiteral,
NumberLike = Number | Enum,
ObjectType = Class | Interface | Reference | Anonymous
ObjectType = Class | Interface | Reference | Tuple | Anonymous
}

// Properties common to all types
Expand Down Expand Up @@ -850,6 +856,11 @@ module ts {
openReferenceChecks: Map<boolean>; // Open type reference check cache
}

export interface TupleType extends ObjectType {
elementTypes: Type[]; // Element types
baseArrayType: TypeReference; // Array<T> where T is best common type of element types
}

// Resolved object type
export interface ResolvedObjectType extends ObjectType {
members: SymbolTable; // Properties by name
Expand Down
Loading

0 comments on commit 5b25524

Please sign in to comment.