Fold String.prototype.slice() for string literals and string-typed va…

…riables

Also bail out if start > end. The compiler was crashing in these cases.

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

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

@@ -115,7 +115,8 @@ private Node tryFoldKnownStringMethods(Node subtree, Node callTarget) {
             case "substr":
               return tryFoldStringSubstr(subtree, stringNode, firstArg);
             case "substring":
-              return tryFoldStringSubstring(subtree, stringNode, firstArg);
+            case "slice":
+              return tryFoldStringSubstringOrSlice(subtree, stringNode, firstArg);
             case "charAt":
               return tryFoldStringCharAt(subtree, stringNode, firstArg);
             case "charCodeAt":
@@ -129,7 +130,8 @@ private Node tryFoldKnownStringMethods(Node subtree, Node callTarget) {
         case "substr":
           return tryReplaceSubstrWithCharAt(subtree, callTarget, firstArg);
         case "substring":
-          return tryReplaceSubstringWithCharAt(subtree, callTarget, firstArg);
+        case "slice":
+          return tryReplaceSubstringOrSliceWithCharAt(subtree, callTarget, firstArg);
       }
     }
     return subtree;
@@ -558,9 +560,9 @@ private Node tryFoldStringSubstr(Node n, Node stringNode, Node arg1) {
   }
 
   /**
-   * Try to fold .substring() calls on strings
+   * Try to fold .substring() or .slice() calls on strings
    */
-  private Node tryFoldStringSubstring(Node n, Node stringNode, Node arg1) {
+  private Node tryFoldStringSubstringOrSlice(Node n, Node stringNode, Node arg1) {
     Preconditions.checkArgument(n.isCall());
     Preconditions.checkArgument(stringNode.isString());
     Preconditions.checkArgument(arg1 != null);
@@ -598,10 +600,9 @@ private Node tryFoldStringSubstring(Node n, Node stringNode, Node arg1) {
     // specify the behavior in some of these cases, but we haven't
     // done a thorough investigation that it is correctly implemented
     // in all browsers.
-    if ((end > stringAsString.length()) ||
-        (start > stringAsString.length()) ||
-        (end < 0) ||
-        (start < 0)) {
+    if ((end > stringAsString.length()) || (start > stringAsString.length())
+        || (start < 0) || (end < 0)
+        || (start > end)) {
       return n;
     }
 
@@ -627,7 +628,7 @@ private Node tryReplaceSubstrWithCharAt(Node n, Node callTarget, Node firstArg)
     return n;
   }
 
-  private Node tryReplaceSubstringWithCharAt(Node n, Node callTarget, Node firstArg) {
+  private Node tryReplaceSubstringOrSliceWithCharAt(Node n, Node callTarget, Node firstArg) {
     if (n.getChildCount() != 3) {
       return n;
     }
@@ -642,18 +643,19 @@ private Node tryReplaceSubstringWithCharAt(Node n, Node callTarget, Node firstAr
     }
     int start = maybeStart.intValue();
     int end = maybeEnd.intValue();
-    if (Math.abs(start - end) == 1) {
-      return replaceWithCharAt(n, callTarget, start < end ? firstArg : secondArg);
+    if (start >= end) {
+      return n; // Bail out for simplicity
+    }
+    if (end - start == 1) {
+      return replaceWithCharAt(n, callTarget, firstArg);
     }
     return n;
   }
 
   private Node replaceWithCharAt(Node n, Node callTarget, Node firstArg) {
     // TODO(moz): Maybe correct the arity of the function type here.
-    n.detachChildren();
     callTarget.getLastChild().setString("charAt");
-    n.addChildToFront(callTarget);
-    n.addChildToBack(firstArg);
+    firstArg.getNext().detach();
     reportCodeChange();
     return n;
   }

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

@@ -152,13 +152,28 @@ public void testFoldStringSubstring() {
     fold("x = 'abcde'['substring'](1,3)", "x = 'bc'");
     fold("x = 'abcde'.substring(2)", "x = 'cde'");
 
-    // we should be leaving negative indexes alone for now
+    // we should be leaving negative, out-of-bound, and inverted indices alone for now
     foldSame("x = 'abcde'.substring(-1)");
     foldSame("x = 'abcde'.substring(1, -2)");
     foldSame("x = 'abcde'.substring(1, 2, 3)");
+    foldSame("x = 'abcde'.substring(2, 0)");
     foldSame("x = 'a'.substring(0, 2)");
   }
 
+  public void testFoldStringSlice() {
+    fold("x = 'abcde'.slice(0,2)", "x = 'ab'");
+    fold("x = 'abcde'.slice(1,2)", "x = 'b'");
+    fold("x = 'abcde'['slice'](1,3)", "x = 'bc'");
+    fold("x = 'abcde'.slice(2)", "x = 'cde'");
+
+    // we should be leaving negative, out-of-bound, and inverted indices alone for now
+    foldSame("x = 'abcde'.slice(-1)");
+    foldSame("x = 'abcde'.slice(1, -2)");
+    foldSame("x = 'abcde'.slice(1, 2, 3)");
+    foldSame("x = 'abcde'.slice(2, 0)");
+    foldSame("x = 'a'.slice(0, 2)");
+  }
+
   public void testFoldStringCharAt() {
     fold("x = 'abcde'.charAt(0)", "x = 'a'");
     fold("x = 'abcde'.charAt(1)", "x = 'b'");
@@ -310,19 +325,28 @@ public void testFoldParseOctalNumbers() {
     fold("x = parseInt(021, 8)", "x = 15");
   }
 
-  // TODO(moz): Fold String.prototype.slice()
   public void testReplaceWithCharAt() {
     enableTypeCheck();
     foldStringTyped("a.substring(0, 1)", "a.charAt(0)");
-    foldStringTyped("a.substring(2, 1)", "a.charAt(1)");
     foldSameStringTyped("a.substring(1, 2, 3)");
     foldSameStringTyped("a.substring(i, i + 1)"); // TODO(moz): Fold cases like these
     foldSameStringTyped("a.substring()");
     foldSameStringTyped("a.substring(1)");
     foldSameStringTyped("a.substring(1, 3, 4)");
     foldSameStringTyped("a.substring(-1, 3)");
+    foldSameStringTyped("a.substring(2, 1)");
     foldSameStringTyped("a.substring(3, 1)");
 
+    foldStringTyped("a.slice(0, 1)", "a.charAt(0)");
+    foldSameStringTyped("a.slice(1, 2, 3)");
+    foldSameStringTyped("a.slice(i, i + 1)"); // TODO(moz): Fold cases like these
+    foldSameStringTyped("a.slice()");
+    foldSameStringTyped("a.slice(1)");
+    foldSameStringTyped("a.slice(1, 3, 4)");
+    foldSameStringTyped("a.slice(-1, 3)");
+    foldSameStringTyped("a.slice(2, 1)");
+    foldSameStringTyped("a.slice(3, 1)");
+
     foldStringTyped("a.substr(0, 1)", "a.charAt(0)");
     foldStringTyped("a.substr(2, 1)", "a.charAt(2)");
     foldStringTyped("a.substr(-2, 1)", "a.charAt(-2)");