Merge pull request #5148 from weswigham/duplicate-export-behavior

Make export var or export *'s with duplicate identifiers an error

Author: weswigham

src/compiler/checker.ts

@@ -1103,39 +1103,81 @@ namespace ts {
             return links.resolvedExports || (links.resolvedExports = getExportsForModule(moduleSymbol));
         }
 
-        function extendExportSymbols(target: SymbolTable, source: SymbolTable) {
+        interface ExportCollisionTracker {
+            specifierText: string;
+            exportsWithDuplicate: ExportDeclaration[];
+        }
+
+        /**
+         * Extends one symbol table with another while collecting information on name collisions for error message generation into the `lookupTable` argument
+         * Not passing `lookupTable` and `exportNode` disables this collection, and just extends the tables
+         */
+        function extendExportSymbols(target: SymbolTable, source: SymbolTable, lookupTable?: Map<ExportCollisionTracker>, exportNode?: ExportDeclaration) {
             for (const id in source) {
                 if (id !== "default" && !hasProperty(target, id)) {
                     target[id] = source[id];
+                    if (lookupTable && exportNode) {
+                        lookupTable[id] = {
+                            specifierText: getTextOfNode(exportNode.moduleSpecifier)
+                        } as ExportCollisionTracker;
+                    }
+                }
+                else if (lookupTable && exportNode && id !== "default" && hasProperty(target, id) && resolveSymbol(target[id]) !== resolveSymbol(source[id])) {
+                    if (!lookupTable[id].exportsWithDuplicate) {
+                        lookupTable[id].exportsWithDuplicate = [exportNode];
+                    }
+                    else {
+                        lookupTable[id].exportsWithDuplicate.push(exportNode);
+                    }
                 }
             }
         }
 
         function getExportsForModule(moduleSymbol: Symbol): SymbolTable {
-            let result: SymbolTable;
             const visitedSymbols: Symbol[] = [];
-            visit(moduleSymbol);
-            return result || moduleSymbol.exports;
+            return visit(moduleSymbol) || moduleSymbol.exports;
 
             // The ES6 spec permits export * declarations in a module to circularly reference the module itself. For example,
             // module 'a' can 'export * from "b"' and 'b' can 'export * from "a"' without error.
-            function visit(symbol: Symbol) {
-                if (symbol && symbol.flags & SymbolFlags.HasExports && !contains(visitedSymbols, symbol)) {
-                    visitedSymbols.push(symbol);
-                    if (symbol !== moduleSymbol) {
-                        if (!result) {
-                            result = cloneSymbolTable(moduleSymbol.exports);
+            function visit(symbol: Symbol): SymbolTable {
+                if (!(symbol && symbol.flags & SymbolFlags.HasExports && !contains(visitedSymbols, symbol))) {
+                    return;
+                }
+                visitedSymbols.push(symbol);
+                const symbols = cloneSymbolTable(symbol.exports);
+                // All export * declarations are collected in an __export symbol by the binder
+                const exportStars = symbol.exports["__export"];
+                if (exportStars) {
+                    const nestedSymbols: SymbolTable = {};
+                    const lookupTable: Map<ExportCollisionTracker> = {};
+                    for (const node of exportStars.declarations) {
+                        const resolvedModule = resolveExternalModuleName(node, (node as ExportDeclaration).moduleSpecifier);
+                        const exportedSymbols = visit(resolvedModule);
+                        extendExportSymbols(
+                            nestedSymbols,
+                            exportedSymbols,
+                            lookupTable,
+                            node as ExportDeclaration
+                        );
+                    }
+                    for (const id in lookupTable) {
+                        const { exportsWithDuplicate } = lookupTable[id];
+                        // It's not an error if the file with multiple `export *`s with duplicate names exports a member with that name itself
+                        if (id === "export=" || !(exportsWithDuplicate && exportsWithDuplicate.length) || hasProperty(symbols, id)) {
+                            continue;
                         }
-                        extendExportSymbols(result, symbol.exports);
-                    }
-                    // All export * declarations are collected in an __export symbol by the binder
-                    const exportStars = symbol.exports["__export"];
-                    if (exportStars) {
-                        for (const node of exportStars.declarations) {
-                            visit(resolveExternalModuleName(node, (<ExportDeclaration>node).moduleSpecifier));
+                        for (const node of exportsWithDuplicate) {
+                            diagnostics.add(createDiagnosticForNode(
+                                node,
+                                Diagnostics.Module_0_has_already_exported_a_member_named_1_Consider_explicitly_re_exporting_to_resolve_the_ambiguity,
+                                lookupTable[id].specifierText,
+                                id
+                            ));
                         }
                     }
+                    extendExportSymbols(symbols, nestedSymbols);
                 }
+                return symbols;
             }
         }
 
@@ -14132,8 +14174,29 @@ namespace ts {
                     const declaration = getDeclarationOfAliasSymbol(exportEqualsSymbol) || exportEqualsSymbol.valueDeclaration;
                     error(declaration, Diagnostics.An_export_assignment_cannot_be_used_in_a_module_with_other_exported_elements);
                 }
+                // Checks for export * conflicts
+                const exports = getExportsOfModule(moduleSymbol);
+                for (const id in exports) {
+                    if (id === "__export") {
+                        continue;
+                    }
+                    const { declarations, flags } = exports[id];
+                    // 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)
+                    if (!(flags & (SymbolFlags.Namespace | SymbolFlags.Interface | SymbolFlags.Enum)) && declarations.length > 1) {
+                        const exportedDeclarations: Declaration[] = filter(declarations, isNotOverload);
+                        if (exportedDeclarations.length > 1) {
+                            for (const declaration of exportedDeclarations) {
+                                diagnostics.add(createDiagnosticForNode(declaration, Diagnostics.Cannot_redeclare_exported_variable_0, id));
+                            }
+                        }
+                    }
+                }
                 links.exportsChecked = true;
             }
+
+            function isNotOverload(declaration: Declaration): boolean {
+                return declaration.kind !== SyntaxKind.FunctionDeclaration || !!(declaration as FunctionDeclaration).body;
+            }
         }
 
         function checkTypePredicate(node: TypePredicateNode) {

src/compiler/diagnosticMessages.json

@@ -844,6 +844,10 @@
         "category": "Error",
         "code": 2307
     },
+    "Module {0} has already exported a member named '{1}'. Consider explicitly re-exporting to resolve the ambiguity.": {
+        "category": "Error",
+        "code": 2308
+    },
     "An export assignment cannot be used in a module with other exported elements.": {
         "category": "Error",
         "code": 2309
@@ -900,6 +904,10 @@
         "category": "Error",
         "code": 2322
     },
+    "Cannot redeclare exported variable '{0}'.": {
+        "category": "Error",
+        "code": 2323
+    },
     "Property '{0}' is missing in type '{1}'.": {
         "category": "Error",
         "code": 2324

src/compiler/emitter.ts

@@ -5795,9 +5795,11 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
 
                 if (!shouldHoistDeclarationInSystemJsModule(node)) {
                     // do not emit var if variable was already hoisted
-                    if (!(node.flags & NodeFlags.Export) || isES6ExportedDeclaration(node)) {
+
+                    const isES6ExportedEnum = isES6ExportedDeclaration(node);
+                    if (!(node.flags & NodeFlags.Export) || (isES6ExportedEnum && isFirstDeclarationOfKind(node, node.symbol && node.symbol.declarations, SyntaxKind.EnumDeclaration))) {
                         emitStart(node);
-                        if (isES6ExportedDeclaration(node)) {
+                        if (isES6ExportedEnum) {
                             write("export ");
                         }
                         write("var ");
@@ -5894,6 +5896,10 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
                 return languageVersion === ScriptTarget.ES6 && !!(resolver.getNodeCheckFlags(node) & NodeCheckFlags.LexicalModuleMergesWithClass);
             }
 
+            function isFirstDeclarationOfKind(node: Declaration, declarations: Declaration[], kind: SyntaxKind) {
+                return !forEach(declarations, declaration => declaration.kind === kind && declaration.pos < node.pos);
+            }
+
             function emitModuleDeclaration(node: ModuleDeclaration) {
                 // Emit only if this module is non-ambient.
                 const shouldEmit = shouldEmitModuleDeclaration(node);
@@ -5905,15 +5911,18 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
                 const emitVarForModule = !hoistedInDeclarationScope && !isModuleMergedWithES6Class(node);
 
                 if (emitVarForModule) {
-                    emitStart(node);
-                    if (isES6ExportedDeclaration(node)) {
-                        write("export ");
+                    const isES6ExportedNamespace = isES6ExportedDeclaration(node);
+                    if (!isES6ExportedNamespace || isFirstDeclarationOfKind(node, node.symbol && node.symbol.declarations, SyntaxKind.ModuleDeclaration)) {
+                        emitStart(node);
+                        if (isES6ExportedNamespace) {
+                            write("export ");
+                        }
+                        write("var ");
+                        emit(node.name);
+                        write(";");
+                        emitEnd(node);
+                        writeLine();
                     }
-                    write("var ");
-                    emit(node.name);
-                    write(";");
-                    emitEnd(node);
-                    writeLine();
                 }
 
                 emitStart(node);

tests/baselines/reference/enumExportMergingES6.js

@@ -0,0 +1,23 @@
+//// [enumExportMergingES6.ts]
+export enum Animals {
+	Cat = 1
+}
+export enum Animals {
+	Dog = 2
+}
+export enum Animals {
+	CatDog = Cat | Dog
+}
+
+
+//// [enumExportMergingES6.js]
+export var Animals;
+(function (Animals) {
+    Animals[Animals["Cat"] = 1] = "Cat";
+})(Animals || (Animals = {}));
+(function (Animals) {
+    Animals[Animals["Dog"] = 2] = "Dog";
+})(Animals || (Animals = {}));
+(function (Animals) {
+    Animals[Animals["CatDog"] = 3] = "CatDog";
+})(Animals || (Animals = {}));

tests/baselines/reference/enumExportMergingES6.symbols

@@ -0,0 +1,22 @@
+=== tests/cases/conformance/enums/enumExportMergingES6.ts ===
+export enum Animals {
+>Animals : Symbol(Animals, Decl(enumExportMergingES6.ts, 0, 0), Decl(enumExportMergingES6.ts, 2, 1), Decl(enumExportMergingES6.ts, 5, 1))
+
+	Cat = 1
+>Cat : Symbol(Animals.Cat, Decl(enumExportMergingES6.ts, 0, 21))
+}
+export enum Animals {
+>Animals : Symbol(Animals, Decl(enumExportMergingES6.ts, 0, 0), Decl(enumExportMergingES6.ts, 2, 1), Decl(enumExportMergingES6.ts, 5, 1))
+
+	Dog = 2
+>Dog : Symbol(Animals.Dog, Decl(enumExportMergingES6.ts, 3, 21))
+}
+export enum Animals {
+>Animals : Symbol(Animals, Decl(enumExportMergingES6.ts, 0, 0), Decl(enumExportMergingES6.ts, 2, 1), Decl(enumExportMergingES6.ts, 5, 1))
+
+	CatDog = Cat | Dog
+>CatDog : Symbol(Animals.CatDog, Decl(enumExportMergingES6.ts, 6, 21))
+>Cat : Symbol(Animals.Cat, Decl(enumExportMergingES6.ts, 0, 21))
+>Dog : Symbol(Animals.Dog, Decl(enumExportMergingES6.ts, 3, 21))
+}
+

tests/baselines/reference/enumExportMergingES6.types

@@ -0,0 +1,25 @@
+=== tests/cases/conformance/enums/enumExportMergingES6.ts ===
+export enum Animals {
+>Animals : Animals
+
+	Cat = 1
+>Cat : Animals
+>1 : number
+}
+export enum Animals {
+>Animals : Animals
+
+	Dog = 2
+>Dog : Animals
+>2 : number
+}
+export enum Animals {
+>Animals : Animals
+
+	CatDog = Cat | Dog
+>CatDog : Animals
+>Cat | Dog : number
+>Cat : Animals
+>Dog : Animals
+}
+

tests/baselines/reference/es6ImportDefaultBindingFollowedWithNamedImport1WithExport.errors.txt

@@ -1,39 +1,51 @@
 tests/cases/compiler/client.ts(1,1): error TS1191: An import declaration cannot have modifiers.
+tests/cases/compiler/client.ts(2,12): error TS2323: Cannot redeclare exported variable 'x1'.
 tests/cases/compiler/client.ts(3,1): error TS1191: An import declaration cannot have modifiers.
 tests/cases/compiler/client.ts(3,34): error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'a'.
+tests/cases/compiler/client.ts(4,12): error TS2323: Cannot redeclare exported variable 'x1'.
 tests/cases/compiler/client.ts(5,1): error TS1191: An import declaration cannot have modifiers.
 tests/cases/compiler/client.ts(5,34): error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'a'.
+tests/cases/compiler/client.ts(6,12): error TS2323: Cannot redeclare exported variable 'x1'.
 tests/cases/compiler/client.ts(7,1): error TS1191: An import declaration cannot have modifiers.
 tests/cases/compiler/client.ts(7,34): error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'x'.
 tests/cases/compiler/client.ts(7,37): error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'a'.
+tests/cases/compiler/client.ts(8,12): error TS2323: Cannot redeclare exported variable 'x1'.
 tests/cases/compiler/client.ts(9,1): error TS1191: An import declaration cannot have modifiers.
 tests/cases/compiler/client.ts(9,34): error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'x'.
+tests/cases/compiler/client.ts(10,12): error TS2323: Cannot redeclare exported variable 'x1'.
 tests/cases/compiler/client.ts(11,1): error TS1191: An import declaration cannot have modifiers.
 tests/cases/compiler/client.ts(11,34): error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'm'.
+tests/cases/compiler/client.ts(12,12): error TS2323: Cannot redeclare exported variable 'x1'.
 
 
 ==== tests/cases/compiler/server.ts (0 errors) ====
 
     var a = 10;
     export default a;
 
-==== tests/cases/compiler/client.ts (12 errors) ====
+==== tests/cases/compiler/client.ts (18 errors) ====
     export import defaultBinding1, { } from "./server";
     ~~~~~~
 !!! error TS1191: An import declaration cannot have modifiers.
     export var x1: number = defaultBinding1;
+               ~~
+!!! error TS2323: Cannot redeclare exported variable 'x1'.
     export import defaultBinding2, { a } from "./server";
     ~~~~~~
 !!! error TS1191: An import declaration cannot have modifiers.
                                      ~
 !!! error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'a'.
     export var x1: number = defaultBinding2;
+               ~~
+!!! error TS2323: Cannot redeclare exported variable 'x1'.
     export import defaultBinding3, { a as b } from "./server";
     ~~~~~~
 !!! error TS1191: An import declaration cannot have modifiers.
                                      ~
 !!! error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'a'.
     export var x1: number = defaultBinding3;
+               ~~
+!!! error TS2323: Cannot redeclare exported variable 'x1'.
     export import defaultBinding4, { x, a as y } from "./server";
     ~~~~~~
 !!! 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
                                         ~
 !!! error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'a'.
     export var x1: number = defaultBinding4;
+               ~~
+!!! error TS2323: Cannot redeclare exported variable 'x1'.
     export import defaultBinding5, { x as z,  } from "./server";
     ~~~~~~
 !!! error TS1191: An import declaration cannot have modifiers.
                                      ~
 !!! error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'x'.
     export var x1: number = defaultBinding5;
+               ~~
+!!! error TS2323: Cannot redeclare exported variable 'x1'.
     export import defaultBinding6, { m,  } from "./server";
     ~~~~~~
 !!! error TS1191: An import declaration cannot have modifiers.
                                      ~
 !!! error TS2305: Module '"tests/cases/compiler/server"' has no exported member 'm'.
     export var x1: number = defaultBinding6;
+               ~~
+!!! error TS2323: Cannot redeclare exported variable 'x1'.