Skip to content

Commit 7cda045

Browse files
sandersnsandersn
authored
Always export typedefs (microsoft#23723)
* Always export typedefs This actually just required deleting a check in declareModuleMembers and checking for external AND commonjs modules in a couple of places. However, while experimenting with this feature, I discovered that even previously-exported typedefs would only be exported if they came after a commonjs export node. So I added a commonjs check to the pass in the parser. It will not catch nested module.exports, but it will catch top-level assignments. The new test tests both changes. * Post-bind typedef instead of pre-checking for commonjs * Duplicate identifier errors * Fix class type reference resolution+update baselines * Move to a type-based check for duplicate identifiers
1 parent 0bbf4d5 commit 7cda045

23 files changed

Lines changed: 650 additions & 21 deletions

‎src/compiler/binder.ts‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ namespace ts {
118118
let thisParentContainer: Node; // Container one level up
119119
let blockScopeContainer: Node;
120120
let lastContainer: Node;
121+
let delayedTypedefs: { typedef: JSDocTypedefTag, container: Node, lastContainer: Node, blockScopeContainer: Node, parent: Node }[];
121122
let seenThisKeyword: boolean;
122123

123124
// state used by control flow analysis
@@ -176,6 +177,7 @@ namespace ts {
176177
bind(file);
177178
file.symbolCount = symbolCount;
178179
file.classifiableNames = classifiableNames;
180+
delayedBindJSDocTypedefTag();
179181
}
180182

181183
file = undefined;
@@ -186,6 +188,7 @@ namespace ts {
186188
thisParentContainer = undefined;
187189
blockScopeContainer = undefined;
188190
lastContainer = undefined;
191+
delayedTypedefs = undefined;
189192
seenThisKeyword = false;
190193
currentFlow = undefined;
191194
currentBreakTarget = undefined;
@@ -450,8 +453,7 @@ namespace ts {
450453
// and this case is specially handled. Module augmentations should only be merged with original module definition
451454
// and should never be merged directly with other augmentation, and the latter case would be possible if automatic merge is allowed.
452455
if (node.kind === SyntaxKind.JSDocTypedefTag) Debug.assert(isInJavaScriptFile(node)); // We shouldn't add symbols for JSDoc nodes if not in a JS file.
453-
const isJSDocTypedefInJSDocNamespace = isJSDocTypedefTag(node) && node.name && node.name.kind === SyntaxKind.Identifier && node.name.isInJSDocNamespace;
454-
if ((!isAmbientModule(node) && (hasExportModifier || container.flags & NodeFlags.ExportContext)) || isJSDocTypedefInJSDocNamespace) {
456+
if ((!isAmbientModule(node) && (hasExportModifier || container.flags & NodeFlags.ExportContext)) || isJSDocTypedefTag(node)) {
455457
if (hasModifier(node, ModifierFlags.Default) && !getDeclarationName(node)) {
456458
return declareSymbol(container.symbol.exports, container.symbol, node, symbolFlags, symbolExcludes); // No local symbol for an unnamed default!
457459
}
@@ -1727,7 +1729,7 @@ namespace ts {
17271729
declareModuleMember(node, symbolFlags, symbolExcludes);
17281730
break;
17291731
case SyntaxKind.SourceFile:
1730-
if (isExternalModule(<SourceFile>container)) {
1732+
if (isExternalOrCommonJsModule(<SourceFile>container)) {
17311733
declareModuleMember(node, symbolFlags, symbolExcludes);
17321734
break;
17331735
}
@@ -1745,6 +1747,24 @@ namespace ts {
17451747
bindBlockScopedDeclaration(node, SymbolFlags.BlockScopedVariable, SymbolFlags.BlockScopedVariableExcludes);
17461748
}
17471749

1750+
function delayedBindJSDocTypedefTag() {
1751+
if (!delayedTypedefs) {
1752+
return;
1753+
}
1754+
const saveContainer = container;
1755+
const saveLastContainer = lastContainer;
1756+
const saveBlockScopeContainer = blockScopeContainer;
1757+
const saveParent = parent;
1758+
for (const delay of delayedTypedefs) {
1759+
({ container, lastContainer, blockScopeContainer, parent } = delay);
1760+
bindBlockScopedDeclaration(delay.typedef, SymbolFlags.TypeAlias, SymbolFlags.TypeAliasExcludes);
1761+
}
1762+
container = saveContainer;
1763+
lastContainer = saveLastContainer;
1764+
blockScopeContainer = saveBlockScopeContainer;
1765+
parent = saveParent;
1766+
}
1767+
17481768
// The binder visits every node in the syntax tree so it is a convenient place to perform a single localized
17491769
// check for reserved words used as identifiers in strict mode code.
17501770
function checkStrictModeIdentifier(node: Identifier) {
@@ -2194,7 +2214,7 @@ namespace ts {
21942214
case SyntaxKind.JSDocTypedefTag: {
21952215
const { fullName } = node as JSDocTypedefTag;
21962216
if (!fullName || fullName.kind === SyntaxKind.Identifier) {
2197-
return bindBlockScopedDeclaration(<Declaration>node, SymbolFlags.TypeAlias, SymbolFlags.TypeAliasExcludes);
2217+
(delayedTypedefs || (delayedTypedefs = [])).push({ typedef: node as JSDocTypedefTag, container, lastContainer, blockScopeContainer, parent });
21982218
}
21992219
break;
22002220
}
@@ -2304,7 +2324,10 @@ namespace ts {
23042324
return s;
23052325
});
23062326
if (symbol) {
2307-
declareSymbol(symbol.exports, symbol, lhs, SymbolFlags.Property | SymbolFlags.ExportValue, SymbolFlags.None);
2327+
const flags = isClassExpression(node.right) ?
2328+
SymbolFlags.Property | SymbolFlags.ExportValue | SymbolFlags.Class :
2329+
SymbolFlags.Property | SymbolFlags.ExportValue;
2330+
declareSymbol(symbol.exports, symbol, lhs, flags, SymbolFlags.None);
23082331
}
23092332
}
23102333

‎src/compiler/checker.ts‎

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7626,23 +7626,38 @@ namespace ts {
76267626
return unknownType;
76277627
}
76287628

7629-
// A jsdoc TypeReference may have resolved to a value (as opposed to a type). If
7630-
// the symbol is a constructor function, return the inferred class type; otherwise,
7631-
// the type of this reference is just the type of the value we resolved to.
7629+
const jsdocType = getJSDocTypeReference(node, symbol, typeArguments);
7630+
if (jsdocType) {
7631+
return jsdocType;
7632+
}
7633+
7634+
// Resolve the type reference as a Type for the purpose of reporting errors.
7635+
resolveTypeReferenceName(getTypeReferenceName(node), SymbolFlags.Type);
7636+
return getTypeOfSymbol(symbol);
7637+
}
7638+
7639+
/**
7640+
* A jsdoc TypeReference may have resolved to a value (as opposed to a type). If
7641+
* the symbol is a constructor function, return the inferred class type; otherwise,
7642+
* the type of this reference is just the type of the value we resolved to.
7643+
*/
7644+
function getJSDocTypeReference(node: NodeWithTypeArguments, symbol: Symbol, typeArguments: Type[]): Type | undefined {
76327645
const assignedType = getAssignedClassType(symbol);
76337646
const valueType = getTypeOfSymbol(symbol);
7634-
const referenceType = valueType.symbol && !isInferredClassType(valueType) && getTypeReferenceTypeWorker(node, valueType.symbol, typeArguments);
7647+
const referenceType = valueType.symbol && valueType.symbol !== symbol && !isInferredClassType(valueType) && getTypeReferenceTypeWorker(node, valueType.symbol, typeArguments);
76357648
if (referenceType || assignedType) {
76367649
return referenceType && assignedType ? getIntersectionType([assignedType, referenceType]) : referenceType || assignedType;
76377650
}
7638-
7639-
// Resolve the type reference as a Type for the purpose of reporting errors.
7640-
resolveTypeReferenceName(getTypeReferenceName(node), SymbolFlags.Type);
7641-
return valueType;
76427651
}
76437652

76447653
function getTypeReferenceTypeWorker(node: NodeWithTypeArguments, symbol: Symbol, typeArguments: Type[]): Type | undefined {
76457654
if (symbol.flags & (SymbolFlags.Class | SymbolFlags.Interface)) {
7655+
if (symbol.valueDeclaration && isBinaryExpression(symbol.valueDeclaration.parent)) {
7656+
const jsdocType = getJSDocTypeReference(node, symbol, typeArguments);
7657+
if (jsdocType) {
7658+
return jsdocType;
7659+
}
7660+
}
76467661
return getTypeFromClassOrInterfaceReference(node, symbol, typeArguments);
76477662
}
76487663

@@ -20031,6 +20046,7 @@ namespace ts {
2003120046
getUnionType([removeDefinitelyFalsyTypes(leftType), rightType], UnionReduction.Subtype) :
2003220047
leftType;
2003320048
case SyntaxKind.EqualsToken:
20049+
checkSpecialAssignment(left, right);
2003420050
checkAssignmentOperator(rightType);
2003520051
return getRegularTypeOfObjectLiteral(rightType);
2003620052
case SyntaxKind.CommaToken:
@@ -20040,6 +20056,24 @@ namespace ts {
2004020056
return rightType;
2004120057
}
2004220058

20059+
function checkSpecialAssignment(left: Node, right: Expression) {
20060+
const special = getSpecialPropertyAssignmentKind(left.parent as BinaryExpression);
20061+
if (special === SpecialPropertyAssignmentKind.ModuleExports) {
20062+
const rightType = checkExpression(right, checkMode);
20063+
for (const prop of getPropertiesOfObjectType(rightType)) {
20064+
const propType = getTypeOfSymbol(prop);
20065+
if (propType.symbol && propType.symbol.flags & SymbolFlags.Class) {
20066+
const name = prop.escapedName;
20067+
const symbol = resolveName(prop.valueDeclaration, name, SymbolFlags.Type, undefined, name, /*isUse*/ false);
20068+
if (symbol) {
20069+
grammarErrorOnNode(symbol.declarations[0], Diagnostics.Duplicate_identifier_0, unescapeLeadingUnderscores(name));
20070+
return grammarErrorOnNode(prop.valueDeclaration, Diagnostics.Duplicate_identifier_0, unescapeLeadingUnderscores(name));
20071+
}
20072+
}
20073+
}
20074+
}
20075+
}
20076+
2004320077
function isEvalNode(node: Expression) {
2004420078
return node.kind === SyntaxKind.Identifier && (node as Identifier).escapedText === "eval";
2004520079
}

‎src/harness/harness.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ namespace Utils {
217217
for (const childName in node) {
218218
if (childName === "parent" || childName === "nextContainer" || childName === "modifiers" || childName === "externalModuleIndicator" ||
219219
// for now ignore jsdoc comments
220-
childName === "jsDocComment" || childName === "checkJsDirective") {
220+
childName === "jsDocComment" || childName === "checkJsDirective" || childName === "commonJsModuleIndicator") {
221221
continue;
222222
}
223223
const child = (<any>node)[childName];

‎tests/baselines/reference/jsdocTypedefNoCrash.symbols‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@
44
* }}
55
*/
66
export const foo = 5;
7-
>foo : Symbol(foo, Decl(export.js, 4, 12))
7+
>foo : Symbol(foo, Decl(export.js, 4, 12), Decl(export.js, 1, 3))
88

‎tests/baselines/reference/typedefCrossModule.symbols‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
=== tests/cases/conformance/jsdoc/commonjs.d.ts ===
2+
declare var module: { exports: any};
3+
>module : Symbol(module, Decl(commonjs.d.ts, 0, 11))
4+
>exports : Symbol(exports, Decl(commonjs.d.ts, 0, 21))
5+
6+
=== tests/cases/conformance/jsdoc/mod1.js ===
7+
/// <reference path="./commonjs.d.ts"/>
8+
/** @typedef {{ type: "a", x: 1 }} A */
9+
/** @typedef {{ type: "b", y: 1 }} B */
10+
/** @typedef {A | B} Both */
11+
module.exports = C
12+
>module.exports : Symbol(exports, Decl(commonjs.d.ts, 0, 21))
13+
>module : Symbol(export=, Decl(mod1.js, 0, 0))
14+
>exports : Symbol(export=, Decl(mod1.js, 0, 0))
15+
>C : Symbol(C, Decl(mod1.js, 4, 18))
16+
17+
function C() {
18+
>C : Symbol(C, Decl(mod1.js, 4, 18))
19+
20+
this.p = 1
21+
>p : Symbol(C.p, Decl(mod1.js, 5, 14))
22+
}
23+
24+
=== tests/cases/conformance/jsdoc/mod2.js ===
25+
/// <reference path="./commonjs.d.ts"/>
26+
/** @typedef {{ type: "a", x: 1 }} A */
27+
/** @typedef {{ type: "b", y: 1 }} B */
28+
/** @typedef {A | B} Both */
29+
30+
export function C() {
31+
>C : Symbol(C, Decl(mod2.js, 0, 0))
32+
33+
this.p = 1
34+
>p : Symbol(C.p, Decl(mod2.js, 5, 21))
35+
}
36+
37+
=== tests/cases/conformance/jsdoc/mod3.js ===
38+
/// <reference path="./commonjs.d.ts"/>
39+
/** @typedef {{ type: "a", x: 1 }} A */
40+
/** @typedef {{ type: "b", y: 1 }} B */
41+
/** @typedef {A | B} Both */
42+
43+
exports.C = function() {
44+
>exports.C : Symbol(C, Decl(mod3.js, 0, 0))
45+
>exports : Symbol(C, Decl(mod3.js, 0, 0))
46+
>C : Symbol(C, Decl(mod3.js, 0, 0))
47+
48+
this.p = 1
49+
>p : Symbol(C.p, Decl(mod3.js, 5, 24))
50+
}
51+
52+
=== tests/cases/conformance/jsdoc/use.js ===
53+
/** @type {import('./mod1').Both} */
54+
var both1 = { type: 'a', x: 1 };
55+
>both1 : Symbol(both1, Decl(use.js, 1, 3))
56+
>type : Symbol(type, Decl(use.js, 1, 13))
57+
>x : Symbol(x, Decl(use.js, 1, 24))
58+
59+
/** @type {import('./mod2').Both} */
60+
var both2 = both1;
61+
>both2 : Symbol(both2, Decl(use.js, 3, 3))
62+
>both1 : Symbol(both1, Decl(use.js, 1, 3))
63+
64+
/** @type {import('./mod3').Both} */
65+
var both3 = both2;
66+
>both3 : Symbol(both3, Decl(use.js, 5, 3))
67+
>both2 : Symbol(both2, Decl(use.js, 3, 3))
68+
69+
70+

‎tests/baselines/reference/typedefCrossModule.types‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
=== tests/cases/conformance/jsdoc/commonjs.d.ts ===
2+
declare var module: { exports: any};
3+
>module : { exports: any; }
4+
>exports : any
5+
6+
=== tests/cases/conformance/jsdoc/mod1.js ===
7+
/// <reference path="./commonjs.d.ts"/>
8+
/** @typedef {{ type: "a", x: 1 }} A */
9+
/** @typedef {{ type: "b", y: 1 }} B */
10+
/** @typedef {A | B} Both */
11+
module.exports = C
12+
>module.exports = C : typeof C
13+
>module.exports : any
14+
>module : { exports: any; }
15+
>exports : any
16+
>C : typeof C
17+
18+
function C() {
19+
>C : typeof C
20+
21+
this.p = 1
22+
>this.p = 1 : 1
23+
>this.p : any
24+
>this : any
25+
>p : any
26+
>1 : 1
27+
}
28+
29+
=== tests/cases/conformance/jsdoc/mod2.js ===
30+
/// <reference path="./commonjs.d.ts"/>
31+
/** @typedef {{ type: "a", x: 1 }} A */
32+
/** @typedef {{ type: "b", y: 1 }} B */
33+
/** @typedef {A | B} Both */
34+
35+
export function C() {
36+
>C : typeof C
37+
38+
this.p = 1
39+
>this.p = 1 : 1
40+
>this.p : any
41+
>this : any
42+
>p : any
43+
>1 : 1
44+
}
45+
46+
=== tests/cases/conformance/jsdoc/mod3.js ===
47+
/// <reference path="./commonjs.d.ts"/>
48+
/** @typedef {{ type: "a", x: 1 }} A */
49+
/** @typedef {{ type: "b", y: 1 }} B */
50+
/** @typedef {A | B} Both */
51+
52+
exports.C = function() {
53+
>exports.C = function() { this.p = 1} : typeof C
54+
>exports.C : typeof C
55+
>exports : typeof import("tests/cases/conformance/jsdoc/mod3")
56+
>C : typeof C
57+
>function() { this.p = 1} : typeof C
58+
59+
this.p = 1
60+
>this.p = 1 : 1
61+
>this.p : any
62+
>this : any
63+
>p : any
64+
>1 : 1
65+
}
66+
67+
=== tests/cases/conformance/jsdoc/use.js ===
68+
/** @type {import('./mod1').Both} */
69+
var both1 = { type: 'a', x: 1 };
70+
>both1 : { type: "a"; x: 1; } | { type: "b"; y: 1; }
71+
>{ type: 'a', x: 1 } : { type: "a"; x: 1; }
72+
>type : "a"
73+
>'a' : "a"
74+
>x : 1
75+
>1 : 1
76+
77+
/** @type {import('./mod2').Both} */
78+
var both2 = both1;
79+
>both2 : { type: "a"; x: 1; } | { type: "b"; y: 1; }
80+
>both1 : { type: "a"; x: 1; }
81+
82+
/** @type {import('./mod3').Both} */
83+
var both3 = both2;
84+
>both3 : { type: "a"; x: 1; } | { type: "b"; y: 1; }
85+
>both2 : { type: "a"; x: 1; }
86+
87+
88+

0 commit comments

Comments
 (0)