Skip to content

Commit 360e1b1

Browse files
committed
Type this in more constructor functions
Previously, `this: this` in constructor functions only when there was an explicit `@constructor` tag on the function. Now, `this: this` for any function that's known to be a constructor function. This improves completions inside constructor functions; also note that previously the compiler *did* type `this: this` inside methods of constructor functions, so this fix makes us more consistent. This is reflected in the large number of baselines that improve. The fix is a simple switch to `isJSConstructor`, which is the standard way to detect constructor functions. I'm not sure why the original PR didn't use this method. I remember discussing this limitation in the original bug, #25979, and I guess I decided that it made sense. But I was heavily primed by the bug's framing of the problem in terms of `noImplicitThis`, which *should* require an explicit `@constructor` tag. With better typing comes better detection of `@readonly` assignment; I had to fix the readonly detection code to use `isJSConstructor` as well.
1 parent 2d09da6 commit 360e1b1

File tree

94 files changed

+298
-141
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

94 files changed

+298
-141
lines changed

src/compiler/checker.ts

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22416,30 +22416,21 @@ namespace ts {
2241622416
const isInJS = isInJSFile(node);
2241722417
if (isFunctionLike(container) &&
2241822418
(!isInParameterInitializerBeforeContainingFunction(node) || getThisParameter(container))) {
22419+
let thisType: Type | undefined;
2241922420
// Note: a parameter initializer should refer to class-this unless function-this is explicitly annotated.
2242022421
// If this is a function in a JS file, it might be a class method.
2242122422
const className = getClassNameFromPrototypeMethod(container);
2242222423
if (isInJS && className) {
2242322424
const classSymbol = checkExpression(className).symbol;
2242422425
if (classSymbol && classSymbol.members && (classSymbol.flags & SymbolFlags.Function)) {
22425-
const classType = (getDeclaredTypeOfSymbol(classSymbol) as InterfaceType).thisType;
22426-
if (classType) {
22427-
return getFlowTypeOfReference(node, classType);
22428-
}
22426+
thisType = (getDeclaredTypeOfSymbol(classSymbol) as InterfaceType).thisType;
2242922427
}
2243022428
}
22431-
// Check if it's a constructor definition, can be either a variable decl or function decl
22432-
// i.e.
22433-
// * /** @constructor */ function [name]() { ... }
22434-
// * /** @constructor */ var x = function() { ... }
22435-
else if (isInJS &&
22436-
(container.kind === SyntaxKind.FunctionExpression || container.kind === SyntaxKind.FunctionDeclaration) &&
22437-
getJSDocClassTag(container)) {
22438-
const classType = (getDeclaredTypeOfSymbol(getMergedSymbol(container.symbol)) as InterfaceType).thisType!;
22439-
return getFlowTypeOfReference(node, classType);
22429+
else if (isJSConstructor(container)) {
22430+
thisType = (getDeclaredTypeOfSymbol(getMergedSymbol(container.symbol)) as InterfaceType).thisType;
2244022431
}
2244122432

22442-
const thisType = getThisTypeOfDeclaration(container) || getContextualThisParameterType(container);
22433+
thisType ||= getThisTypeOfDeclaration(container) || getContextualThisParameterType(container);
2244322434
if (thisType) {
2244422435
return getFlowTypeOfReference(node, thisType);
2244522436
}
@@ -28543,7 +28534,7 @@ namespace ts {
2854328534
expr.expression.kind === SyntaxKind.ThisKeyword) {
2854428535
// Look for if this is the constructor for the class that `symbol` is a property of.
2854528536
const ctor = getContainingFunction(expr);
28546-
if (!(ctor && ctor.kind === SyntaxKind.Constructor)) {
28537+
if (!(ctor && (ctor.kind === SyntaxKind.Constructor || isJSConstructor(ctor)))) {
2854728538
return true;
2854828539
}
2854928540
if (symbol.valueDeclaration) {

tests/baselines/reference/callbackCrossModule.symbols

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ function C() {
1313
>C : Symbol(C, Decl(mod1.js, 4, 18))
1414

1515
this.p = 1
16+
>this.p : Symbol(C.p, Decl(mod1.js, 5, 14))
17+
>this : Symbol(C, Decl(mod1.js, 4, 18))
1618
>p : Symbol(C.p, Decl(mod1.js, 5, 14))
1719
}
1820

tests/baselines/reference/callbackCrossModule.types

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ function C() {
1616
this.p = 1
1717
>this.p = 1 : 1
1818
>this.p : any
19-
>this : any
19+
>this : this
2020
>p : any
2121
>1 : 1
2222
}

tests/baselines/reference/chainedPrototypeAssignment.symbols

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,17 @@ var A = function A() {
4242
>A : Symbol(A, Decl(mod.js, 1, 7))
4343

4444
this.a = 1
45+
>this.a : Symbol(A.a, Decl(mod.js, 1, 22))
46+
>this : Symbol(A, Decl(mod.js, 1, 7))
4547
>a : Symbol(A.a, Decl(mod.js, 1, 22))
4648
}
4749
var B = function B() {
4850
>B : Symbol(B, Decl(mod.js, 4, 3), Decl(mod.js, 9, 13))
4951
>B : Symbol(B, Decl(mod.js, 4, 7))
5052

5153
this.b = 2
54+
>this.b : Symbol(B.b, Decl(mod.js, 4, 22))
55+
>this : Symbol(B, Decl(mod.js, 4, 7))
5256
>b : Symbol(B.b, Decl(mod.js, 4, 22))
5357
}
5458
exports.A = A

tests/baselines/reference/chainedPrototypeAssignment.types

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ var A = function A() {
5252
this.a = 1
5353
>this.a = 1 : 1
5454
>this.a : any
55-
>this : any
55+
>this : this
5656
>a : any
5757
>1 : 1
5858
}
@@ -64,7 +64,7 @@ var B = function B() {
6464
this.b = 2
6565
>this.b = 2 : 2
6666
>this.b : any
67-
>this : any
67+
>this : this
6868
>b : any
6969
>2 : 2
7070
}

tests/baselines/reference/classCanExtendConstructorFunction.symbols

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,8 @@ function Soup(flavour) {
193193
>flavour : Symbol(flavour, Decl(generic.js, 4, 14))
194194

195195
this.flavour = flavour
196+
>this.flavour : Symbol(Soup.flavour, Decl(generic.js, 4, 24))
197+
>this : Symbol(Soup, Decl(generic.js, 0, 0))
196198
>flavour : Symbol(Soup.flavour, Decl(generic.js, 4, 24))
197199
>flavour : Symbol(flavour, Decl(generic.js, 4, 14))
198200
}

tests/baselines/reference/classCanExtendConstructorFunction.types

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ function Soup(flavour) {
238238
this.flavour = flavour
239239
>this.flavour = flavour : T
240240
>this.flavour : any
241-
>this : any
241+
>this : this
242242
>flavour : any
243243
>flavour : T
244244
}

tests/baselines/reference/commonjsAccessExports.symbols

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ exports.x;
1818
>Cls : Symbol(Cls, Decl(a.js, 4, 1))
1919

2020
this.x = 0;
21+
>this.x : Symbol(Cls.x, Decl(a.js, 6, 30))
22+
>this : Symbol(Cls, Decl(a.js, 6, 17))
2123
>x : Symbol(Cls.x, Decl(a.js, 6, 30))
2224
}
2325
}

tests/baselines/reference/commonjsAccessExports.types

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ exports.x;
2424
this.x = 0;
2525
>this.x = 0 : 0
2626
>this.x : any
27-
>this : any
27+
>this : this
2828
>x : any
2929
>0 : 0
3030
}

tests/baselines/reference/constructorFunctionMergeWithClass.symbols

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ var SomeClass = function () {
33
>SomeClass : Symbol(SomeClass, Decl(file1.js, 0, 3), Decl(file2.js, 0, 0), Decl(file2.js, 0, 19))
44

55
this.otherProp = 0;
6+
>this.otherProp : Symbol(SomeClass.otherProp, Decl(file1.js, 0, 29))
7+
>this : Symbol(SomeClass, Decl(file1.js, 0, 15))
68
>otherProp : Symbol(SomeClass.otherProp, Decl(file1.js, 0, 29))
79

810
};

tests/baselines/reference/constructorFunctionMergeWithClass.types

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ var SomeClass = function () {
66
this.otherProp = 0;
77
>this.otherProp = 0 : 0
88
>this.otherProp : any
9-
>this : any
9+
>this : this
1010
>otherProp : any
1111
>0 : 0
1212

tests/baselines/reference/constructorFunctions.symbols

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@ function C1() {
33
>C1 : Symbol(C1, Decl(index.js, 0, 0))
44

55
if (!(this instanceof C1)) return new C1();
6+
>this : Symbol(C1, Decl(index.js, 0, 0))
67
>C1 : Symbol(C1, Decl(index.js, 0, 0))
78
>C1 : Symbol(C1, Decl(index.js, 0, 0))
89

910
this.x = 1;
11+
>this.x : Symbol(C1.x, Decl(index.js, 1, 47))
12+
>this : Symbol(C1, Decl(index.js, 0, 0))
1013
>x : Symbol(C1.x, Decl(index.js, 1, 47))
1114
}
1215

@@ -22,10 +25,13 @@ var C2 = function () {
2225
>C2 : Symbol(C2, Decl(index.js, 8, 3))
2326

2427
if (!(this instanceof C2)) return new C2();
28+
>this : Symbol(C2, Decl(index.js, 8, 8))
2529
>C2 : Symbol(C2, Decl(index.js, 8, 3))
2630
>C2 : Symbol(C2, Decl(index.js, 8, 3))
2731

2832
this.x = 1;
33+
>this.x : Symbol(C2.x, Decl(index.js, 9, 47))
34+
>this : Symbol(C2, Decl(index.js, 8, 8))
2935
>x : Symbol(C2.x, Decl(index.js, 9, 47))
3036

3137
};

tests/baselines/reference/constructorFunctions.types

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,15 @@ function C1() {
66
>!(this instanceof C1) : boolean
77
>(this instanceof C1) : boolean
88
>this instanceof C1 : boolean
9-
>this : any
9+
>this : this
1010
>C1 : typeof C1
1111
>new C1() : C1
1212
>C1 : typeof C1
1313

1414
this.x = 1;
1515
>this.x = 1 : 1
1616
>this.x : any
17-
>this : any
17+
>this : this
1818
>x : any
1919
>1 : 1
2020
}
@@ -37,15 +37,15 @@ var C2 = function () {
3737
>!(this instanceof C2) : boolean
3838
>(this instanceof C2) : boolean
3939
>this instanceof C2 : boolean
40-
>this : any
40+
>this : this
4141
>C2 : typeof C2
4242
>new C2() : C2
4343
>C2 : typeof C2
4444

4545
this.x = 1;
4646
>this.x = 1 : 1
4747
>this.x : any
48-
>this : any
48+
>this : this
4949
>x : any
5050
>1 : 1
5151

tests/baselines/reference/constructorFunctions2.symbols

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ const a = new A().id;
2121

2222
const B = function() { this.id = 1; }
2323
>B : Symbol(B, Decl(index.js, 3, 5))
24+
>this.id : Symbol(B.id, Decl(index.js, 3, 22))
25+
>this : Symbol(B, Decl(index.js, 3, 9))
2426
>id : Symbol(B.id, Decl(index.js, 3, 22))
2527

2628
B.prototype.m = function() { this.x = 2; }
@@ -49,6 +51,8 @@ b.x;
4951
=== tests/cases/conformance/salsa/other.js ===
5052
function A() { this.id = 1; }
5153
>A : Symbol(A, Decl(other.js, 0, 0))
54+
>this.id : Symbol(A.id, Decl(other.js, 0, 14))
55+
>this : Symbol(A, Decl(other.js, 0, 0))
5256
>id : Symbol(A.id, Decl(other.js, 0, 14))
5357

5458
module.exports = A;

tests/baselines/reference/constructorFunctions2.types

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const B = function() { this.id = 1; }
2626
>function() { this.id = 1; } : typeof B
2727
>this.id = 1 : 1
2828
>this.id : any
29-
>this : any
29+
>this : this
3030
>id : any
3131
>1 : 1
3232

@@ -64,7 +64,7 @@ function A() { this.id = 1; }
6464
>A : typeof A
6565
>this.id = 1 : 1
6666
>this.id : any
67-
>this : any
67+
>this : this
6868
>id : any
6969
>1 : 1
7070

tests/baselines/reference/constructorFunctions3.symbols

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ function Instance() {
33
>Instance : Symbol(Instance, Decl(a.js, 0, 0))
44

55
this.i = 'simple'
6+
>this.i : Symbol(Instance.i, Decl(a.js, 0, 21))
7+
>this : Symbol(Instance, Decl(a.js, 0, 0))
68
>i : Symbol(Instance.i, Decl(a.js, 0, 21))
79
}
810
var i = new Instance();
@@ -19,6 +21,8 @@ function StaticToo() {
1921
>StaticToo : Symbol(StaticToo, Decl(a.js, 5, 2), Decl(a.js, 9, 1))
2022

2123
this.i = 'more complex'
24+
>this.i : Symbol(StaticToo.i, Decl(a.js, 7, 22))
25+
>this : Symbol(StaticToo, Decl(a.js, 5, 2), Decl(a.js, 9, 1))
2226
>i : Symbol(StaticToo.i, Decl(a.js, 7, 22))
2327
}
2428
StaticToo.property = 'yep'
@@ -41,10 +45,14 @@ function A () {
4145
>A : Symbol(A, Decl(a.js, 13, 10), Decl(a.js, 24, 1))
4246

4347
this.x = 1
48+
>this.x : Symbol(A.x, Decl(a.js, 16, 15))
49+
>this : Symbol(A, Decl(a.js, 13, 10), Decl(a.js, 24, 1))
4450
>x : Symbol(A.x, Decl(a.js, 16, 15))
4551

4652
/** @type {1} */
4753
this.second = 1
54+
>this.second : Symbol(A.second, Decl(a.js, 17, 14))
55+
>this : Symbol(A, Decl(a.js, 13, 10), Decl(a.js, 24, 1))
4856
>second : Symbol(A.second, Decl(a.js, 17, 14))
4957
}
5058
/** @param {number} n */

tests/baselines/reference/constructorFunctions3.types

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ function Instance() {
55
this.i = 'simple'
66
>this.i = 'simple' : "simple"
77
>this.i : any
8-
>this : any
8+
>this : this
99
>i : any
1010
>'simple' : "simple"
1111
}
@@ -26,7 +26,7 @@ function StaticToo() {
2626
this.i = 'more complex'
2727
>this.i = 'more complex' : "more complex"
2828
>this.i : any
29-
>this : any
29+
>this : this
3030
>i : any
3131
>'more complex' : "more complex"
3232
}
@@ -55,16 +55,16 @@ function A () {
5555
this.x = 1
5656
>this.x = 1 : 1
5757
>this.x : any
58-
>this : any
58+
>this : this
5959
>x : any
6060
>1 : 1
6161

6262
/** @type {1} */
6363
this.second = 1
6464
>this.second = 1 : 1
65-
>this.second : any
66-
>this : any
67-
>second : any
65+
>this.second : 1
66+
>this : this
67+
>second : 1
6868
>1 : 1
6969
}
7070
/** @param {number} n */

tests/baselines/reference/constructorFunctionsStrict.symbols

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ function C(x) {
55
>x : Symbol(x, Decl(a.js, 1, 11))
66

77
this.x = x
8+
>this.x : Symbol(C.x, Decl(a.js, 1, 15))
9+
>this : Symbol(C, Decl(a.js, 0, 0))
810
>x : Symbol(C.x, Decl(a.js, 1, 15))
911
>x : Symbol(x, Decl(a.js, 1, 11))
1012
}
@@ -41,13 +43,16 @@ function A(x) {
4143
>x : Symbol(x, Decl(a.js, 12, 11))
4244

4345
if (!(this instanceof A)) {
46+
>this : Symbol(A, Decl(a.js, 9, 15))
4447
>A : Symbol(A, Decl(a.js, 9, 15))
4548

4649
return new A(x)
4750
>A : Symbol(A, Decl(a.js, 9, 15))
4851
>x : Symbol(x, Decl(a.js, 12, 11))
4952
}
5053
this.x = x
54+
>this.x : Symbol(A.x, Decl(a.js, 15, 5))
55+
>this : Symbol(A, Decl(a.js, 9, 15))
5156
>x : Symbol(A.x, Decl(a.js, 15, 5))
5257
>x : Symbol(x, Decl(a.js, 12, 11))
5358
}

tests/baselines/reference/constructorFunctionsStrict.types

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ function C(x) {
77
this.x = x
88
>this.x = x : number
99
>this.x : any
10-
>this : any
10+
>this : this
1111
>x : any
1212
>x : number
1313
}
@@ -56,7 +56,7 @@ function A(x) {
5656
>!(this instanceof A) : boolean
5757
>(this instanceof A) : boolean
5858
>this instanceof A : boolean
59-
>this : any
59+
>this : this
6060
>A : typeof A
6161

6262
return new A(x)
@@ -67,7 +67,7 @@ function A(x) {
6767
this.x = x
6868
>this.x = x : number
6969
>this.x : any
70-
>this : any
70+
>this : this
7171
>x : any
7272
>x : number
7373
}

tests/baselines/reference/expandoFunctionContextualTypesJs.symbols

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,14 @@ function foo() {
5858
* @type {MyComponentProps}
5959
*/
6060
this.props = { color: "red" };
61+
>this.props : Symbol(foo.props, Decl(input.js, 35, 16))
62+
>this : Symbol(foo, Decl(input.js, 33, 28))
6163
>props : Symbol(foo.props, Decl(input.js, 35, 16))
6264
>color : Symbol(color, Decl(input.js, 39, 18))
6365

6466
expectLiteral(this);
6567
>expectLiteral : Symbol(expectLiteral, Decl(input.js, 27, 27))
68+
>this : Symbol(foo, Decl(input.js, 33, 28))
6669
}
6770

6871
/**

0 commit comments

Comments
 (0)