Make remove unused vars compatible with rest parameters

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=164866596

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

@@ -2763,7 +2763,6 @@ public static void removeChild(Node parent, Node node) {
       parent.replaceChild(node, IR.empty());
     } else if (parent.isObjectPattern()) {
       // Remove the name from the object pattern
-
       parent.removeChild(node);
     } else if (parent.isArrayPattern()) {
       if (node == parent.getLastChild()) {
@@ -2779,6 +2778,9 @@ public static void removeChild(Node parent, Node node) {
         // want to remove it from the AST
         removeChild(parent.getParent(), parent);
       }
+    } else if (parent.isRest()) {
+      // Rest params can only ever have one child node
+      parent.detach();
     } else if (parent.isParamList()) {
       parent.removeChild(node);
     } else if (parent.isDefaultValue()) {

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

@@ -536,7 +536,9 @@ private void removeUnusedDestructuringNames(Node argList, Scope fparamScope) {
         if (toRemove.getParent().isStringKey()) {
           toRemove = toRemove.getParent();
         }
-        NodeUtil.deleteNode(toRemove, compiler);
+        NodeUtil.markFunctionsDeleted(toRemove, compiler);
+        compiler.reportChangeToEnclosingScope(toRemove.getParent());
+        NodeUtil.removeChild(toRemove.getParent(), toRemove);
       }
     }
   }
@@ -553,30 +555,33 @@ private void removeUnusedDestructuringNames(Node argList, Scope fparamScope) {
   private void maybeRemoveUnusedTrailingParameters(Node argList, Scope fparamScope) {
     Node lastArg;
     while ((lastArg = argList.getLastChild()) != null) {
-      Node toRemove = lastArg;
+      Node lValue = lastArg;
       if (lastArg.isDefaultValue()) {
-        toRemove = lastArg.getFirstChild();
-        Node defaultValueSecondChild = toRemove.getNext();
+        lValue = lastArg.getFirstChild();
+        Node defaultValueSecondChild = lValue.getNext();
         if (NodeUtil.mayHaveSideEffects(defaultValueSecondChild)) {
           break;
         }
       }
 
-      if (toRemove.isDestructuringPattern()) {
-        if (!toRemove.hasChildren()) {
+      if (lValue.isRest()) {
+          lValue = lValue.getFirstChild();
+      }
+
+      if (lValue.isDestructuringPattern()) {
+        if (lValue.hasChildren()) {
+          break;
+        } else {
           // Remove empty destructuring patterns and their associated object literal assignment
           // if it exists and if the right hand side does not have side effects. Note, a
           // destructuring pattern with a "leftover" property key as in {a:{}} is not considered
           // empty in this case!
           NodeUtil.deleteNode(lastArg, compiler);
           continue;
-        } else {
-          break;
         }
       }
 
-      // Remove unreferenced parameters
-      Var var = fparamScope.getVar(toRemove.getString());
+      Var var = fparamScope.getVar(lValue.getString());
       if (!referenced.contains(var)) {
         NodeUtil.deleteNode(lastArg, compiler);
       } else {
@@ -1006,14 +1011,14 @@ private void removeUnreferencedVars() {
               || toRemove.isArrayPattern() // Array Pattern
               || parent.isObjectPattern() // Object Pattern
               || toRemove.isClass()
-              || ((toRemove.isDefaultValue()
+              || (toRemove.isDefaultValue()
                       && NodeUtil.getEnclosingScopeRoot(toRemove).isFunction())
-                  || ((parent.isDefaultValue())
-                      && NodeUtil.getEnclosingScopeRoot(parent).isFunction())),
+              || (toRemove.isRest() && NodeUtil.getEnclosingScopeRoot(toRemove).isFunction()),
           "We should only declare Vars and functions and function args and classes");
 
       if ((toRemove.isParamList() && parent.isFunction())
-          || (toRemove.isDefaultValue() && NodeUtil.getEnclosingScopeRoot(parent).isFunction())) {
+          || (toRemove.isDefaultValue() && NodeUtil.getEnclosingScopeRoot(toRemove).isFunction())
+          || (toRemove.isRest() && NodeUtil.getEnclosingScopeRoot(toRemove).isFunction())) {
         // Don't remove function arguments here. That's a special case
         // that's taken care of in removeUnreferencedFunctionArgs.
       } else if (toRemove.isComputedProp()) {

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

@@ -312,6 +312,30 @@ public void testArrayDestructuringParams() {
     test("function f([x] = [foo()]) {}; f();", "function f([] = [foo()]) {}; f();");
   }
 
+  public void testRestParams() {
+    test(
+        "function foo(...args) {/**rest param unused*/}; foo();",
+        "function foo(       ) {/**rest param unused*/}; foo();");
+
+    testSame("function foo(a, ...args) { args[0]; }; foo();");
+
+    // Rest param in pattern
+    testSame(
+        LINE_JOINER.join(
+            "function countArgs(x, ...{length}) {",
+            "  return length;",
+            "}",
+          "alert(countArgs(1, 1, 1, 1, 1));"));
+
+    test(
+        " function foo([...rest]) {/**rest unused*/}; foo();",
+        " function foo() {/**rest unused*/}; foo();");
+
+    test(
+        " function foo([x, ...rest]) { x; }; foo();",
+        " function foo([x]) { x; }; foo();");
+  }
+
   public void testFunctionsDeadButEscaped() {
     testSame("function b(a) { a = 1; print(arguments[0]) }; b(6)");
     testSame("function b(a) { var c = 2; a = c; print(arguments[0]) }; b(6)");