Modify NodeTraversal so that is is possible to easily inspect the uns…

…ubstantiated Scope root stack. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=162539254
google · Jul 20, 2017 · f2d4742 · f2d4742
1 parent 89e476c
commit f2d4742
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 11 deletions.
diff --git a/src/com/google/javascript/jscomp/NodeTraversal.java b/src/com/google/javascript/jscomp/NodeTraversal.java
@@ -24,6 +24,7 @@
 import com.google.javascript.rhino.Node;
 import com.google.javascript.rhino.Token;
 import java.util.ArrayDeque;
+import java.util.ArrayList;
 import java.util.Deque;
 import java.util.List;
 import java.util.Set;
@@ -54,7 +55,7 @@ public class NodeTraversal {
    * A stack of scope roots. All scopes that have not been created
    * are represented in this Deque.
    */
-  private final ArrayDeque<Node> scopeRoots = new ArrayDeque<>();
+  private final ArrayList<Node> scopeRoots = new ArrayList<>();
 
   /**
    * Stack containing the control flow graphs (CFG) that have been created. There are fewer CFGs
@@ -837,7 +838,7 @@ private void recordScopeRoot(Node node) {
   private void pushScope(Node node) {
     checkState(curNode != null);
     checkState(node != null);
-    scopeRoots.push(node);
+    scopeRoots.add(node);
     recordScopeRoot(node);
     if (scopeCallback != null) {
       scopeCallback.enterScope(this);
@@ -874,8 +875,11 @@ private void popScope(boolean quietly) {
     if (!quietly && scopeCallback != null) {
       scopeCallback.exitScope(this);
     }
-    Node scopeRoot = scopeRoots.pollFirst();
+    Node scopeRoot;
-    if (scopeRoot == null) {
+    int roots = scopeRoots.size();
+    if (roots > 0) {
+      scopeRoot = scopeRoots.remove(roots - 1);
+    } else {
       scopeRoot = scopes.pop().getRootNode();
     }
     if (NodeUtil.isValidCfgRoot(scopeRoot)) {
@@ -887,16 +891,30 @@ private void popScope(boolean quietly) {
   public Scope getScope() {
     Scope scope = scopes.peek();
 
-    Node root = null;
+    for (int i = 0; i < scopeRoots.size(); i++) {
-    while ((root = scopeRoots.pollLast()) != null) {
+      scope = scopeCreator.createScope(scopeRoots.get(i), scope);
-      scope = scopeCreator.createScope(root, scope);
       scopes.push(scope);
     }
+    scopeRoots.clear();
 
     // No need to call compiler.setScope; the top scopeRoot is now the top scope
     return scope;
   }
 
+
+
+  public Node getClosestHoistScopeRoot() {
+    int roots = scopeRoots.size();
+    for (int i = roots; i > 0; i--) {
+      Node rootNode = scopeRoots.get(i - 1);
+      if (Scope.isHoistScopeRootNode(rootNode)) {
+        return rootNode;
+      }
+    }
+
+    return scopes.peek().getClosestHoistScope().getRootNode();
+  }
+
   public Scope getClosestHoistScope() {
     // TODO(moz): This should not call getScope(). We should find the root of the closest hoist
     // scope and effectively getScope() from there, which avoids scanning inner scopes that might
@@ -929,12 +947,12 @@ public ControlFlowGraph<Node> getControlFlowGraph() {
 
   /** Returns the current scope's root. */
   public Node getScopeRoot() {
-    Node root = scopeRoots.peek();
+    int roots = scopeRoots.size();
-    if (root == null) {
+    if (roots > 0) {
+      return scopeRoots.get(roots - 1);
+    } else {
       Scope s = scopes.peek();
       return s != null ? s.getRootNode() : null;
-    } else {
-      return root;
     }
   }
 

diff --git a/src/com/google/javascript/jscomp/Scope.java b/src/com/google/javascript/jscomp/Scope.java
@@ -311,6 +311,18 @@ boolean isHoistScope() {
     return isFunctionScope() || isFunctionBlockScope() || isGlobal() || isModuleScope();
   }
 
+  public static boolean isHoistScopeRootNode(Node n) {
+    switch (n.getToken()) {
+      case FUNCTION:
+      case MODULE_BODY:
+      case ROOT:
+      case SCRIPT:
+        return true;
+      default:
+        return NodeUtil.isFunctionBlock(n);
+    }
+  }
+
   /**
    * If a var were declared in this scope, return the scope it would be hoisted to.
    *

diff --git a/test/com/google/javascript/jscomp/NodeTraversalTest.java b/test/com/google/javascript/jscomp/NodeTraversalTest.java
@@ -162,6 +162,37 @@ public void visit(NodeTraversal t, Node n, Node parent) {
     );
   }
 
+  public void testGetHoistScopeRoot() {
+    Compiler compiler = new Compiler();
+    String code = LINE_JOINER.join(
+        "function foo() {",
+        "  if (true) { var XXX; }",
+        "}");
+    Node tree = parse(compiler, code);
+    NodeTraversal.traverseEs6(compiler, tree,
+        new NodeTraversal.Callback() {
+
+          @Override
+          public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
+            return true;
+          }
+
+          @Override
+          public void visit(NodeTraversal t, Node n, Node parent) {
+            if (n.isName() && n.getString().equals("XXX")) {
+              Node root = t.getClosestHoistScopeRoot();
+              assertThat(NodeUtil.isFunctionBlock(root)).isTrue();
+
+              t.getScope();  // force scope creation
+
+              root = t.getClosestHoistScopeRoot();
+              assertThat(NodeUtil.isFunctionBlock(root)).isTrue();
+            }
+          }
+        }
+    );
+  }
+
   private static class NameChangingCallback implements NodeTraversal.Callback {
     @Override
     public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {