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