From d892fd408fecc86cf2f262821344b8c2429be392 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 24 Oct 2019 13:12:44 -0700 Subject: [PATCH] Fix expando handling in getTypeReferenceType (#34712) * Fix expando handling in getTypeReferenceType getExpandoSymbol looks for the initialiser of a symbol when it is an expando value (IIFEs, function exprs, class exprs and empty object literals) and returns the symbol. Previously, however, it returned the symbol of the initialiser without merging with the declaration symbol itself. This missed, in particular, the prototype assignment in the following pattern: ```js var x = function x() { this.y = 1 } x.prototype = { z() { } } /** @type {x} */ var xx; xx.z // missed! ``` getJSDocValueReference had weird try-again code that relied on calling getTypeOfSymbol, which *does* correctly merge the symbols. This PR re-removes that code and instead makes getExpandoSymbol call mergeJSSymbols itself. * Remove extra newline --- src/compiler/checker.ts | 13 +++--- .../jsdocTypeReferenceToMergedClass.symbols | 45 +++++++++---------- .../jsdocTypeReferenceToMergedClass.types | 19 ++++---- .../jsdoc/jsdocTypeReferenceToMergedClass.ts | 12 +++-- 4 files changed, 42 insertions(+), 47 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index ed3057895fa..052c89a7b09 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -2629,7 +2629,12 @@ namespace ts { return undefined; } const init = isVariableDeclaration(decl) ? getDeclaredExpandoInitializer(decl) : getAssignedExpandoInitializer(decl); - return init && getSymbolOfNode(init) || undefined; + if (init) { + const initSymbol = getSymbolOfNode(init); + if (initSymbol) { + return mergeJSSymbols(initSymbol, symbol); + } + } } @@ -10766,9 +10771,7 @@ namespace ts { && isCallExpression(decl.initializer) && isRequireCall(decl.initializer, /*requireStringLiteralLikeArgument*/ true) && valueType.symbol; - const isImportType = node.kind === SyntaxKind.ImportType; - const isDelayedMergeClass = symbol !== valueType.symbol && getMergedSymbol(symbol) === valueType.symbol; - if (isRequireAlias || isImportType || isDelayedMergeClass) { + if (isRequireAlias || node.kind === SyntaxKind.ImportType) { typeType = getTypeReferenceType(node, valueType.symbol); } } @@ -24997,7 +25000,7 @@ namespace ts { } function mergeJSSymbols(target: Symbol, source: Symbol | undefined) { - if (source && (hasEntries(source.exports) || hasEntries(source.members))) { + if (source) { const links = getSymbolLinks(source); if (!links.inferredClassSymbol || !links.inferredClassSymbol.has("" + getSymbolId(target))) { const inferred = isTransientSymbol(target) ? target : cloneSymbol(target) as TransientSymbol; diff --git a/tests/baselines/reference/jsdocTypeReferenceToMergedClass.symbols b/tests/baselines/reference/jsdocTypeReferenceToMergedClass.symbols index 229163c5d78..86dcb536419 100644 --- a/tests/baselines/reference/jsdocTypeReferenceToMergedClass.symbols +++ b/tests/baselines/reference/jsdocTypeReferenceToMergedClass.symbols @@ -1,35 +1,32 @@ -=== tests/cases/conformance/jsdoc/test.js === +=== tests/cases/conformance/jsdoc/jsdocTypeReferenceToMergedClass.js === // https://github.com/microsoft/TypeScript/issues/34685 -/** @param {Workspace.Project} p */ -function demo(p) { ->demo : Symbol(demo, Decl(test.js, 0, 0)) ->p : Symbol(p, Decl(test.js, 3, 14)) - - p.isServiceProject() ->p.isServiceProject : Symbol(isServiceProject, Decl(mod1.js, 3, 31)) ->p : Symbol(p, Decl(test.js, 3, 14)) ->isServiceProject : Symbol(isServiceProject, Decl(mod1.js, 3, 31)) -} -=== tests/cases/conformance/jsdoc/mod1.js === -// Note: mod1.js needs to appear second to trigger the bug var Workspace = {} ->Workspace : Symbol(Workspace, Decl(mod1.js, 1, 3), Decl(mod1.js, 1, 18), Decl(mod1.js, 2, 37)) +>Workspace : Symbol(Workspace, Decl(jsdocTypeReferenceToMergedClass.js, 2, 3), Decl(jsdocTypeReferenceToMergedClass.js, 5, 20), Decl(jsdocTypeReferenceToMergedClass.js, 7, 37)) + +/** @type {Workspace.Project} */ +var p; +>p : Symbol(p, Decl(jsdocTypeReferenceToMergedClass.js, 4, 3)) + +p.isServiceProject() +>p.isServiceProject : Symbol(isServiceProject, Decl(jsdocTypeReferenceToMergedClass.js, 8, 31)) +>p : Symbol(p, Decl(jsdocTypeReferenceToMergedClass.js, 4, 3)) +>isServiceProject : Symbol(isServiceProject, Decl(jsdocTypeReferenceToMergedClass.js, 8, 31)) Workspace.Project = function wp() { } ->Workspace.Project : Symbol(Workspace.Project, Decl(mod1.js, 1, 18), Decl(mod1.js, 3, 10)) ->Workspace : Symbol(Workspace, Decl(mod1.js, 1, 3), Decl(mod1.js, 1, 18), Decl(mod1.js, 2, 37)) ->Project : Symbol(Workspace.Project, Decl(mod1.js, 1, 18), Decl(mod1.js, 3, 10)) ->wp : Symbol(wp, Decl(mod1.js, 2, 19)) +>Workspace.Project : Symbol(Workspace.Project, Decl(jsdocTypeReferenceToMergedClass.js, 5, 20), Decl(jsdocTypeReferenceToMergedClass.js, 8, 10)) +>Workspace : Symbol(Workspace, Decl(jsdocTypeReferenceToMergedClass.js, 2, 3), Decl(jsdocTypeReferenceToMergedClass.js, 5, 20), Decl(jsdocTypeReferenceToMergedClass.js, 7, 37)) +>Project : Symbol(Workspace.Project, Decl(jsdocTypeReferenceToMergedClass.js, 5, 20), Decl(jsdocTypeReferenceToMergedClass.js, 8, 10)) +>wp : Symbol(wp, Decl(jsdocTypeReferenceToMergedClass.js, 7, 19)) Workspace.Project.prototype = { ->Workspace.Project.prototype : Symbol(Workspace.Project.prototype, Decl(mod1.js, 2, 37)) ->Workspace.Project : Symbol(Workspace.Project, Decl(mod1.js, 1, 18), Decl(mod1.js, 3, 10)) ->Workspace : Symbol(Workspace, Decl(mod1.js, 1, 3), Decl(mod1.js, 1, 18), Decl(mod1.js, 2, 37)) ->Project : Symbol(Workspace.Project, Decl(mod1.js, 1, 18), Decl(mod1.js, 3, 10)) ->prototype : Symbol(Workspace.Project.prototype, Decl(mod1.js, 2, 37)) +>Workspace.Project.prototype : Symbol(Workspace.Project.prototype, Decl(jsdocTypeReferenceToMergedClass.js, 7, 37)) +>Workspace.Project : Symbol(Workspace.Project, Decl(jsdocTypeReferenceToMergedClass.js, 5, 20), Decl(jsdocTypeReferenceToMergedClass.js, 8, 10)) +>Workspace : Symbol(Workspace, Decl(jsdocTypeReferenceToMergedClass.js, 2, 3), Decl(jsdocTypeReferenceToMergedClass.js, 5, 20), Decl(jsdocTypeReferenceToMergedClass.js, 7, 37)) +>Project : Symbol(Workspace.Project, Decl(jsdocTypeReferenceToMergedClass.js, 5, 20), Decl(jsdocTypeReferenceToMergedClass.js, 8, 10)) +>prototype : Symbol(Workspace.Project.prototype, Decl(jsdocTypeReferenceToMergedClass.js, 7, 37)) isServiceProject() {} ->isServiceProject : Symbol(isServiceProject, Decl(mod1.js, 3, 31)) +>isServiceProject : Symbol(isServiceProject, Decl(jsdocTypeReferenceToMergedClass.js, 8, 31)) } diff --git a/tests/baselines/reference/jsdocTypeReferenceToMergedClass.types b/tests/baselines/reference/jsdocTypeReferenceToMergedClass.types index c4e380f8c0e..3fd1661fa29 100644 --- a/tests/baselines/reference/jsdocTypeReferenceToMergedClass.types +++ b/tests/baselines/reference/jsdocTypeReferenceToMergedClass.types @@ -1,22 +1,19 @@ -=== tests/cases/conformance/jsdoc/test.js === +=== tests/cases/conformance/jsdoc/jsdocTypeReferenceToMergedClass.js === // https://github.com/microsoft/TypeScript/issues/34685 -/** @param {Workspace.Project} p */ -function demo(p) { ->demo : (p: wp) => void +var Workspace = {} +>Workspace : typeof Workspace +>{} : {} + +/** @type {Workspace.Project} */ +var p; >p : wp - p.isServiceProject() +p.isServiceProject() >p.isServiceProject() : void >p.isServiceProject : () => void >p : wp >isServiceProject : () => void -} -=== tests/cases/conformance/jsdoc/mod1.js === -// Note: mod1.js needs to appear second to trigger the bug -var Workspace = {} ->Workspace : typeof Workspace ->{} : {} Workspace.Project = function wp() { } >Workspace.Project = function wp() { } : typeof wp diff --git a/tests/cases/conformance/jsdoc/jsdocTypeReferenceToMergedClass.ts b/tests/cases/conformance/jsdoc/jsdocTypeReferenceToMergedClass.ts index e559ffd27ca..931ad69cfb0 100644 --- a/tests/cases/conformance/jsdoc/jsdocTypeReferenceToMergedClass.ts +++ b/tests/cases/conformance/jsdoc/jsdocTypeReferenceToMergedClass.ts @@ -3,14 +3,12 @@ // @allowJs: true // @checkJs: true -// @Filename: test.js -/** @param {Workspace.Project} p */ -function demo(p) { - p.isServiceProject() -} -// @Filename: mod1.js -// Note: mod1.js needs to appear second to trigger the bug +// @Filename: jsdocTypeReferenceToMergedClass.js var Workspace = {} +/** @type {Workspace.Project} */ +var p; +p.isServiceProject() + Workspace.Project = function wp() { } Workspace.Project.prototype = { isServiceProject() {}