Skip to content

Type this in more constructor functions #39447

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 6 commits into from
Jul 8, 2020
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
37 changes: 12 additions & 25 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22436,30 +22436,23 @@ namespace ts {
const isInJS = isInJSFile(node);
if (isFunctionLike(container) &&
(!isInParameterInitializerBeforeContainingFunction(node) || getThisParameter(container))) {
let thisType = getThisTypeOfDeclaration(container) || isInJS && getTypeForThisExpressionFromJSDoc(container);
// Note: a parameter initializer should refer to class-this unless function-this is explicitly annotated.
// If this is a function in a JS file, it might be a class method.
const className = getClassNameFromPrototypeMethod(container);
if (isInJS && className) {
const classSymbol = checkExpression(className).symbol;
if (classSymbol && classSymbol.members && (classSymbol.flags & SymbolFlags.Function)) {
const classType = (getDeclaredTypeOfSymbol(classSymbol) as InterfaceType).thisType;
if (classType) {
return getFlowTypeOfReference(node, classType);
if (!thisType) {
const className = getClassNameFromPrototypeMethod(container);
if (isInJS && className) {
const classSymbol = checkExpression(className).symbol;
if (classSymbol && classSymbol.members && (classSymbol.flags & SymbolFlags.Function)) {
thisType = (getDeclaredTypeOfSymbol(classSymbol) as InterfaceType).thisType;
}
}
}
// Check if it's a constructor definition, can be either a variable decl or function decl
// i.e.
// * /** @constructor */ function [name]() { ... }
// * /** @constructor */ var x = function() { ... }
else if (isInJS &&
(container.kind === SyntaxKind.FunctionExpression || container.kind === SyntaxKind.FunctionDeclaration) &&
getJSDocClassTag(container)) {
const classType = (getDeclaredTypeOfSymbol(getMergedSymbol(container.symbol)) as InterfaceType).thisType!;
return getFlowTypeOfReference(node, classType);
else if (isJSConstructor(container)) {
thisType = (getDeclaredTypeOfSymbol(getMergedSymbol(container.symbol)) as InterfaceType).thisType;
}
thisType ||= getContextualThisParameterType(container);
}

const thisType = getThisTypeOfDeclaration(container) || getContextualThisParameterType(container);
if (thisType) {
return getFlowTypeOfReference(node, thisType);
}
Expand All @@ -22471,12 +22464,6 @@ namespace ts {
return getFlowTypeOfReference(node, type);
}

if (isInJS) {
const type = getTypeForThisExpressionFromJSDoc(container);
if (type && type !== errorType) {
return getFlowTypeOfReference(node, type);
}
}
if (isSourceFile(container)) {
// look up in the source file's locals or exports
if (container.commonJsModuleIndicator) {
Expand Down Expand Up @@ -28570,7 +28557,7 @@ namespace ts {
expr.expression.kind === SyntaxKind.ThisKeyword) {
// Look for if this is the constructor for the class that `symbol` is a property of.
const ctor = getContainingFunction(expr);
if (!(ctor && ctor.kind === SyntaxKind.Constructor)) {
if (!(ctor && (ctor.kind === SyntaxKind.Constructor || isJSConstructor(ctor)))) {
return true;
}
if (symbol.valueDeclaration) {
Expand Down
5 changes: 0 additions & 5 deletions src/services/codefixes/fixImplicitThis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,5 @@ namespace ts.codefix {
return [Diagnostics.Convert_function_declaration_0_to_arrow_function, name!.text];
}
}
// No outer 'this', add a @class tag if in a JS constructor function
else if (isSourceFileJS(sourceFile) && isPropertyAccessExpression(token.parent) && isAssignmentExpression(token.parent.parent)) {
addJSDocTags(changes, sourceFile, fn, [factory.createJSDocClassTag(/*tagName*/ undefined)]);
return Diagnostics.Add_class_tag;
}
}
}
5 changes: 4 additions & 1 deletion tests/baselines/reference/assignmentToVoidZero2.errors.txt
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
tests/cases/conformance/salsa/assignmentToVoidZero2.js(2,9): error TS2339: Property 'k' does not exist on type 'typeof import("tests/cases/conformance/salsa/assignmentToVoidZero2")'.
tests/cases/conformance/salsa/assignmentToVoidZero2.js(5,3): error TS2339: Property 'y' does not exist on type 'typeof o'.
tests/cases/conformance/salsa/assignmentToVoidZero2.js(6,9): error TS2339: Property 'y' does not exist on type 'typeof o'.
tests/cases/conformance/salsa/assignmentToVoidZero2.js(10,10): error TS2339: Property 'q' does not exist on type 'C'.
tests/cases/conformance/salsa/assignmentToVoidZero2.js(13,9): error TS2339: Property 'q' does not exist on type 'C'.
tests/cases/conformance/salsa/importer.js(1,13): error TS2305: Module '"./assignmentToVoidZero2"' has no exported member 'k'.


==== tests/cases/conformance/salsa/assignmentToVoidZero2.js (4 errors) ====
==== tests/cases/conformance/salsa/assignmentToVoidZero2.js (5 errors) ====
exports.j = 1;
exports.k = void 0;
~
Expand All @@ -22,6 +23,8 @@ tests/cases/conformance/salsa/importer.js(1,13): error TS2305: Module '"./assign
function C() {
this.p = 1
this.q = void 0
~
!!! error TS2339: Property 'q' does not exist on type 'C'.
}
var c = new C()
c.p + c.q
Expand Down
3 changes: 3 additions & 0 deletions tests/baselines/reference/assignmentToVoidZero2.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,12 @@ function C() {
>C : Symbol(C, Decl(assignmentToVoidZero2.js, 5, 9))

this.p = 1
>this.p : Symbol(C.p, Decl(assignmentToVoidZero2.js, 7, 14))
>this : Symbol(C, Decl(assignmentToVoidZero2.js, 5, 9))
>p : Symbol(C.p, Decl(assignmentToVoidZero2.js, 7, 14))

this.q = void 0
>this : Symbol(C, Decl(assignmentToVoidZero2.js, 5, 9))
}
var c = new C()
>c : Symbol(c, Decl(assignmentToVoidZero2.js, 11, 3))
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/assignmentToVoidZero2.types
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ function C() {
this.p = 1
>this.p = 1 : 1
>this.p : any
>this : any
>this : this
>p : any
>1 : 1

this.q = void 0
>this.q = void 0 : undefined
>this.q : any
>this : any
>this : this
>q : any
>void 0 : undefined
>0 : 0
Expand Down
2 changes: 2 additions & 0 deletions tests/baselines/reference/callbackCrossModule.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ function C() {
>C : Symbol(C, Decl(mod1.js, 4, 18))

this.p = 1
>this.p : Symbol(C.p, Decl(mod1.js, 5, 14))
>this : Symbol(C, Decl(mod1.js, 4, 18))
>p : Symbol(C.p, Decl(mod1.js, 5, 14))
}

Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/callbackCrossModule.types
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ function C() {
this.p = 1
>this.p = 1 : 1
>this.p : any
>this : any
>this : this
>p : any
>1 : 1
}
Expand Down
4 changes: 4 additions & 0 deletions tests/baselines/reference/chainedPrototypeAssignment.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,17 @@ var A = function A() {
>A : Symbol(A, Decl(mod.js, 1, 7))

this.a = 1
>this.a : Symbol(A.a, Decl(mod.js, 1, 22))
>this : Symbol(A, Decl(mod.js, 1, 7))
>a : Symbol(A.a, Decl(mod.js, 1, 22))
}
var B = function B() {
>B : Symbol(B, Decl(mod.js, 4, 3), Decl(mod.js, 9, 13))
>B : Symbol(B, Decl(mod.js, 4, 7))

this.b = 2
>this.b : Symbol(B.b, Decl(mod.js, 4, 22))
>this : Symbol(B, Decl(mod.js, 4, 7))
>b : Symbol(B.b, Decl(mod.js, 4, 22))
}
exports.A = A
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/chainedPrototypeAssignment.types
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ var A = function A() {
this.a = 1
>this.a = 1 : 1
>this.a : any
>this : any
>this : this
>a : any
>1 : 1
}
Expand All @@ -64,7 +64,7 @@ var B = function B() {
this.b = 2
>this.b = 2 : 2
>this.b : any
>this : any
>this : this
>b : any
>2 : 2
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ function Soup(flavour) {
>flavour : Symbol(flavour, Decl(generic.js, 4, 14))

this.flavour = flavour
>this.flavour : Symbol(Soup.flavour, Decl(generic.js, 4, 24))
>this : Symbol(Soup, Decl(generic.js, 0, 0))
>flavour : Symbol(Soup.flavour, Decl(generic.js, 4, 24))
>flavour : Symbol(flavour, Decl(generic.js, 4, 14))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ function Soup(flavour) {
this.flavour = flavour
>this.flavour = flavour : T
>this.flavour : any
>this : any
>this : this
>flavour : any
>flavour : T
}
Expand Down
2 changes: 2 additions & 0 deletions tests/baselines/reference/commonjsAccessExports.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ exports.x;
>Cls : Symbol(Cls, Decl(a.js, 4, 1))

this.x = 0;
>this.x : Symbol(Cls.x, Decl(a.js, 6, 30))
>this : Symbol(Cls, Decl(a.js, 6, 17))
>x : Symbol(Cls.x, Decl(a.js, 6, 30))
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/commonjsAccessExports.types
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ exports.x;
this.x = 0;
>this.x = 0 : 0
>this.x : any
>this : any
>this : this
>x : any
>0 : 0
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ var SomeClass = function () {
>SomeClass : Symbol(SomeClass, Decl(file1.js, 0, 3), Decl(file2.js, 0, 0), Decl(file2.js, 0, 19))

this.otherProp = 0;
>this.otherProp : Symbol(SomeClass.otherProp, Decl(file1.js, 0, 29))
>this : Symbol(SomeClass, Decl(file1.js, 0, 15))
>otherProp : Symbol(SomeClass.otherProp, Decl(file1.js, 0, 29))

};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ var SomeClass = function () {
this.otherProp = 0;
>this.otherProp = 0 : 0
>this.otherProp : any
>this : any
>this : this
>otherProp : any
>0 : 0

Expand Down
6 changes: 6 additions & 0 deletions tests/baselines/reference/constructorFunctions.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ function C1() {
>C1 : Symbol(C1, Decl(index.js, 0, 0))

if (!(this instanceof C1)) return new C1();
>this : Symbol(C1, Decl(index.js, 0, 0))
>C1 : Symbol(C1, Decl(index.js, 0, 0))
>C1 : Symbol(C1, Decl(index.js, 0, 0))

this.x = 1;
>this.x : Symbol(C1.x, Decl(index.js, 1, 47))
>this : Symbol(C1, Decl(index.js, 0, 0))
>x : Symbol(C1.x, Decl(index.js, 1, 47))
}

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

if (!(this instanceof C2)) return new C2();
>this : Symbol(C2, Decl(index.js, 8, 8))
>C2 : Symbol(C2, Decl(index.js, 8, 3))
>C2 : Symbol(C2, Decl(index.js, 8, 3))

this.x = 1;
>this.x : Symbol(C2.x, Decl(index.js, 9, 47))
>this : Symbol(C2, Decl(index.js, 8, 8))
>x : Symbol(C2.x, Decl(index.js, 9, 47))

};
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/constructorFunctions.types
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ function C1() {
>!(this instanceof C1) : boolean
>(this instanceof C1) : boolean
>this instanceof C1 : boolean
>this : any
>this : this
>C1 : typeof C1
>new C1() : C1
>C1 : typeof C1

this.x = 1;
>this.x = 1 : 1
>this.x : any
>this : any
>this : this
>x : any
>1 : 1
}
Expand All @@ -37,15 +37,15 @@ var C2 = function () {
>!(this instanceof C2) : boolean
>(this instanceof C2) : boolean
>this instanceof C2 : boolean
>this : any
>this : this
>C2 : typeof C2
>new C2() : C2
>C2 : typeof C2

this.x = 1;
>this.x = 1 : 1
>this.x : any
>this : any
>this : this
>x : any
>1 : 1

Expand Down
4 changes: 4 additions & 0 deletions tests/baselines/reference/constructorFunctions2.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ const a = new A().id;

const B = function() { this.id = 1; }
>B : Symbol(B, Decl(index.js, 3, 5))
>this.id : Symbol(B.id, Decl(index.js, 3, 22))
>this : Symbol(B, Decl(index.js, 3, 9))
>id : Symbol(B.id, Decl(index.js, 3, 22))

B.prototype.m = function() { this.x = 2; }
Expand Down Expand Up @@ -49,6 +51,8 @@ b.x;
=== tests/cases/conformance/salsa/other.js ===
function A() { this.id = 1; }
>A : Symbol(A, Decl(other.js, 0, 0))
>this.id : Symbol(A.id, Decl(other.js, 0, 14))
>this : Symbol(A, Decl(other.js, 0, 0))
>id : Symbol(A.id, Decl(other.js, 0, 14))

module.exports = A;
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/constructorFunctions2.types
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const B = function() { this.id = 1; }
>function() { this.id = 1; } : typeof B
>this.id = 1 : 1
>this.id : any
>this : any
>this : this
>id : any
>1 : 1

Expand Down Expand Up @@ -64,7 +64,7 @@ function A() { this.id = 1; }
>A : typeof A
>this.id = 1 : 1
>this.id : any
>this : any
>this : this
>id : any
>1 : 1

Expand Down
8 changes: 8 additions & 0 deletions tests/baselines/reference/constructorFunctions3.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ function Instance() {
>Instance : Symbol(Instance, Decl(a.js, 0, 0))

this.i = 'simple'
>this.i : Symbol(Instance.i, Decl(a.js, 0, 21))
>this : Symbol(Instance, Decl(a.js, 0, 0))
>i : Symbol(Instance.i, Decl(a.js, 0, 21))
}
var i = new Instance();
Expand All @@ -19,6 +21,8 @@ function StaticToo() {
>StaticToo : Symbol(StaticToo, Decl(a.js, 5, 2), Decl(a.js, 9, 1))

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

this.x = 1
>this.x : Symbol(A.x, Decl(a.js, 16, 15))
>this : Symbol(A, Decl(a.js, 13, 10), Decl(a.js, 24, 1))
>x : Symbol(A.x, Decl(a.js, 16, 15))

/** @type {1} */
this.second = 1
>this.second : Symbol(A.second, Decl(a.js, 17, 14))
>this : Symbol(A, Decl(a.js, 13, 10), Decl(a.js, 24, 1))
>second : Symbol(A.second, Decl(a.js, 17, 14))
}
/** @param {number} n */
Expand Down
Loading