Skip to content

Type 'this' in function properties of object literals #8382

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

Merged
merged 2 commits into from
Apr 29, 2016
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 3 additions & 2 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8206,10 +8206,11 @@ namespace ts {
if (signature.thisType) {
return signature.thisType;
}
if (container.parent && container.parent.kind === SyntaxKind.ObjectLiteralExpression) {
const parentObject = container.parent && container.parent.kind === SyntaxKind.PropertyAssignment ? container.parent.parent : container.parent;
if (parentObject && parentObject.kind === SyntaxKind.ObjectLiteralExpression) {
// Note: this works because object literal methods are deferred,
// which means that the type of the containing object literal is already known.
const type = checkExpressionCached(<ObjectLiteralExpression>container.parent);
const type = checkExpressionCached(<ObjectLiteralExpression>parentObject);
if (type) {
return type;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
tests/cases/compiler/commentsOnObjectLiteral2.ts(1,14): error TS2304: Cannot find name 'makeClass'.
tests/cases/compiler/commentsOnObjectLiteral2.ts(9,17): error TS2339: Property 'name' does not exist on type '{ initialize: (name: any) => void; }'.


==== tests/cases/compiler/commentsOnObjectLiteral2.ts (1 errors) ====
==== tests/cases/compiler/commentsOnObjectLiteral2.ts (2 errors) ====
var Person = makeClass(
~~~~~~~~~
!!! error TS2304: Cannot find name 'makeClass'.
Expand All @@ -13,6 +14,8 @@ tests/cases/compiler/commentsOnObjectLiteral2.ts(1,14): error TS2304: Cannot fin
*/
initialize: function(name) {
this.name = name;
~~~~
!!! error TS2339: Property 'name' does not exist on type '{ initialize: (name: any) => void; }'.
} /* trailing comment 1*/,
}
);
5 changes: 5 additions & 0 deletions tests/baselines/reference/fatarrowfunctions.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,11 @@ var messenger = {

setTimeout(() => { this.message.toString(); }, 3000);
>setTimeout : Symbol(setTimeout, Decl(fatarrowfunctions.ts, 34, 1))
>this.message.toString : Symbol(String.toString, Decl(lib.d.ts, --, --))
>this.message : Symbol(message, Decl(fatarrowfunctions.ts, 38, 17))
>this : Symbol(, Decl(fatarrowfunctions.ts, 38, 15))
>message : Symbol(message, Decl(fatarrowfunctions.ts, 38, 17))
>toString : Symbol(String.toString, Decl(lib.d.ts, --, --))
}
};

12 changes: 6 additions & 6 deletions tests/baselines/reference/fatarrowfunctions.types
Original file line number Diff line number Diff line change
Expand Up @@ -234,12 +234,12 @@ var messenger = {
>setTimeout(() => { this.message.toString(); }, 3000) : number
>setTimeout : (expression: any, msec?: number, language?: any) => number
>() => { this.message.toString(); } : () => void
>this.message.toString() : any
>this.message.toString : any
>this.message : any
>this : any
>message : any
>toString : any
>this.message.toString() : string
>this.message.toString : () => string
>this.message : string
>this : { message: string; start: () => void; }
>message : string
>toString : () => string
>3000 : number
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,17 @@ var messenger = {

var _self = this;
>_self : Symbol(_self, Decl(fatarrowfunctionsInFunctions.ts, 5, 11))
>this : Symbol(, Decl(fatarrowfunctionsInFunctions.ts, 2, 15))

setTimeout(function() {
>setTimeout : Symbol(setTimeout, Decl(fatarrowfunctionsInFunctions.ts, 0, 0))

_self.message.toString();
>_self.message.toString : Symbol(String.toString, Decl(lib.d.ts, --, --))
>_self.message : Symbol(message, Decl(fatarrowfunctionsInFunctions.ts, 2, 17))
>_self : Symbol(_self, Decl(fatarrowfunctionsInFunctions.ts, 5, 11))
>message : Symbol(message, Decl(fatarrowfunctionsInFunctions.ts, 2, 17))
>toString : Symbol(String.toString, Decl(lib.d.ts, --, --))

}, 3000);
}
Expand Down
16 changes: 8 additions & 8 deletions tests/baselines/reference/fatarrowfunctionsInFunctions.types
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,21 @@ var messenger = {
>function() { var _self = this; setTimeout(function() { _self.message.toString(); }, 3000); } : () => void

var _self = this;
>_self : any
>this : any
>_self : { message: string; start: () => void; }
>this : { message: string; start: () => void; }

setTimeout(function() {
>setTimeout(function() { _self.message.toString(); }, 3000) : number
>setTimeout : (expression: any, msec?: number, language?: any) => number
>function() { _self.message.toString(); } : () => void

_self.message.toString();
>_self.message.toString() : any
>_self.message.toString : any
>_self.message : any
>_self : any
>message : any
>toString : any
>_self.message.toString() : string
>_self.message.toString : () => string
>_self.message : string
>_self : { message: string; start: () => void; }
>message : string
>toString : () => string

}, 3000);
>3000 : number
Expand Down
16 changes: 10 additions & 6 deletions tests/baselines/reference/looseThisTypeInFunctions.errors.txt
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
tests/cases/conformance/types/thisType/looseThisTypeInFunctions.ts(21,1): error TS2322: Type '(this: C, m: number) => number' is not assignable to type '(this: void, m: number) => number'.
The 'this' types of each signature are incompatible.
Type 'void' is not assignable to type 'C'.
tests/cases/conformance/types/thisType/looseThisTypeInFunctions.ts(25,27): error TS2339: Property 'length' does not exist on type 'number'.
tests/cases/conformance/types/thisType/looseThisTypeInFunctions.ts(33,28): error TS2339: Property 'length' does not exist on type 'number'.
tests/cases/conformance/types/thisType/looseThisTypeInFunctions.ts(37,9): error TS2684: The 'this' context of type 'void' is not assignable to method's 'this' of type 'I'.
tests/cases/conformance/types/thisType/looseThisTypeInFunctions.ts(46,20): error TS2339: Property 'length' does not exist on type 'number'.


==== tests/cases/conformance/types/thisType/looseThisTypeInFunctions.ts (4 errors) ====
==== tests/cases/conformance/types/thisType/looseThisTypeInFunctions.ts (5 errors) ====
interface I {
n: number;
explicitThis(this: this, m: number): number;
Expand All @@ -32,12 +33,14 @@ tests/cases/conformance/types/thisType/looseThisTypeInFunctions.ts(46,20): error
!!! error TS2322: Type '(this: C, m: number) => number' is not assignable to type '(this: void, m: number) => number'.
!!! error TS2322: The 'this' types of each signature are incompatible.
!!! error TS2322: Type 'void' is not assignable to type 'C'.
let o = {
let o = {
n: 101,
explicitThis: function (m: number) {
return m + this.n.length; // ok, this.n: any
explicitThis: function (m: number) {
return m + this.n.length; // error, 'length' does not exist on 'number'
~~~~~~
!!! error TS2339: Property 'length' does not exist on type 'number'.
},
implicitThis(m: number): number { return m; }
implicitThis(m: number): number { return m; }
};
let i: I = o;
let o2: I = {
Expand All @@ -63,4 +66,5 @@ tests/cases/conformance/types/thisType/looseThisTypeInFunctions.ts(46,20): error
return this.n.length; // error, this.n: number
~~~~~~
!!! error TS2339: Property 'length' does not exist on type 'number'.
}
}

13 changes: 7 additions & 6 deletions tests/baselines/reference/looseThisTypeInFunctions.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ class C implements I {
}
let c = new C();
c.explicitVoid = c.explicitThis; // error, 'void' is missing everything
let o = {
let o = {
n: 101,
explicitThis: function (m: number) {
return m + this.n.length; // ok, this.n: any
explicitThis: function (m: number) {
return m + this.n.length; // error, 'length' does not exist on 'number'
},
implicitThis(m: number): number { return m; }
implicitThis(m: number): number { return m; }
};
let i: I = o;
let o2: I = {
Expand All @@ -45,7 +45,8 @@ o.implicitThis = c.explicitThis; // ok, implicitThis(this:any) is assignable to
o.implicitThis = i.explicitThis;
i.explicitThis = function(m) {
return this.n.length; // error, this.n: number
}
}


//// [looseThisTypeInFunctions.js]
var C = (function () {
Expand All @@ -67,7 +68,7 @@ c.explicitVoid = c.explicitThis; // error, 'void' is missing everything
var o = {
n: 101,
explicitThis: function (m) {
return m + this.n.length; // ok, this.n: any
return m + this.n.length; // error, 'length' does not exist on 'number'
Copy link
Member

Choose a reason for hiding this comment

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

These comments are unhelpful to future test updates, please just remove them instead of changing them in the future.

Copy link
Member

Choose a reason for hiding this comment

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

What I mean is, you run the tests, find out something is wrong, realize you have to update the test itself as well because the comment is now lying to you, and also update the baselines 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.

What about the case that you run the tests, see that it changed, and have to figure out whether or not the change is an improvement? In that case the comment helps you figure out if the test should still pass, and why.

I've hit several old-ish tests that have accumulated multiple failures -- I couldn't tell if they were supposed to fail or even what the test was supposed to show.

},
implicitThis: function (m) { return m; }
};
Expand Down
7 changes: 7 additions & 0 deletions tests/baselines/reference/selfInLambdas.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,15 @@ var o = {
>onmousemove : Symbol(Window.onmousemove, Decl(selfInLambdas.ts, 6, 18))

this.counter++
>this.counter : Symbol(counter, Decl(selfInLambdas.ts, 10, 9))
>this : Symbol(, Decl(selfInLambdas.ts, 10, 7))
>counter : Symbol(counter, Decl(selfInLambdas.ts, 10, 9))

var f = () => this.counter;
>f : Symbol(f, Decl(selfInLambdas.ts, 18, 15))
>this.counter : Symbol(counter, Decl(selfInLambdas.ts, 10, 9))
>this : Symbol(, Decl(selfInLambdas.ts, 10, 7))
>counter : Symbol(counter, Decl(selfInLambdas.ts, 10, 9))

}

Expand Down
16 changes: 8 additions & 8 deletions tests/baselines/reference/selfInLambdas.types
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,16 @@ var o = {

this.counter++
>this.counter++ : number
>this.counter : any
>this : any
>counter : any
>this.counter : number
>this : { counter: number; start: () => void; }
>counter : number

var f = () => this.counter;
>f : () => any
>() => this.counter : () => any
>this.counter : any
>this : any
>counter : any
>f : () => number
>() => this.counter : () => number
>this.counter : number
>this : { counter: number; start: () => void; }
>counter : number

}

Expand Down
3 changes: 3 additions & 0 deletions tests/baselines/reference/thisBinding2.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ var messenger = {
return setTimeout(() => { var x = this.message; }, 3000);
>setTimeout : Symbol(setTimeout, Decl(thisBinding2.ts, 12, 1))
>x : Symbol(x, Decl(thisBinding2.ts, 17, 37))
>this.message : Symbol(message, Decl(thisBinding2.ts, 14, 17))
>this : Symbol(, Decl(thisBinding2.ts, 14, 15))
>message : Symbol(message, Decl(thisBinding2.ts, 14, 17))
}
};

8 changes: 4 additions & 4 deletions tests/baselines/reference/thisBinding2.types
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ var messenger = {
>setTimeout(() => { var x = this.message; }, 3000) : number
>setTimeout : (expression: any, msec?: number, language?: any) => number
>() => { var x = this.message; } : () => void
>x : any
>this.message : any
>this : any
>message : any
>x : string
>this.message : string
>this : { message: string; start: () => number; }
>message : string
>3000 : number
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class A {

a: function() { return this; },
>a : Symbol(a, Decl(thisInPropertyBoundDeclarations.ts, 33, 13))
>this : Symbol(, Decl(thisInPropertyBoundDeclarations.ts, 33, 11))

};

Expand All @@ -79,6 +80,7 @@ class A {
return {
a: function() { return this; },
>a : Symbol(a, Decl(thisInPropertyBoundDeclarations.ts, 38, 16))
>this : Symbol(, Decl(thisInPropertyBoundDeclarations.ts, 38, 14))

};
};
Expand Down
12 changes: 6 additions & 6 deletions tests/baselines/reference/thisInPropertyBoundDeclarations.types
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ class A {
>{ a: function() { return this; }, } : { a: () => any; }

a: function() { return this; },
>a : () => any
>function() { return this; } : () => any
>this : any
>a : () => { a: any; }
>function() { return this; } : () => { a: any; }
>this : { a: () => any; }

};

Expand All @@ -98,9 +98,9 @@ class A {
>{ a: function() { return this; }, } : { a: () => any; }

a: function() { return this; },
>a : () => any
>function() { return this; } : () => any
>this : any
>a : () => { a: any; }
>function() { return this; } : () => { a: any; }
>this : { a: () => any; }

};
};
Expand Down
7 changes: 7 additions & 0 deletions tests/baselines/reference/thisTypeInObjectLiterals.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@ let o = {
d: "bar",
m() {
return this.d.length;
},
f: function() {
return this.d.length;
}
}

let mutuallyRecursive = {
a: 100,
start() {
Expand Down Expand Up @@ -35,6 +39,9 @@ var o = {
d: "bar",
m: function () {
return this.d.length;
},
f: function () {
return this.d.length;
}
};
var mutuallyRecursive = {
Expand Down
Loading