From dfedb24f759f9d0cb13732a82d20dd70ab0e1e22 Mon Sep 17 00:00:00 2001 From: James Keane Date: Tue, 31 Jul 2018 16:52:39 -0400 Subject: [PATCH] Jsdoc `@constructor` - in constructor properly infer `this` as class instance (#25980) * Properly infer `this` in tagged `@constructor`s. `c.prototype.method = function() { this }` was already supported. This commit add support to two more kinds relying on the JSDoc `@constructor` tag. These are: 1. `/** @constructor */ function Example() { this }` 2. `/** @constructor */ var Example = function() { this }` * Update the baseline for js constructorFunctions. C3 and C4 `this` was set as `any`, now it is properly showing as the class type. * Fix lint errors * Add circular initialisers to constructo fn tests. * Error (`TS2348`) if calling tagged js constructors When calling a JS function explicitly tagged with either `@class` or `@constructor` the checker should throw a TS2348 not callable error. * Don't resolve jsdoc classes with construct sigs. This undoes the last commit that sought to change how js functions tagged with `@class` were inferred. For some reason, currently unknown, giving those functions construct signatures causes issues in property assignment/member resolution (as seen in the `typeFromPropertyAssignment12` test case). Instead of changing the signature resolution, the error is explicitly generated in `resolveCallExpression` for those functions. --- src/compiler/checker.ts | 20 ++++++- .../reference/constructorFunctions.errors.txt | 56 +++++++++++++++++++ .../reference/constructorFunctions.symbols | 22 ++++++++ .../reference/constructorFunctions.types | 34 ++++++++++- .../conformance/salsa/constructorFunctions.ts | 6 ++ 5 files changed, 134 insertions(+), 4 deletions(-) create mode 100644 tests/baselines/reference/constructorFunctions.errors.txt diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 5c46f3e7db..868957d0fc 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -15404,8 +15404,8 @@ namespace ts { (!isInParameterInitializerBeforeContainingFunction(node) || getThisParameter(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. Check if it's the RHS - // of a x.prototype.y = function [name]() { .... } + // If this is a function in a JS file, it might be a class method. + // Check if it's the RHS of a x.prototype.y = function [name]() { .... } if (container.kind === SyntaxKind.FunctionExpression && container.parent.kind === SyntaxKind.BinaryExpression && getSpecialPropertyAssignmentKind(container.parent as BinaryExpression) === SpecialPropertyAssignmentKind.PrototypeProperty) { @@ -15419,6 +15419,17 @@ namespace ts { return getFlowTypeOfReference(node, getInferredClassType(classSymbol)); } } + // 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 ((container.kind === SyntaxKind.FunctionExpression || container.kind === SyntaxKind.FunctionDeclaration) && + getJSDocClassTag(container)) { + const classType = getJavaScriptClassType(container.symbol); + if (classType) { + return getFlowTypeOfReference(node, classType); + } + } const thisType = getThisTypeOfDeclaration(container) || getContextualThisParameterType(container); if (thisType) { @@ -19497,6 +19508,11 @@ namespace ts { } return resolveErrorCall(node); } + // If the function is explicitly marked with `@class`, then it must be constructed. + if (callSignatures.some(sig => isInJavaScriptFile(sig.declaration) && !!getJSDocClassTag(sig.declaration!))) { + error(node, Diagnostics.Value_of_type_0_is_not_callable_Did_you_mean_to_include_new, typeToString(funcType)); + return resolveErrorCall(node); + } return resolveCall(node, callSignatures, candidatesOutArray, isForSignatureHelp); } diff --git a/tests/baselines/reference/constructorFunctions.errors.txt b/tests/baselines/reference/constructorFunctions.errors.txt new file mode 100644 index 0000000000..8b2fc319c5 --- /dev/null +++ b/tests/baselines/reference/constructorFunctions.errors.txt @@ -0,0 +1,56 @@ +tests/cases/conformance/salsa/index.js(22,15): error TS2348: Value of type 'typeof C3' is not callable. Did you mean to include 'new'? +tests/cases/conformance/salsa/index.js(30,15): error TS2348: Value of type 'typeof C4' is not callable. Did you mean to include 'new'? + + +==== tests/cases/conformance/salsa/index.js (2 errors) ==== + function C1() { + if (!(this instanceof C1)) return new C1(); + this.x = 1; + } + + const c1_v1 = C1(); + const c1_v2 = new C1(); + + var C2 = function () { + if (!(this instanceof C2)) return new C2(); + this.x = 1; + }; + + const c2_v1 = C2(); + const c2_v2 = new C2(); + + /** @class */ + function C3() { + if (!(this instanceof C3)) return new C3(); + }; + + const c3_v1 = C3(); + ~~~~ +!!! error TS2348: Value of type 'typeof C3' is not callable. Did you mean to include 'new'? + const c3_v2 = new C3(); + + /** @class */ + var C4 = function () { + if (!(this instanceof C4)) return new C4(); + }; + + const c4_v1 = C4(); + ~~~~ +!!! error TS2348: Value of type 'typeof C4' is not callable. Did you mean to include 'new'? + const c4_v2 = new C4(); + + var c5_v1; + c5_v1 = function f() { }; + new c5_v1(); + + var c5_v2; + c5_v2 = class { }; + new c5_v2(); + + /** @class */ + function C6() { + this.functions = [x => x, x => x + 1, x => x - 1] + }; + + var c6_v1 = new C6(); + \ No newline at end of file diff --git a/tests/baselines/reference/constructorFunctions.symbols b/tests/baselines/reference/constructorFunctions.symbols index abd77f3ea2..21dd0fdebf 100644 --- a/tests/baselines/reference/constructorFunctions.symbols +++ b/tests/baselines/reference/constructorFunctions.symbols @@ -43,6 +43,7 @@ function C3() { >C3 : Symbol(C3, Decl(index.js, 14, 23)) if (!(this instanceof C3)) return new C3(); +>this : Symbol(C3, Decl(index.js, 14, 23)) >C3 : Symbol(C3, Decl(index.js, 14, 23)) >C3 : Symbol(C3, Decl(index.js, 14, 23)) @@ -61,6 +62,7 @@ var C4 = function () { >C4 : Symbol(C4, Decl(index.js, 25, 3)) if (!(this instanceof C4)) return new C4(); +>this : Symbol(C4, Decl(index.js, 25, 8)) >C4 : Symbol(C4, Decl(index.js, 25, 3)) >C4 : Symbol(C4, Decl(index.js, 25, 3)) @@ -93,4 +95,24 @@ c5_v2 = class { }; new c5_v2(); >c5_v2 : Symbol(c5_v2, Decl(index.js, 36, 3)) +/** @class */ +function C6() { +>C6 : Symbol(C6, Decl(index.js, 38, 12)) + + this.functions = [x => x, x => x + 1, x => x - 1] +>this.functions : Symbol(C6.functions, Decl(index.js, 41, 15)) +>this : Symbol(C6, Decl(index.js, 38, 12)) +>functions : Symbol(C6.functions, Decl(index.js, 41, 15)) +>x : Symbol(x, Decl(index.js, 42, 20)) +>x : Symbol(x, Decl(index.js, 42, 20)) +>x : Symbol(x, Decl(index.js, 42, 27)) +>x : Symbol(x, Decl(index.js, 42, 27)) +>x : Symbol(x, Decl(index.js, 42, 39)) +>x : Symbol(x, Decl(index.js, 42, 39)) + +}; + +var c6_v1 = new C6(); +>c6_v1 : Symbol(c6_v1, Decl(index.js, 45, 3)) +>C6 : Symbol(C6, Decl(index.js, 38, 12)) diff --git a/tests/baselines/reference/constructorFunctions.types b/tests/baselines/reference/constructorFunctions.types index 7b8d6b0e2e..582ba35cfe 100644 --- a/tests/baselines/reference/constructorFunctions.types +++ b/tests/baselines/reference/constructorFunctions.types @@ -69,7 +69,7 @@ function C3() { >!(this instanceof C3) : boolean >(this instanceof C3) : boolean >this instanceof C3 : boolean ->this : any +>this : C3 >C3 : typeof C3 >new C3() : C3 >C3 : typeof C3 @@ -95,7 +95,7 @@ var C4 = function () { >!(this instanceof C4) : boolean >(this instanceof C4) : boolean >this instanceof C4 : boolean ->this : any +>this : C4 >C4 : typeof C4 >new C4() : C4 >C4 : typeof C4 @@ -137,4 +137,34 @@ new c5_v2(); >new c5_v2() : c5_v2 >c5_v2 : typeof c5_v2 +/** @class */ +function C6() { +>C6 : typeof C6 + + this.functions = [x => x, x => x + 1, x => x - 1] +>this.functions = [x => x, x => x + 1, x => x - 1] : ((x: any) => any)[] +>this.functions : ((x: any) => any)[] +>this : C6 +>functions : ((x: any) => any)[] +>[x => x, x => x + 1, x => x - 1] : ((x: any) => any)[] +>x => x : (x: any) => any +>x : any +>x : any +>x => x + 1 : (x: any) => any +>x : any +>x + 1 : any +>x : any +>1 : 1 +>x => x - 1 : (x: any) => number +>x : any +>x - 1 : number +>x : any +>1 : 1 + +}; + +var c6_v1 = new C6(); +>c6_v1 : C6 +>new C6() : C6 +>C6 : typeof C6 diff --git a/tests/cases/conformance/salsa/constructorFunctions.ts b/tests/cases/conformance/salsa/constructorFunctions.ts index 6c166a22f7..155f8d5c27 100644 --- a/tests/cases/conformance/salsa/constructorFunctions.ts +++ b/tests/cases/conformance/salsa/constructorFunctions.ts @@ -43,3 +43,9 @@ var c5_v2; c5_v2 = class { }; new c5_v2(); +/** @class */ +function C6() { + this.functions = [x => x, x => x + 1, x => x - 1] +}; + +var c6_v1 = new C6();