From b2271329b0d22d08fa4b2042ff2b3145cc6a521f Mon Sep 17 00:00:00 2001
From: blickly <blickly@google.com>
Date: Fri, 13 May 2016 17:37:19 -0700
Subject: [PATCH] Don't create a binary namespace for legacy modules.

Update module rewriting to not create a binary module name for those modules
declared with goog.module.declareLegacyNamespace. This also fixes issues
where the wrong goog.exportSymbol name would be generated in some legacy
goog.module cases (see the new integration tests).
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=122312797
---
 .../jscomp/ClosureRewriteModule.java          | 165 ++++++++++--------
 .../jscomp/ClosureRewriteModuleTest.java      | 137 ++++++++++-----
 .../javascript/jscomp/IntegrationTest.java    |  61 +++++++
 3 files changed, 244 insertions(+), 119 deletions(-)

diff --git a/src/com/google/javascript/jscomp/ClosureRewriteModule.java b/src/com/google/javascript/jscomp/ClosureRewriteModule.java
index ac86de985df..9ebc2dd8b0e 100644
--- a/src/com/google/javascript/jscomp/ClosureRewriteModule.java
+++ b/src/com/google/javascript/jscomp/ClosureRewriteModule.java
@@ -39,6 +39,8 @@
 import java.util.Map;
 import java.util.Set;
 
+import javax.annotation.Nullable;
+
 /**
  * Process aliases in goog.modules.
  * <pre>
@@ -210,7 +212,6 @@ private final class ScriptDescription {
     boolean isModule;
     boolean declareLegacyNamespace;
     String legacyNamespace; // "a.b.c"
-    String binaryNamespace; // "module$exports$a$b$c
     String contentsPrefix; // "module$contents$a$b$c_
     final Set<String> topLevelNames = new HashSet<>(); // For prefixed content renaming.
     final Deque<ScriptDescription> childScripts = new LinkedList<>(); // For goog.loadModule()
@@ -235,6 +236,14 @@ public void addChildScript(ScriptDescription childScript) {
     public ScriptDescription removeFirstChildScript() {
       return childScripts.removeFirst();
     }
+
+    // "module$exports$a$b$c" for non-legacy modules
+    @Nullable String getBinaryNamespace() {
+      if (this.declareLegacyNamespace) {
+        return null;
+      }
+      return MODULE_EXPORTS_PREFIX + this.legacyNamespace.replace('.', '$');
+    }
   }
 
   private class ScriptRecorder extends AbstractPreOrderCallback {
@@ -411,11 +420,7 @@ public void visit(Node typeRefNode) {
                 currentScript.legacyNamespacesByAlias.containsKey(prefixTypeName);
             if (nameIsAnAlias) {
               String legacyNamespace = currentScript.legacyNamespacesByAlias.get(prefixTypeName);
-              boolean targetNamespaceIsAGoogModule = rewriteState.containsModule(legacyNamespace);
-              String aliasedNamespace =
-                  targetNamespaceIsAGoogModule
-                      ? toBinaryNamespace(legacyNamespace)
-                      : legacyNamespace;
+              String aliasedNamespace = rewriteState.getExportedNamespace(legacyNamespace);
               safeSetString(typeRefNode, aliasedNamespace + suffix);
               return;
             }
@@ -428,9 +433,10 @@ public void visit(Node typeRefNode) {
               return;
             }
 
-            boolean targetIsAGoogModule = rewriteState.containsModule(prefixTypeName);
+
+            String binaryNamespaceIfModule = rewriteState.getBinaryNamespace(prefixTypeName);
             if (legacyScriptNamespacesAndPrefixes.contains(prefixTypeName)
-                && !targetIsAGoogModule) {
+                && binaryNamespaceIfModule == null) {
               // This thing is definitely coming from a legacy script and so the fully qualified
               // type name will always resolve as is.
               return;
@@ -439,8 +445,8 @@ public void visit(Node typeRefNode) {
             // If the typeName is a reference to a fully qualified legacy namespace like
             // "foo.bar.Baz" of something that is actually a module then rewrite the JsDoc reference
             // to "module$exports$Bar".
-            if (targetIsAGoogModule) {
-              safeSetString(typeRefNode, toBinaryNamespace(prefixTypeName) + suffix);
+            if (binaryNamespaceIfModule != null) {
+              safeSetString(typeRefNode, binaryNamespaceIfModule + suffix);
               return;
             }
 
@@ -477,6 +483,16 @@ boolean isLegacyModule(String legacyNamespace) {
       return scriptDescriptionsByGoogModuleNamespace.get(legacyNamespace).declareLegacyNamespace;
     }
 
+    @Nullable String getBinaryNamespace(String legacyNamespace) {
+      ScriptDescription script = scriptDescriptionsByGoogModuleNamespace.get(legacyNamespace);
+      return script == null ? null : script.getBinaryNamespace();
+    }
+
+    String getExportedNamespace(String legacyNamespace) {
+      String binaryNamespace = getBinaryNamespace(legacyNamespace);
+      return binaryNamespace != null ? binaryNamespace : legacyNamespace;
+    }
+
     void trackModule(String legacyNamespace, ScriptDescription description) {
       scriptDescriptionsByGoogModuleNamespace.put(legacyNamespace, description);
       moduleNamesByRootNode.put(description.rootNode, legacyNamespace);
@@ -576,7 +592,6 @@ private void recordGoogModule(NodeTraversal t, Node call) {
     currentScript.isModule = true;
     currentScript.rootNode = NodeUtil.getEnclosingStatement(call).getParent();
     currentScript.legacyNamespace = legacyNamespace;
-    currentScript.binaryNamespace = toBinaryNamespace(legacyNamespace);
     currentScript.contentsPrefix = toModuleContentsPrefix(legacyNamespace);
 
     // If some other script is advertising itself as a goog.module() with this same namespace.
@@ -726,7 +741,7 @@ private void recordGoogModuleGet(NodeTraversal t, Node call) {
 
     if (aliasName != null
         && isTopLevel(t, NodeUtil.getEnclosingStatement(call), ScopeType.EXEC_CONTEXT)) {
-      // Record alias -> binaryNamespace associations for later inlining.
+      // Record alias -> legacyNamespace associations for later inlining.
       recordImportAlias(aliasName, legacyNamespace);
     }
   }
@@ -751,7 +766,10 @@ private void recordTopLevelVarNames(Node varNode) {
   }
 
   private void maybeRecordExportDeclaration(Node n) {
-    if (!currentScript.isModule || !n.getString().equals("exports") || !isAssignTarget(n)) {
+    if (!currentScript.isModule
+        || currentScript.declareLegacyNamespace
+        || !n.getString().equals("exports")
+        || !isAssignTarget(n)) {
       return;
     }
 
@@ -810,8 +828,10 @@ private void updateGoogRequire(NodeTraversal t, Node call) {
     Node statementNode = NodeUtil.getEnclosingStatement(call);
     String legacyNamespace = legacyNamespaceNode.getString();
 
-    boolean targetIsGoogModule = rewriteState.containsModule(legacyNamespace);
-    boolean targetIsAGoogModuleGetted =
+    boolean targetIsNonLegacyGoogModule =
+        rewriteState.containsModule(legacyNamespace)
+            && !rewriteState.isLegacyModule(legacyNamespace);
+    boolean targetIsGoogModuleGetted =
         currentScript.googModuleGettedNamespaces.contains(legacyNamespaceNode.getString());
     boolean importHasAlias = currentScript.legacyNamespacesByAlias.containsValue(legacyNamespace);
     boolean isDestructuring = statementNode.getFirstChild().isDestructuringLhs();
@@ -821,22 +841,20 @@ private void updateGoogRequire(NodeTraversal t, Node call) {
             && call.getGrandparent().isName()
             && NodeUtil.isNameDeclaration(call.getGrandparent().getParent());
 
-    if (currentScript.isModule || (targetIsGoogModule && targetIsAGoogModuleGetted)) {
+    if (currentScript.isModule || (targetIsNonLegacyGoogModule && targetIsGoogModuleGetted)) {
       if (isDestructuring) {
         // Rewrite
         //   "var {a, b} = goog.require('foo.Bar');" to
         //   "var {a, b} = module$exports$foo$Bar;" or
         //   "var {a, b} = foo.Bar;"
-        Node replacementNamespaceName =
-            NodeUtil.newQName(
-                compiler,
-                targetIsGoogModule ? toBinaryNamespace(legacyNamespace) : legacyNamespace);
+        String exportedNamespace = rewriteState.getExportedNamespace(legacyNamespace);
+        Node replacementNamespaceName = NodeUtil.newQName(compiler, exportedNamespace);
         replacementNamespaceName.putProp(Node.ORIGINALNAME_PROP, legacyNamespace);
         replacementNamespaceName.putBooleanProp(Node.GOOG_MODULE_REQUIRE, true);
         replacementNamespaceName.srcrefTree(call);
         call.getParent().replaceChild(call, replacementNamespaceName);
         markConst(statementNode);
-      } else if (targetIsGoogModule) {
+      } else if (targetIsNonLegacyGoogModule) {
         if (immediatePropertyAccess || !isTopLevel(t, statementNode, ScopeType.EXEC_CONTEXT)) {
           // Rewrite
           //   "var Foo = goog.require("bar.Foo").Foo;" to
@@ -844,7 +862,7 @@ private void updateGoogRequire(NodeTraversal t, Node call) {
           // or
           //   "function() {var Foo = goog.require("bar.Foo");}" to
           //   "function() {var Foo = module$exports$bar$Foo;}"
-          Node binaryNamespaceName = IR.name(toBinaryNamespace(legacyNamespace));
+          Node binaryNamespaceName = IR.name(rewriteState.getBinaryNamespace(legacyNamespace));
           binaryNamespaceName.putProp(Node.ORIGINALNAME_PROP, legacyNamespace);
           call.getParent().replaceChild(call, binaryNamespaceName);
         } else if (importHasAlias || !rewriteState.isLegacyModule(legacyNamespace)) {
@@ -874,7 +892,7 @@ private void updateGoogRequire(NodeTraversal t, Node call) {
           statementNode.getParent().replaceChild(statementNode, IR.exprResult(call));
         }
       }
-      if (targetIsGoogModule) {
+      if (targetIsNonLegacyGoogModule) {
         // Add goog.require() and namespace name to preprocessor table because they're removed
         // by current pass. If target is not a module then goog.require() is retained for
         // ProcessClosurePrimitives pass and symbols will be added there instead.
@@ -906,9 +924,11 @@ private void updateGoogModuleGetCall(Node call) {
       // In a non-module script goog.module.get() is used inside of a goog.scope() and it's
       // imported alias need only be made available within that scope.
       // Replace "goog.module.get('pkg.Foo');" with "module$exports$pkg$Foo;".
-      Node binaryNamespaceName = IR.name(toBinaryNamespace(legacyNamespace));
-      binaryNamespaceName.putProp(Node.ORIGINALNAME_PROP, legacyNamespace);
-      call.getParent().replaceChild(call, binaryNamespaceName);
+      Node exportedNamespaceName =
+          NodeUtil.newQName(compiler, rewriteState.getExportedNamespace(legacyNamespace))
+              .srcrefTree(call);
+      exportedNamespaceName.putProp(Node.ORIGINALNAME_PROP, legacyNamespace);
+      call.getParent().replaceChild(call, exportedNamespaceName);
     }
     compiler.reportCodeChange();
   }
@@ -924,7 +944,11 @@ private void updateExportsPropertyAssignment(Node getpropNode) {
     // Update "exports.foo = Foo" to "module$exports$pkg$Foo.foo = Foo";
     Node exportsNameNode = getpropNode.getFirstChild();
     Preconditions.checkState(exportsNameNode.getString().equals("exports"));
-    safeSetString(exportsNameNode, currentScript.binaryNamespace);
+    String exportedNamespace =
+        currentScript.declareLegacyNamespace
+            ? currentScript.legacyNamespace
+            : currentScript.getBinaryNamespace();
+    safeSetMaybeQualifiedString(exportsNameNode, exportedNamespace);
 
     Node jsdocNode = parent.isAssign() ? parent : getpropNode;
     markConstAndCopyJsDoc(jsdocNode, jsdocNode, parent.getLastChild());
@@ -958,9 +982,7 @@ private void maybeUpdateTopLevelName(NodeTraversal t, Node nameNode) {
     boolean nameIsAnAlias = currentScript.legacyNamespacesByAlias.containsKey(name);
     if (nameIsAnAlias && var.getNode() != nameNode) {
       String legacyNamespace = currentScript.legacyNamespacesByAlias.get(name);
-      boolean targetNamespaceIsAGoogModule = rewriteState.containsModule(legacyNamespace);
-      String namespaceToInline =
-          targetNamespaceIsAGoogModule ? toBinaryNamespace(legacyNamespace) : legacyNamespace;
+      String namespaceToInline = rewriteState.getExportedNamespace(legacyNamespace);
       safeSetMaybeQualifiedString(nameNode, namespaceToInline);
 
       // Make sure this action won't shadow a local variable.
@@ -1044,39 +1066,54 @@ private void maybeUpdateExportDeclToNode(NodeTraversal t, Node target, Node valu
    * In module "foo.Bar", rewrite "exports = Bar" to "var module$exports$foo$Bar = Bar".
    */
   private void maybeUpdateExportDeclaration(NodeTraversal t, Node n) {
-    if (!currentScript.isModule || !n.getString().equals("exports") || !isAssignTarget(n)) {
+    if (!currentScript.isModule
+        || !n.getString().equals("exports")
+        || !isAssignTarget(n)) {
       return;
     }
 
     // Rewrite "exports = ..." as "var module$exports$foo$Bar = ..."
     Node assignNode = n.getParent();
-    Node exprResultNode = assignNode.getParent();
     Node rhs = assignNode.getLastChild();
-    rhs.detachFromParent();
-    Node binaryNamespaceName = IR.name(currentScript.binaryNamespace);
-    binaryNamespaceName.putProp(Node.ORIGINALNAME_PROP, currentScript.legacyNamespace);
-    Node exportsObjectCreationNode = IR.var(binaryNamespaceName, rhs);
-    exportsObjectCreationNode.srcrefTree(exprResultNode);
-    exportsObjectCreationNode.putBooleanProp(Node.IS_NAMESPACE, true);
-    exprResultNode.getParent().replaceChild(exprResultNode, exportsObjectCreationNode);
+    Node jsdocNode;
+    if (currentScript.declareLegacyNamespace) {
+      Node legacyQname = NodeUtil.newQName(compiler, currentScript.legacyNamespace).srcrefTree(n);
+      assignNode.replaceChild(n, legacyQname);
+      jsdocNode = assignNode;
+    } else {
+      rhs.detachFromParent();
+      Node exprResultNode = assignNode.getParent();
+      Node binaryNamespaceName = IR.name(currentScript.getBinaryNamespace());
+      binaryNamespaceName.putProp(Node.ORIGINALNAME_PROP, currentScript.legacyNamespace);
+      Node exportsObjectCreationNode = IR.var(binaryNamespaceName, rhs);
+      exportsObjectCreationNode.srcrefTree(exprResultNode);
+      exportsObjectCreationNode.putBooleanProp(Node.IS_NAMESPACE, true);
+      exprResultNode.getParent().replaceChild(exprResultNode, exportsObjectCreationNode);
+      jsdocNode = exportsObjectCreationNode;
+      currentScript.hasCreatedExportObject = true;
+    }
+    markConstAndCopyJsDoc(assignNode, jsdocNode, rhs);
     compiler.reportCodeChange();
-    currentScript.hasCreatedExportObject = true;
 
-    markConstAndCopyJsDoc(assignNode, exportsObjectCreationNode, rhs);
-    maybeExportLegacyNamespaceAfter(exportsObjectCreationNode, assignNode);
     maybeUpdateExportObjectLiteral(t, rhs);
     return;
   }
 
   private void maybeUpdateExportNameRef(Node n) {
-    if (!currentScript.isModule || !"exports".equals(n.getString())) {
+    if (!currentScript.isModule || !"exports".equals(n.getString()) || n.getParent() == null) {
       return;
     }
     if (n.getParent().isParamList()) {
       return;
     }
 
-    safeSetString(n, currentScript.binaryNamespace);
+    if (currentScript.declareLegacyNamespace) {
+      Node legacyQname = NodeUtil.newQName(compiler, currentScript.legacyNamespace).srcrefTree(n);
+      n.getParent().replaceChild(n, legacyQname);
+      return;
+    }
+
+    safeSetString(n, currentScript.getBinaryNamespace());
 
     // Either this module is going to create it's own exports object at some point or else if it's
     // going to be defensively created automatically then that should have occurred at the top of
@@ -1110,7 +1147,8 @@ private void updateEndScript() {
 
   private void updateEndModule() {
     Preconditions.checkState(currentScript.isModule);
-    Preconditions.checkState(currentScript.hasCreatedExportObject);
+    Preconditions.checkState(
+        currentScript.declareLegacyNamespace || currentScript.hasCreatedExportObject);
   }
 
   /**
@@ -1138,7 +1176,11 @@ private void popScript() {
    * Add the missing "var module$exports$pkg$Foo = {};" line.
    */
   private void exportTheEmptyBinaryNamespaceAt(Node atNode, AddAt addAt) {
-    Node binaryNamespaceName = IR.name(currentScript.binaryNamespace);
+    if (currentScript.declareLegacyNamespace) {
+      return;
+    }
+
+    Node binaryNamespaceName = IR.name(currentScript.getBinaryNamespace());
     binaryNamespaceName.putProp(Node.ORIGINALNAME_PROP, currentScript.legacyNamespace);
     Node binaryNamespaceExportNode = IR.var(binaryNamespaceName, IR.objectlit());
     if (addAt == AddAt.BEFORE) {
@@ -1151,33 +1193,6 @@ private void exportTheEmptyBinaryNamespaceAt(Node atNode, AddAt addAt) {
     markConst(binaryNamespaceExportNode);
     compiler.reportCodeChange();
     currentScript.hasCreatedExportObject = true;
-
-    maybeExportLegacyNamespaceAfter(binaryNamespaceExportNode, null);
-  }
-
-  /**
-   * Maybe add a "foo.Bar = module$exports$foo$Bar;" statement to satisfy legacy namespace refs.
-   */
-  private void maybeExportLegacyNamespaceAfter(Node afterNode, Node jsDocSourceNode) {
-    if (!currentScript.declareLegacyNamespace) {
-      return;
-    }
-
-    Node binaryNamespaceName = IR.name(currentScript.binaryNamespace);
-    binaryNamespaceName.putProp(Node.ORIGINALNAME_PROP, currentScript.legacyNamespace);
-    Node assignmentNode =
-        IR.assign(NodeUtil.newQName(compiler, currentScript.legacyNamespace), binaryNamespaceName);
-    Node binaryToLegacyBridgeStatement = IR.exprResult(assignmentNode);
-    binaryToLegacyBridgeStatement.srcrefTree(afterNode);
-    if (jsDocSourceNode != null) {
-      markConstAndCopyJsDoc(jsDocSourceNode, assignmentNode, binaryNamespaceName);
-    } else {
-      markConst(assignmentNode);
-    }
-    // Do NOT mark this statement 'IS_NAMESPACE' because that would prevent
-    // ProcessClosurePrimitives from properly desugaring goog.provide().
-    afterNode.getParent().addChildAfter(binaryToLegacyBridgeStatement, afterNode);
-    compiler.reportCodeChange();
   }
 
   /**
@@ -1337,10 +1352,6 @@ private boolean isTopLevel(NodeTraversal t, Node n, ScopeType scopeType) {
     }
   }
 
-  private static String toBinaryNamespace(String legacyNamespace) {
-    return MODULE_EXPORTS_PREFIX + legacyNamespace.replace('.', '$');
-  }
-
   private static String toModuleContentsPrefix(String legacyNamespace) {
     return MODULE_CONTENTS_PREFIX + legacyNamespace.replace('.', '$') + "_";
   }
diff --git a/test/com/google/javascript/jscomp/ClosureRewriteModuleTest.java b/test/com/google/javascript/jscomp/ClosureRewriteModuleTest.java
index 0b29bb637c7..b20039c2d40 100644
--- a/test/com/google/javascript/jscomp/ClosureRewriteModuleTest.java
+++ b/test/com/google/javascript/jscomp/ClosureRewriteModuleTest.java
@@ -168,15 +168,7 @@ public void testDestructuringImports() {
   }
 
   public void testDeclareLegacyNamespace() {
-    test(
-        LINE_JOINER.join(
-            "goog.module('ns.a');",
-            "goog.module.declareLegacyNamespace();"),
-
-        LINE_JOINER.join(
-            "goog.provide('ns.a');",
-            "/** @const */ var module$exports$ns$a = {};",
-            "/** @const */ ns.a = module$exports$ns$a;"));
+    test("goog.module('ns.a'); goog.module.declareLegacyNamespace();", "goog.provide('ns.a');");
   }
 
   public void testSideEffectOnlyModuleImport() {
@@ -234,23 +226,17 @@ public void testSideEffectOnlyImportOfGoogProvide() {
   public void testSideEffectOnlyImportOfLegacyGoogModule() {
     test(
         new String[] {
-            LINE_JOINER.join(
-                "goog.module('ns.b');",
-                "goog.module.declareLegacyNamespace();",
-                "",
-                "alert('hello world');"),
-            LINE_JOINER.join(
-                "goog.module('ns.a');",
-                "",
-                "goog.require('ns.b');")},
-
+          LINE_JOINER.join(
+              "goog.module('ns.b');",
+              "goog.module.declareLegacyNamespace();",
+              "",
+              "alert('hello world');"),
+          LINE_JOINER.join("goog.module('ns.a');", "", "goog.require('ns.b');")
+        },
         new String[] {
-            LINE_JOINER.join(
-                "goog.provide('ns.b');",
-                "/** @const */ var module$exports$ns$b = {};",
-                "/** @const */ ns.b = module$exports$ns$b;",
-                "alert('hello world');"),
-            "/** @const */ var module$exports$ns$a = {}; goog.require('ns.b');"});
+          "goog.provide('ns.b'); alert('hello world');",
+          "/** @const */ var module$exports$ns$a = {}; goog.require('ns.b');"
+        });
   }
 
   public void testBundle1() {
@@ -314,12 +300,7 @@ public void testBundle3() {
             "  var b = goog.require('ns.b');",
             "  return exports;",
             "});"),
-
-        LINE_JOINER.join(
-            "/** @const */ var module$exports$ns$b = {};",
-            "goog.provide('ns.a');",
-            "/** @const */ var module$exports$ns$a = {};",
-            "/** @const */ ns.a = module$exports$ns$a;"));
+        LINE_JOINER.join("/** @const */ var module$exports$ns$b = {};", "goog.provide('ns.a');"));
   }
 
   public void testBundle4() {
@@ -359,15 +340,13 @@ public void testBundle5() {
             "  var xid = exports;",
             "  return exports;",
             "});"),
-
         LINE_JOINER.join(
             "/** @const */ var module$exports$goog$asserts = {};",
             "goog.provide('xid');",
-            "/** @const */ var module$exports$xid = function(id) {",
+            "/** @const */ xid = function(id) {",
             "  return module$contents$xid_xid.internal_(id);",
             "};",
-            "/** @const */ xid = module$exports$xid;",
-            "var module$contents$xid_xid = module$exports$xid"));
+            "var module$contents$xid_xid = xid"));
   }
 
   public void testGoogScope1() {
@@ -1277,11 +1256,9 @@ public void testPublicExport() {
             "goog.module('a.b.c');",
             "goog.module.declareLegacyNamespace();",
             "/** @public */ exports = 5;"),
-
         LINE_JOINER.join(
             "goog.provide('a.b.c');",
-            "/** @const @public */ var module$exports$a$b$c = 5;",
-            "/** @const @public */ a.b.c = module$exports$a$b$c;"));
+            "/** @const @public */ a.b.c = 5;"));
   }
 
   public void testGoogModuleReferencedWithGlobalName() {
@@ -1329,18 +1306,94 @@ public void testGoogModuleValidReferences() {
           "/** @const */ var module$exports$a$b$c={};",
           "goog.scope(function() { var c = module$exports$a$b$c; use(c); });"
         });
+  }
 
+  public void testLegacyGoogModuleValidReferences() {
     test(
         new String[] {
           "goog.module('a.b.c'); goog.module.declareLegacyNamespace();",
           "goog.require('a.b.c'); use(a.b.c);"
         },
+        new String[] {
+            "goog.provide('a.b.c');",
+            "goog.require('a.b.c'); use(a.b.c);"
+        });
+
+    test(
+        new String[] {
+          "goog.module('a.b.c'); goog.module.declareLegacyNamespace();",
+          "goog.module('x.y.z'); var c = goog.require('a.b.c'); use(c);"
+        },
+        new String[] {
+          "goog.provide('a.b.c');",
+          "/** @const */ var module$exports$x$y$z={}; goog.require('a.b.c'); use(a.b.c);"
+        });
+
+    test(
         new String[] {
           LINE_JOINER.join(
-              "goog.provide('a.b.c');",
-              "/** @const */ var module$exports$a$b$c={};",
-              "/** @const */ a.b.c = module$exports$a$b$c"),
-          "goog.require('a.b.c'); use(a.b.c);"
+              "goog.module('a.b.Foo');",
+              "goog.module.declareLegacyNamespace();",
+              "",
+              "/** @constructor */ exports = function() {};"),
+          "/** @param {a.b.Foo} x */ function f(x) {}"
+        },
+        new String[] {
+          "goog.provide('a.b.Foo'); /** @constructor */ a.b.Foo = function() {};",
+          "/** @param {a.b.Foo} x */ function f(x) {}"
+        });
+
+    test(
+        new String[] {
+          LINE_JOINER.join(
+              "goog.module('a.b.c');",
+              "goog.module.declareLegacyNamespace();",
+              "",
+              "exports = function() {};"),
+          "function f() { return goog.module.get('a.b.c'); }"
+        },
+        new String[] {
+          "goog.provide('a.b.c'); /** @const */ a.b.c = function() {};",
+          "function f() { return a.b.c; }"
+        });
+
+    test(
+        new String[] {
+          LINE_JOINER.join(
+              "goog.module('a.b.Foo');",
+              "goog.module.declareLegacyNamespace();",
+              "",
+              "/** @constructor */ function Foo() {};",
+              "",
+              "exports = Foo;"),
+          "/** @param {a.b.Foo} x */ function f(x) {}"
+        },
+        new String[] {
+            LINE_JOINER.join(
+                "goog.provide('a.b.Foo');",
+                "/** @constructor */ function module$contents$a$b$Foo_Foo() {};",
+                "/** @const */ a.b.Foo = module$contents$a$b$Foo_Foo;"),
+          "/** @param {a.b.Foo} x */ function f(x) {}"
+        });
+
+  }public void testABC(){
+    test(
+        new String[] {
+          LINE_JOINER.join(
+              "goog.module('a.b');",
+              "goog.module.declareLegacyNamespace();",
+              "",
+              "/** @constructor */ function Foo() {};",
+              "",
+              "exports.Foo = Foo;"),
+          "/** @param {a.b.Foo} x */ function f(x) {}"
+        },
+        new String[] {
+            LINE_JOINER.join(
+                "goog.provide('a.b');",
+                "/** @constructor */ function module$contents$a$b_Foo() {};",
+                "/** @const */ a.b.Foo = module$contents$a$b_Foo;"),
+          "/** @param {a.b.Foo} x */ function f(x) {}"
         });
   }
 
diff --git a/test/com/google/javascript/jscomp/IntegrationTest.java b/test/com/google/javascript/jscomp/IntegrationTest.java
index b26ae2c970e..db301fe7a22 100644
--- a/test/com/google/javascript/jscomp/IntegrationTest.java
+++ b/test/com/google/javascript/jscomp/IntegrationTest.java
@@ -3105,6 +3105,67 @@ public void testGoogModuleOuterLegacyInner() {
         ClosureRewriteModule.QUALIFIED_REFERENCE_TO_GOOG_MODULE);
   }
 
+  public void testLegacyGoogModuleExport() {
+    CompilerOptions options = new CompilerOptions();
+    options.setClosurePass(true);
+    options.setCodingConvention(new ClosureCodingConvention());
+    options.setGenerateExports(true);
+
+    test(
+        options,
+        new String[] {
+          LINE_JOINER.join(
+              "var goog = {};",
+              "goog.exportSymbol = function(path, symbol) {};"),
+          LINE_JOINER.join(
+              "goog.module('foo.example.ClassName');",
+              "goog.module.declareLegacyNamespace();",
+              "",
+              "/** @constructor */ function ClassName() {}",
+              "",
+              "/** @export */",
+              "exports = ClassName;"),
+        },
+        new String[] {
+          LINE_JOINER.join(
+              "var goog = {};",
+              "goog.exportSymbol = function(path, symbol) {};"),
+          LINE_JOINER.join(
+              "var foo = {};",
+              "foo.example = {};",
+              "function module$contents$foo$example$ClassName_ClassName() {}",
+              "foo.example.ClassName = module$contents$foo$example$ClassName_ClassName;",
+              "goog.exportSymbol('foo.example.ClassName', foo.example.ClassName);"),
+        });
+
+    test(
+        options,
+        new String[] {
+          LINE_JOINER.join(
+              "var goog = {};",
+              "goog.exportSymbol = function(path, symbol) {};"),
+          LINE_JOINER.join(
+              "goog.module('foo.ns');",
+              "goog.module.declareLegacyNamespace();",
+              "",
+              "/** @constructor */ function ClassName() {}",
+              "",
+              "/** @export */",
+              "exports.ExportedName = ClassName;"),
+        },
+        new String[] {
+          LINE_JOINER.join(
+              "var goog = {};",
+              "goog.exportSymbol = function(path, symbol) {};"),
+          LINE_JOINER.join(
+              "var foo = {};",
+              "foo.ns = {};",
+              "function module$contents$foo$ns_ClassName() {}",
+              "foo.ns.ExportedName = module$contents$foo$ns_ClassName;",
+              "goog.exportSymbol('foo.ns.ExportedName', foo.ns.ExportedName);"),
+        });
+  }
+
   public void testUnboundedArrayLiteralInfiniteLoop() {
     CompilerOptions options = createCompilerOptions();
     options.setIdeMode(true);