Skip to content

Commit 969634b

Browse files
authored
Restore delayed merge check to getTypeFromJSDocValueReference (microsoft#34706)
* 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 microsoft#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. * Add bug number
1 parent 07d3a2e commit 969634b

File tree

4 files changed

+96
-1
lines changed

4 files changed

+96
-1
lines changed

src/compiler/checker.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10766,7 +10766,9 @@ namespace ts {
1076610766
&& isCallExpression(decl.initializer)
1076710767
&& isRequireCall(decl.initializer, /*requireStringLiteralLikeArgument*/ true)
1076810768
&& valueType.symbol;
10769-
if (isRequireAlias || node.kind === SyntaxKind.ImportType) {
10769+
const isImportType = node.kind === SyntaxKind.ImportType;
10770+
const isDelayedMergeClass = symbol !== valueType.symbol && getMergedSymbol(symbol) === valueType.symbol;
10771+
if (isRequireAlias || isImportType || isDelayedMergeClass) {
1077010772
typeType = getTypeReferenceType(node, valueType.symbol);
1077110773
}
1077210774
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
=== tests/cases/conformance/jsdoc/test.js ===
2+
// https://github.com/microsoft/TypeScript/issues/34685
3+
4+
/** @param {Workspace.Project} p */
5+
function demo(p) {
6+
>demo : Symbol(demo, Decl(test.js, 0, 0))
7+
>p : Symbol(p, Decl(test.js, 3, 14))
8+
9+
p.isServiceProject()
10+
>p.isServiceProject : Symbol(isServiceProject, Decl(mod1.js, 3, 31))
11+
>p : Symbol(p, Decl(test.js, 3, 14))
12+
>isServiceProject : Symbol(isServiceProject, Decl(mod1.js, 3, 31))
13+
}
14+
=== tests/cases/conformance/jsdoc/mod1.js ===
15+
// Note: mod1.js needs to appear second to trigger the bug
16+
var Workspace = {}
17+
>Workspace : Symbol(Workspace, Decl(mod1.js, 1, 3), Decl(mod1.js, 1, 18), Decl(mod1.js, 2, 37))
18+
19+
Workspace.Project = function wp() { }
20+
>Workspace.Project : Symbol(Workspace.Project, Decl(mod1.js, 1, 18), Decl(mod1.js, 3, 10))
21+
>Workspace : Symbol(Workspace, Decl(mod1.js, 1, 3), Decl(mod1.js, 1, 18), Decl(mod1.js, 2, 37))
22+
>Project : Symbol(Workspace.Project, Decl(mod1.js, 1, 18), Decl(mod1.js, 3, 10))
23+
>wp : Symbol(wp, Decl(mod1.js, 2, 19))
24+
25+
Workspace.Project.prototype = {
26+
>Workspace.Project.prototype : Symbol(Workspace.Project.prototype, Decl(mod1.js, 2, 37))
27+
>Workspace.Project : Symbol(Workspace.Project, Decl(mod1.js, 1, 18), Decl(mod1.js, 3, 10))
28+
>Workspace : Symbol(Workspace, Decl(mod1.js, 1, 3), Decl(mod1.js, 1, 18), Decl(mod1.js, 2, 37))
29+
>Project : Symbol(Workspace.Project, Decl(mod1.js, 1, 18), Decl(mod1.js, 3, 10))
30+
>prototype : Symbol(Workspace.Project.prototype, Decl(mod1.js, 2, 37))
31+
32+
isServiceProject() {}
33+
>isServiceProject : Symbol(isServiceProject, Decl(mod1.js, 3, 31))
34+
}
35+
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
=== tests/cases/conformance/jsdoc/test.js ===
2+
// https://github.com/microsoft/TypeScript/issues/34685
3+
4+
/** @param {Workspace.Project} p */
5+
function demo(p) {
6+
>demo : (p: wp) => void
7+
>p : wp
8+
9+
p.isServiceProject()
10+
>p.isServiceProject() : void
11+
>p.isServiceProject : () => void
12+
>p : wp
13+
>isServiceProject : () => void
14+
}
15+
=== tests/cases/conformance/jsdoc/mod1.js ===
16+
// Note: mod1.js needs to appear second to trigger the bug
17+
var Workspace = {}
18+
>Workspace : typeof Workspace
19+
>{} : {}
20+
21+
Workspace.Project = function wp() { }
22+
>Workspace.Project = function wp() { } : typeof wp
23+
>Workspace.Project : typeof wp
24+
>Workspace : typeof Workspace
25+
>Project : typeof wp
26+
>function wp() { } : typeof wp
27+
>wp : typeof wp
28+
29+
Workspace.Project.prototype = {
30+
>Workspace.Project.prototype = { isServiceProject() {}} : { isServiceProject(): void; }
31+
>Workspace.Project.prototype : { isServiceProject(): void; }
32+
>Workspace.Project : typeof wp
33+
>Workspace : typeof Workspace
34+
>Project : typeof wp
35+
>prototype : { isServiceProject(): void; }
36+
>{ isServiceProject() {}} : { isServiceProject(): void; }
37+
38+
isServiceProject() {}
39+
>isServiceProject : () => void
40+
}
41+
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// https://github.com/microsoft/TypeScript/issues/34685
2+
// @noEmit: true
3+
// @allowJs: true
4+
// @checkJs: true
5+
6+
// @Filename: test.js
7+
/** @param {Workspace.Project} p */
8+
function demo(p) {
9+
p.isServiceProject()
10+
}
11+
// @Filename: mod1.js
12+
// Note: mod1.js needs to appear second to trigger the bug
13+
var Workspace = {}
14+
Workspace.Project = function wp() { }
15+
Workspace.Project.prototype = {
16+
isServiceProject() {}
17+
}

0 commit comments

Comments
 (0)