From 056a656937944f9f7366f630d278b2ececa39cd4 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 24 Oct 2019 08:37:10 -0700 Subject: [PATCH 1/2] Restore delayed merge check to getTypeFromJSDocValueReference This is needed when a function merges with a prototype assignment. The resulting *merged* symbol is a constructor function marked with SymbolFlags.Class. However, the merge doesn't happen until getTypeOfFuncClassEnumModule is called, which, in the getTypeReferenceType code path, doesn't happen until getTypeFromJSDocValueReference. That means the check for SymbolFlags.Class is missed. Previously, getTypeFromJSDocValueReference had a weird check `symbol !== getTypeOfSymbol(symbol).symbol`, which, if true, ran getTypeReferenceType again on `getTypeOfSymbol(symbol).symbol`. For JS "aliases", this had the effect of dereferencing the alias, and for function-prototype merges, this had the effect of ... just trying again after the merge had happened. This is a confusing way to run things. getTypeReferenceType should instead detect a function-prototype merge, cause it to happen, and *then* run the rest of its code instead of relying on try-again logic at the very end. However, for the RC, I want to fix this code by restoring the old check, with an additional check to make sure that #33106 doesn't break again: ```ts const valueType = getTypeOfSymbol(symbol) symbol !== valueType.symbol && getMergedSymbol(symbol) === valueType.symbol ``` I'll work on the real fix afterwards and put it into 3.8. --- src/compiler/checker.ts | 4 +- .../jsdocTypeReferenceToMergedClass.symbols | 33 ++++++++++++++++ .../jsdocTypeReferenceToMergedClass.types | 39 +++++++++++++++++++ .../jsdoc/jsdocTypeReferenceToMergedClass.ts | 16 ++++++++ 4 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/jsdocTypeReferenceToMergedClass.symbols create mode 100644 tests/baselines/reference/jsdocTypeReferenceToMergedClass.types create mode 100644 tests/cases/conformance/jsdoc/jsdocTypeReferenceToMergedClass.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index f30e765ab42c3..ed3057895fae8 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -10766,7 +10766,9 @@ namespace ts { && isCallExpression(decl.initializer) && isRequireCall(decl.initializer, /*requireStringLiteralLikeArgument*/ true) && valueType.symbol; - if (isRequireAlias || node.kind === SyntaxKind.ImportType) { + const isImportType = node.kind === SyntaxKind.ImportType; + const isDelayedMergeClass = symbol !== valueType.symbol && getMergedSymbol(symbol) === valueType.symbol; + if (isRequireAlias || isImportType || isDelayedMergeClass) { typeType = getTypeReferenceType(node, valueType.symbol); } } diff --git a/tests/baselines/reference/jsdocTypeReferenceToMergedClass.symbols b/tests/baselines/reference/jsdocTypeReferenceToMergedClass.symbols new file mode 100644 index 0000000000000..25aebdb79d41d --- /dev/null +++ b/tests/baselines/reference/jsdocTypeReferenceToMergedClass.symbols @@ -0,0 +1,33 @@ +=== tests/cases/conformance/jsdoc/test.js === +/** @param {Workspace.Project} p */ +function demo(p) { +>demo : Symbol(demo, Decl(test.js, 0, 0)) +>p : Symbol(p, Decl(test.js, 1, 14)) + + p.isServiceProject() +>p.isServiceProject : Symbol(isServiceProject, Decl(mod1.js, 3, 31)) +>p : Symbol(p, Decl(test.js, 1, 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.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.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)) + + isServiceProject() {} +>isServiceProject : Symbol(isServiceProject, Decl(mod1.js, 3, 31)) +} + diff --git a/tests/baselines/reference/jsdocTypeReferenceToMergedClass.types b/tests/baselines/reference/jsdocTypeReferenceToMergedClass.types new file mode 100644 index 0000000000000..1e4e7ef654d9b --- /dev/null +++ b/tests/baselines/reference/jsdocTypeReferenceToMergedClass.types @@ -0,0 +1,39 @@ +=== tests/cases/conformance/jsdoc/test.js === +/** @param {Workspace.Project} p */ +function demo(p) { +>demo : (p: wp) => void +>p : wp + + 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 +>Workspace.Project : typeof wp +>Workspace : typeof Workspace +>Project : typeof wp +>function wp() { } : typeof wp +>wp : typeof wp + +Workspace.Project.prototype = { +>Workspace.Project.prototype = { isServiceProject() {}} : { isServiceProject(): void; } +>Workspace.Project.prototype : { isServiceProject(): void; } +>Workspace.Project : typeof wp +>Workspace : typeof Workspace +>Project : typeof wp +>prototype : { isServiceProject(): void; } +>{ isServiceProject() {}} : { isServiceProject(): void; } + + isServiceProject() {} +>isServiceProject : () => void +} + diff --git a/tests/cases/conformance/jsdoc/jsdocTypeReferenceToMergedClass.ts b/tests/cases/conformance/jsdoc/jsdocTypeReferenceToMergedClass.ts new file mode 100644 index 0000000000000..d840b051d9bd1 --- /dev/null +++ b/tests/cases/conformance/jsdoc/jsdocTypeReferenceToMergedClass.ts @@ -0,0 +1,16 @@ +// @noEmit: true +// @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 +var Workspace = {} +Workspace.Project = function wp() { } +Workspace.Project.prototype = { + isServiceProject() {} +} From d1515f4ee0c6b7c2099565afa00630c39b58553b Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 24 Oct 2019 08:59:57 -0700 Subject: [PATCH 2/2] Add bug number --- .../reference/jsdocTypeReferenceToMergedClass.symbols | 6 ++++-- .../reference/jsdocTypeReferenceToMergedClass.types | 2 ++ .../conformance/jsdoc/jsdocTypeReferenceToMergedClass.ts | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/baselines/reference/jsdocTypeReferenceToMergedClass.symbols b/tests/baselines/reference/jsdocTypeReferenceToMergedClass.symbols index 25aebdb79d41d..229163c5d788b 100644 --- a/tests/baselines/reference/jsdocTypeReferenceToMergedClass.symbols +++ b/tests/baselines/reference/jsdocTypeReferenceToMergedClass.symbols @@ -1,12 +1,14 @@ === tests/cases/conformance/jsdoc/test.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, 1, 14)) +>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, 1, 14)) +>p : Symbol(p, Decl(test.js, 3, 14)) >isServiceProject : Symbol(isServiceProject, Decl(mod1.js, 3, 31)) } === tests/cases/conformance/jsdoc/mod1.js === diff --git a/tests/baselines/reference/jsdocTypeReferenceToMergedClass.types b/tests/baselines/reference/jsdocTypeReferenceToMergedClass.types index 1e4e7ef654d9b..c4e380f8c0eb7 100644 --- a/tests/baselines/reference/jsdocTypeReferenceToMergedClass.types +++ b/tests/baselines/reference/jsdocTypeReferenceToMergedClass.types @@ -1,4 +1,6 @@ === tests/cases/conformance/jsdoc/test.js === +// https://github.com/microsoft/TypeScript/issues/34685 + /** @param {Workspace.Project} p */ function demo(p) { >demo : (p: wp) => void diff --git a/tests/cases/conformance/jsdoc/jsdocTypeReferenceToMergedClass.ts b/tests/cases/conformance/jsdoc/jsdocTypeReferenceToMergedClass.ts index d840b051d9bd1..e559ffd27ca6f 100644 --- a/tests/cases/conformance/jsdoc/jsdocTypeReferenceToMergedClass.ts +++ b/tests/cases/conformance/jsdoc/jsdocTypeReferenceToMergedClass.ts @@ -1,3 +1,4 @@ +// https://github.com/microsoft/TypeScript/issues/34685 // @noEmit: true // @allowJs: true // @checkJs: true