Skip to content

Commit 60396d8

Browse files
weswighamweswigham
committed
Fix alias naming and structure bugs in js declarations
1 parent 88f3593 commit 60396d8

11 files changed

+690
-82
lines changed

‎src/compiler/checker.ts‎

Lines changed: 53 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4967,7 +4967,7 @@ namespace ts {
49674967
const oldcontext = context;
49684968
context = {
49694969
...oldcontext,
4970-
usedSymbolNames: mapMap(symbolTable, (_symbol, name) => [unescapeLeadingUnderscores(name), true]),
4970+
usedSymbolNames: createMap(),
49714971
remappedSymbolNames: createMap(),
49724972
tracker: {
49734973
...oldcontext.tracker,
@@ -4991,6 +4991,10 @@ namespace ts {
49914991
context.usedSymbolNames!.set(name, true);
49924992
});
49934993
}
4994+
forEachEntry(symbolTable, (symbol, name) => {
4995+
const baseName = unescapeLeadingUnderscores(name);
4996+
void getInternalSymbolName(symbol, baseName); // Called to cache values into `usedSymbolNames` and `remappedSymbolNames`
4997+
});
49944998
let addingDeclare = !bundled;
49954999
const exportEquals = symbolTable.get(InternalSymbolName.ExportEquals);
49965000
if (exportEquals && symbolTable.size > 1 && exportEquals.flags & SymbolFlags.Alias) {
@@ -5203,7 +5207,11 @@ namespace ts {
52035207
isPrivate = true;
52045208
}
52055209
const modifierFlags = (!isPrivate ? ModifierFlags.Export : 0) | (isDefault && !needsPostExportDefault ? ModifierFlags.Default : 0);
5206-
if (symbol.flags & SymbolFlags.Function) {
5210+
const isConstMergedWithNS = symbol.flags & SymbolFlags.Module &&
5211+
symbol.flags & (SymbolFlags.BlockScopedVariable | SymbolFlags.FunctionScopedVariable | SymbolFlags.Property) &&
5212+
symbol.escapedName !== InternalSymbolName.ExportEquals;
5213+
const isConstMergedWithNSPrintableAsSignatureMerge = isConstMergedWithNS && isTypeRepresentableAsFunctionNamespaceMerge(getTypeOfSymbol(symbol), symbol);
5214+
if (symbol.flags & SymbolFlags.Function || isConstMergedWithNSPrintableAsSignatureMerge) {
52075215
serializeAsFunctionNamespaceMerge(getTypeOfSymbol(symbol), symbol, getInternalSymbolName(symbol, symbolName), modifierFlags);
52085216
}
52095217
if (symbol.flags & SymbolFlags.TypeAlias) {
@@ -5214,7 +5222,8 @@ namespace ts {
52145222
if (symbol.flags & (SymbolFlags.BlockScopedVariable | SymbolFlags.FunctionScopedVariable | SymbolFlags.Property)
52155223
&& symbol.escapedName !== InternalSymbolName.ExportEquals
52165224
&& !(symbol.flags & SymbolFlags.Prototype)
5217-
&& !(symbol.flags & SymbolFlags.Class)) {
5225+
&& !(symbol.flags & SymbolFlags.Class)
5226+
&& !isConstMergedWithNSPrintableAsSignatureMerge) {
52185227
serializeVariableOrProperty(symbol, symbolName, isPrivate, needsPostExportDefault, propertyAsAlias, modifierFlags);
52195228
}
52205229
if (symbol.flags & SymbolFlags.Enum) {
@@ -5231,7 +5240,7 @@ namespace ts {
52315240
serializeAsClass(symbol, getInternalSymbolName(symbol, symbolName), modifierFlags);
52325241
}
52335242
}
5234-
if (symbol.flags & (SymbolFlags.ValueModule | SymbolFlags.NamespaceModule)) {
5243+
if ((symbol.flags & (SymbolFlags.ValueModule | SymbolFlags.NamespaceModule) && (!isConstMergedWithNS || isTypeOnlyNamespace(symbol))) || isConstMergedWithNSPrintableAsSignatureMerge) {
52355244
serializeModule(symbol, symbolName, modifierFlags);
52365245
}
52375246
if (symbol.flags & SymbolFlags.Interface) {
@@ -5258,7 +5267,9 @@ namespace ts {
52585267
}
52595268

52605269
function includePrivateSymbol(symbol: Symbol) {
5270+
if (some(symbol.declarations, isParameterDeclaration)) return;
52615271
Debug.assertDefined(deferredPrivates);
5272+
getUnusedName(unescapeLeadingUnderscores(symbol.escapedName), symbol); // Call to cache unique name for symbol
52625273
deferredPrivates!.set("" + getSymbolId(symbol), symbol);
52635274
}
52645275

@@ -5332,29 +5343,40 @@ namespace ts {
53325343
), modifierFlags);
53335344
}
53345345

5346+
function getNamespaceMembersForSerialization(symbol: Symbol) {
5347+
return !symbol.exports ? [] : filter(arrayFrom((symbol.exports).values()), p => !((p.flags & SymbolFlags.Prototype) || (p.escapedName === "prototype")));
5348+
}
5349+
5350+
function isTypeOnlyNamespace(symbol: Symbol) {
5351+
return every(getNamespaceMembersForSerialization(symbol), m => !(resolveSymbol(m).flags & SymbolFlags.Value));
5352+
}
5353+
53355354
function serializeModule(symbol: Symbol, symbolName: string, modifierFlags: ModifierFlags) {
5336-
const members = !symbol.exports ? [] : filter(arrayFrom((symbol.exports).values()), p => !((p.flags & SymbolFlags.Prototype) || (p.escapedName === "prototype")));
5355+
const members = getNamespaceMembersForSerialization(symbol);
53375356
// Split NS members up by declaration - members whose parent symbol is the ns symbol vs those whose is not (but were added in later via merging)
53385357
const locationMap = arrayToMultiMap(members, m => m.parent && m.parent === symbol ? "real" : "merged");
53395358
const realMembers = locationMap.get("real") || emptyArray;
53405359
const mergedMembers = locationMap.get("merged") || emptyArray;
53415360
// TODO: `suppressNewPrivateContext` is questionable -we need to simply be emitting privates in whatever scope they were declared in, rather
5342-
// than whatever scope we traverse to them in. That's a bit of a complex rewrite, since we're not _actually_ tracking privates at all in advance,
5361+
// than whatever scope we traverse to them in. That's a bit ofgetUnusedName a complex rewrite, since we're not _actually_ tracking privates at all in advance,
53435362
// so we don't even have placeholders to fill in.
53445363
if (length(realMembers)) {
53455364
const localName = getInternalSymbolName(symbol, symbolName);
5346-
serializeAsNamespaceDeclaration(realMembers, localName, modifierFlags, !!(symbol.flags & SymbolFlags.Function));
5365+
serializeAsNamespaceDeclaration(realMembers, localName, modifierFlags, !!(symbol.flags & (SymbolFlags.Function | SymbolFlags.Assignment)));
53475366
}
53485367
if (length(mergedMembers)) {
53495368
const localName = getInternalSymbolName(symbol, symbolName);
5350-
forEach(mergedMembers, includePrivateSymbol);
53515369
const nsBody = createModuleBlock([createExportDeclaration(
53525370
/*decorators*/ undefined,
53535371
/*modifiers*/ undefined,
53545372
createNamedExports(map(filter(mergedMembers, n => n.escapedName !== InternalSymbolName.ExportEquals), s => {
53555373
const name = unescapeLeadingUnderscores(s.escapedName);
53565374
const localName = getInternalSymbolName(s, name);
5357-
return createExportSpecifier(name === localName ? undefined : localName, name);
5375+
const aliasDecl = s.declarations && getDeclarationOfAliasSymbol(s);
5376+
const target = aliasDecl && getTargetOfAliasDeclaration(aliasDecl, /*dontRecursivelyResolve*/ true);
5377+
includePrivateSymbol(target || s);
5378+
const targetName = target ? getInternalSymbolName(target, unescapeLeadingUnderscores(target.escapedName)) : localName;
5379+
return createExportSpecifier(name === targetName ? undefined : targetName, name);
53585380
}))
53595381
)]);
53605382
addResult(createModuleDeclaration(
@@ -5629,6 +5651,7 @@ namespace ts {
56295651
serializeMaybeAliasAssignment(symbol);
56305652
break;
56315653
case SyntaxKind.BinaryExpression:
5654+
case SyntaxKind.PropertyAccessExpression:
56325655
// Could be best encoded as though an export specifier or as though an export assignment
56335656
// If name is default or export=, do an export assignment
56345657
// Otherwise do an export specifier
@@ -5639,10 +5662,6 @@ namespace ts {
56395662
serializeExportSpecifier(localName, targetName);
56405663
}
56415664
break;
5642-
case SyntaxKind.PropertyAccessExpression:
5643-
// A PAE alias is _always_ going to exist as an append to a top-level export, where our top level
5644-
// handling should always be sufficient to encode the export action itself
5645-
break;
56465665
default:
56475666
return Debug.failBadSyntaxKind(node, "Unhandled alias declaration kind in symbol serializer!");
56485667
}
@@ -5671,7 +5690,8 @@ namespace ts {
56715690
const aliasDecl = symbol.declarations && getDeclarationOfAliasSymbol(symbol);
56725691
// serialize what the alias points to, preserve the declaration's initializer
56735692
const target = aliasDecl && getTargetOfAliasDeclaration(aliasDecl, /*dontRecursivelyResolve*/ true);
5674-
if (target) {
5693+
// If the target resolves and resolves to a thing defined in this file, emit as an alias, otherwise emit as a const
5694+
if (target && length(target.declarations) && some(target.declarations, d => getSourceFileOfNode(d) === getSourceFileOfNode(enclosingDeclaration))) {
56755695
// In case `target` refers to a namespace member, look at the declaration and serialize the leftmost symbol in it
56765696
// eg, `namespace A { export class B {} }; exports = A.B;`
56775697
// Technically, this is all that's required in the case where the assignment is an entity name expression
@@ -6111,11 +6131,8 @@ namespace ts {
61116131
return context.remappedSymbolNames!.get("" + getSymbolId(symbol))!;
61126132
}
61136133
}
6114-
if (input === InternalSymbolName.Default) {
6115-
input = "_default";
6116-
}
6117-
else if (input === InternalSymbolName.ExportEquals) {
6118-
input = "_exports";
6134+
if (symbol) {
6135+
input = getNameCandidateWorker(symbol, input);
61196136
}
61206137
let i = 0;
61216138
const original = input;
@@ -6130,17 +6147,29 @@ namespace ts {
61306147
return input;
61316148
}
61326149

6133-
function getInternalSymbolName(symbol: Symbol, localName: string) {
6134-
if (context.remappedSymbolNames!.has("" + getSymbolId(symbol))) {
6135-
return context.remappedSymbolNames!.get("" + getSymbolId(symbol))!;
6136-
}
6150+
function getNameCandidateWorker(symbol: Symbol, localName: string) {
61376151
if (localName === InternalSymbolName.Default || localName === InternalSymbolName.Class || localName === InternalSymbolName.Function) {
61386152
const flags = context.flags;
61396153
context.flags |= NodeBuilderFlags.InInitialEntityName;
61406154
const nameCandidate = getNameOfSymbolAsWritten(symbol, context);
61416155
context.flags = flags;
6142-
localName = isIdentifierText(nameCandidate, languageVersion) && !isStringANonContextualKeyword(nameCandidate) ? nameCandidate : getUnusedName(`_default`, symbol);
6156+
localName = nameCandidate.length > 0 && isSingleOrDoubleQuote(nameCandidate.charCodeAt(0)) ? stripQuotes(nameCandidate) : nameCandidate;
6157+
}
6158+
if (localName === InternalSymbolName.Default) {
6159+
localName = "_default";
6160+
}
6161+
else if (localName === InternalSymbolName.ExportEquals) {
6162+
localName = "_exports";
6163+
}
6164+
localName = isIdentifierText(localName, languageVersion) && !isStringANonContextualKeyword(localName) ? localName : "_" + localName.replace(/[^a-zA-Z0-9]/g, "_");
6165+
return localName;
6166+
}
6167+
6168+
function getInternalSymbolName(symbol: Symbol, localName: string) {
6169+
if (context.remappedSymbolNames!.has("" + getSymbolId(symbol))) {
6170+
return context.remappedSymbolNames!.get("" + getSymbolId(symbol))!;
61436171
}
6172+
localName = getNameCandidateWorker(symbol, localName);
61446173
// The result of this is going to be used as the symbol's name - lock it in, so `getUnusedName` will also pick it up
61456174
context.remappedSymbolNames!.set("" + getSymbolId(symbol), localName);
61466175
return localName;

‎tests/baselines/reference/jsDeclarationsExportAssignedClassExpressionShadowing.js‎

Lines changed: 2 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
//// [index.js]
2-
// TODO: Fixup
32
class A {
43
member = new Q();
54
}
@@ -15,7 +14,6 @@ module.exports.Another = Q;
1514

1615

1716
//// [index.js]
18-
// TODO: Fixup
1917
var A = /** @class */ (function () {
2018
function A() {
2119
this.member = new Q();
@@ -43,44 +41,11 @@ declare class Q {
4341
x: A;
4442
}
4543
declare namespace Q {
46-
export { Another };
44+
export { Q_1 as Another };
4745
}
4846
declare class A {
4947
member: Q;
5048
}
51-
declare var Another: typeof Q;
52-
declare class Q {
49+
declare class Q_1 {
5350
x: number;
5451
}
55-
56-
57-
//// [DtsFileErrors]
58-
59-
60-
out/index.d.ts(2,15): error TS2300: Duplicate identifier 'Q'.
61-
out/index.d.ts(5,19): error TS2300: Duplicate identifier 'Q'.
62-
out/index.d.ts(12,15): error TS2300: Duplicate identifier 'Q'.
63-
64-
65-
==== ./out/index.d.ts (3 errors) ====
66-
export = Q;
67-
declare class Q {
68-
~
69-
!!! error TS2300: Duplicate identifier 'Q'.
70-
x: A;
71-
}
72-
declare namespace Q {
73-
~
74-
!!! error TS2300: Duplicate identifier 'Q'.
75-
export { Another };
76-
}
77-
declare class A {
78-
member: Q;
79-
}
80-
declare var Another: typeof Q;
81-
declare class Q {
82-
~
83-
!!! error TS2300: Duplicate identifier 'Q'.
84-
x: number;
85-
}
86-

‎tests/baselines/reference/jsDeclarationsExportAssignedClassExpressionShadowing.symbols‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,36 @@
11
=== tests/cases/conformance/jsdoc/declarations/index.js ===
2-
// TODO: Fixup
32
class A {
43
>A : Symbol(A, Decl(index.js, 0, 0))
54

65
member = new Q();
7-
>member : Symbol(A.member, Decl(index.js, 1, 9))
8-
>Q : Symbol(Q, Decl(index.js, 3, 1))
6+
>member : Symbol(A.member, Decl(index.js, 0, 9))
7+
>Q : Symbol(Q, Decl(index.js, 2, 1))
98
}
109
class Q {
11-
>Q : Symbol(Q, Decl(index.js, 3, 1))
10+
>Q : Symbol(Q, Decl(index.js, 2, 1))
1211

1312
x = 42;
14-
>x : Symbol(Q.x, Decl(index.js, 4, 9))
13+
>x : Symbol(Q.x, Decl(index.js, 3, 9))
1514
}
1615
module.exports = class Q {
1716
>module.exports : Symbol("tests/cases/conformance/jsdoc/declarations/index", Decl(index.js, 0, 0))
18-
>module : Symbol(export=, Decl(index.js, 6, 1))
19-
>exports : Symbol(export=, Decl(index.js, 6, 1))
20-
>Q : Symbol(Q, Decl(index.js, 7, 16))
17+
>module : Symbol(export=, Decl(index.js, 5, 1))
18+
>exports : Symbol(export=, Decl(index.js, 5, 1))
19+
>Q : Symbol(Q, Decl(index.js, 6, 16))
2120

2221
constructor() {
2322
this.x = new A();
24-
>this.x : Symbol(Q.x, Decl(index.js, 8, 19))
25-
>this : Symbol(Q, Decl(index.js, 7, 16))
26-
>x : Symbol(Q.x, Decl(index.js, 8, 19))
23+
>this.x : Symbol(Q.x, Decl(index.js, 7, 19))
24+
>this : Symbol(Q, Decl(index.js, 6, 16))
25+
>x : Symbol(Q.x, Decl(index.js, 7, 19))
2726
>A : Symbol(A, Decl(index.js, 0, 0))
2827
}
2928
}
3029
module.exports.Another = Q;
3130
>module.exports.Another : Symbol(Another)
32-
>module.exports : Symbol(Another, Decl(index.js, 11, 1))
33-
>module : Symbol(module, Decl(index.js, 6, 1))
31+
>module.exports : Symbol(Another, Decl(index.js, 10, 1))
32+
>module : Symbol(module, Decl(index.js, 5, 1))
3433
>exports : Symbol("tests/cases/conformance/jsdoc/declarations/index", Decl(index.js, 0, 0))
35-
>Another : Symbol(Another, Decl(index.js, 11, 1))
36-
>Q : Symbol(Q, Decl(index.js, 3, 1))
34+
>Another : Symbol(Another, Decl(index.js, 10, 1))
35+
>Q : Symbol(Q, Decl(index.js, 2, 1))
3736

‎tests/baselines/reference/jsDeclarationsExportAssignedClassExpressionShadowing.types‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
=== tests/cases/conformance/jsdoc/declarations/index.js ===
2-
// TODO: Fixup
32
class A {
43
>A : A
54

‎tests/baselines/reference/jsDeclarationsFunctionsCjs.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,7 @@ export namespace b {
121121
}
122122
export function c(): void;
123123
export namespace c {
124-
class Cls {
125-
}
124+
export { Cls };
126125
}
127126
export function d(a: number, b: number): string;
128127
export function e<T, U>(a: T, b: U): T & U;
@@ -159,3 +158,6 @@ export function i(): void;
159158
export function ii(): void;
160159
export function jj(): void;
161160
export function j(): void;
161+
declare class Cls {
162+
}
163+
export {};

0 commit comments

Comments
 (0)