Merge pull request #17856 from Microsoft/synthesizedNamespaces

[release-2.5] Fix emit for leading 'var' declarations for synthesized namespaces

Author: DanielRosenwasser

src/compiler/transformers/ts.ts

@@ -151,7 +151,17 @@ namespace ts {
                         break;
                     }
 
-                    recordEmittedDeclarationInScope(node);
+                    // Record these declarations provided that they have a name.
+                    if ((node as ClassDeclaration | FunctionDeclaration).name) {
+                        recordEmittedDeclarationInScope(node as ClassDeclaration | FunctionDeclaration);
+                    }
+                    else {
+                        // These nodes should always have names unless they are default-exports;
+                        // however, class declaration parsing allows for undefined names, so syntactically invalid
+                        // programs may also have an undefined name.
+                        Debug.assert(node.kind === SyntaxKind.ClassDeclaration || hasModifier(node, ModifierFlags.Default));
+                    }
+
                     break;
             }
         }
@@ -2639,36 +2649,33 @@ namespace ts {
         /**
          * Records that a declaration was emitted in the current scope, if it was the first
          * declaration for the provided symbol.
-         *
-         * NOTE: if there is ever a transformation above this one, we may not be able to rely
-         *       on symbol names.
          */
-        function recordEmittedDeclarationInScope(node: Node) {
-            const name = node.symbol && node.symbol.escapedName;
-            if (name) {
-                if (!currentScopeFirstDeclarationsOfName) {
-                    currentScopeFirstDeclarationsOfName = createUnderscoreEscapedMap<Node>();
-                }
+        function recordEmittedDeclarationInScope(node: FunctionDeclaration | ClassDeclaration | ModuleDeclaration | EnumDeclaration) {
+            if (!currentScopeFirstDeclarationsOfName) {
+                currentScopeFirstDeclarationsOfName = createUnderscoreEscapedMap<Node>();
+            }
 
-                if (!currentScopeFirstDeclarationsOfName.has(name)) {
-                    currentScopeFirstDeclarationsOfName.set(name, node);
-                }
+            const name = declaredNameInScope(node);
+            if (!currentScopeFirstDeclarationsOfName.has(name)) {
+                currentScopeFirstDeclarationsOfName.set(name, node);
             }
         }
 
         /**
-         * Determines whether a declaration is the first declaration with the same name emitted
-         * in the current scope.
+         * Determines whether a declaration is the first declaration with
+         * the same name emitted in the current scope.
          */
-        function isFirstEmittedDeclarationInScope(node: Node) {
+        function isFirstEmittedDeclarationInScope(node: ModuleDeclaration | EnumDeclaration) {
             if (currentScopeFirstDeclarationsOfName) {
-                const name = node.symbol && node.symbol.escapedName;
-                if (name) {
-                    return currentScopeFirstDeclarationsOfName.get(name) === node;
-                }
+                const name = declaredNameInScope(node);
+                return currentScopeFirstDeclarationsOfName.get(name) === node;
             }
+            return true;
+        }
 
-            return false;
+        function declaredNameInScope(node: FunctionDeclaration | ClassDeclaration | ModuleDeclaration | EnumDeclaration): __String {
+            Debug.assertNode(node.name, isIdentifier);
+            return (node.name as Identifier).escapedText;
         }
 
         /**
@@ -2746,7 +2753,7 @@ namespace ts {
                 return createNotEmittedStatement(node);
             }
 
-            Debug.assert(isIdentifier(node.name), "TypeScript module should have an Identifier name.");
+            Debug.assertNode(node.name, isIdentifier, "A TypeScript namespace should have an Identifier name.");
             enableSubstitutionForNamespaceExports();
 
             const statements: Statement[] = [];

src/compiler/types.ts

@@ -2923,7 +2923,7 @@ namespace ts {
 
     export interface Symbol {
         flags: SymbolFlags;                     // Symbol flags
-        escapedName: __String;                           // Name of symbol
+        escapedName: __String;                  // Name of symbol
         declarations?: Declaration[];           // Declarations associated with this symbol
         valueDeclaration?: Declaration;         // First value declaration of the symbol
         members?: SymbolTable;                  // Class, interface or literal instance members

src/harness/unittests/transform.ts

@@ -74,6 +74,70 @@ namespace ts {
                 }
             }).outputText;
         });
+
+        testBaseline("rewrittenNamespace", () => {
+            return ts.transpileModule(`namespace Reflect { const x = 1; }`, {
+                transformers: {
+                    before: [forceNamespaceRewrite],
+                },
+                compilerOptions: {
+                    newLine: NewLineKind.CarriageReturnLineFeed,
+                }
+            }).outputText;
+        });
+
+        testBaseline("rewrittenNamespaceFollowingClass", () => {
+            return ts.transpileModule(`
+            class C { foo = 10; static bar = 20 }
+            namespace C { export let x = 10; }
+            `, {
+                transformers: {
+                    before: [forceNamespaceRewrite],
+                },
+                compilerOptions: {
+                    target: ts.ScriptTarget.ESNext,
+                    newLine: NewLineKind.CarriageReturnLineFeed,
+                }
+            }).outputText;
+        });
+
+        testBaseline("synthesizedClassAndNamespaceCombination", () => {
+            return ts.transpileModule("", {
+                transformers: {
+                    before: [replaceWithClassAndNamespace],
+                },
+                compilerOptions: {
+                    target: ts.ScriptTarget.ESNext,
+                    newLine: NewLineKind.CarriageReturnLineFeed,
+                }
+            }).outputText;
+
+            function replaceWithClassAndNamespace() {
+                return (sourceFile: ts.SourceFile) => {
+                    const result = getMutableClone(sourceFile);
+                    result.statements = ts.createNodeArray([
+                        ts.createClassDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, "Foo", /*typeParameters*/ undefined, /*heritageClauses*/ undefined, /*members*/ undefined),
+                        ts.createModuleDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, createIdentifier("Foo"), createModuleBlock([createEmptyStatement()]))
+                    ]);
+                    return result;
+                };
+            }
+        });
+
+        function forceNamespaceRewrite(context: ts.TransformationContext) {
+            return (sourceFile: ts.SourceFile): ts.SourceFile => {
+                return visitNode(sourceFile);
+
+                function visitNode<T extends ts.Node>(node: T): T {
+                    if (node.kind === ts.SyntaxKind.ModuleBlock) {
+                        const block = node as T & ts.ModuleBlock;
+                        const statements = ts.createNodeArray([...block.statements]);
+                        return ts.updateModuleBlock(block, statements) as typeof block;
+                    }
+                    return ts.visitEachChild(node, visitNode, context);
+                }
+            };
+        }
     });
 }
 

tests/baselines/reference/defaultExportsCannotMerge01.js

@@ -36,7 +36,6 @@ function Decl() {
     return 0;
 }
 exports.default = Decl;
-var Decl;
 (function (Decl) {
     Decl.x = 10;
     Decl.y = 20;

tests/baselines/reference/defaultExportsCannotMerge04.js

@@ -18,6 +18,5 @@ Object.defineProperty(exports, "__esModule", { value: true });
 function Foo() {
 }
 exports.default = Foo;
-var Foo;
 (function (Foo) {
 })(Foo || (Foo = {}));

tests/baselines/reference/parserEnumDeclaration4.js

@@ -3,6 +3,7 @@ enum void {
 }
 
 //// [parserEnumDeclaration4.js]
+var ;
 (function () {
 })( || ( = {}));
 void {};

tests/baselines/reference/transformApi/transformsCorrectly.rewrittenNamespace.js

@@ -0,0 +1,4 @@
+var Reflect;
+(function (Reflect) {
+    var x = 1;
+})(Reflect || (Reflect = {}));

tests/baselines/reference/transformApi/transformsCorrectly.rewrittenNamespaceFollowingClass.js

@@ -0,0 +1,9 @@
+class C {
+    constructor() {
+        this.foo = 10;
+    }
+}
+C.bar = 20;
+(function (C) {
+    C.x = 10;
+})(C || (C = {}));

tests/baselines/reference/transformApi/transformsCorrectly.synthesizedClassAndNamespaceCombination.js

@@ -0,0 +1,5 @@
+class Foo {
+}
+(function (Foo) {
+    ;
+})(Foo || (Foo = {}));