Merge pull request #16686 from Microsoft/fix16467

Improve JavaScript type from constructor imported via require

Author: rbuckton

src/compiler/checker.ts

@@ -16388,8 +16388,8 @@ namespace ts {
          * Indicates whether a declaration can be treated as a constructor in a JavaScript
          * file.
          */
-        function isJavaScriptConstructor(node: Declaration): boolean {
-            if (isInJavaScriptFile(node)) {
+        function isJavaScriptConstructor(node: Declaration | undefined): boolean {
+            if (node && isInJavaScriptFile(node)) {
                 // If the node has a @class tag, treat it like a constructor.
                 if (getJSDocClassTag(node)) return true;
 
@@ -16404,6 +16404,21 @@ namespace ts {
             return false;
         }
 
+        function getJavaScriptClassType(symbol: Symbol): Type | undefined {
+            if (isDeclarationOfFunctionOrClassExpression(symbol)) {
+                symbol = getSymbolOfNode((<VariableDeclaration>symbol.valueDeclaration).initializer);
+            }
+            if (isJavaScriptConstructor(symbol.valueDeclaration)) {
+                return getInferredClassType(symbol);
+            }
+            if (symbol.flags & SymbolFlags.Variable) {
+                const valueType = getTypeOfSymbol(symbol);
+                if (valueType.symbol && !isInferredClassType(valueType) && isJavaScriptConstructor(valueType.symbol.valueDeclaration)) {
+                    return getInferredClassType(valueType.symbol);
+                }
+            }
+        }
+
         function getInferredClassType(symbol: Symbol) {
             const links = getSymbolLinks(symbol);
             if (!links.inferredClassType) {
@@ -16447,16 +16462,14 @@ namespace ts {
                     // in a JS file
                     // Note:JS inferred classes might come from a variable declaration instead of a function declaration.
                     // In this case, using getResolvedSymbol directly is required to avoid losing the members from the declaration.
-                    let funcSymbol = node.expression.kind === SyntaxKind.Identifier ?
+                    const funcSymbol = node.expression.kind === SyntaxKind.Identifier ?
                         getResolvedSymbol(node.expression as Identifier) :
                         checkExpression(node.expression).symbol;
-                    if (funcSymbol && isDeclarationOfFunctionOrClassExpression(funcSymbol)) {
-                        funcSymbol = getSymbolOfNode((<VariableDeclaration>funcSymbol.valueDeclaration).initializer);
-                    }
-                    if (funcSymbol && funcSymbol.flags & SymbolFlags.Function && (funcSymbol.members || getJSDocClassTag(funcSymbol.valueDeclaration))) {
-                        return getInferredClassType(funcSymbol);
+                    const type = funcSymbol && getJavaScriptClassType(funcSymbol);
+                    if (type) {
+                        return type;
                     }
-                    else if (noImplicitAny) {
+                    if (noImplicitAny) {
                         error(node, Diagnostics.new_expression_whose_target_lacks_a_construct_signature_implicitly_has_an_any_type);
                     }
                     return anyType;

tests/baselines/reference/constructorFunctions2.symbols

@@ -0,0 +1,41 @@
+=== tests/cases/conformance/salsa/node.d.ts ===
+declare function require(id: string): any;
+>require : Symbol(require, Decl(node.d.ts, 0, 0))
+>id : Symbol(id, Decl(node.d.ts, 0, 25))
+
+declare var module: any, exports: any;
+>module : Symbol(module, Decl(node.d.ts, 1, 11))
+>exports : Symbol(exports, Decl(node.d.ts, 1, 24))
+
+=== tests/cases/conformance/salsa/index.js ===
+const A = require("./other");
+>A : Symbol(A, Decl(index.js, 0, 5))
+>require : Symbol(require, Decl(node.d.ts, 0, 0))
+>"./other" : Symbol("tests/cases/conformance/salsa/other", Decl(other.js, 0, 0))
+
+const a = new A().id;
+>a : Symbol(a, Decl(index.js, 1, 5))
+>new A().id : Symbol(A.id, Decl(other.js, 0, 14))
+>A : Symbol(A, Decl(index.js, 0, 5))
+>id : Symbol(A.id, Decl(other.js, 0, 14))
+
+const B = function() { this.id = 1; }
+>B : Symbol(B, Decl(index.js, 3, 5))
+>id : Symbol(B.id, Decl(index.js, 3, 22))
+
+const b = new B().id;
+>b : Symbol(b, Decl(index.js, 4, 5))
+>new B().id : Symbol(B.id, Decl(index.js, 3, 22))
+>B : Symbol(B, Decl(index.js, 3, 5))
+>id : Symbol(B.id, Decl(index.js, 3, 22))
+
+=== tests/cases/conformance/salsa/other.js ===
+function A() { this.id = 1; }
+>A : Symbol(A, Decl(other.js, 0, 0))
+>id : Symbol(A.id, Decl(other.js, 0, 14))
+
+module.exports = A;
+>module : Symbol(export=, Decl(other.js, 0, 29))
+>exports : Symbol(export=, Decl(other.js, 0, 29))
+>A : Symbol(A, Decl(other.js, 0, 0))
+

tests/baselines/reference/constructorFunctions2.types

@@ -0,0 +1,55 @@
+=== tests/cases/conformance/salsa/node.d.ts ===
+declare function require(id: string): any;
+>require : (id: string) => any
+>id : string
+
+declare var module: any, exports: any;
+>module : any
+>exports : any
+
+=== tests/cases/conformance/salsa/index.js ===
+const A = require("./other");
+>A : () => void
+>require("./other") : () => void
+>require : (id: string) => any
+>"./other" : "./other"
+
+const a = new A().id;
+>a : number
+>new A().id : number
+>new A() : { id: number; }
+>A : () => void
+>id : number
+
+const B = function() { this.id = 1; }
+>B : () => void
+>function() { this.id = 1; } : () => void
+>this.id = 1 : 1
+>this.id : any
+>this : any
+>id : any
+>1 : 1
+
+const b = new B().id;
+>b : number
+>new B().id : number
+>new B() : { id: number; }
+>B : () => void
+>id : number
+
+=== tests/cases/conformance/salsa/other.js ===
+function A() { this.id = 1; }
+>A : () => void
+>this.id = 1 : 1
+>this.id : any
+>this : any
+>id : any
+>1 : 1
+
+module.exports = A;
+>module.exports = A : () => void
+>module.exports : any
+>module : any
+>exports : any
+>A : () => void
+

tests/cases/conformance/salsa/constructorFunctions2.ts

@@ -0,0 +1,18 @@
+// @allowJs: true
+// @checkJs: true
+// @noEmit: true
+// @module: commonjs
+// @filename: node.d.ts
+declare function require(id: string): any;
+declare var module: any, exports: any;
+
+// @filename: index.js
+const A = require("./other");
+const a = new A().id;
+
+const B = function() { this.id = 1; }
+const b = new B().id;
+
+// @filename: other.js
+function A() { this.id = 1; }
+module.exports = A;