Skip to content

Commit a4a5b38

Browse files
committed
Report circular JSDoc type references (microsoft#27404)
JSDoc types references can often be to values, which can often be circular in ways that types tied to declarations cannot. I decided to create a separate property on SymbolLinks rather than reusing declaredType, although I'm not sure that's strictly required.
1 parent 6afa880 commit a4a5b38

7 files changed

+71
-4
lines changed

src/compiler/checker.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,7 @@ namespace ts {
666666
ResolvedReturnType,
667667
ImmediateBaseConstraint,
668668
EnumTagType,
669+
JSDocTypeReference,
669670
}
670671

671672
const enum CheckMode {
@@ -4503,6 +4504,8 @@ namespace ts {
45034504
return !!(<Signature>target).resolvedReturnType;
45044505
case TypeSystemPropertyName.ImmediateBaseConstraint:
45054506
return !!(<Type>target).immediateBaseConstraint;
4507+
case TypeSystemPropertyName.JSDocTypeReference:
4508+
return !!getSymbolLinks(target as Symbol).resolvedJSDocType;
45064509
}
45074510
return Debug.assertNever(propertyName);
45084511
}
@@ -8275,7 +8278,7 @@ namespace ts {
82758278
return type;
82768279
}
82778280

8278-
// JS are 'string' or 'number', not an enum type.
8281+
// JS enums are 'string' or 'number', not an enum type.
82798282
const enumTag = isInJSFile(node) && symbol.valueDeclaration && getJSDocEnumTag(symbol.valueDeclaration);
82808283
if (enumTag) {
82818284
const links = getNodeLinks(enumTag);
@@ -8318,12 +8321,21 @@ namespace ts {
83188321
* the type of this reference is just the type of the value we resolved to.
83198322
*/
83208323
function getJSDocTypeReference(node: NodeWithTypeArguments, symbol: Symbol, typeArguments: Type[] | undefined): Type | undefined {
8324+
if (!pushTypeResolution(symbol, TypeSystemPropertyName.JSDocTypeReference)) {
8325+
return errorType;
8326+
}
83218327
const assignedType = getAssignedClassType(symbol);
83228328
const valueType = getTypeOfSymbol(symbol);
83238329
const referenceType = valueType.symbol && valueType.symbol !== symbol && !isInferredClassType(valueType) && getTypeReferenceTypeWorker(node, valueType.symbol, typeArguments);
8330+
if (!popTypeResolution()) {
8331+
getSymbolLinks(symbol).resolvedJSDocType = errorType;
8332+
error(node, Diagnostics.JSDoc_type_0_circularly_references_itself, symbolToString(symbol));
8333+
return errorType;
8334+
}
83248335
if (referenceType || assignedType) {
83258336
// TODO: GH#18217 (should the `|| assignedType` be at a lower precedence?)
8326-
return (referenceType && assignedType ? getIntersectionType([assignedType, referenceType]) : referenceType || assignedType)!;
8337+
const type = (referenceType && assignedType ? getIntersectionType([assignedType, referenceType]) : referenceType || assignedType)!;
8338+
return getSymbolLinks(symbol).resolvedJSDocType = type;
83278339
}
83288340
}
83298341

@@ -8460,7 +8472,7 @@ namespace ts {
84608472
symbol = resolveTypeReferenceName(getTypeReferenceName(node), meaning);
84618473
type = getTypeReferenceType(node, symbol);
84628474
}
8463-
// Cache both the resolved symbol and the resolved type. The resolved symbol is needed in when we check the
8475+
// Cache both the resolved symbol and the resolved type. The resolved symbol is needed when we check the
84648476
// type reference in checkTypeReferenceNode.
84658477
links.resolvedSymbol = symbol;
84668478
links.resolvedType = type;

src/compiler/diagnosticMessages.json

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2116,6 +2116,10 @@
21162116
"category": "Error",
21172117
"code": 2586
21182118
},
2119+
"JSDoc type '{0}' circularly references itself.": {
2120+
"category": "Error",
2121+
"code": 2587
2122+
},
21192123
"JSX element attributes type '{0}' may not be a union type.": {
21202124
"category": "Error",
21212125
"code": 2600
@@ -4684,4 +4688,4 @@
46844688
"category": "Message",
46854689
"code": 95068
46864690
}
4687-
}
4691+
}

src/compiler/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3536,6 +3536,7 @@ namespace ts {
35363536
type?: Type; // Type of value symbol
35373537
uniqueESSymbolType?: Type; // UniqueESSymbol type for a symbol
35383538
declaredType?: Type; // Type of class, interface, enum, type alias, or type parameter
3539+
resolvedJSDocType?: Type; // Resolved type of a JSDoc type reference
35393540
typeParameters?: TypeParameter[]; // Type parameters of type alias (undefined if non-generic)
35403541
outerTypeParameters?: TypeParameter[]; // Outer type parameters of anonymous object type
35413542
inferredClassType?: Type; // Type of an inferred ES5 class
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
tests/cases/conformance/jsdoc/bug27346.js(2,4): error TS8030: The type of a function declaration must match the function's signature.
2+
tests/cases/conformance/jsdoc/bug27346.js(2,11): error TS2587: JSDoc type 'MyClass' circularly references itself.
3+
4+
5+
==== tests/cases/conformance/jsdoc/bug27346.js (2 errors) ====
6+
/**
7+
* @type {MyClass}
8+
~~~~~~~~~~~~~~~
9+
!!! error TS8030: The type of a function declaration must match the function's signature.
10+
~~~~~~~
11+
!!! error TS2587: JSDoc type 'MyClass' circularly references itself.
12+
*/
13+
function MyClass() { }
14+
MyClass.prototype = {};
15+
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
=== tests/cases/conformance/jsdoc/bug27346.js ===
2+
/**
3+
* @type {MyClass}
4+
*/
5+
function MyClass() { }
6+
>MyClass : Symbol(MyClass, Decl(bug27346.js, 0, 0), Decl(bug27346.js, 3, 22))
7+
8+
MyClass.prototype = {};
9+
>MyClass.prototype : Symbol(MyClass.prototype, Decl(bug27346.js, 3, 22))
10+
>MyClass : Symbol(MyClass, Decl(bug27346.js, 0, 0), Decl(bug27346.js, 3, 22))
11+
>prototype : Symbol(MyClass.prototype, Decl(bug27346.js, 3, 22))
12+
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
=== tests/cases/conformance/jsdoc/bug27346.js ===
2+
/**
3+
* @type {MyClass}
4+
*/
5+
function MyClass() { }
6+
>MyClass : typeof MyClass
7+
8+
MyClass.prototype = {};
9+
>MyClass.prototype = {} : {}
10+
>MyClass.prototype : {}
11+
>MyClass : typeof MyClass
12+
>prototype : {}
13+
>{} : {}
14+
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// @allowJs: true
2+
// @noEmit: true
3+
// @checkJs: true
4+
// @Filename: bug27346.js
5+
/**
6+
* @type {MyClass}
7+
*/
8+
function MyClass() { }
9+
MyClass.prototype = {};

0 commit comments

Comments
 (0)