changed method hasParamWithNumberObjectLit, to check whether *all* ch…

…ildren of a parameter is a number string_key or not, rather than only verifying the first one. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=163071055
google · Jul 26, 2017 · 26782fb · 26782fb
1 parent b2a1d45
commit 26782fb
Show file tree

Hide file tree

Showing 2 changed files with 93 additions and 7 deletions.
diff --git a/src/com/google/javascript/jscomp/InlineFunctions.java b/src/com/google/javascript/jscomp/InlineFunctions.java
@@ -330,7 +330,7 @@ private void maybeAddFunction(Function fn, JSModule module) {
         functionState.setInline(false);
       }
 
-      if (hasParamWithNumberObjectLit(fnNode)) {
+      if (hasParamWithInvalidPropertyNameIdentifier(fnNode)) {
         functionState.setInline(false);
       }
 
@@ -716,17 +716,22 @@ private void resolveInlineConflicts() {
   }
 
   /**
-   * @return whether the function has a param with an OBJECT_PATTERN STRING_KEY that is a number.
+   * @return whether the function has a param with an OBJECT_PATTERN STRING_KEY that is a number,
+   *     which is not a valid JavaScript identifier or if it is a quoted string.
    *     Prevents such functions from being inlined.
+   *     These cases are currently chosen to not be inlined and is not an inherent limitation.
+   *     TODO(bellashim): For invalid property names, invoke property values using bracket notation
+   *        and inline those functions.
    */
-  private static boolean hasParamWithNumberObjectLit(Node fnNode) {
+  private static boolean hasParamWithInvalidPropertyNameIdentifier(Node fnNode) {
-    Predicate<Node> hasParamWithNumberObjectLitPredicate =
+    Predicate<Node> hasParamWithInvalidPropertyNameIdentifierPredicate =
         new Predicate<Node>() {
           @Override
           public boolean apply(Node input) {
             if (input.isObjectPattern()) {
-              for (char c : input.getFirstChild().getString().toCharArray()) {
+              for (Node prop = input.getFirstChild(); prop != null; prop = prop.getNext()) {
-                if (Character.isDigit(c)) {
+                char first = prop.getString().charAt(0);
+                if (Character.isDigit(first) || prop.isQuotedString()) {
                   return true;
                 }
               }
@@ -735,7 +740,7 @@ public boolean apply(Node input) {
           }
         };
 
-    return NodeUtil.has(fnNode, hasParamWithNumberObjectLitPredicate,
+    return NodeUtil.has(fnNode, hasParamWithInvalidPropertyNameIdentifierPredicate,
         Predicates.<Node>alwaysTrue());
   }
 

diff --git a/test/com/google/javascript/jscomp/InlineFunctionsTest.java b/test/com/google/javascript/jscomp/InlineFunctionsTest.java
@@ -2638,6 +2638,14 @@ public void testRestObjectPattern5() {
         "[null,null,null,3,null].x");
   }
 
+  public void testRestObjectPattern6() {
+    testSame(
+        LINE_JOINER.join(
+            "function f(...{p: x, 3:y}) {",
+            "  return p;",
+            "}"));
+  }
+
   public void testObjectPatternParam1() {
     test(
         LINE_JOINER.join(
@@ -2795,6 +2803,71 @@ public void testObjectPatternParam13() {
         "x.b + x.c");
   }
 
+  public void testObjectPatternParam14() {
+    testSame(
+        LINE_JOINER.join(
+            "function f({3:x}) {",
+            "  return x;",
+            "}",
+            "f({3:1});"));
+  }
+
+  public void testObjectPatternParam15() {
+    testSame(
+        LINE_JOINER.join(
+            "function f({x:{3:y}}) {",
+            "  return y;",
+            "}",
+            "f({x:{3:1}});"));
+  }
+
+  public void testObjectPatternParam16() {
+    testSame(
+      LINE_JOINER.join(
+          "function f({p: x, 3: y}) {",
+          "  return x;",
+          "}"));
+  }
+
+  public void testObjectPatternParam17() {
+    test(
+      LINE_JOINER.join(
+          "function f({prop1, prop2}) {",
+          "  return prop1;",
+          "}",
+          "f({prop1:5, prop2:6});"),
+      LINE_JOINER.join(
+          "{var prop1$jscomp$inline_0={prop1:5,prop2:6}.prop1;",
+          "prop1$jscomp$inline_0}"));
+  }
+
+  public void testObjectPatternParam18() {
+   testSame(
+       LINE_JOINER.join(
+           "function f({'foo bar':x}) {",
+           "  return x;",
+           "}",
+           "f({'foo bar': 1});"));
+  }
+
+  public void testObjectPatternParam19() {
+    testSame(
+        LINE_JOINER.join(
+            "function f({'foo_bar':x}) {",
+            "  return x;",
+            "}",
+            "f({'foo_bar': 1});"));
+  }
+
+  public void testObjectPatternParam20() {
+    testSame(
+        LINE_JOINER.join(
+            "function f({'_foobar':x}) {",
+            "  return x;",
+            "}",
+            "f({'_foobar': 1});"));
+  }
+
   public void testDefaultObjectPatternParam1() {
     test(
         LINE_JOINER.join(
@@ -2842,6 +2915,14 @@ public void testDefaultObjectPatternParam4() {
             "b$jscomp$inline_1+c$jscomp$inline_2}"));
   }
 
+  public void testDefaultObjectPatternParam5() {
+    testSame(
+        LINE_JOINER.join(
+            "function f({p:x, 3:y} = {p:1, 3:2}) {",
+            " return x;",
+            "}"));
+  }
+
   public void testDefaultParam1() {
     test(
         LINE_JOINER.join(