Change JSType#toString to thread through a StringBuilder and construc…

…t the type string only at the end. Currently, types that contain nested types (PrototypeObjectType, UnionType, and TemplatizedType) call JSType#toString or its friends instead of appending to one StringBuilder. This can create unnecessarily many strings. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=144236240
google · Jan 12, 2017 · cee6a33 · cee6a33
1 parent b5615b1
commit cee6a33
Show file tree

Hide file tree

Showing 22 changed files with 135 additions and 151 deletions.
diff --git a/src/com/google/javascript/rhino/jstype/AllType.java b/src/com/google/javascript/rhino/jstype/AllType.java
@@ -81,8 +81,8 @@ public TernaryValue testForEquality(JSType that) {
   }
 
   @Override
-  String toStringHelper(boolean forAnnotations) {
-    return "*";
+  StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
+    return sb.append("*");
   }
 
   @Override

diff --git a/src/com/google/javascript/rhino/jstype/ArrowType.java b/src/com/google/javascript/rhino/jstype/ArrowType.java
@@ -287,8 +287,8 @@ boolean hasUnknownParamsOrReturn() {
   }
 
   @Override
-  String toStringHelper(boolean forAnnotations) {
-    return "[ArrowType]";
+  StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
+    return sb.append("[ArrowType]");
   }
 
   @Override

diff --git a/src/com/google/javascript/rhino/jstype/BooleanType.java b/src/com/google/javascript/rhino/jstype/BooleanType.java
@@ -99,8 +99,8 @@ public JSType autoboxesTo() {
   }
 
   @Override
-  String toStringHelper(boolean forAnnotations) {
-    return getDisplayName();
+  StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
+    return sb.append(getDisplayName());
   }
 
   @Override

diff --git a/src/com/google/javascript/rhino/jstype/EnumElementType.java b/src/com/google/javascript/rhino/jstype/EnumElementType.java
@@ -146,10 +146,11 @@ public int hashCode() {
   }
 
   @Override
-  String toStringHelper(boolean forAnnotations) {
-    return forAnnotations ?
-        primitiveType.toString() :
-        (getReferenceName() + "<" + primitiveType + ">");
+  StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
+    if (forAnnotations) {
+      return sb.append(this.primitiveType);
+    }
+    return sb.append(getReferenceName()).append("<").append(this.primitiveType).append(">");
   }
 
   @Override

diff --git a/src/com/google/javascript/rhino/jstype/EnumType.java b/src/com/google/javascript/rhino/jstype/EnumType.java
@@ -44,7 +44,6 @@
 
 import com.google.javascript.rhino.ErrorReporter;
 import com.google.javascript.rhino.Node;
-
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
@@ -133,8 +132,8 @@ protected boolean isSubtype(JSType that,
   }
 
   @Override
-  String toStringHelper(boolean forAnnotations) {
-    return forAnnotations ? "Object" : getReferenceName();
+  StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
+    return sb.append(forAnnotations ? "Object" : getReferenceName());
   }
 
   @Override

diff --git a/src/com/google/javascript/rhino/jstype/FunctionType.java b/src/com/google/javascript/rhino/jstype/FunctionType.java
@@ -1008,79 +1008,77 @@ public boolean hasEqualCallType(FunctionType otherType) {
    * {@code this:T} if the function expects a known type for {@code this}.
    */
   @Override
-  String toStringHelper(boolean forAnnotations) {
+  StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
     if (!isPrettyPrint() ||
         this == registry.getNativeType(JSTypeNative.FUNCTION_INSTANCE_TYPE)) {
-      return "Function";
+      return sb.append("Function");
     }
 
     setPrettyPrint(false);
 
-    StringBuilder b = new StringBuilder(32);
-    b.append("function (");
+    sb.append("function (");
     int paramNum = call.parameters.getChildCount();
     boolean hasKnownTypeOfThis = !(typeOfThis instanceof UnknownType);
     if (hasKnownTypeOfThis) {
       if (isConstructor()) {
-        b.append("new:");
+        sb.append("new:");
       } else {
-        b.append("this:");
+        sb.append("this:");
       }
-      b.append(typeOfThis.toStringHelper(forAnnotations));
+      typeOfThis.appendTo(sb, forAnnotations);
     }
     if (paramNum > 0) {
       if (hasKnownTypeOfThis) {
-        b.append(", ");
+        sb.append(", ");
       }
       Node p = call.parameters.getFirstChild();
-      appendArgString(b, p, forAnnotations);
+      appendArgString(sb, p, forAnnotations);
 
       p = p.getNext();
       while (p != null) {
-        b.append(", ");
-        appendArgString(b, p, forAnnotations);
+        sb.append(", ");
+        appendArgString(sb, p, forAnnotations);
         p = p.getNext();
       }
     }
-    b.append("): ");
-    b.append(call.returnType.toNonNullString(forAnnotations));
+    sb.append("): ");
+    call.returnType.appendAsNonNull(sb, forAnnotations);
 
     setPrettyPrint(true);
-    return b.toString();
+    return sb;
   }
 
-  private void appendArgString(
-      StringBuilder b, Node p, boolean forAnnotations) {
+  private void appendArgString(StringBuilder sb, Node p, boolean forAnnotations) {
     if (p.isVarArgs()) {
-      appendVarArgsString(b, p.getJSType(), forAnnotations);
+      appendVarArgsString(sb, p.getJSType(), forAnnotations);
     } else if (p.isOptionalArg()) {
-      appendOptionalArgString(b, p.getJSType(), forAnnotations);
+      appendOptionalArgString(sb, p.getJSType(), forAnnotations);
     } else {
-      b.append(p.getJSType().toNonNullString(forAnnotations));
+      p.getJSType().appendAsNonNull(sb, forAnnotations);
     }
   }
 
   /** Gets the string representation of a var args param. */
-  private void appendVarArgsString(StringBuilder builder, JSType paramType,
-      boolean forAnnotations) {
+  private void appendVarArgsString(
+      StringBuilder sb, JSType paramType, boolean forAnnotations) {
     if (paramType.isUnionType()) {
       // Remove the optionality from the var arg.
       paramType = paramType.toMaybeUnionType().getRestrictedUnion(
           registry.getNativeType(JSTypeNative.VOID_TYPE));
     }
-    builder.append("...").append(
-        paramType.toNonNullString(forAnnotations));
+    sb.append("...");
+    paramType.appendAsNonNull(sb, forAnnotations);
   }
 
   /** Gets the string representation of an optional param. */
   private void appendOptionalArgString(
-      StringBuilder builder, JSType paramType, boolean forAnnotations) {
+      StringBuilder sb, JSType paramType, boolean forAnnotations) {
     if (paramType.isUnionType()) {
       // Remove the optionality from the var arg.
       paramType = paramType.toMaybeUnionType().getRestrictedUnion(
           registry.getNativeType(JSTypeNative.VOID_TYPE));
     }
-    builder.append(paramType.toNonNullString(forAnnotations)).append("=");
+    paramType.appendAsNonNull(sb, forAnnotations).append("=");
   }
 
   /**

diff --git a/src/com/google/javascript/rhino/jstype/InstanceObjectType.java b/src/com/google/javascript/rhino/jstype/InstanceObjectType.java
@@ -93,12 +93,10 @@ boolean defineProperty(String name, JSType type, boolean inferred,
   }
 
   @Override
-  String toStringHelper(boolean forAnnotations) {
-    if (constructor.hasReferenceName()) {
-      return constructor.getReferenceName();
-    } else {
-      return super.toStringHelper(forAnnotations);
-    }
+  StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
+    return constructor.hasReferenceName()
+        ? sb.append(constructor.getReferenceName())
+            : super.appendTo(sb, forAnnotations);
   }
 
   @Override

diff --git a/src/com/google/javascript/rhino/jstype/JSType.java b/src/com/google/javascript/rhino/jstype/JSType.java
@@ -1578,7 +1578,7 @@ public TypePair(JSType typeA, JSType typeB) {
    */
   @Override
   public String toString() {
-    return toStringHelper(false);
+    return appendTo(new StringBuilder(), false).toString();
   }
 
   /**
@@ -1592,29 +1592,27 @@ public String toDebugHashCodeString() {
   /**
    * A string representation of this type, suitable for printing
    * in type annotations at code generation time.
+   *
+   * Don't call from this package; use appendAsNonNull instead.
    */
   public final String toAnnotationString() {
-    return toStringHelper(true);
+    return appendTo(new StringBuilder(), true).toString();
   }
 
-  public final String toNonNullString(boolean forAnnotations) {
-    if (forAnnotations) {
-      return toNonNullAnnotationString();
-    } else {
-      return toStringHelper(false);
-    }
+  // Don't call from this package; use appendAsNonNull instead.
+  public final String toNonNullAnnotationString() {
+    return appendAsNonNull(new StringBuilder(), true).toString();
   }
 
-  public final String toNonNullAnnotationString() {
-    return !isUnknownType() && !isTemplateType() && !isRecordType() && !isFunctionType()
-        && isObject() ? "!" + toAnnotationString() : toAnnotationString();
+  final StringBuilder appendAsNonNull(StringBuilder sb, boolean forAnnotations) {
+    if (forAnnotations && isObject()
+        && !isUnknownType() && !isTemplateType() && !isRecordType() && !isFunctionType()) {
+      sb.append("!");
+    }
+    return appendTo(sb, forAnnotations);
   }
 
-  /**
-   * @param forAnnotations Whether this is for use in code generator
-   *     annotations. Otherwise, it's for warnings.
-   */
-  abstract String toStringHelper(boolean forAnnotations);
+  abstract StringBuilder appendTo(StringBuilder sb, boolean forAnnotations);
 
   /**
    * Modify this type so that it matches the specified type.

diff --git a/src/com/google/javascript/rhino/jstype/NamedType.java b/src/com/google/javascript/rhino/jstype/NamedType.java
@@ -43,7 +43,6 @@
 import com.google.common.base.Predicate;
 import com.google.javascript.rhino.ErrorReporter;
 import com.google.javascript.rhino.Node;
-
 import java.util.ArrayList;
 import java.util.List;
 
@@ -153,8 +152,8 @@ public String getReferenceName() {
   }
 
   @Override
-  String toStringHelper(boolean forAnnotations) {
-    return reference;
+  StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
+    return sb.append(this.reference);
   }
 
   @Override

diff --git a/src/com/google/javascript/rhino/jstype/NoObjectType.java b/src/com/google/javascript/rhino/jstype/NoObjectType.java
@@ -162,8 +162,8 @@ public <T> T visit(Visitor<T> visitor) {
   }
 
   @Override
-  String toStringHelper(boolean forAnnotations) {
-    return forAnnotations ? "?" : "NoObject";
+  StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
+    return sb.append(forAnnotations ? "?" : "NoObject");
   }
 
   @Override

diff --git a/src/com/google/javascript/rhino/jstype/NoResolvedType.java b/src/com/google/javascript/rhino/jstype/NoResolvedType.java
@@ -89,7 +89,7 @@ protected boolean isSubtype(JSType that,
   }
 
   @Override
-  String toStringHelper(boolean forAnnotations) {
-    return forAnnotations ? "?" : "NoResolvedType";
+  StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
+    return sb.append(forAnnotations ? "?" : "NoResolvedType");
   }
 }
diff --git a/src/com/google/javascript/rhino/jstype/NoType.java b/src/com/google/javascript/rhino/jstype/NoType.java
@@ -122,7 +122,7 @@ public <T> T visit(Visitor<T> visitor) {
   }
 
   @Override
-  String toStringHelper(boolean forAnnotations) {
-    return forAnnotations ? "?" : "None";
+  StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
+    return sb.append(forAnnotations ? "?" : "None");
   }
 }
diff --git a/src/com/google/javascript/rhino/jstype/NullType.java b/src/com/google/javascript/rhino/jstype/NullType.java
@@ -100,8 +100,8 @@ public TernaryValue testForEquality(JSType that) {
   }
 
   @Override
-  String toStringHelper(boolean forAnnotations) {
-    return getDisplayName();
+  StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
+    return sb.append(getDisplayName());
   }
 
   @Override

diff --git a/src/com/google/javascript/rhino/jstype/NumberType.java b/src/com/google/javascript/rhino/jstype/NumberType.java
@@ -93,8 +93,8 @@ public boolean matchesObjectContext() {
   }
 
   @Override
-  String toStringHelper(boolean forAnnotations) {
-    return getDisplayName();
+  StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
+    return sb.append(getDisplayName());
   }
 
   @Override