Fold cases like foo.substring(i - 1, i) to foo.charAt(i - 1) in J2clR…

…eplaceKnownMethodsPass There's some usage of this syntax. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=139516879
google · Nov 18, 2016 · c215a7d · c215a7d
1 parent 86bb55e
commit c215a7d
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 5 deletions.
diff --git a/src/com/google/javascript/jscomp/J2clReplaceKnownMethodsPass.java b/src/com/google/javascript/jscomp/J2clReplaceKnownMethodsPass.java
@@ -69,17 +69,29 @@ private void tryReplaceSubstringOrSliceWithCharAtForNameNodes(Node n) {
     Node callTarget = n.getFirstChild();
     Node firstArg = callTarget.getNext();
     Node secondArg = firstArg.getNext();
+    if (firstArg.getJSType() == null || !firstArg.getJSType().isNumberValueType()
+        || secondArg.getJSType() == null || !secondArg.getJSType().isNumberValueType()) {
+      return;
+    }
     if (firstArg.isName() && secondArg.isAdd()
         && secondArg.getFirstChild().isName()
         && secondArg.getFirstChild().getString().equals(firstArg.getString())
-        && firstArg.getJSType() != null && firstArg.getJSType().isNumberValueType()) {
-      Double maybeOne = NodeUtil.getNumberValue(secondArg.getSecondChild());
-      if (maybeOne != null && maybeOne.intValue() == 1) { // substring(i, i + 1)
-        replaceWithCharAt(callTarget, firstArg);
-      }
+        && isOne(secondArg.getSecondChild())) {
+      // substring(i, i + 1)
+      replaceWithCharAt(callTarget, firstArg);
+    } else if (firstArg.isSub() && secondArg.isName()
+        && firstArg.getFirstChild().isName()
+        && firstArg.getFirstChild().getString().equals(secondArg.getString())
+        && isOne(firstArg.getSecondChild())) {
+      // substring(i - 1, i)
+      replaceWithCharAt(callTarget, firstArg);
     }
   }
 
+  private boolean isOne(Node n) {
+    return n.isNumber() && n.getDouble() == 1.0;
+  }
+
   private void replaceWithCharAt(Node callTarget, Node firstArg) {
     // TODO(moz): Maybe correct the arity of the function type here.
     callTarget.getLastChild().setString("charAt");

diff --git a/src/com/google/javascript/rhino/Node.java b/src/com/google/javascript/rhino/Node.java
@@ -2829,6 +2829,10 @@ public boolean isAdd() {
     return this.token == Token.ADD;
   }
 
+  public boolean isSub() {
+    return this.token == Token.SUB;
+  }
+
   public boolean isAnd() {
     return this.token == Token.AND;
   }

diff --git a/test/com/google/javascript/jscomp/J2clReplaceKnownMethodsPassTest.java b/test/com/google/javascript/jscomp/J2clReplaceKnownMethodsPassTest.java
@@ -47,6 +47,8 @@ public CompilerPass getProcessor(final Compiler compiler) {
   public void testReplaceWithCharAt() {
     foldStringTyped(
         "var /** number */ i; a.substring(i, i + 1)", "var /** number */ i; a.charAt(i)");
+    foldStringTyped(
+        "var /** number */ i; a.substring(i - 1, i)", "var /** number */ i; a.charAt(i - 1)");
     foldStringTyped(
         "var /** number */ i; ''.substring(i, i + 1)", "var /** number */ i; ''.charAt(i)");
     foldSameStringTyped("var /** number */ i; a.substring(i, 2 + 1)");