Make sure to rewrite externs + sources together in goog.module rewriting

This is necessary to make sure that externs references to source is rewritten correctly, which comes up more often inside type-summary files. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=200302347
google · Jun 13, 2018 · efa389a · efa389a
1 parent 50de7ab
commit efa389a
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 13 deletions.
diff --git a/src/com/google/javascript/jscomp/ClosureRewriteModule.java b/src/com/google/javascript/jscomp/ClosureRewriteModule.java
@@ -25,6 +25,7 @@
 import com.google.common.base.Strings;
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Multimap;
 import com.google.javascript.jscomp.NodeTraversal.Callback;
@@ -693,21 +694,17 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
   @Override
   public void process(Node externs, Node root) {
     Deque<ScriptDescription> scriptDescriptions = new ArrayDeque<>();
-    processAllFiles(scriptDescriptions, externs);
-    processAllFiles(scriptDescriptions, root);
+    processAllFiles(scriptDescriptions, Iterables.concat(externs.children(), root.children()));
   }
 
-  private void processAllFiles(Deque<ScriptDescription> scriptDescriptions, Node scriptParent) {
-    if (scriptParent == null) {
-      return;
-    }
-    NodeTraversal.traverse(compiler, scriptParent, new UnwrapGoogLoadModule());
-
+  private void processAllFiles(
+      Deque<ScriptDescription> scriptDescriptions, Iterable<Node> scriptNodes) {
     // Record all the scripts first so that the googModuleNamespaces global state can be complete
     // before doing any updating also queue up scriptDescriptions for later use in ScriptUpdater
     // runs.
-    for (Node c = scriptParent.getFirstChild(); c != null; c = c.getNext()) {
+    for (Node c : scriptNodes) {
       checkState(c.isScript(), c);
+      NodeTraversal.traverse(compiler, c, new UnwrapGoogLoadModule());
       pushScript(new ScriptDescription());
       currentScript.rootNode = c;
       scriptDescriptions.addLast(currentScript);
@@ -723,7 +720,7 @@ private void processAllFiles(Deque<ScriptDescription> scriptDescriptions, Node s
 
     // Update scripts using the now complete googModuleNamespaces global state and unspool the
     // scriptDescriptions that were queued up by all the recording.
-    for (Node c = scriptParent.getFirstChild(); c != null; c = c.getNext()) {
+    for (Node c : scriptNodes) {
       pushScript(scriptDescriptions.removeFirst());
       NodeTraversal.traverse(compiler, c, new ScriptUpdater());
       popScript();

diff --git a/test/com/google/javascript/jscomp/CompilerTestCase.java b/test/com/google/javascript/jscomp/CompilerTestCase.java
@@ -1457,10 +1457,10 @@ private void testInternal(
         }
 
         if (rewriteClosureCode && i == 0) {
-          new ClosureRewriteClass(compiler).process(null, mainRoot);
-          new ClosureRewriteModule(compiler, null, null).process(null, mainRoot);
+          new ClosureRewriteClass(compiler).process(externsRoot, mainRoot);
+          new ClosureRewriteModule(compiler, null, null).process(externsRoot, mainRoot);
           new ScopedAliases(compiler, null, CompilerOptions.NULL_ALIAS_TRANSFORMATION_HANDLER)
-              .process(null, mainRoot);
+              .process(externsRoot, mainRoot);
           hasCodeChanged = hasCodeChanged || recentChange.hasCodeChanged();
         }
 

diff --git a/test/com/google/javascript/jscomp/IntegrationTest.java b/test/com/google/javascript/jscomp/IntegrationTest.java
@@ -4671,6 +4671,28 @@ public void testIjsWithGoogScopeWorks() {
         });
   }
 
+  public void testExternsReferencesToGoogModuleTypesAreRewritten() {
+    CompilerOptions options = createCompilerOptions();
+    options.setClosurePass(true);
+    options.setCheckTypes(true);
+
+    test(
+        options,
+        new String[] {
+            LINE_JOINER.join(
+                "/** @externs */",
+                "/** @constructor @template T */ function Bar() {}",
+                "/** @const {!Bar<!ns.Foo>} */ var B;",
+                ""),
+            LINE_JOINER.join(
+                "goog.module('ns');",
+                "",
+                "exports.Foo = class {}",
+                ""),
+        },
+        (String[]) null);
+  }
+
   // GitHub issue #250: https://github.com/google/closure-compiler/issues/250
   public void testInlineStringConcat() {
     CompilerOptions options = createCompilerOptions();