InlineFunctions compatible with REST parameters by injecting blocks w…

…ith temporary variables that are removed in later passes. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=159293956
google · Jun 19, 2017 · 183c87f · 183c87f
1 parent b004cad
commit 183c87f
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 21 deletions.
diff --git a/src/com/google/javascript/jscomp/FunctionArgumentInjector.java b/src/com/google/javascript/jscomp/FunctionArgumentInjector.java
@@ -19,6 +19,7 @@
 import com.google.common.base.Predicate;
 import com.google.common.base.Supplier;
 import com.google.javascript.jscomp.NodeUtil.Visitor;
+import com.google.javascript.rhino.IR;
 import com.google.javascript.rhino.Node;
 import com.google.javascript.rhino.Token;
 import java.util.HashSet;
@@ -126,13 +127,29 @@ static LinkedHashMap<String, Node> getFunctionCallParameterMap(
       argMap.put(THIS_MARKER, NodeUtil.newUndefinedNode(callNode));
     }
 
-    for (Node fnArg : NodeUtil.getFunctionParameters(fnNode).children()) {
+    for (Node fnParam : NodeUtil.getFunctionParameters(fnNode).children()) {
       if (cArg != null) {
-        argMap.put(fnArg.getString(), cArg);
+        if (fnParam.isRest()) {
-        cArg = cArg.getNext();
+          Node array = IR.arraylit();
+          array.useSourceInfoIfMissingFromForTree(cArg);
+          while (cArg != null) {
+            array.addChildToBack(cArg.cloneTree());
+            cArg = cArg.getNext();
+          }
+          argMap.put(fnParam.getFirstChild().getString(), array);
+        } else {
+          argMap.put(fnParam.getString(), cArg);
+          cArg = cArg.getNext();
+        }
       } else {
-        Node srcLocation = callNode;
+        if (fnParam.isRest()) {
-        argMap.put(fnArg.getString(), NodeUtil.newUndefinedNode(srcLocation));
+          //No arguments for REST parameters
+          Node array = IR.arraylit();
+          argMap.put(fnParam.getFirstChild().getString(), array);
+        } else {
+          Node srcLocation = callNode;
+          argMap.put(fnParam.getString(), NodeUtil.newUndefinedNode(srcLocation));
+        }
       }
     }
 

diff --git a/src/com/google/javascript/jscomp/InlineFunctions.java b/src/com/google/javascript/jscomp/InlineFunctions.java
@@ -266,16 +266,6 @@ private void maybeAddFunction(Function fn, JSModule module) {
       return;
     }
     Node fnNode = fn.getFunctionNode();
-    int i = 0;
-    while (fnNode.getSecondChild().getChildAtIndex(i) != null) {
-      Node param = fnNode.getSecondChild().getChildAtIndex(i);
-      if (param.isRest()) {
-        functionState.setInline(false);
-        return;
-      }
-      i++;
-    }
-
     if (enforceMaxSizeAfterInlining
         && !isAlwaysInlinable(fnNode)
         && maxSizeAfterInlining <= NodeUtil.countAstSizeUpToLimit(fnNode, maxSizeAfterInlining)) {

diff --git a/test/com/google/javascript/jscomp/InlineFunctionsTest.java b/test/com/google/javascript/jscomp/InlineFunctionsTest.java
@@ -2549,23 +2549,54 @@ public void testFunctionProperty() {
   }
 
   public void testArrowFunctionRestParam1() {
-    testSame(
+    test(
         LINE_JOINER.join(
             "function foo() {",
             "  var f = (...args) => args[0];",
             "  return f(8);",
             "}",
-            "foo();"));
+            "foo();"),
+        "[8][0]");
   }
 
   public void testArrowFunctionRestParam2() {
-    testSame(
+    test(
+        LINE_JOINER.join(
+            "function foo() {",
+            "  var f = (x, ...args) => x + args[0];",
+            "  return f(1, 8);",
+            "}",
+            "foo();"),
+        LINE_JOINER.join(
+            "{",
+            "  var JSCompiler_inline_result$jscomp$inline_0;",
+            "  {",
+            "     var args$jscomp$inline_1 = [8];",
+            "     JSCompiler_inline_result$jscomp$inline_0 = 1 + args$jscomp$inline_1[0];",
+            "  }",
+            "  JSCompiler_inline_result$jscomp$inline_0",
+            "}"));
+  }
+
+  public void testArrowFunctionRestParam3() {
+    test(
         LINE_JOINER.join(
             "function foo() {",
-              "  var f = (x, ...args) => x + args[0];",
+            "  var f = (...args) => args[0].bar;",
-              "  return f(1, 8);",
+            "  return f({bar: 8});",
             "}",
-            "foo();"));
+            "foo();"),
+        "[{bar: 8}][0].bar");
   }
 
+  //TODO(bellashim):Make the test below pass
+  public void disabled_testRestParam1() {
+    test(
+        LINE_JOINER.join(
+            "function countArgs(...{length}) {",
+            "  return length;",
+            "}",
+            "countArgs(1, 1, 1, 1, 1);"),
+        "[1, 1, 1, 1, 1].length;");
+  }
 }
diff --git a/test/com/google/javascript/jscomp/IntegrationTest.java b/test/com/google/javascript/jscomp/IntegrationTest.java
@@ -4329,6 +4329,25 @@ public void testCheckVarsOnInAdvancedMode() {
     test(options, "var x = foobar;", VarCheck.UNDEFINED_VAR_ERROR);
   }
 
+  // TODO(bellashim): Add a non-transpiling version of this test when InlineFunctions
+  // can run in that mode.
+  public void testInlineRestParam() {
+    CompilerOptions options = createCompilerOptions();
+    options.setLanguageIn(LanguageMode.ECMASCRIPT_2017);
+    options.setLanguageOut(LanguageMode.ECMASCRIPT5);
+    CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options);
+    test(
+        options,
+        LINE_JOINER.join(
+            "function foo() {",
+            "  var f = (...args) => args[0];",
+            "  return f(8);",
+            "}",
+            "alert(foo());"),
+        "alert(function(c){for(var b=[],a=0;a<arguments.a;++a)b[a-0]"
+            + "=arguments[a];return b[0]}(8))");
+  }
+
   /** Creates a CompilerOptions object with google coding conventions. */
   @Override
   protected CompilerOptions createCompilerOptions() {