Skip to content

Commit

Permalink
Follow symbol through commonjs require for inferred class type
Browse files Browse the repository at this point in the history
  • Loading branch information
rbuckton committed Aug 25, 2017
1 parent 13eeb34 commit ec8f5cf
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 9 deletions.
31 changes: 22 additions & 9 deletions src/compiler/checker.ts
Expand Up @@ -16327,8 +16327,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;

Expand All @@ -16343,6 +16343,21 @@ namespace ts {
return false;
}

function getJavaScriptClassType(symbol: Symbol): Type | undefined {
if (symbol && 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) {
Expand Down Expand Up @@ -16386,16 +16401,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;
Expand Down
41 changes: 41 additions & 0 deletions 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))

55 changes: 55 additions & 0 deletions 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

18 changes: 18 additions & 0 deletions 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;

0 comments on commit ec8f5cf

Please sign in to comment.