Skip to content

Commit 0fe7acc

Browse files
ortaorta
committed
Only do an extra JSDoc type reference lookup when on a class - fixes microsoft#36830 
1 parent 9f332db commit 0fe7acc

8 files changed

Lines changed: 77 additions & 12 deletions

‎src/compiler/checker.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11616,9 +11616,15 @@ namespace ts {
1161611616
}
1161711617
isRequireAlias = isCallExpression(expr) && isRequireCall(expr, /*requireStringLiteralLikeArgument*/ true) && !!valueType.symbol;
1161811618
}
11619-
const isImportTypeWithQualifier = node.kind === SyntaxKind.ImportType && (node as ImportTypeNode).qualifier;
11619+
11620+
// We need to do an extra hop to find the correct type when type being referred is a class, because
11621+
// a class and its type are interwoven in the type
11622+
const isImportOfClassTypeWithQualifier = node.kind === SyntaxKind.ImportType
11623+
&& (node as ImportTypeNode).qualifier
11624+
&& valueType.flags & SymbolFlags.Class;
1162011625
// valueType might not have a symbol, eg, {import('./b').STRING_LITERAL}
11621-
if (valueType.symbol && (isRequireAlias || isImportTypeWithQualifier)) {
11626+
if (valueType.symbol && (isRequireAlias || isImportOfClassTypeWithQualifier)) {
11627+
// follow fake alias (commonjs or class value->type)
1162211628
typeType = getTypeReferenceType(node, valueType.symbol);
1162311629
}
1162411630
}
@@ -13305,9 +13311,7 @@ namespace ts {
1330513311
links.resolvedSymbol = unknownSymbol;
1330613312
return links.resolvedType = errorType;
1330713313
}
13308-
const targetMeaning = node.isTypeOf || isJSDocTypeExpression(node.parent) ? SymbolFlags.Value
13309-
: node.flags & NodeFlags.JSDoc ? SymbolFlags.Value | SymbolFlags.Type
13310-
: SymbolFlags.Type;
13314+
const targetMeaning = node.isTypeOf ? SymbolFlags.Value : node.flags & NodeFlags.JSDoc ? SymbolFlags.Value | SymbolFlags.Type : SymbolFlags.Type;
1331113315
// TODO: Future work: support unions/generics/whatever via a deferred import-type
1331213316
const innerModuleSymbol = resolveExternalModuleName(node, node.argument.literal);
1331313317
if (!innerModuleSymbol) {

‎tests/baselines/reference/jsDocImplicitTypeOfForValueImport.symbols‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
=== tests/cases/compiler/lib/index.js ===
2+
// 36830
3+
//
4+
5+
/** @type {import('../folder/index').zzz} */
6+
export function xxx() {
7+
>xxx : Symbol(xxx, Decl(index.js, 0, 0))
8+
9+
return null;
10+
}
11+
12+
=== tests/cases/compiler/folder/index.d.ts ===
13+
export function zzz(): string;
14+
>zzz : Symbol(zzz, Decl(index.d.ts, 0, 0))
15+

‎tests/baselines/reference/jsDocImplicitTypeOfForValueImport.types‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
=== tests/cases/compiler/lib/index.js ===
2+
// 36830
3+
//
4+
5+
/** @type {import('../folder/index').zzz} */
6+
export function xxx() {
7+
>xxx : () => string
8+
9+
return null;
10+
>null : null
11+
}
12+
13+
=== tests/cases/compiler/folder/index.d.ts ===
14+
export function zzz(): string;
15+
>zzz : () => string
16+

‎tests/baselines/reference/jsdocImportTypeReferenceToClassAlias.errors.txt‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
tests/cases/conformance/jsdoc/test.js(4,7): error TS2339: Property 's' does not exist on type 'typeof C'.
2+
3+
4+
==== tests/cases/conformance/jsdoc/mod1.js (0 errors) ====
5+
class C {
6+
s() { }
7+
}
8+
module.exports.C = C
9+
10+
==== tests/cases/conformance/jsdoc/test.js (1 errors) ====
11+
/** @typedef {import('./mod1').C} X */
12+
/** @param {X} c */
13+
function demo(c) {
14+
c.s
15+
~
16+
!!! error TS2339: Property 's' does not exist on type 'typeof C'.
17+
}
18+

‎tests/baselines/reference/jsdocImportTypeReferenceToClassAlias.symbols‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@ function demo(c) {
2121
>c : Symbol(c, Decl(test.js, 2, 14))
2222

2323
c.s
24-
>c.s : Symbol(C.s, Decl(mod1.js, 0, 9))
2524
>c : Symbol(c, Decl(test.js, 2, 14))
26-
>s : Symbol(C.s, Decl(mod1.js, 0, 9))
2725
}
2826

‎tests/baselines/reference/jsdocImportTypeReferenceToClassAlias.types‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@ module.exports.C = C
1919
/** @param {X} c */
2020
function demo(c) {
2121
>demo : (c: X) => void
22-
>c : C
22+
>c : typeof C
2323

2424
c.s
25-
>c.s : () => void
26-
>c : C
27-
>s : () => void
25+
>c.s : any
26+
>c : typeof C
27+
>s : any
2828
}
2929

‎tests/cases/compiler/jsDocImplicitTypeOfForValueImport.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// 36830
2+
//
3+
// @allowJs: true
4+
// @noEmit: true
5+
// @checkJs: true
6+
7+
// @filename: lib/index.js
8+
/** @type {import('../folder/index').zzz} */
9+
export function xxx() {
10+
return null;
11+
}
12+
13+
// @filename: folder/index.d.ts
14+
export function zzz(): string;

‎tests/cases/fourslash/fourslash.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828

2929
//---------------------------------------
3030
// For API editors:
31-
// When editting this file, and only while editing this file, enable the reference comments
31+
// When editing this file, and only while editing this file, enable the reference comments
3232
// and comment out the declarations in this section to get proper type information.
3333
// Undo these changes before compiling/committing/editing any other fourslash tests.
3434
// The test suite will likely crash if you try 'jake runtests' with reference comments enabled.

0 commit comments

Comments
 (0)