Cleanup traversals: There are a number of places that addChildrenX is…

… called when addChildX should be. Also a few places that call "getNext()" is called on detached nodes which is a clear code smell. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=128122683
google · Jul 22, 2016 · 911677d · 911677d
1 parent 16c96ae
commit 911677d
Show file tree

Hide file tree

Showing 19 changed files with 38 additions and 30 deletions.
diff --git a/src/com/google/javascript/jscomp/CheckSideEffects.java b/src/com/google/javascript/jscomp/CheckSideEffects.java
@@ -195,7 +195,7 @@ private void addExtern() {
     CompilerInput input = compiler.getSynthesizedExternsInput();
     name.setStaticSourceFile(input.getSourceFile());
     var.setStaticSourceFile(input.getSourceFile());
-    input.getAstRoot(compiler).addChildrenToBack(var);
+    input.getAstRoot(compiler).addChildToBack(var);
     compiler.reportCodeChange();
   }
 

diff --git a/src/com/google/javascript/jscomp/Es6ConvertSuper.java b/src/com/google/javascript/jscomp/Es6ConvertSuper.java
@@ -76,7 +76,7 @@ private void addSyntheticConstructor(Node classNode) {
             IR.getprop(superClass.cloneTree(), IR.string("apply")),
             IR.thisNode(),
             IR.name("arguments")));
-        body.addChildrenToFront(exprResult);
+        body.addChildToFront(exprResult);
       }
       Node constructor = IR.function(
           IR.name(""),

diff --git a/src/com/google/javascript/jscomp/Es6TemplateLiterals.java b/src/com/google/javascript/jscomp/Es6TemplateLiterals.java
@@ -98,7 +98,7 @@ static void visitTaggedTemplateLiteral(NodeTraversal t, Node n) {
         TEMPLATELIT_VAR + t.getCompiler().getUniqueNameIdSupplier().get());
     Node var = IR.var(callsiteId, siteObject).useSourceInfoIfMissingFromForTree(n);
     Node script = NodeUtil.getEnclosingScript(n);
-    script.addChildrenToFront(var);
+    script.addChildToFront(var);
 
     // Define the "raw" property on the introduced variable.
     Node defineRaw = IR.exprResult(IR.assign(IR.getprop(

diff --git a/src/com/google/javascript/jscomp/Es6TypedToEs6Converter.java b/src/com/google/javascript/jscomp/Es6TypedToEs6Converter.java
@@ -736,7 +736,7 @@ private Node convertDeclaredTypeToJSDoc(Node type) {
           member.setDeclaredTypeExpression(null);
           colon.addChildToBack(member.detachFromParent());
           colon.addChildToBack(memberType);
-          lb.addChildrenToBack(colon);
+          lb.addChildToBack(colon);
         }
         return new Node(Token.LC, lb);
       }

diff --git a/src/com/google/javascript/jscomp/ExtractPrototypeMemberDeclarations.java b/src/com/google/javascript/jscomp/ExtractPrototypeMemberDeclarations.java
@@ -19,6 +19,7 @@
 import com.google.javascript.jscomp.NodeTraversal.AbstractShallowCallback;
 import com.google.javascript.rhino.IR;
 import com.google.javascript.rhino.Node;
+
 import java.util.LinkedList;
 import java.util.List;
 
@@ -155,7 +156,7 @@ private void doExtraction(GatherExtractionInfo info) {
       Node var = NodeUtil.newVarNode(prototypeAlias, null)
           .useSourceInfoIfMissingFromForTree(injectionPoint);
 
-      injectionPoint.addChildrenToFront(var);
+      injectionPoint.addChildToFront(var);
     }
     // Go through all extraction instances and extract each of them.
     for (ExtractionInstance instance : info.instances) {

diff --git a/src/com/google/javascript/jscomp/FunctionRewriter.java b/src/com/google/javascript/jscomp/FunctionRewriter.java
@@ -96,7 +96,7 @@ public void process(Node externs, Node root) {
         }
 
         Node addingRoot = compiler.getNodeForCodeInsertion(null);
-        addingRoot.addChildrenToFront(helperCode);
+        addingRoot.addChildToFront(helperCode);
         compiler.reportCodeChange();
       }
     }

diff --git a/src/com/google/javascript/jscomp/FunctionToBlockMutator.java b/src/com/google/javascript/jscomp/FunctionToBlockMutator.java
@@ -354,7 +354,7 @@ private static Node replaceReturns(
         Node label = IR.label(name, block).srcref(block);
 
         Node newRoot = IR.block().srcref(block);
-        newRoot.addChildrenToBack(label);
+        newRoot.addChildToBack(label);
 
 
         // The label is now the root.
@@ -388,7 +388,7 @@ private static void addDummyAssignment(Node node, String resultName) {
     Node resultNode = createAssignStatementNode(resultName, retVal);
     resultNode.useSourceInfoIfMissingFromForTree(node);
 
-    node.addChildrenToBack(resultNode);
+    node.addChildToBack(resultNode);
   }
 
   /**

diff --git a/src/com/google/javascript/jscomp/MinimizedCondition.java b/src/com/google/javascript/jscomp/MinimizedCondition.java
@@ -336,7 +336,7 @@ private static MeasuredNode addNode(Node parent, MeasuredNode... children) {
       int cost = 0;
       boolean changed = false;
       for (MeasuredNode child : children) {
-        parent.addChildrenToBack(child.node);
+        parent.addChildToBack(child.node);
         cost += child.length;
         changed = changed || child.changed;
       }

diff --git a/src/com/google/javascript/jscomp/NodeUtil.java b/src/com/google/javascript/jscomp/NodeUtil.java
@@ -35,6 +35,7 @@
 import com.google.javascript.rhino.dtoa.DToA;
 import com.google.javascript.rhino.jstype.JSType;
 import com.google.javascript.rhino.jstype.TernaryValue;
+
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -43,6 +44,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+
 import javax.annotation.Nullable;
 
 /**
@@ -2604,7 +2606,7 @@ public static void removeChild(Node parent, Node node) {
   static void maybeAddFinally(Node tryNode) {
     Preconditions.checkState(tryNode.isTry());
     if (!NodeUtil.hasFinally(tryNode)) {
-      tryNode.addChildrenToBack(IR.block().srcref(tryNode));
+      tryNode.addChildToBack(IR.block().srcref(tryNode));
     }
   }
 
@@ -3621,7 +3623,6 @@ static Node newUndefinedNode(Node srcReferenceNode) {
   static Node newVarNode(String name, Node value) {
     Node nodeName = IR.name(name);
     if (value != null) {
-      Preconditions.checkState(value.getNext() == null);
       nodeName.addChildToBack(value);
       nodeName.srcref(value);
     }

diff --git a/src/com/google/javascript/jscomp/OptimizeParameters.java b/src/com/google/javascript/jscomp/OptimizeParameters.java
@@ -512,7 +512,7 @@ private boolean eliminateParamsAfter(Node fnNode, Node argNode) {
       eliminateParamsAfter(fnNode, argNode.getNext());
       argNode.detachFromParent();
       Node var = IR.var(argNode).useSourceInfoIfMissingFrom(argNode);
-      fnNode.getLastChild().addChildrenToFront(var);
+      fnNode.getLastChild().addChildToFront(var);
       compiler.reportCodeChange();
       return true;
     }

diff --git a/src/com/google/javascript/jscomp/PeepholeMinimizeConditions.java b/src/com/google/javascript/jscomp/PeepholeMinimizeConditions.java
@@ -164,8 +164,10 @@ private void tryJoinForCondition(Node n) {
    */
   private Node tryReplaceIf(Node n) {
 
+    Node next = null;
     for (Node child = n.getFirstChild();
-         child != null; child = child.getNext()){
+         child != null; child = next){
+      next = child.getNext();
       if (child.isIf()){
         Node cond = child.getFirstChild();
         Node thenBranch = cond.getNext();
@@ -225,6 +227,8 @@ && isReturnBlock(thenBranch)
           n.replaceChild(child, returnNode);
           n.removeChild(nextNode);
           reportCodeChange();
+          // everything else in the block is dead code.
+          break;
         } else if (elseBranch != null && statementMustExitParent(thenBranch)) {
           child.removeChild(elseBranch);
           n.addChildAfter(elseBranch, child);
@@ -591,7 +595,7 @@ private Node tryMinimizeIf(Node n) {
                       unnegatedCond.getNode(),
                       innerCond.detachFromParent())
                       .srcref(placeholder));
-              n.addChildrenToBack(innerThenBranch.detachFromParent());
+              n.addChildToBack(innerThenBranch.detachFromParent());
               reportCodeChange();
               // Not worth trying to fold the current IF-ELSE into && because
               // the inner IF-ELSE wasn't able to be folded into && anyways.
@@ -702,13 +706,14 @@ private Node tryMinimizeIf(Node n) {
       if (name1.hasChildren()
           && maybeName2.isName()
           && name1.getString().equals(maybeName2.getString())) {
-        Node thenExpr = name1.removeChildren();
+        Preconditions.checkState(name1.hasOneChild());
+        Node thenExpr = name1.removeFirstChild();
         Node elseExpr = elseAssign.getLastChild().detachFromParent();
         placeholder.detachFromParent();
         Node hookNode = IR.hook(shortCond.getNode(), thenExpr, elseExpr)
                             .srcref(n);
         var.detachFromParent();
-        name1.addChildrenToBack(hookNode);
+        name1.addChildToBack(hookNode);
         parent.replaceChild(n, var);
         reportCodeChange();
         return var;
@@ -728,12 +733,13 @@ private Node tryMinimizeIf(Node n) {
           && maybeName1.isName()
           && maybeName1.getString().equals(name2.getString())) {
         Node thenExpr = thenAssign.getLastChild().detachFromParent();
-        Node elseExpr = name2.removeChildren();
+        Preconditions.checkState(name2.hasOneChild());
+        Node elseExpr = name2.removeFirstChild();
         placeholder.detachFromParent();
         Node hookNode = IR.hook(shortCond.getNode(), thenExpr, elseExpr)
                             .srcref(n);
         var.detachFromParent();
-        name2.addChildrenToBack(hookNode);
+        name2.addChildToBack(hookNode);
         parent.replaceChild(n, var);
         reportCodeChange();
 

diff --git a/src/com/google/javascript/jscomp/ProcessCommonJSModules.java b/src/com/google/javascript/jscomp/ProcessCommonJSModules.java
@@ -385,7 +385,7 @@ private void visitRequireEnsureCall(Node n) {
       callbackFunction.getSecondChild().removeChildren();
       n.removeChildren();
       n.putBooleanProp(Node.FREE_CALL, true);
-      n.addChildrenToFront(callbackFunction);
+      n.addChildToFront(callbackFunction);
 
       compiler.reportCodeChange();
     }

diff --git a/src/com/google/javascript/jscomp/ReorderConstantExpression.java b/src/com/google/javascript/jscomp/ReorderConstantExpression.java
@@ -54,8 +54,8 @@ Node optimizeSubtree(Node subtree) {
         Node firstNode = subtree.getFirstChild().detachFromParent();
         Node lastNode = subtree.getLastChild().detachFromParent();
 
-        subtree.addChildrenToFront(lastNode);
-        subtree.addChildrenToBack(firstNode);
+        subtree.addChildToFront(lastNode);
+        subtree.addChildToBack(firstNode);
         reportCodeChange();
       }
     }

diff --git a/src/com/google/javascript/jscomp/RescopeGlobalSymbols.java b/src/com/google/javascript/jscomp/RescopeGlobalSymbols.java
@@ -117,7 +117,7 @@ private boolean isCrossModuleName(String name) {
   private void addExternForGlobalSymbolNamespace() {
     Node varNode = IR.var(IR.name(globalSymbolNamespace));
     CompilerInput input = compiler.getSynthesizedExternsInput();
-    input.getAstRoot(compiler).addChildrenToBack(varNode);
+    input.getAstRoot(compiler).addChildToBack(varNode);
     compiler.reportCodeChange();
   }
 

diff --git a/src/com/google/javascript/jscomp/StripCode.java b/src/com/google/javascript/jscomp/StripCode.java
@@ -160,8 +160,10 @@ public void visit(NodeTraversal t, Node n, Node parent) {
      */
     void removeVarDeclarationsByNameOrRvalue(NodeTraversal t, Node n,
         Node parent) {
+      Node next = null;
       for (Node nameNode = n.getFirstChild(); nameNode != null;
-          nameNode = nameNode.getNext()) {
+          nameNode = next) {
+        next = nameNode.getNext();
         String name = nameNode.getString();
         if (isStripName(name) ||
             isCallWhoseReturnValueShouldBeStripped(nameNode.getFirstChild())) {

diff --git a/src/com/google/javascript/rhino/TypeDeclarationsIR.java b/src/com/google/javascript/rhino/TypeDeclarationsIR.java
@@ -205,7 +205,7 @@ public static TypeDeclarationNode functionType(
 
     if (restName != null) {
       Node rest = new Node(Token.REST, IR.name(restName));
-      node.addChildrenToBack(maybeAddType(rest, restType));
+      node.addChildToBack(maybeAddType(rest, restType));
     }
     return node;
   }

diff --git a/test/com/google/javascript/jscomp/CombinedCompilerPassTest.java b/test/com/google/javascript/jscomp/CombinedCompilerPassTest.java
@@ -67,9 +67,9 @@ private static Node createPostOrderAlphabet() {
     d.addChildToBack(b);
     d.addChildToBack(c);
 
-    h.addChildrenToBack(e);
-    h.addChildrenToBack(f);
-    h.addChildrenToBack(g);
+    h.addChildToBack(e);
+    h.addChildToBack(f);
+    h.addChildToBack(g);
 
     l.addChildToBack(i);
     l.addChildToBack(j);

diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceWithTypeSyntaxTranspilationTest.java b/test/com/google/javascript/jscomp/NewTypeInferenceWithTypeSyntaxTranspilationTest.java
@@ -16,8 +16,6 @@
 
 package com.google.javascript.jscomp;
 
-import static com.google.javascript.jscomp.CompilerTestCase.LINE_JOINER;
-
 import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
 import com.google.javascript.jscomp.newtypes.JSTypeCreatorFromJSDoc;
 

diff --git a/test/com/google/javascript/jscomp/RecordFunctionInformationTest.java b/test/com/google/javascript/jscomp/RecordFunctionInformationTest.java
@@ -72,7 +72,7 @@ public void testMotionPreservesOriginalSourceName() {
 
     Node nodeG = mainRoot.getFirstChild().getLastChild();
     mainRoot.getFirstChild().removeChild(nodeG);
-    mainRoot.getLastChild().addChildrenToBack(nodeG.cloneTree());
+    mainRoot.getLastChild().addChildToBack(nodeG.cloneTree());
 
     FunctionInformationMap.Builder expected =
       FunctionInformationMap.newBuilder();