Do the goog.loadModule inlining while processing the AST rather than

having to keep around extra state to do it at the end. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=139781248
google · Nov 21, 2016 · 9904fad · 9904fad
1 parent b587fa1
commit 9904fad
Showing 1 changed file with 34 additions and 31 deletions.
diff --git a/src/com/google/javascript/jscomp/ClosureRewriteModule.java b/src/com/google/javascript/jscomp/ClosureRewriteModule.java
@@ -386,10 +386,13 @@ private class ScriptUpdater implements Callback {
     @Override
     public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
       switch (n.getToken()) {
-        case CALL:
+        case EXPR_RESULT:
-          if (NodeUtil.isCallTo(n, "goog.loadModule") && n.getLastChild().isFunction()) {
+          if (isGoogLoadModuleStatement(n)) {
-            updateGoogLoadModule(n);
+            updateGoogLoadModuleEarly(n);
           }
+          break;
+
+        case CALL:
           if (NodeUtil.isCallTo(n, "goog.module")) {
             updateGoogModule(n);
           }
@@ -434,6 +437,12 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
     @Override
     public void visit(NodeTraversal t, Node n, Node parent) {
       switch (n.getToken()) {
+        case EXPR_RESULT:
+          if (isGoogLoadModuleStatement(n)) {
+            updateGoogLoadModuleLate(n);
+          }
+          break;
+
         case SCRIPT:
           updateEndScript();
           break;
@@ -449,6 +458,13 @@ public void visit(NodeTraversal t, Node n, Node parent) {
     }
   }
 
+  private static boolean isGoogLoadModuleStatement(Node exprResult) {
+    Preconditions.checkArgument(exprResult.isExprResult());
+    Node call = exprResult.getFirstChild();
+    return call != null
+        && NodeUtil.isCallTo(call, "goog.loadModule") && call.getLastChild().isFunction();
+  }
+
   /**
    * Checks that imports of goog.module provided files are used correctly.
    */
@@ -540,7 +556,6 @@ public void visit(Node typeRefNode) {
   // Per script state needed for rewriting including nested goog.loadModule() calls.
   private Deque<ScriptDescription> scriptStack = new LinkedList<>();
   private ScriptDescription currentScript = null;
-  private List<Node> loadModuleStatements = new ArrayList<>(); // Statements to be inlined.
 
   // Global state tracking an association between the dotted names of goog.module()s and whether
   // the goog.module declares itself as a legacy namespace.
@@ -632,8 +647,6 @@ private void processAllFiles(Deque<ScriptDescription> scriptDescriptions, Node s
       pushScript(scriptDescriptions.removeFirst());
       NodeTraversal.traverseEs6(compiler, c, new ScriptUpdater());
       popScript();
-
-      inlineGoogLoadModuleCalls();
     }
   }
 
@@ -654,8 +667,6 @@ public void hotSwapScript(Node scriptRoot, Node originalRoot) {
     NodeTraversal.traverseEs6(compiler, scriptRoot, new ScriptUpdater());
     popScript();
 
-    inlineGoogLoadModuleCalls();
-
     reportUnrecognizedRequires();
   }
 
@@ -880,13 +891,23 @@ static void inlineModuleIntoGlobal(Node scriptNode) {
     scriptNode.addChildrenToBack(moduleNode.removeChildren());
   }
 
-  private void updateGoogLoadModule(Node call) {
+  private void updateGoogLoadModuleEarly(Node exprResultNode) {
     pushScript(currentScript.removeFirstChildScript());
+    Node moduleScopeRoot = exprResultNode.getFirstChild().getLastChild().getLastChild();
+    Preconditions.checkState(NodeUtil.isModuleScopeRoot(moduleScopeRoot),
+        "goog.loadModule called with non-module contents: %s", moduleScopeRoot);
+    currentScript.rootNode = moduleScopeRoot;
+  }
+
+  private void updateGoogLoadModuleLate(Node exprResultNode) {
+    Node call = exprResultNode.removeFirstChild();
+    Node moduleBlockNode = call.getLastChild().getLastChild();
 
-    currentScript.rootNode = call.getLastChild().getLastChild();
+    exprResultNode.setToken(Token.BLOCK);
-    // These should be removed and replaced with the block in the function being passed as a
+    exprResultNode.addChildrenToBack(moduleBlockNode.removeChildren());
-    // parameter, but it's not safe to do this in the middle of AST traversal.
+    currentScript.rootNode = exprResultNode;
-    loadModuleStatements.add(NodeUtil.getEnclosingStatement(call));
+    NodeUtil.tryMergeBlock(exprResultNode);
+    compiler.reportCodeChange();
   }
 
   private void updateGoogModule(Node call) {
@@ -1354,24 +1375,6 @@ private void exportTheEmptyBinaryNamespaceAt(Node atNode, AddAt addAt) {
     currentScript.hasCreatedExportObject = true;
   }
 
-  /**
-   * Rewrite "goog.loadModule(function(exports) { asdf; });" to "{ asdf; }"
-   *
-   * Can't be done in updateGoogLoadModule() because it runs in the middle of AST traversal and
-   * performing a block merge in the middle of AST traversal will either hide the block contents
-   * from the traversal or will screw up the NodeTraversal.getScope() creation.
-   */
-  private void inlineGoogLoadModuleCalls() {
-    for (Node loadModuleStatement : loadModuleStatements) {
-      Node moduleBlockNode = loadModuleStatement.getFirstChild().getLastChild().getLastChild();
-      moduleBlockNode.detach();
-      loadModuleStatement.replaceWith(moduleBlockNode);
-      NodeUtil.tryMergeBlock(moduleBlockNode);
-      compiler.reportCodeChange();
-    }
-    loadModuleStatements.clear();
-  }
-
   static void checkAndSetStrictModeDirective(NodeTraversal t, Node n) {
     Preconditions.checkState(n.isScript(), n);