Type this in more constructor functions (#39447)

* 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.

* Remove `Add @Class tag` fix for noImplicitThis.

The new rules mean that it never applies. It's possible that it should
apply to functions like

```js
function f() {
  this.init()
}
```

In which `init` is never defined, but I think this program is incomplete
enough that not offering the fix is fine.

* Fix precedence of `@this`

Previously, both `@class` and `@this` in a jsdoc would cause the `@this`
annotation to be ignored. This became a worse problem with this PR,
because `this` is correctly typed even without the annotation.

This commit makes sure that `@this` is checked first and used if
present.

Author: sandersn

src/compiler/checker.ts

@@ -22440,30 +22440,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);
                 }
@@ -22475,12 +22468,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) {
@@ -28574,7 +28561,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) {

src/services/codefixes/fixImplicitThis.ts

@@ -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;
-        }
     }
 }

tests/baselines/reference/assignmentToVoidZero2.errors.txt

@@ -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;
             ~
@@ -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

tests/baselines/reference/assignmentToVoidZero2.symbols

@@ -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))

tests/baselines/reference/assignmentToVoidZero2.types

@@ -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

tests/baselines/reference/callbackCrossModule.symbols

@@ -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))
 }
 

tests/baselines/reference/callbackCrossModule.types

@@ -16,7 +16,7 @@ function C() {
     this.p = 1
 >this.p = 1 : 1
 >this.p : any
->this : any
+>this : this
 >p : any
 >1 : 1
 }

tests/baselines/reference/chainedPrototypeAssignment.symbols

@@ -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

tests/baselines/reference/chainedPrototypeAssignment.types

@@ -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
 }
@@ -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
 }

tests/baselines/reference/classCanExtendConstructorFunction.symbols

@@ -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))
 }

tests/baselines/reference/classCanExtendConstructorFunction.types

@@ -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
 }

tests/baselines/reference/commonjsAccessExports.symbols

@@ -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))
     }
 }

tests/baselines/reference/commonjsAccessExports.types

@@ -24,7 +24,7 @@ exports.x;
         this.x = 0;
 >this.x = 0 : 0
 >this.x : any
->this : any
+>this : this
 >x : any
 >0 : 0
     }

tests/baselines/reference/constructorFunctionMergeWithClass.symbols

@@ -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))
 
 };

tests/baselines/reference/constructorFunctionMergeWithClass.types

@@ -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
 

tests/baselines/reference/constructorFunctions.symbols

@@ -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))
 }
 
@@ -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))
 
 };

tests/baselines/reference/constructorFunctions.types

@@ -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
 }
@@ -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
 

tests/baselines/reference/constructorFunctions2.symbols

@@ -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; }
@@ -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;

tests/baselines/reference/constructorFunctions2.types

@@ -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
 
@@ -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
 

tests/baselines/reference/constructorFunctions3.symbols

@@ -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();
@@ -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'
@@ -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 */