From cba570eedc32a3edffcb245e0789e816fe08a647 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 24 Oct 2019 10:18:22 -0700 Subject: [PATCH 1/2] 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. --- src/compiler/checker.ts | 14 +++--- .../jsdocTypeReferenceToMergedClass.symbols | 45 +++++++++---------- .../jsdocTypeReferenceToMergedClass.types | 19 ++++---- .../jsdoc/jsdocTypeReferenceToMergedClass.ts | 12 +++-- 4 files changed, 43 insertions(+), 47 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index ed3057895fae8..d0ba7a25820ec 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); + } + } } @@ -10723,6 +10728,7 @@ namespace ts { if (symbol === unknownSymbol) { return errorType; } + symbol = getExpandoSymbol(symbol) || symbol; if (symbol.flags & (SymbolFlags.Class | SymbolFlags.Interface)) { return getTypeFromClassOrInterfaceReference(node, symbol); @@ -10766,9 +10772,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 +25001,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 229163c5d788b..86dcb53641926 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 c4e380f8c0eb7..3fd1661fa29a3 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 e559ffd27ca6f..931ad69cfb072 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() {} From a15996db01dff9bc9747ba243177781549962607 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 24 Oct 2019 10:55:40 -0700 Subject: [PATCH 2/2] Remove extra newline --- src/compiler/checker.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index d0ba7a25820ec..052c89a7b09d5 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -10728,7 +10728,6 @@ namespace ts { if (symbol === unknownSymbol) { return errorType; } - symbol = getExpandoSymbol(symbol) || symbol; if (symbol.flags & (SymbolFlags.Class | SymbolFlags.Interface)) { return getTypeFromClassOrInterfaceReference(node, symbol);