Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic type aliases #3397

Merged
merged 8 commits into from
Jun 9, 2015
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ module ts {
case SyntaxKind.ArrowFunction:
case SyntaxKind.ModuleDeclaration:
case SyntaxKind.SourceFile:
case SyntaxKind.TypeAliasDeclaration:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a container with locals, but classes and interfaces are not?

return ContainerFlags.IsContainerWithLocals;

case SyntaxKind.CatchClause:
Expand Down Expand Up @@ -424,6 +425,7 @@ module ts {
case SyntaxKind.FunctionDeclaration:
case SyntaxKind.FunctionExpression:
case SyntaxKind.ArrowFunction:
case SyntaxKind.TypeAliasDeclaration:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this get its type parameters added to .locals, but Class and Interface do not? Are the type parameters added to the locals for generic signatures as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type parameters of classes and interfaces are in the members collection, whereas type parameters of generic signatures are in the locals collection. Since a type alias doesn't have any notion of members, locals seems like the best choice. Also, it is a bit less work in resolveName because we don't have to add an extra case to the switch (we look for locals on every ancestor node).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move the class and interface type parameters to the locals as well (and create a locals if we don't have one already)? Seems really strange that they are stored in the members, and certainly not consistent with type aliases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I'd like to leave it this way. We wouldn't gain anything from the move, in fact it would just introduce another symbol table that needs to be visited. Also, the spec actually calls for type parameters and members to be in the same declaration space (even though it isn't observable right now because all members are values, but it would be if we ever support local types in classes and/or interfaces).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always thought of locals as entities that could be accessed without qualification (they are in scope), whereas members require qualification (they are not in scope). This is why it seemed natural that type parameters would be locals, while class elements would be members. If that is not the difference between locals and members, what is the difference?

// All the children of these container types are never visible through another
// symbol (i.e. through another symbol's 'exports' or 'members'). Instead,
// they're only accessed 'lexically' (i.e. from code that exists underneath
Expand Down
140 changes: 79 additions & 61 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2577,8 +2577,8 @@ module ts {
function getLocalTypeParametersOfClassOrInterface(symbol: Symbol): TypeParameter[] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or type alias

let result: TypeParameter[];
for (let node of symbol.declarations) {
if (node.kind === SyntaxKind.InterfaceDeclaration || node.kind === SyntaxKind.ClassDeclaration) {
let declaration = <InterfaceDeclaration>node;
if (node.kind === SyntaxKind.InterfaceDeclaration || node.kind === SyntaxKind.ClassDeclaration || node.kind === SyntaxKind.TypeAliasDeclaration) {
let declaration = <InterfaceDeclaration | TypeAliasDeclaration>node;
if (declaration.typeParameters) {
result = appendTypeParameters(result, declaration.typeParameters);
}
Expand All @@ -2593,6 +2593,15 @@ module ts {
return concatenate(getOuterTypeParametersOfClassOrInterface(symbol), getLocalTypeParametersOfClassOrInterface(symbol));
}

function getTypeParametersOfTypeAlias(symbol: Symbol): TypeParameter[] {
let links = getSymbolLinks(symbol);
if (!links.typeParameters) {
let declaration = <TypeAliasDeclaration>getDeclarationOfKind(symbol, SyntaxKind.TypeAliasDeclaration);
links.typeParameters = declaration.typeParameters ? appendTypeParameters(undefined, declaration.typeParameters) : emptyArray;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the outer type parameters? Does a type alias not need them? It would be good to have a comment explaining why.

}
return links.typeParameters;
}

function getBaseTypes(type: InterfaceType): ObjectType[] {
let typeWithBaseTypes = <InterfaceTypeWithBaseTypes>type;
if (!typeWithBaseTypes.baseTypes) {
Expand Down Expand Up @@ -3513,72 +3522,79 @@ module ts {
}
}

function getTypeFromTypeReferenceOrExpressionWithTypeArguments(node: TypeReferenceNode | ExpressionWithTypeArguments): Type {
let links = getNodeLinks(node);
if (!links.resolvedType) {
let type: Type;

// We don't currently support heritage clauses with complex expressions in them.
// For these cases, we just set the type to be the unknownType.
if (node.kind !== SyntaxKind.ExpressionWithTypeArguments || isSupportedExpressionWithTypeArguments(<ExpressionWithTypeArguments>node)) {
let typeNameOrExpression = node.kind === SyntaxKind.TypeReference
? (<TypeReferenceNode>node).typeName
: (<ExpressionWithTypeArguments>node).expression;

let symbol = resolveEntityName(typeNameOrExpression, SymbolFlags.Type);
if (symbol) {
if ((symbol.flags & SymbolFlags.TypeParameter) && isTypeParameterReferenceIllegalInConstraint(node, symbol)) {
// TypeScript 1.0 spec (April 2014): 3.4.1
// Type parameters declared in a particular type parameter list
// may not be referenced in constraints in that type parameter list
// Implementation: such type references are resolved to 'unknown' type that usually denotes error
type = unknownType;
}
else {
type = createTypeReferenceIfGeneric(
getDeclaredTypeOfSymbol(symbol),
node, node.typeArguments);
}
}
function getTypeFromClassOrInterfaceReference(node: TypeReferenceNode | ExpressionWithTypeArguments, symbol: Symbol): Type {
let type = getDeclaredTypeOfSymbol(symbol);
let typeParameters = (<InterfaceType>type).localTypeParameters;
if (typeParameters) {
if (!node.typeArguments || node.typeArguments.length !== typeParameters.length) {
error(node, Diagnostics.Generic_type_0_requires_1_type_argument_s, typeToString(type, /*enclosingDeclaration*/ undefined, TypeFormatFlags.WriteArrayAsGenericType), typeParameters.length);
return unknownType;
}

links.resolvedType = type || unknownType;
}

return links.resolvedType;
}

function createTypeReferenceIfGeneric(type: Type, node: Node, typeArguments: NodeArray<TypeNode>): Type {
if (type.flags & (TypeFlags.Class | TypeFlags.Interface) && type.flags & TypeFlags.Reference) {
// In a type reference, the outer type parameters of the referenced class or interface are automatically
// supplied as type arguments and the type reference only specifies arguments for the local type parameters
// of the class or interface.
let localTypeParameters = (<InterfaceType>type).localTypeParameters;
let expectedTypeArgCount = localTypeParameters ? localTypeParameters.length : 0;
let typeArgCount = typeArguments ? typeArguments.length : 0;
if (typeArgCount === expectedTypeArgCount) {
// When no type arguments are expected we already have the right type because all outer type parameters
// have themselves as default type arguments.
if (typeArgCount) {
return createTypeReference(<GenericType>type, concatenate((<InterfaceType>type).outerTypeParameters,
map(typeArguments, getTypeFromTypeNode)));
}
return createTypeReference(<GenericType>type, concatenate((<InterfaceType>type).outerTypeParameters,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happened to our fun optimization?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is still there, I just simplified the code. If the declared type has no local type parameters, we simply return the declared type (which may still have outer type parameters).

map(node.typeArguments, getTypeFromTypeNode)));
}
if (node.typeArguments) {
error(node, Diagnostics.Type_0_is_not_generic, typeToString(type));
return unknownType;
}
return type;
}

function getTypeFromTypeAliasReference(node: TypeReferenceNode | ExpressionWithTypeArguments, symbol: Symbol): Type {
let type = getDeclaredTypeOfSymbol(symbol);
let typeParameters = getTypeParametersOfTypeAlias(symbol);
if (typeParameters.length) {
if (!node.typeArguments || node.typeArguments.length !== typeParameters.length) {
error(node, Diagnostics.Generic_type_0_requires_1_type_argument_s, symbolToString(symbol), typeParameters.length);
return unknownType;
}
else {
error(node, Diagnostics.Generic_type_0_requires_1_type_argument_s, typeToString(type, /*enclosingDeclaration*/ undefined, TypeFormatFlags.WriteArrayAsGenericType), expectedTypeArgCount);
return undefined;
let typeArguments = map(node.typeArguments, getTypeFromTypeNode);
let id = getTypeListId(typeArguments);
let links = getSymbolLinks(symbol);
if (!links.instantiations) {
links.instantiations = {};
// The declared type corresponds to an instantiation with its own type parameters as type arguments
links.instantiations[getTypeListId(typeParameters)] = type;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this first instantiation (with its own type parameters) be done when you process the type alias declaration itself? Like in getDeclaredTypeOfTypeAlias?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good point.

}
return links.instantiations[id] || (links.instantiations[id] = instantiateType(type, createTypeMapper(typeParameters, typeArguments)));
}
else {
if (typeArguments) {
error(node, Diagnostics.Type_0_is_not_generic, typeToString(type));
return undefined;
}
if (node.typeArguments) {
error(node, Diagnostics.Type_0_is_not_generic, symbolToString(symbol));
return unknownType;
}

return type;
}

function getTypeFromTypeParameterReference(node: TypeReferenceNode | ExpressionWithTypeArguments, symbol: Symbol): Type {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this really need to take the node and the symbol separately?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, no. But since we need both, we'd have to turn around and do work to find the other.

// TypeScript 1.0 spec (April 2014): 3.4.1
// Type parameters declared in a particular type parameter list
// may not be referenced in constraints in that type parameter list
// Implementation: such type references are resolved to 'unknown' type that usually denotes error
return isTypeParameterReferenceIllegalInConstraint(node, symbol) ? unknownType : getDeclaredTypeOfSymbol(symbol);
}

function getTypeFromTypeReference(node: TypeReferenceNode | ExpressionWithTypeArguments): Type {
let links = getNodeLinks(node);
if (!links.resolvedType) {
// We only support expressions that are simple qualified names. For other expressions this produces undefined. let typeNameOrExpression = node.kind === SyntaxKind.TypeReference ? (<TypeReferenceNode>node).typeName :
isSupportedExpressionWithTypeArguments(<ExpressionWithTypeArguments>node) ? (<ExpressionWithTypeArguments>node).expression :
undefined;
let symbol = typeNameOrExpression && resolveEntityName(typeNameOrExpression, SymbolFlags.Type) || unknownSymbol;
let type = symbol.flags & (SymbolFlags.Class | SymbolFlags.Interface) ? getTypeFromClassOrInterfaceReference(node, symbol) :
symbol.flags & SymbolFlags.TypeParameter ? getTypeFromTypeParameterReference(node, symbol) :
symbol.flags & SymbolFlags.TypeAlias ? getTypeFromTypeAliasReference(node, symbol) :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do we check that the type parameter reference does not have type arguments?

getDeclaredTypeOfSymbol(symbol);
// Cache both the resolved symbol and the resolved type. The resolved symbol is needed in when we check the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we may now accidentally allow type arguments to enums, aliases, and type parameters.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. Will fix. Guess we have no tests for that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not surprised. It is not an obvious case to test. Can you add tests for this as well?

// type reference in checkTypeReferenceOrExpressionWithTypeArguments.
links.resolvedSymbol = symbol;
links.resolvedType = type;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resolvedSymbol should already be cached on the node links of typeNameOrExpression. I would say just call resolveEntityName on that in checkTypeReferenceOrExpressionWithTypeArguments. Seems more elegant than special casing this here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I do see why you did it that way.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the resolved symbol isn't cached by resolveEntityName, so we'd end up doing the work twice.

}
return links.resolvedType;
}

function getTypeFromTypeQueryNode(node: TypeQueryNode): Type {
let links = getNodeLinks(node);
if (!links.resolvedType) {
Expand Down Expand Up @@ -3848,9 +3864,9 @@ module ts {
case SyntaxKind.StringLiteral:
return getTypeFromStringLiteral(<StringLiteral>node);
case SyntaxKind.TypeReference:
return getTypeFromTypeReferenceOrExpressionWithTypeArguments(<TypeReferenceNode>node);
return getTypeFromTypeReference(<TypeReferenceNode>node);
case SyntaxKind.ExpressionWithTypeArguments:
return getTypeFromTypeReferenceOrExpressionWithTypeArguments(<ExpressionWithTypeArguments>node);
return getTypeFromTypeReference(<ExpressionWithTypeArguments>node);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this diff is wrong.

case SyntaxKind.TypeQuery:
return getTypeFromTypeQueryNode(<TypeQueryNode>node);
case SyntaxKind.ArrayType:
Expand Down Expand Up @@ -8766,13 +8782,15 @@ module ts {
// Grammar checking
checkGrammarTypeArguments(node, node.typeArguments);

let type = getTypeFromTypeReferenceOrExpressionWithTypeArguments(node);
let type = getTypeFromTypeReference(node);
if (type !== unknownType && node.typeArguments) {
// Do type argument local checks only if referenced type is successfully resolved
let symbol = getNodeLinks(node).resolvedSymbol;
let typeParameters = symbol.flags & SymbolFlags.TypeAlias ? getTypeParametersOfTypeAlias(symbol) : (<TypeReference>type).target.localTypeParameters;
let len = node.typeArguments.length;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This diff is completely incorrect.

for (let i = 0; i < len; i++) {
checkSourceElement(node.typeArguments[i]);
let constraint = getConstraintOfTypeParameter((<TypeReference>type).target.typeParameters[i]);
let constraint = getConstraintOfTypeParameter(typeParameters[i]);
if (produceDiagnostics && constraint) {
let typeArgument = (<TypeReference>type).typeArguments[i];
checkTypeAssignableTo(typeArgument, constraint, node, Diagnostics.Type_0_does_not_satisfy_the_constraint_1);
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ module ts {
return visitNodes(cbNodes, node.decorators) ||
visitNodes(cbNodes, node.modifiers) ||
visitNode(cbNode, (<TypeAliasDeclaration>node).name) ||
visitNodes(cbNodes, (<TypeAliasDeclaration>node).typeParameters) ||
visitNode(cbNode, (<TypeAliasDeclaration>node).type);
case SyntaxKind.EnumDeclaration:
return visitNodes(cbNodes, node.decorators) ||
Expand Down Expand Up @@ -4547,6 +4548,7 @@ module ts {
setModifiers(node, modifiers);
parseExpected(SyntaxKind.TypeKeyword);
node.name = parseIdentifier();
node.typeParameters = parseTypeParameters();
parseExpected(SyntaxKind.EqualsToken);
node.type = parseType();
parseSemicolon();
Expand Down
5 changes: 4 additions & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,7 @@ module ts {

export interface TypeAliasDeclaration extends Declaration, Statement {
name: Identifier;
typeParameters?: NodeArray<TypeParameterDeclaration>;
type: TypeNode;
}

Expand Down Expand Up @@ -1525,7 +1526,9 @@ module ts {
export interface SymbolLinks {
target?: Symbol; // Resolved (non-alias) target of an alias
type?: Type; // Type of value symbol
declaredType?: Type; // Type of class, interface, enum, or type parameter
declaredType?: Type; // Type of class, interface, enum, type alias, or type parameter
typeParameters?: TypeParameter[]; // Type parameters of type alias (empty array if none)
instantiations?: Map<Type>; // Instantiations of generic type alias
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to move the typeParameters and instantiations here for classes and interfaces as well? Seems like it would be nice if they were all in one place.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will think about it. It is generally more convenient to have them on the Type object because that's what we're operating on in most cases. Getting back to the SymbolLinks from the type requires us to do getSymbolLinks(type.symbol) which adds a bit of overhead.

mapper?: TypeMapper; // Type mapper for instantiation alias
referenced?: boolean; // True if alias symbol has been referenced as a value
unionType?: UnionType; // Containing union type for union property
Expand Down
120 changes: 120 additions & 0 deletions tests/baselines/reference/genericTypeAliases.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
//// [genericTypeAliases.ts]
type Tree<T> = T | { left: Tree<T>, right: Tree<T> };

var tree: Tree<number> = {
left: {
left: 0,
right: {
left: 1,
right: 2
},
},
right: 3
};

type Lazy<T> = T | (() => T);

var ls: Lazy<string>;
ls = "eager";
ls = () => "lazy";

type Foo<T> = T | { x: Foo<T> };
type Bar<U> = U | { x: Bar<U> };

// Deeply instantiated generics
var x: Foo<string>;
var y: Bar<string>;
x = y;
y = x;

x = "string";
x = { x: "hello" };
x = { x: { x: "world" } };

var z: Foo<number>;
z = 42;
z = { x: 42 };
z = { x: { x: 42 } };

type Strange<T> = string; // Type parameter not used
var s: Strange<number>;
s = "hello";

interface Tuple<A, B> {
a: A;
b: B;
}

type Pair<T> = Tuple<T, T>;

interface TaggedPair<T> extends Pair<T> {
tag: string;
}

var p: TaggedPair<number>;
p.a = 1;
p.b = 2;
p.tag = "test";

function f<A>() {
type Foo<T> = T | { x: Foo<T> };
var x: Foo<A[]>;
return x;
}

function g<B>() {
type Bar<U> = U | { x: Bar<U> };
var x: Bar<B[]>;
return x;
}

// Deeply instantiated generics
var a = f<string>();
var b = g<string>();
a = b;


//// [genericTypeAliases.js]
var tree = {
left: {
left: 0,
right: {
left: 1,
right: 2
}
},
right: 3
};
var ls;
ls = "eager";
ls = function () { return "lazy"; };
// Deeply instantiated generics
var x;
var y;
x = y;
y = x;
x = "string";
x = { x: "hello" };
x = { x: { x: "world" } };
var z;
z = 42;
z = { x: 42 };
z = { x: { x: 42 } };
var s;
s = "hello";
var p;
p.a = 1;
p.b = 2;
p.tag = "test";
function f() {
var x;
return x;
}
function g() {
var x;
return x;
}
// Deeply instantiated generics
var a = f();
var b = g();
a = b;
Loading