Various simplifications to remove Node.isFor from our codebase.

Also added a Node.isAnyFor method for the cases where one wants to check if a node represents some for, be it a for-in, for-of, or for(;;) ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=141342706
google · Dec 8, 2016 · f9fa184 · f9fa184
1 parent 1392055
commit f9fa184
Show file tree

Hide file tree

Showing 16 changed files with 53 additions and 52 deletions.
diff --git a/src/com/google/javascript/jscomp/BranchCoverageInstrumentationCallback.java b/src/com/google/javascript/jscomp/BranchCoverageInstrumentationCallback.java
@@ -80,7 +80,7 @@ public void visit(NodeTraversal traversal, Node node, Node parent) {
             fileName, new FileInstrumentationData(fileName, createArrayName(traversal)));
       }
       processBranchInfo(node, instrumentationData.get(fileName), getChildrenBlocks(node));
-    } else if (node.isFor() || node.isWhile() || node.isDo()) {
+    } else if (NodeUtil.isLoopStructure(node)) {
       List<Node> blocks = getChildrenBlocks(node);
       ControlFlowGraph<Node> cfg = traversal.getControlFlowGraph();
       for (DiGraph.DiGraphEdge<Node, ControlFlowGraph.Branch> outEdge : cfg.getOutEdges(node)) {

diff --git a/src/com/google/javascript/jscomp/CoalesceVariableNames.java b/src/com/google/javascript/jscomp/CoalesceVariableNames.java
@@ -347,7 +347,7 @@ private static void removeVarDeclaration(Node name) {
         Node assign = IR.assign(name, value).srcref(name);
 
         // We don't need to wrapped it with EXPR node if it is within a FOR.
-        if (!parent.isFor()) {
+        if (!parent.isVanillaFor()) {
           assign = NodeUtil.newExpr(assign);
         }
         parent.replaceChild(var, assign);

diff --git a/src/com/google/javascript/jscomp/ConvertToTypedInterface.java b/src/com/google/javascript/jscomp/ConvertToTypedInterface.java
@@ -302,7 +302,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
             Node body = NodeUtil.getLoopCodeBlock(n);
             parent.addChildAfter(body.detach(), n);
             NodeUtil.removeChild(parent, n);
-            Node initializer = n.isFor() ? n.getFirstChild() : IR.empty();
+            Node initializer = NodeUtil.isAnyFor(n) ? n.getFirstChild() : IR.empty();
             if (initializer.isVar() && initializer.hasOneChild()) {
               parent.addChildBefore(initializer.detach(), body);
               processName(initializer.getFirstChild(), initializer);

diff --git a/src/com/google/javascript/jscomp/DeadAssignmentsElimination.java b/src/com/google/javascript/jscomp/DeadAssignmentsElimination.java
@@ -226,7 +226,7 @@ private void tryRemoveAssignment(NodeTraversal t, Node n, Node exprRoot,
       // Ignore declarations that don't initialize a value. Dead code removal will kill those nodes.
       // Also ignore the var declaration if it's in a for-loop instantiation since there's not a
       // safe place to move the side-effects.
-      if (isDeclarationNode && (rhs == null || parent.getParent().isFor())) {
+      if (isDeclarationNode && (rhs == null || NodeUtil.isAnyFor(parent.getParent()))) {
         return;
       }
 
@@ -291,9 +291,7 @@ private void tryRemoveAssignment(NodeTraversal t, Node n, Node exprRoot,
               IR.voidNode(IR.number(0).srcref(n)));
         } else if (n.isComma() && n != parent.getLastChild()) {
           parent.removeChild(n);
-        } else if (parent.isFor()
-            && !parent.isForIn()
-            && NodeUtil.getConditionExpression(parent) != n) {
+        } else if (parent.isVanillaFor() && NodeUtil.getConditionExpression(parent) != n) {
           parent.replaceChild(n, IR.empty());
         } else {
           // Cannot replace x = a++ with x = a because that's not valid

diff --git a/src/com/google/javascript/jscomp/DefinitionsRemover.java b/src/com/google/javascript/jscomp/DefinitionsRemover.java
@@ -422,7 +422,7 @@ public void performRemove() {
           "AST should be normalized first");
       Node parent = var.getParent();
       Node rValue = name.removeFirstChild();
-      Preconditions.checkState(!parent.isFor());
+      Preconditions.checkState(!NodeUtil.isLoopStructure(parent));
       parent.replaceChild(var, NodeUtil.newExpr(rValue));
     }
 

diff --git a/src/com/google/javascript/jscomp/Denormalize.java b/src/com/google/javascript/jscomp/Denormalize.java
@@ -96,8 +96,7 @@ private void maybeCollapseIntoForStatements(Node n, Node parent) {
           compiler.reportCodeChange();
         }
       }
-    } else if (nextSibling.isFor()
-        && nextSibling.getFirstChild().isEmpty()) {
+    } else if (nextSibling.isVanillaFor() && nextSibling.getFirstChild().isEmpty()) {
 
       // Does the current node contain an in operator?  If so, embedding
       // the expression in a for loop can cause some JavaScript parsers (such

diff --git a/src/com/google/javascript/jscomp/Es6RewriteGenerators.java b/src/com/google/javascript/jscomp/Es6RewriteGenerators.java
@@ -767,7 +767,7 @@ private void visitLoop(String label) {
       body = currentStatement.removeFirstChild();
       initializer = IR.empty();
       incr = IR.empty();
-    } else if (currentStatement.isFor()) {
+    } else if (currentStatement.isVanillaFor()) {
       initializer = currentStatement.removeFirstChild();
       if (initializer.isAssign()) {
         initializer = IR.exprResult(initializer);

diff --git a/src/com/google/javascript/jscomp/Es6SyntacticScopeCreator.java b/src/com/google/javascript/jscomp/Es6SyntacticScopeCreator.java
@@ -106,7 +106,10 @@ private void scanRoot(Node n) {
           declareVar(classNameNode);
         }
       }
-    } else if (n.isBlock() || n.isFor() || n.isForOf() || n.isSwitch() || n.isModuleBody()) {
+    } else if (n.isBlock()
+        || NodeUtil.isAnyFor(n)
+        || n.isSwitch()
+        || n.isModuleBody()) {
       if (scope.getParent() != null) {
         inputId = NodeUtil.getInputId(n);
       }
@@ -262,8 +265,14 @@ private static boolean isShadowingDisallowed(Scope s, String name) {
   private boolean isNodeAtCurrentLexicalScope(Node n) {
     Node parent = n.getParent();
     Node grandparent = parent.getParent();
-    Preconditions.checkState(parent.isBlock() || parent.isFor() || parent.isForOf()
-        || parent.isScript() || parent.isModuleBody() || parent.isLabel(), parent);
+    Preconditions.checkState(
+        parent.isBlock()
+            || (parent.isVanillaFor() || parent.isForIn())
+            || parent.isForOf()
+            || parent.isScript()
+            || parent.isModuleBody()
+            || parent.isLabel(),
+        parent);
 
     if (parent.isSyntheticBlock()
         && grandparent != null && (grandparent.isCase() || grandparent.isDefaultCase())) {

diff --git a/src/com/google/javascript/jscomp/InlineVariables.java b/src/com/google/javascript/jscomp/InlineVariables.java
@@ -638,7 +638,7 @@ private boolean canMoveModerately(
      */
     private boolean isValidDeclaration(Reference declaration) {
       return (declaration.getParent().isVar()
-          && !declaration.getGrandparent().isFor())
+              && !NodeUtil.isLoopStructure(declaration.getGrandparent()))
           || NodeUtil.isFunctionDeclaration(declaration.getParent());
     }
 

diff --git a/src/com/google/javascript/jscomp/NameAnalyzer.java b/src/com/google/javascript/jscomp/NameAnalyzer.java
@@ -566,7 +566,7 @@ private void recordAssignment(NodeTraversal t, Node n, Node recordNode) {
       Node parent = n.getParent();
       NameInformation ns = createNameInformation(t, nameNode);
       if (ns != null) {
-        if (parent.isFor() && !parent.isForIn()) {
+        if (parent.isVanillaFor()) {
           // Patch for assignments that appear in the init,
           // condition or iteration part of a FOR loop.  Without
           // this change, all 3 of those parts try to claim the for
@@ -790,10 +790,10 @@ private void addSimplifiedExpression(Node n, Node parent) {
         if (value != null) {
           addSimplifiedChildren(value);
         }
-      } else if (n.isAssign() &&
-          (parent.isExprResult() ||
-           parent.isFor() ||
-           parent.isReturn())) {
+      } else if (n.isAssign()
+          && (parent.isExprResult()
+              || parent.isVanillaFor()
+              || parent.isReturn())) {
         for (Node child : n.children()) {
           addSimplifiedChildren(child);
         }
@@ -816,20 +816,18 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
       // arguments to function calls with side effects or are used in
       // control structure predicates.  These names are always
       // referenced when the enclosing function is called.
-      if (n.isFor()) {
-        if (!n.isForIn()) {
-          Node decl = n.getFirstChild();
-          Node pred = decl.getNext();
-          Node step = pred.getNext();
-          addSimplifiedExpression(decl, n);
-          addSimplifiedExpression(pred, n);
-          addSimplifiedExpression(step, n);
-        } else { // n.getChildCount() == 3
-          Node decl = n.getFirstChild();
-          Node iter = decl.getNext();
-          addAllChildren(decl);
-          addAllChildren(iter);
-        }
+      if (n.isVanillaFor()) {
+        Node decl = n.getFirstChild();
+        Node pred = decl.getNext();
+        Node step = pred.getNext();
+        addSimplifiedExpression(decl, n);
+        addSimplifiedExpression(pred, n);
+        addSimplifiedExpression(step, n);
+      } else if (n.isForIn()) {
+        Node decl = n.getFirstChild();
+        Node iter = decl.getNext();
+        addAllChildren(decl);
+        addAllChildren(iter);
       }
 
       if (parent.isVar() ||
@@ -1817,7 +1815,7 @@ private void replaceWithRhs(Node parent, Node n) {
       newReplacements.add(valueExpr);
       changeProxy.replaceWith(
           parent, n, collapseReplacements(newReplacements));
-    } else if (n.isAssign() && !parent.isFor()) {
+    } else if (n.isAssign() && !(parent.isVanillaFor() || parent.isForIn())) {
       // assignment appears in a RHS expression.  we have already
       // considered names in the assignment's RHS as being referenced;
       // replace the assignment with its RHS.
@@ -1857,7 +1855,7 @@ private void replaceTopLevelExpressionWithRhs(Node parent, Node n) {
         break;
       case ASSIGN:
         Preconditions.checkArgument(
-            parent.isFor(),
+            parent.isVanillaFor() || parent.isForIn(),
             "Unsupported assignment in replaceWithRhs. parent: %s",
             parent.getToken());
         break;
@@ -1872,7 +1870,7 @@ private void replaceTopLevelExpressionWithRhs(Node parent, Node n) {
       replacements.addAll(getSideEffectNodes(rhs));
     }
 
-    if (parent.isFor()) {
+    if (parent.isVanillaFor() || parent.isForIn()) {
       // tweak replacements array s.t. it is a single expression node.
       if (replacements.isEmpty()) {
         replacements.add(IR.empty());

diff --git a/src/com/google/javascript/jscomp/NodeUtil.java b/src/com/google/javascript/jscomp/NodeUtil.java
@@ -2373,6 +2373,10 @@ static boolean isEnhancedFor(Node n) {
     return n.isForOf() || n.isForIn();
   }
 
+  static boolean isAnyFor(Node n) {
+    return n.isVanillaFor() || n.isForIn() || n.isForOf();
+  }
+
   /** Use Node.isForIn instead. */
   @Deprecated
   public static boolean isForIn(Node n) {
@@ -2685,8 +2689,7 @@ public static void removeChild(Node parent, Node node) {
       parent.removeChild(node);
       // A LABEL without children can not be referred to, remove it.
       removeChild(parent.getParent(), parent);
-    } else if (parent.isFor()
-        && parent.getChildCount() == 4) {
+    } else if (parent.isVanillaFor()) {
       // Only Token.FOR can have an Token.EMPTY other control structure
       // need something for the condition. Others need to be replaced
       // or the structure removed.

diff --git a/src/com/google/javascript/jscomp/Normalize.java b/src/com/google/javascript/jscomp/Normalize.java
@@ -823,7 +823,7 @@ private void replaceVarWithAssignment(Node n, Node parent, Node grandparent) {
         // It is an empty reference remove it.
         if (NodeUtil.isStatementBlock(grandparent)) {
           grandparent.removeChild(parent);
-        } else if (grandparent.isFor()) {
+        } else if (grandparent.isForIn()) {
           // This is the "for (var a in b)..." case.  We don't need to worry
           // about initializers in "for (var a;;)..." as those are moved out
           // as part of the other normalizations.

diff --git a/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java b/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java
@@ -59,7 +59,6 @@ Node optimizeSubtree(Node subtree) {
       case WHILE:
         return tryFoldWhile(subtree);
       case FOR:
-      case FOR_IN:
         {
           Node condition = NodeUtil.getConditionExpression(subtree);
           if (condition != null) {
@@ -876,11 +875,7 @@ Node tryFoldWhile(Node n) {
    * Removes FORs that always evaluate to false.
    */
   Node tryFoldFor(Node n) {
-    Preconditions.checkArgument(n.isFor());
-    // If this is a FOR-IN loop skip it.
-    if (n.isForIn()) {
-      return n;
-    }
+    Preconditions.checkArgument(n.isVanillaFor());
 
     Node init = n.getFirstChild();
     Node cond = init.getNext();

diff --git a/src/com/google/javascript/jscomp/RemoveUnusedVars.java b/src/com/google/javascript/jscomp/RemoveUnusedVars.java
@@ -844,9 +844,7 @@ private void removeUnreferencedVars() {
           toRemove.getFirstChild().setString("");
         }
         // Don't remove bleeding functions.
-      } else if (parent != null &&
-          parent.isFor() &&
-          parent.getChildCount() < 4) {
+      } else if (parent != null && parent.isForIn()) {
         // foreach iterations have 3 children. Leave them alone.
       } else if (toRemove.isVar() &&
           nameNode.hasChildren() &&

diff --git a/src/com/google/javascript/jscomp/RescopeGlobalSymbols.java b/src/com/google/javascript/jscomp/RescopeGlobalSymbols.java
@@ -21,7 +21,6 @@
 import com.google.javascript.jscomp.NodeTraversal.Callback;
 import com.google.javascript.rhino.IR;
 import com.google.javascript.rhino.Node;
-
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
@@ -461,7 +460,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
         if (!c.isName()) {
           allName = false;
         }
-        if (c.isAssign() || parent.isFor()) {
+        if (c.isAssign() || NodeUtil.isAnyFor(parent)) {
           interestingChildren.add(c);
         }
       }
@@ -472,7 +471,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
         return;
       }
       for (Node c : interestingChildren) {
-        if (parent.isFor() && parent.getFirstChild() == n) {
+        if (NodeUtil.isAnyFor(parent) && parent.getFirstChild() == n) {
           commas.add(c.cloneTree());
         } else {
           // Var statement outside of for-loop.

diff --git a/src/com/google/javascript/rhino/Node.java b/src/com/google/javascript/rhino/Node.java
@@ -2955,6 +2955,8 @@ public boolean isFalse() {
     return this.token == Token.FALSE;
   }
 
+  /** Use isVanillaFor, isForIn, or NodeUtil.isAnyFor instead */
+  @Deprecated
   public boolean isFor() {
     return this.isVanillaFor() || this.isForIn();
   }