Refactor some shared code into a new AstManipulations class

A followup will use this code in a fourth pass so the StatementFusion class seems like the wrong home for it.

PiperOrigin-RevId: 325844462

Author: lauraharker

src/com/google/javascript/jscomp/AstManipulations.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2020 The Closure Compiler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.javascript.jscomp;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+import com.google.javascript.rhino.Node;
+import com.google.javascript.rhino.Token;
+
+/** Utilities to abstract away certain common AST manipulations */
+final class AstManipulations {
+
+  // Non-instantiable
+  private AstManipulations() {}
+
+  /**
+   * Returns a single node equivalent to executing <expr1, expr2>.
+   *
+   * <p>Requires that expr1 and expr2 are detached (i.e. have no parent nodes). If exp2 is EMPTY
+   * this just returns exp1.
+   */
+  static Node fuseExpressions(Node exp1, Node exp2) {
+    checkArgument(exp1.getParent() == null, "Expected detached node, got %s", exp1);
+    checkArgument(exp2.getParent() == null, "Expected detached node, got %s", exp2);
+    if (exp2.isEmpty()) {
+      return exp1;
+    }
+    Node comma = new Node(Token.COMMA, exp1);
+    comma.useSourceInfoIfMissingFrom(exp2);
+
+    // We can just join the new comma expression with another comma but
+    // lets keep all the comma's in a straight line. That way we can use
+    // tree comparison.
+    if (exp2.isComma()) {
+      Node leftMostChild = exp2;
+      while (leftMostChild.isComma()) {
+        leftMostChild = leftMostChild.getFirstChild();
+      }
+      Node parent = leftMostChild.getParent();
+      comma.addChildToBack(leftMostChild.detach());
+      parent.addChildToFront(comma);
+      return exp2;
+    } else {
+      comma.addChildToBack(exp2);
+      return comma;
+    }
+  }
+}

src/com/google/javascript/jscomp/StatementFusion.java

@@ -19,7 +19,6 @@
 import static com.google.common.base.Preconditions.checkArgument;
 
 import com.google.javascript.rhino.Node;
-import com.google.javascript.rhino.Token;
 
 /**
  * Tries to fuse all the statements in a block into a one statement by using COMMAs or statements.
@@ -33,7 +32,7 @@
  * program, and so it makes sense to prefer fusing statements with semicolons rather than commas.
  * This assumption has never been validated on a real program.
  */
-class StatementFusion extends AbstractPeepholeOptimization {
+final class StatementFusion extends AbstractPeepholeOptimization {
 
   @Override
   Node optimizeSubtree(Node n) {
@@ -118,8 +117,7 @@ private static Node fuseIntoOneStatement(Node parent, Node first, Node last) {
 
     Node next = null;
     for (Node cur = first.getNext(); cur != last; cur = next) {
-      commaTree = fuseExpressionIntoExpression(
-          commaTree, cur.removeFirstChild());
+      commaTree = AstManipulations.fuseExpressions(commaTree, cur.removeFirstChild());
       next = cur.getNext();
       parent.removeChild(cur);
     }
@@ -162,41 +160,15 @@ private static void fuseExpressionIntoControlFlowStatement(
     }
   }
 
-  // exp1, exp1
-  static Node fuseExpressionIntoExpression(Node exp1, Node exp2) {
-    if (exp2.isEmpty()) {
-      return exp1;
-    }
-    Node comma = new Node(Token.COMMA, exp1);
-    comma.useSourceInfoIfMissingFrom(exp2);
-
-    // We can just join the new comma expression with another comma but
-    // lets keep all the comma's in a straight line. That way we can use
-    // tree comparison.
-    if (exp2.isComma()) {
-      Node leftMostChild = exp2;
-      while (leftMostChild.isComma()) {
-        leftMostChild = leftMostChild.getFirstChild();
-      }
-      Node parent = leftMostChild.getParent();
-      comma.addChildToBack(leftMostChild.detach());
-      parent.addChildToFront(comma);
-      return exp2;
-    } else {
-      comma.addChildToBack(exp2);
-      return comma;
-    }
-  }
-
-  protected static void fuseExpressionIntoFirstChild(Node exp, Node stmt) {
+  private static void fuseExpressionIntoFirstChild(Node exp, Node stmt) {
     Node val = stmt.removeFirstChild();
-    Node comma = fuseExpressionIntoExpression(exp, val);
+    Node comma = AstManipulations.fuseExpressions(exp, val);
     stmt.addChildToFront(comma);
   }
 
-  protected static void fuseExpressionIntoSecondChild(Node exp, Node stmt) {
+  private static void fuseExpressionIntoSecondChild(Node exp, Node stmt) {
     Node val = stmt.getSecondChild().detach();
-    Node comma = fuseExpressionIntoExpression(exp, val);
+    Node comma = AstManipulations.fuseExpressions(exp, val);
     stmt.addChildAfter(comma, stmt.getFirstChild());
   }
 }

test/com/google/javascript/jscomp/AstManipulationsTest.java

@@ -0,0 +1,94 @@
+/*
+ * Copyright 2020 The Closure Compiler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.javascript.jscomp;
+
+import static com.google.javascript.rhino.testing.NodeSubject.assertNode;
+
+import com.google.javascript.rhino.IR;
+import com.google.javascript.rhino.Node;
+import com.google.javascript.rhino.Token;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Unit tests for {@link AstManipulations} */
+@RunWith(JUnit4.class)
+public final class AstManipulationsTest {
+
+  @Test
+  public void fuseExpressions_fusesTwoNumbers() {
+    Node one = IR.number(1);
+    Node two = IR.number(2);
+
+    assertNode(AstManipulations.fuseExpressions(one, two))
+        .isEqualTo(IR.comma(one.cloneNode(), two.cloneNode()));
+  }
+
+  @Test
+  public void fuseExpressions_dropsEmptySecondExpression() {
+    Node one = IR.number(1);
+    Node empty = IR.empty();
+
+    assertNode(AstManipulations.fuseExpressions(one, empty)).isEqualTo(one.cloneNode());
+  }
+
+  @Test
+  public void fuseExpressions_keepsEmptyFirstExpression() {
+    Node empty = IR.empty();
+    Node one = IR.number(1);
+
+    // Note: it should be fine to return one instead of this comma
+    assertNode(AstManipulations.fuseExpressions(empty, one))
+        .isEqualTo(new Node(Token.COMMA, empty.cloneNode(), one.cloneNode()));
+  }
+
+  @Test
+  public void fuseExpressions_fusesNumberIntoComma() {
+    Node zero = IR.number(0);
+    Node one = IR.number(1);
+    Node zeroCommaOne = IR.comma(zero, one);
+    Node two = IR.number(2);
+
+    assertNode(AstManipulations.fuseExpressions(zeroCommaOne, two))
+        .isEqualTo(IR.comma(zeroCommaOne.cloneTree(), two.cloneNode()));
+  }
+
+  @Test
+  public void fuseExpressions_fusesCommaIntoNumber() {
+    Node zero = IR.number(0);
+    Node one = IR.number(1);
+    Node two = IR.number(2);
+    Node oneCommaTwo = IR.comma(one, two);
+
+    // Utility keeps commas in the left branch of the comma tree, i.e.
+    //  COMMA
+    //    COMMA
+    //      NUMBER 0
+    //      NUMBER 1
+    //    NUMBER 2
+    // instead of this:
+    //  COMMA
+    //    NUMBER 0
+    //    COMMA
+    //      NUMBER 1
+    //      NUMBER 2
+    // to make unit-testing passes that use this utility easier. Writing 'expected' JS code
+    // for the latter tree requires more parentheses.
+    assertNode(AstManipulations.fuseExpressions(zero, oneCommaTwo))
+        .isEqualTo(IR.comma(IR.comma(zero.cloneNode(), one.cloneNode()), two.cloneNode()));
+  }
+}