Skip to content

Correct this in tuple type parameter constraints #10234

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Aug 11, 2016
Merged

Correct this in tuple type parameter constraints #10234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7e115bb
Use correct this in tuple type parameter constraints
sandersn Aug 9, 2016
d34bbe5
Add explanatory comment to resolveTupleTypeMembers
sandersn Aug 9, 2016
6a8f4cb
Delay tuple type constraint resolution
sandersn Aug 9, 2016
80963ba
Always use thisType when generating tuple id
sandersn Aug 9, 2016
4087808
Fix single-quote lint
sandersn Aug 10, 2016
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
17 changes: 11 additions & 6 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1538,8 +1538,8 @@ namespace ts {

function createType(flags: TypeFlags): Type {
const result = new Type(checker, flags);
result.id = typeCount;
typeCount++;
result.id = typeCount;
return result;
}

Expand Down Expand Up @@ -4000,6 +4000,9 @@ namespace ts {
return createTypeReference((<TypeReference>type).target,
concatenate((<TypeReference>type).typeArguments, [thisArgument || (<TypeReference>type).target.thisType]));
}
if (type.flags & TypeFlags.Tuple) {
return createTupleType((type as TupleType).elementTypes, thisArgument);
Copy link
Member

Choose a reason for hiding this comment

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

This isn't the right approach. You need to create a new tuple type that records its associated this-type, and then use that this-type when (and if) the members are resolved. Member resolution is lazy, but cannot change once it has been performed. The way you have it now, you might be stepping on information that was already recorded by a previous call to resolveTypeMembers.

Copy link
Member Author

Choose a reason for hiding this comment

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

How do I create a new tuple type safely? Can I just say createNewTupleType(type.elementTypes)?

And once I have the tuple type, I think I'll need to store thisType in a separate property so that resolveTupleTypeMembers can use it later, right? (I guess I could still immediately call resultTupleTypeMembers like I do in the current commit.)

Copy link
Member

Choose a reason for hiding this comment

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

You basically have to change createTupleType and the TupleType type to record the this-type information alongside the element types. Also, it would need to be part of the key used for the tuple type in the cache. Once you do that you can then call createTupleType from getTypeWithThisArgument passing along the appropriate this-type.

Copy link
Member

Choose a reason for hiding this comment

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

Don't immediately resolve the members, that defeats the whole purpose of the implementation.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, done.

}
return type;
}

Expand Down Expand Up @@ -4103,7 +4106,8 @@ namespace ts {
function resolveTupleTypeMembers(type: TupleType) {
const arrayElementType = getUnionType(type.elementTypes);
// Make the tuple type itself the 'this' type by including an extra type argument
const arrayType = resolveStructuredTypeMembers(createTypeFromGenericGlobalType(globalArrayType, [arrayElementType, type]));
// (Unless it's provided in the case that the tuple is a type parameter constraint)
const arrayType = resolveStructuredTypeMembers(createTypeFromGenericGlobalType(globalArrayType, [arrayElementType, type.thisType || type]));
const members = createTupleTypeMemberSymbols(type.elementTypes);
addInheritedMembers(members, arrayType.properties);
setObjectTypeMembers(type, members, arrayType.callSignatures, arrayType.constructSignatures, arrayType.stringIndexInfo, arrayType.numberIndexInfo);
Expand Down Expand Up @@ -5230,15 +5234,16 @@ namespace ts {
return links.resolvedType;
}

function createTupleType(elementTypes: Type[]) {
const id = getTypeListId(elementTypes);
return tupleTypes[id] || (tupleTypes[id] = createNewTupleType(elementTypes));
function createTupleType(elementTypes: Type[], thisType?: Type) {
const id = getTypeListId(elementTypes) + "," + (thisType ? thisType.id : 0);
return tupleTypes[id] || (tupleTypes[id] = createNewTupleType(elementTypes, thisType));
Copy link
Member

@ahejlsberg ahejlsberg Aug 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

You need to always append an extra id no matter whether you have a this-type or not. Otherwise you end up confusing tuples with a this-type with next-higher-arity tuples without a this-type. I suggest always using

let id = getTypeListId(elementTypes) + "," + (thisType ? thisType.id : 0);

Then also change createType to increment typeCount before assigning it to the new type's id property.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

}

function createNewTupleType(elementTypes: Type[]) {
function createNewTupleType(elementTypes: Type[], thisType?: Type) {
const propagatedFlags = getPropagatingFlagsOfTypes(elementTypes, /*excludeKinds*/ 0);
const type = <TupleType>createObjectType(TypeFlags.Tuple | propagatedFlags);
type.elementTypes = elementTypes;
type.thisType = thisType;
return type;
}

Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2371,6 +2371,7 @@ namespace ts {

export interface TupleType extends ObjectType {
elementTypes: Type[]; // Element types
thisType?: Type; // This-type of tuple (only needed for tuples that are constraints of type parameters)
}

export interface UnionOrIntersectionType extends Type {
Expand Down
29 changes: 29 additions & 0 deletions tests/baselines/reference/thisInTupleTypeParameterConstraints.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//// [thisInTupleTypeParameterConstraints.ts]
/// <reference no-default-lib="true"/>

interface Boolean {}
interface IArguments {}
interface Function {}
interface Number {}
interface RegExp {}
interface Object {}
interface String {}

interface Array<T> {
// 4 methods will run out of memory if this-types are not instantiated
// correctly for tuple types that are type parameter constraints
map<U>(arg: this): void;
reduceRight<U>(arg: this): void;
reduce<U>(arg: this): void;
reduce2<U>(arg: this): void;
}

declare function f<T extends [(x: number) => number]>(a: T): void;
let x: [(x: number) => number];
f(x);


//// [thisInTupleTypeParameterConstraints.js]
/// <reference no-default-lib="true"/>
var x;
f(x);
66 changes: 66 additions & 0 deletions tests/baselines/reference/thisInTupleTypeParameterConstraints.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
=== tests/cases/compiler/thisInTupleTypeParameterConstraints.ts ===
/// <reference no-default-lib="true"/>

interface Boolean {}
>Boolean : Symbol(Boolean, Decl(thisInTupleTypeParameterConstraints.ts, 0, 0))

interface IArguments {}
>IArguments : Symbol(IArguments, Decl(thisInTupleTypeParameterConstraints.ts, 2, 20))

interface Function {}
>Function : Symbol(Function, Decl(thisInTupleTypeParameterConstraints.ts, 3, 23))

interface Number {}
>Number : Symbol(Number, Decl(thisInTupleTypeParameterConstraints.ts, 4, 21))

interface RegExp {}
>RegExp : Symbol(RegExp, Decl(thisInTupleTypeParameterConstraints.ts, 5, 19))

interface Object {}
>Object : Symbol(Object, Decl(thisInTupleTypeParameterConstraints.ts, 6, 19))

interface String {}
>String : Symbol(String, Decl(thisInTupleTypeParameterConstraints.ts, 7, 19))

interface Array<T> {
>Array : Symbol(Array, Decl(thisInTupleTypeParameterConstraints.ts, 8, 19))
>T : Symbol(T, Decl(thisInTupleTypeParameterConstraints.ts, 10, 16))

// 4 methods will run out of memory if this-types are not instantiated
// correctly for tuple types that are type parameter constraints
map<U>(arg: this): void;
>map : Symbol(Array.map, Decl(thisInTupleTypeParameterConstraints.ts, 10, 20))
>U : Symbol(U, Decl(thisInTupleTypeParameterConstraints.ts, 13, 8))
>arg : Symbol(arg, Decl(thisInTupleTypeParameterConstraints.ts, 13, 11))

reduceRight<U>(arg: this): void;
>reduceRight : Symbol(Array.reduceRight, Decl(thisInTupleTypeParameterConstraints.ts, 13, 28))
>U : Symbol(U, Decl(thisInTupleTypeParameterConstraints.ts, 14, 16))
>arg : Symbol(arg, Decl(thisInTupleTypeParameterConstraints.ts, 14, 19))

reduce<U>(arg: this): void;
>reduce : Symbol(Array.reduce, Decl(thisInTupleTypeParameterConstraints.ts, 14, 36))
>U : Symbol(U, Decl(thisInTupleTypeParameterConstraints.ts, 15, 11))
>arg : Symbol(arg, Decl(thisInTupleTypeParameterConstraints.ts, 15, 14))

reduce2<U>(arg: this): void;
>reduce2 : Symbol(Array.reduce2, Decl(thisInTupleTypeParameterConstraints.ts, 15, 31))
>U : Symbol(U, Decl(thisInTupleTypeParameterConstraints.ts, 16, 12))
>arg : Symbol(arg, Decl(thisInTupleTypeParameterConstraints.ts, 16, 15))
}

declare function f<T extends [(x: number) => number]>(a: T): void;
>f : Symbol(f, Decl(thisInTupleTypeParameterConstraints.ts, 17, 1))
>T : Symbol(T, Decl(thisInTupleTypeParameterConstraints.ts, 19, 19))
>x : Symbol(x, Decl(thisInTupleTypeParameterConstraints.ts, 19, 31))
>a : Symbol(a, Decl(thisInTupleTypeParameterConstraints.ts, 19, 54))
>T : Symbol(T, Decl(thisInTupleTypeParameterConstraints.ts, 19, 19))

let x: [(x: number) => number];
>x : Symbol(x, Decl(thisInTupleTypeParameterConstraints.ts, 20, 3))
>x : Symbol(x, Decl(thisInTupleTypeParameterConstraints.ts, 20, 9))

f(x);
>f : Symbol(f, Decl(thisInTupleTypeParameterConstraints.ts, 17, 1))
>x : Symbol(x, Decl(thisInTupleTypeParameterConstraints.ts, 20, 3))

67 changes: 67 additions & 0 deletions tests/baselines/reference/thisInTupleTypeParameterConstraints.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
=== tests/cases/compiler/thisInTupleTypeParameterConstraints.ts ===
/// <reference no-default-lib="true"/>

interface Boolean {}
>Boolean : Boolean

interface IArguments {}
>IArguments : IArguments

interface Function {}
>Function : Function

interface Number {}
>Number : Number

interface RegExp {}
>RegExp : RegExp

interface Object {}
>Object : Object

interface String {}
>String : String

interface Array<T> {
>Array : T[]
>T : T

// 4 methods will run out of memory if this-types are not instantiated
// correctly for tuple types that are type parameter constraints
map<U>(arg: this): void;
>map : <U>(arg: this) => void
>U : U
>arg : this

reduceRight<U>(arg: this): void;
>reduceRight : <U>(arg: this) => void
>U : U
>arg : this

reduce<U>(arg: this): void;
>reduce : <U>(arg: this) => void
>U : U
>arg : this

reduce2<U>(arg: this): void;
>reduce2 : <U>(arg: this) => void
>U : U
>arg : this
}

declare function f<T extends [(x: number) => number]>(a: T): void;
>f : <T extends [(x: number) => number]>(a: T) => void
>T : T
>x : number
>a : T
>T : T

let x: [(x: number) => number];
>x : [(x: number) => number]
>x : number

f(x);
>f(x) : void
>f : <T extends [(x: number) => number]>(a: T) => void
>x : [(x: number) => number]

22 changes: 22 additions & 0 deletions tests/cases/compiler/thisInTupleTypeParameterConstraints.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/// <reference no-default-lib="true"/>

interface Boolean {}
interface IArguments {}
interface Function {}
interface Number {}
interface RegExp {}
interface Object {}
interface String {}

interface Array<T> {
// 4 methods will run out of memory if this-types are not instantiated
// correctly for tuple types that are type parameter constraints
map<U>(arg: this): void;
reduceRight<U>(arg: this): void;
reduce<U>(arg: this): void;
reduce2<U>(arg: this): void;
}

declare function f<T extends [(x: number) => number]>(a: T): void;
let x: [(x: number) => number];
f(x);