Inlines functions with object pattern that is a child of a rest param…

…eter. Does not inline object patterns that are not children of rest parameters.

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

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

@@ -139,7 +139,14 @@ static LinkedHashMap<String, Node> getFunctionCallParameterMap(
             array.addChildToBack(cArg.cloneTree());
             cArg = cArg.getNext();
           }
-          argMap.put(fnParam.getFirstChild().getString(), array);
+          if (fnParam.getFirstChild().isObjectPattern()) {
+            Node prop = IR.string(fnParam.getFirstFirstChild().getString());
+            Node getProp = IR.getprop(array, prop);
+            getProp.useSourceInfoIfMissingFromForTree(array);
+            argMap.put(fnParam.getFirstFirstChild().getFirstChild().getString(), getProp);
+          } else {
+            argMap.put(fnParam.getFirstChild().getString(), array);
+          }
           return argMap;
         } else if (fnParam.isDefaultValue()) {
           argMap.put(fnParam.getFirstChild().getString(), cArg);
@@ -326,7 +333,15 @@ static void maybeAddTempsForCallArguments(
         //   x( [] );
         //
         //   The parameter in the call to foo should not become "[]".
-        safe = false;
+        if (cArg.isGetProp()) {
+          if (!NodeUtil.mayHaveSideEffects(cArg)) {
+            safe = true;
+          } else {
+            safe = false;
+          }
+        } else {
+          safe = false;
+        }
       } else if (argSideEffects) {
         // Even if there are no references, we still need to evaluate the
         // expression if it has side-effects.

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

@@ -19,6 +19,8 @@
 import static com.google.common.base.Preconditions.checkState;
 
 import com.google.common.base.Preconditions;
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
 import com.google.common.base.Supplier;
 import com.google.javascript.jscomp.FunctionInjector.CanInlineResult;
 import com.google.javascript.jscomp.FunctionInjector.InliningMode;
@@ -317,13 +319,18 @@ private void maybeAddFunction(Function fn, JSModule module) {
             functionState.setInline(false);
           }
         }
+
       }
 
       // Check if block inlining is allowed.
       if (functionState.canInline() && !functionState.canInlineDirectly()
           && !blockFunctionInliningEnabled) {
         functionState.setInline(false);
       }
+
+      if (hasParamWithNumberObjectLit(fnNode)) {
+        functionState.setInline(false);
+      }
     }
   }
 
@@ -695,6 +702,30 @@ private void resolveInlineConflicts() {
     }
   }
 
+  /**
+   * @return whether the function has a param with an OBJECT_PATTERN STRING_KEY that is a number.
+   *     Prevents such functions from being inlined.
+   */
+  private static boolean hasParamWithNumberObjectLit(Node fnNode) {
+    Predicate<Node> hasParamWithNumberObjectLitPredicate =
+        new Predicate<Node>() {
+          @Override
+          public boolean apply(Node input) {
+            if (input.isObjectPattern()) {
+              for (char c : input.getFirstChild().getString().toCharArray()) {
+                if (Character.isDigit(c)) {
+                  return true;
+                }
+              }
+            }
+            return false;
+          }
+        };
+
+    return NodeUtil.has(fnNode, hasParamWithNumberObjectLitPredicate,
+        Predicates.<Node>alwaysTrue());
+  }
+
   /** @see #resolveInlineConflicts */
   private void resolveInlineConflictsForFunction(FunctionState functionState) {
     // Functions that aren't referenced don't cause conflicts.

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

@@ -460,12 +460,11 @@ public void testMaybeAddTempsForCallArgumentsRestParam2() {
         "function foo(x, ...args) {return args;} foo(1, 2);", "foo", ImmutableSet.of("args"));
   }
 
-  //TODO:@BellaShim get this test to pass
-  public void disabled_testMaybeAddTempsForCallArgumentsObjectLit2() {
+  public void testMaybeAddTempsForCallArgumentsObjectLit1() {
     testNeededTemps(
-        "function foo(x, ...{length}) {return length;} foo(1, 1, 1);",
+        "function foo(x, ...{length: length}) {return length;} foo(1, 1, 1);",
         "foo",
-        ImmutableSet.of("length"));
+        EMPTY_STRING_SET);
   }
 
   public void testArgMapWithDefaultParam1() {
@@ -570,9 +569,11 @@ private static Node parse(String js) {
     Compiler compiler = new Compiler();
     CompilerOptions options = new CompilerOptions();
     options.setLanguageIn(ECMASCRIPT_2017);
+
     compiler.initOptions(options);
     Node n = compiler.parseTestCode(js);
     assertEquals(0, compiler.getErrorCount());
     return n;
   }
 }
+

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

@@ -2589,8 +2589,7 @@ public void testArrowFunctionRestParam3() {
         "[{bar: 8}][0].bar");
   }
 
-  //TODO(bellashim):Make the test below pass
-  public void disabled_testRestParam1() {
+  public void testRestObjectPattern1() {
     test(
         LINE_JOINER.join(
             "function countArgs(...{length}) {",
@@ -2600,6 +2599,42 @@ public void disabled_testRestParam1() {
         "[1, 1, 1, 1, 1].length;");
   }
 
+  public void testRestObjectPattern2() {
+    test(
+        LINE_JOINER.join(
+            "function countArgs(x, ...{length}) {",
+            "  return length;",
+            "}",
+            "countArgs(1, 1, 1, 1, 1);"),
+        "[1, 1, 1, 1].length;");
+  }
+
+  public void testRestObjectPattern3() {
+    test(
+        LINE_JOINER.join(
+            "function countArgs(x, ...{length: length}) {",
+            "  return length;",
+            "}",
+            "countArgs(1, 1, 1, 1, 1);"),
+        "[1, 1, 1, 1].length;");
+  }
+
+  public void testRestObjectPattern4() {
+    testSame(
+        LINE_JOINER.join(
+            "function f(...{3: x}) { ",
+            "  return x; ",
+            "}",
+            "f(null,null,null,3,null);"));
+  }
+
+  public void testRestObjectPattern5() {
+    test(
+        LINE_JOINER.join(
+            "function f(...{x: y}) { ", "  return y; ", "} ", "f(null,null,null,3,null);"),
+        "[null,null,null,3,null].x");
+  }
+
   public void testDefaultParam1() {
     test(
         LINE_JOINER.join(

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

@@ -4348,8 +4348,8 @@ public void testInlineRestParam() {
             "  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))");
+        "alert(function(c){for(var b=[],a=0;a<arguments.length;++a)"
+            + "b[a-0]=arguments[a];return b[0]}(8))");
   }
 
   // TODO(bellashim): Add a non-transpiling version of this test when InlineFunctions
@@ -4370,6 +4370,29 @@ public void testDefaultParameters() {
         "var a={a:9}; a=void 0===a?{a:5}:a;alert(3+a.a)");
   }
 
+  // TODO(bellashim): Add a non-transpiling version of this test when InlineFunctions
+  // can run in that mode.
+  public void testRestObjectPatternParameters() {
+    CompilerOptions options = createCompilerOptions();
+    CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options);
+    options.setLanguageIn(LanguageMode.ECMASCRIPT_2017);
+    options.setLanguageOut(LanguageMode.ECMASCRIPT5);
+    externs = DEFAULT_EXTERNS;
+
+    test(
+        options,
+        LINE_JOINER.join(
+            "function countArgs(x, ...{length}) {",
+            "  return length;",
+            "}",
+            "alert(countArgs(1, 1, 1, 1, 1));"),
+        "alert(function (c,d) {"
+            + "  for(var b=[], a=1; a < arguments.length; ++a)"
+            + "    b[a-1] = arguments[a];"
+            + "    return b.length "
+            + "}(1,1,1,1,1))");
+  }
+
   /** Creates a CompilerOptions object with google coding conventions. */
   @Override
   protected CompilerOptions createCompilerOptions() {

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

@@ -90,6 +90,9 @@ abstract class IntegrationTestCase extends TestCase {
                   " */",
                   "function Array(var_args) {}",
                   "",
+                  "/** @type {number} */",
+                  "Array.prototype.length;",
+                  "",
                   "/**",
                   " * @constructor",
                   " * @return {number}",

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

@@ -500,6 +500,7 @@ public void testMayHaveSideEffects() {
 
     assertSideEffect(false, "Math.random();");
     assertSideEffect(true, "Math.random(seed);");
+    assertSideEffect(false, "[1, 1].foo;");
   }
 
   public void testObjectMethodSideEffects() {