Add isHoistScope for improved readability when dealing with scopes

------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=155037058
google · May 4, 2017 · 27039b0 · 27039b0
1 parent 81af455
commit 27039b0
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 6 deletions.
diff --git a/src/com/google/javascript/jscomp/Es6SyntacticScopeCreator.java b/src/com/google/javascript/jscomp/Es6SyntacticScopeCreator.java
@@ -142,7 +142,7 @@ private void declareLHS(Node n) {
   private void scanVars(Node n, boolean scanInnerBlockScopes, boolean firstScan) {
     switch (n.getToken()) {
       case VAR:
-        if (scope.getClosestHoistScope() == scope) {
+        if (scope.isHoistScope()) {
           declareLHS(n);
         }
         return;

diff --git a/src/com/google/javascript/jscomp/HoistVarsOutOfBlocks.java b/src/com/google/javascript/jscomp/HoistVarsOutOfBlocks.java
@@ -75,7 +75,7 @@ public void process(Node externs, Node root) {
   public void afterExitScope(NodeTraversal t, ReferenceMap refMap) {
     // TODO(tbreisacher): Avoid calling t.getScope() here, so that we aren't creating scopes we
     // don't need to.
-    if (t.getScope() != t.getClosestHoistScope()) {
+    if (!t.getScope().isHoistScope()) {
       return;
     }
     this.refMap = refMap;

diff --git a/src/com/google/javascript/jscomp/Scope.java b/src/com/google/javascript/jscomp/Scope.java
@@ -256,6 +256,18 @@ && getRootNode().hasOneChild()
         && getRootNode().getFirstChild().isCatch();
   }
 
+  /**
+   * If a var were declared in this scope, would it belong to this scope (as opposed to some
+   * enclosing scope)?
+   *
+   * We consider function scopes to be hoist scopes. Even though it's impossible to declare a var
+   * inside function parameters, it would make less sense to say that if you did declare one in
+   * the function parameters, it would be hoisted somewhere else.
+   */
+  boolean isHoistScope() {
+    return isFunctionScope() || isFunctionBlockScope() || isGlobal() || isModuleScope();
+  }
+
   /**
    * If a var were declared in this scope, return the scope it would be hoisted to.
    *
@@ -266,10 +278,7 @@ && getRootNode().hasOneChild()
   public Scope getClosestHoistScope() {
     Scope current = this;
     while (current != null) {
-      if (current.isFunctionScope()
-          || current.isFunctionBlockScope()
-          || current.isGlobal()
-          || current.isModuleScope()) {
+      if (current.isHoistScope()) {
         return current;
       }
       current = current.parent;

diff --git a/test/com/google/javascript/jscomp/Es6SyntacticScopeCreatorTest.java b/test/com/google/javascript/jscomp/Es6SyntacticScopeCreatorTest.java
@@ -430,6 +430,7 @@ public void testScopeRootNode() {
     assertEquals(root, globalScope.getRootNode());
     assertFalse(globalScope.isBlockScope());
     assertEquals(globalScope, globalScope.getClosestHoistScope());
+    assertTrue(globalScope.isHoistScope());
 
     Node function = root.getFirstChild();
     checkState(function.isFunction(), function);
@@ -440,6 +441,7 @@ public void testScopeRootNode() {
     assertEquals(fooBlockNode, fooScope.getRootNode());
     assertTrue(fooScope.isBlockScope());
     assertEquals(fooScope, fooScope.getClosestHoistScope());
+    assertTrue(fooScope.isHoistScope());
     assertTrue(fooScope.isDeclared("x", false));
   }