From efa389a346984c92fc22a3f89a7f62117a2facd4 Mon Sep 17 00:00:00 2001
From: blickly <blickly@google.com>
Date: Tue, 12 Jun 2018 16:34:38 -0700
Subject: [PATCH] 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
---
 .../jscomp/ClosureRewriteModule.java          | 17 ++++++--------
 .../javascript/jscomp/CompilerTestCase.java   |  6 ++---
 .../javascript/jscomp/IntegrationTest.java    | 22 +++++++++++++++++++
 3 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/src/com/google/javascript/jscomp/ClosureRewriteModule.java b/src/com/google/javascript/jscomp/ClosureRewriteModule.java
index 25d2a0c884f..ea15a85f106 100644
--- 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
index 34c5fa2592f..5dbe035b6f4 100644
--- 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
index 443ebeeb0a7..f6346d1e515 100644
--- 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();