Skip to content

Commit 9b441d8

Browse files
weswighamweswigham
committed
Merge pull request microsoft#5148 from weswigham/duplicate-export-behavior
Make export var or export *'s with duplicate identifiers an error
2 parents 209ec68 + 835950a commit 9b441d8

34 files changed

Lines changed: 729 additions & 473 deletions

File tree

‎src/compiler/checker.ts‎

Lines changed: 80 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,39 +1103,81 @@ namespace ts {
11031103
return links.resolvedExports || (links.resolvedExports = getExportsForModule(moduleSymbol));
11041104
}
11051105

1106-
function extendExportSymbols(target: SymbolTable, source: SymbolTable) {
1106+
interface ExportCollisionTracker {
1107+
specifierText: string;
1108+
exportsWithDuplicate: ExportDeclaration[];
1109+
}
1110+
1111+
/**
1112+
* Extends one symbol table with another while collecting information on name collisions for error message generation into the `lookupTable` argument
1113+
* Not passing `lookupTable` and `exportNode` disables this collection, and just extends the tables
1114+
*/
1115+
function extendExportSymbols(target: SymbolTable, source: SymbolTable, lookupTable?: Map<ExportCollisionTracker>, exportNode?: ExportDeclaration) {
11071116
for (const id in source) {
11081117
if (id !== "default" && !hasProperty(target, id)) {
11091118
target[id] = source[id];
1119+
if (lookupTable && exportNode) {
1120+
lookupTable[id] = {
1121+
specifierText: getTextOfNode(exportNode.moduleSpecifier)
1122+
} as ExportCollisionTracker;
1123+
}
1124+
}
1125+
else if (lookupTable && exportNode && id !== "default" && hasProperty(target, id) && resolveSymbol(target[id]) !== resolveSymbol(source[id])) {
1126+
if (!lookupTable[id].exportsWithDuplicate) {
1127+
lookupTable[id].exportsWithDuplicate = [exportNode];
1128+
}
1129+
else {
1130+
lookupTable[id].exportsWithDuplicate.push(exportNode);
1131+
}
11101132
}
11111133
}
11121134
}
11131135

11141136
function getExportsForModule(moduleSymbol: Symbol): SymbolTable {
1115-
let result: SymbolTable;
11161137
const visitedSymbols: Symbol[] = [];
1117-
visit(moduleSymbol);
1118-
return result || moduleSymbol.exports;
1138+
return visit(moduleSymbol) || moduleSymbol.exports;
11191139

11201140
// The ES6 spec permits export * declarations in a module to circularly reference the module itself. For example,
11211141
// module 'a' can 'export * from "b"' and 'b' can 'export * from "a"' without error.
1122-
function visit(symbol: Symbol) {
1123-
if (symbol && symbol.flags & SymbolFlags.HasExports && !contains(visitedSymbols, symbol)) {
1124-
visitedSymbols.push(symbol);
1125-
if (symbol !== moduleSymbol) {
1126-
if (!result) {
1127-
result = cloneSymbolTable(moduleSymbol.exports);
1142+
function visit(symbol: Symbol): SymbolTable {
1143+
if (!(symbol && symbol.flags & SymbolFlags.HasExports && !contains(visitedSymbols, symbol))) {
1144+
return;
1145+
}
1146+
visitedSymbols.push(symbol);
1147+
const symbols = cloneSymbolTable(symbol.exports);
1148+
// All export * declarations are collected in an __export symbol by the binder
1149+
const exportStars = symbol.exports["__export"];
1150+
if (exportStars) {
1151+
const nestedSymbols: SymbolTable = {};
1152+
const lookupTable: Map<ExportCollisionTracker> = {};
1153+
for (const node of exportStars.declarations) {
1154+
const resolvedModule = resolveExternalModuleName(node, (node as ExportDeclaration).moduleSpecifier);
1155+
const exportedSymbols = visit(resolvedModule);
1156+
extendExportSymbols(
1157+
nestedSymbols,
1158+
exportedSymbols,
1159+
lookupTable,
1160+
node as ExportDeclaration
1161+
);
1162+
}
1163+
for (const id in lookupTable) {
1164+
const { exportsWithDuplicate } = lookupTable[id];
1165+
// It's not an error if the file with multiple `export *`s with duplicate names exports a member with that name itself
1166+
if (id === "export=" || !(exportsWithDuplicate && exportsWithDuplicate.length) || hasProperty(symbols, id)) {
1167+
continue;
11281168
}
1129-
extendExportSymbols(result, symbol.exports);
1130-
}
1131-
// All export * declarations are collected in an __export symbol by the binder
1132-
const exportStars = symbol.exports["__export"];
1133-
if (exportStars) {
1134-
for (const node of exportStars.declarations) {
1135-
visit(resolveExternalModuleName(node, (<ExportDeclaration>node).moduleSpecifier));
1169+
for (const node of exportsWithDuplicate) {
1170+
diagnostics.add(createDiagnosticForNode(
1171+
node,
1172+
Diagnostics.Module_0_has_already_exported_a_member_named_1_Consider_explicitly_re_exporting_to_resolve_the_ambiguity,
1173+
lookupTable[id].specifierText,
1174+
id
1175+
));
11361176
}
11371177
}
1178+
extendExportSymbols(symbols, nestedSymbols);
11381179
}
1180+
return symbols;
11391181
}
11401182
}
11411183

@@ -14132,8 +14174,29 @@ namespace ts {
1413214174
const declaration = getDeclarationOfAliasSymbol(exportEqualsSymbol) || exportEqualsSymbol.valueDeclaration;
1413314175
error(declaration, Diagnostics.An_export_assignment_cannot_be_used_in_a_module_with_other_exported_elements);
1413414176
}
14177+
// Checks for export * conflicts
14178+
const exports = getExportsOfModule(moduleSymbol);
14179+
for (const id in exports) {
14180+
if (id === "__export") {
14181+
continue;
14182+
}
14183+
const { declarations, flags } = exports[id];
14184+
// ECMA262: 15.2.1.1 It is a Syntax Error if the ExportedNames of ModuleItemList contains any duplicate entries. (TS Exceptions: namespaces, function overloads, enums, and interfaces)
14185+
if (!(flags & (SymbolFlags.Namespace | SymbolFlags.Interface | SymbolFlags.Enum)) && declarations.length > 1) {
14186+
const exportedDeclarations: Declaration[] = filter(declarations, isNotOverload);
14187+
if (exportedDeclarations.length > 1) {
14188+
for (const declaration of exportedDeclarations) {
14189+
diagnostics.add(createDiagnosticForNode(declaration, Diagnostics.Cannot_redeclare_exported_variable_0, id));
14190+
}
14191+
}
14192+
}
14193+
}
1413514194
links.exportsChecked = true;
1413614195
}
14196+
14197+
function isNotOverload(declaration: Declaration): boolean {
14198+
return declaration.kind !== SyntaxKind.FunctionDeclaration || !!(declaration as FunctionDeclaration).body;
14199+
}
1413714200
}
1413814201

1413914202
function checkTypePredicate(node: TypePredicateNode) {

‎src/compiler/diagnosticMessages.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -844,6 +844,10 @@
844844
"category": "Error",
845845
"code": 2307
846846
},
847+
"Module {0} has already exported a member named '{1}'. Consider explicitly re-exporting to resolve the ambiguity.": {
848+
"category": "Error",
849+
"code": 2308
850+
},
847851
"An export assignment cannot be used in a module with other exported elements.": {
848852
"category": "Error",
849853
"code": 2309
@@ -900,6 +904,10 @@
900904
"category": "Error",
901905
"code": 2322
902906
},
907+
"Cannot redeclare exported variable '{0}'.": {
908+
"category": "Error",
909+
"code": 2323
910+
},
903911
"Property '{0}' is missing in type '{1}'.": {
904912
"category": "Error",
905913
"code": 2324

‎src/compiler/emitter.ts‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5795,9 +5795,11 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
57955795

57965796
if (!shouldHoistDeclarationInSystemJsModule(node)) {
57975797
// do not emit var if variable was already hoisted
5798-
if (!(node.flags & NodeFlags.Export) || isES6ExportedDeclaration(node)) {
5798+
5799+
const isES6ExportedEnum = isES6ExportedDeclaration(node);
5800+
if (!(node.flags & NodeFlags.Export) || (isES6ExportedEnum && isFirstDeclarationOfKind(node, node.symbol && node.symbol.declarations, SyntaxKind.EnumDeclaration))) {
57995801
emitStart(node);
5800-
if (isES6ExportedDeclaration(node)) {
5802+
if (isES6ExportedEnum) {
58015803
write("export ");
58025804
}
58035805
write("var ");
@@ -5894,6 +5896,10 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
58945896
return languageVersion === ScriptTarget.ES6 && !!(resolver.getNodeCheckFlags(node) & NodeCheckFlags.LexicalModuleMergesWithClass);
58955897
}
58965898

5899+
function isFirstDeclarationOfKind(node: Declaration, declarations: Declaration[], kind: SyntaxKind) {
5900+
return !forEach(declarations, declaration => declaration.kind === kind && declaration.pos < node.pos);
5901+
}
5902+
58975903
function emitModuleDeclaration(node: ModuleDeclaration) {
58985904
// Emit only if this module is non-ambient.
58995905
const shouldEmit = shouldEmitModuleDeclaration(node);
@@ -5905,15 +5911,18 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
59055911
const emitVarForModule = !hoistedInDeclarationScope && !isModuleMergedWithES6Class(node);
59065912

59075913
if (emitVarForModule) {
5908-
emitStart(node);
5909-
if (isES6ExportedDeclaration(node)) {
5910-
write("export ");
5914+
const isES6ExportedNamespace = isES6ExportedDeclaration(node);
5915+
if (!isES6ExportedNamespace || isFirstDeclarationOfKind(node, node.symbol && node.symbol.declarations, SyntaxKind.ModuleDeclaration)) {
5916+
emitStart(node);
5917+
if (isES6ExportedNamespace) {
5918+
write("export ");
5919+
}
5920+
write("var ");
5921+
emit(node.name);
5922+
write(";");
5923+
emitEnd(node);
5924+
writeLine();
59115925
}
5912-
write("var ");
5913-
emit(node.name);
5914-
write(";");
5915-
emitEnd(node);
5916-
writeLine();
59175926
}
59185927

59195928
emitStart(node);

‎tests/baselines/reference/enumExportMergingES6.js‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
//// [enumExportMergingES6.ts]
2+
export enum Animals {
3+
Cat = 1
4+
}
5+
export enum Animals {
6+
Dog = 2
7+
}
8+
export enum Animals {
9+
CatDog = Cat | Dog
10+
}
11+
12+
13+
//// [enumExportMergingES6.js]
14+
export var Animals;
15+
(function (Animals) {
16+
Animals[Animals["Cat"] = 1] = "Cat";
17+
})(Animals || (Animals = {}));
18+
(function (Animals) {
19+
Animals[Animals["Dog"] = 2] = "Dog";
20+
})(Animals || (Animals = {}));
21+
(function (Animals) {
22+
Animals[Animals["CatDog"] = 3] = "CatDog";
23+
})(Animals || (Animals = {}));

‎tests/baselines/reference/enumExportMergingES6.symbols‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
=== tests/cases/conformance/enums/enumExportMergingES6.ts ===
2+
export enum Animals {
3+
>Animals : Symbol(Animals, Decl(enumExportMergingES6.ts, 0, 0), Decl(enumExportMergingES6.ts, 2, 1), Decl(enumExportMergingES6.ts, 5, 1))
4+
5+
Cat = 1
6+
>Cat : Symbol(Animals.Cat, Decl(enumExportMergingES6.ts, 0, 21))
7+
}
8+
export enum Animals {
9+
>Animals : Symbol(Animals, Decl(enumExportMergingES6.ts, 0, 0), Decl(enumExportMergingES6.ts, 2, 1), Decl(enumExportMergingES6.ts, 5, 1))
10+
11+
Dog = 2
12+
>Dog : Symbol(Animals.Dog, Decl(enumExportMergingES6.ts, 3, 21))
13+
}
14+
export enum Animals {
15+
>Animals : Symbol(Animals, Decl(enumExportMergingES6.ts, 0, 0), Decl(enumExportMergingES6.ts, 2, 1), Decl(enumExportMergingES6.ts, 5, 1))
16+
17+
CatDog = Cat | Dog
18+
>CatDog : Symbol(Animals.CatDog, Decl(enumExportMergingES6.ts, 6, 21))
19+
>Cat : Symbol(Animals.Cat, Decl(enumExportMergingES6.ts, 0, 21))
20+
>Dog : Symbol(Animals.Dog, Decl(enumExportMergingES6.ts, 3, 21))
21+
}
22+

‎tests/baselines/reference/enumExportMergingES6.types‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
=== tests/cases/conformance/enums/enumExportMergingES6.ts ===
2+
export enum Animals {
3+
>Animals : Animals
4+
5+
Cat = 1
6+
>Cat : Animals
7+
>1 : number
8+
}
9+
export enum Animals {
10+
>Animals : Animals
11+
12+
Dog = 2
13+
>Dog : Animals
14+
>2 : number
15+
}
16+
export enum Animals {
17+
>Animals : Animals
18+
19+
CatDog = Cat | Dog
20+
>CatDog : Animals
21+
>Cat | Dog : number
22+
>Cat : Animals
23+
>Dog : Animals
24+
}
25+

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,51 @@
11
tests/cases/compiler/client.ts(1,1): error TS1191: An import declaration cannot have modifiers.
2+
tests/cases/compiler/client.ts(2,12): error TS2323: Cannot redeclare exported variable 'x1'.
23
tests/cases/compiler/client.ts(3,1): error TS1191: An import declaration cannot have modifiers.
34
tests/cases/compiler/client.ts(3,34): error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'a'.
5+
tests/cases/compiler/client.ts(4,12): error TS2323: Cannot redeclare exported variable 'x1'.
46
tests/cases/compiler/client.ts(5,1): error TS1191: An import declaration cannot have modifiers.
57
tests/cases/compiler/client.ts(5,34): error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'a'.
8+
tests/cases/compiler/client.ts(6,12): error TS2323: Cannot redeclare exported variable 'x1'.
69
tests/cases/compiler/client.ts(7,1): error TS1191: An import declaration cannot have modifiers.
710
tests/cases/compiler/client.ts(7,34): error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'x'.
811
tests/cases/compiler/client.ts(7,37): error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'a'.
12+
tests/cases/compiler/client.ts(8,12): error TS2323: Cannot redeclare exported variable 'x1'.
913
tests/cases/compiler/client.ts(9,1): error TS1191: An import declaration cannot have modifiers.
1014
tests/cases/compiler/client.ts(9,34): error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'x'.
15+
tests/cases/compiler/client.ts(10,12): error TS2323: Cannot redeclare exported variable 'x1'.
1116
tests/cases/compiler/client.ts(11,1): error TS1191: An import declaration cannot have modifiers.
1217
tests/cases/compiler/client.ts(11,34): error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'm'.
18+
tests/cases/compiler/client.ts(12,12): error TS2323: Cannot redeclare exported variable 'x1'.
1319

1420

1521
==== tests/cases/compiler/server.ts (0 errors) ====
1622

1723
var a = 10;
1824
export default a;
1925

20-
==== tests/cases/compiler/client.ts (12 errors) ====
26+
==== tests/cases/compiler/client.ts (18 errors) ====
2127
export import defaultBinding1, { } from "./server";
2228
~~~~~~
2329
!!! error TS1191: An import declaration cannot have modifiers.
2430
export var x1: number = defaultBinding1;
31+
~~
32+
!!! error TS2323: Cannot redeclare exported variable 'x1'.
2533
export import defaultBinding2, { a } from "./server";
2634
~~~~~~
2735
!!! error TS1191: An import declaration cannot have modifiers.
2836
~
2937
!!! error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'a'.
3038
export var x1: number = defaultBinding2;
39+
~~
40+
!!! error TS2323: Cannot redeclare exported variable 'x1'.
3141
export import defaultBinding3, { a as b } from "./server";
3242
~~~~~~
3343
!!! error TS1191: An import declaration cannot have modifiers.
3444
~
3545
!!! error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'a'.
3646
export var x1: number = defaultBinding3;
47+
~~
48+
!!! error TS2323: Cannot redeclare exported variable 'x1'.
3749
export import defaultBinding4, { x, a as y } from "./server";
3850
~~~~~~
3951
!!! error TS1191: An import declaration cannot have modifiers.
@@ -42,16 +54,22 @@ tests/cases/compiler/client.ts(11,34): error TS2305: Module '"tests/cases/compil
4254
~
4355
!!! error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'a'.
4456
export var x1: number = defaultBinding4;
57+
~~
58+
!!! error TS2323: Cannot redeclare exported variable 'x1'.
4559
export import defaultBinding5, { x as z, } from "./server";
4660
~~~~~~
4761
!!! error TS1191: An import declaration cannot have modifiers.
4862
~
4963
!!! error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'x'.
5064
export var x1: number = defaultBinding5;
65+
~~
66+
!!! error TS2323: Cannot redeclare exported variable 'x1'.
5167
export import defaultBinding6, { m, } from "./server";
5268
~~~~~~
5369
!!! error TS1191: An import declaration cannot have modifiers.
5470
~
5571
!!! error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'm'.
5672
export var x1: number = defaultBinding6;
73+
~~
74+
!!! error TS2323: Cannot redeclare exported variable 'x1'.
5775

0 commit comments

Comments
 (0)