[NTI] Add a ToStringContext for adapting how JSTypes are stringified.

This addresses two problems at once: 1. Issue #2499 where type variables are ambiguated, making warnings difficult to understand 2. Migrating `TypedCodeGenerator` to NTI requires printing types in a "for annotations" mode Rather than add various booleans, the context object allows plumbing any configuration all the way through the recursive stringify operation, and is extensible if further configuration is needed later. Moreover, context instances may be passed to multiple stringify operations (i.e. to ensure typevars are printed consistently throughout an entire message). ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=156936000
google · May 25, 2017 · 269e13b · 269e13b
1 parent 1623666
commit 269e13b
Show file tree

Hide file tree

Showing 7 changed files with 101 additions and 38 deletions.
diff --git a/src/com/google/javascript/jscomp/newtypes/EnumType.java b/src/com/google/javascript/jscomp/newtypes/EnumType.java
@@ -235,4 +235,10 @@ static boolean areSubtypes(JSType t1, JSType t2, SubtypeCache subSuperMap) {
   public Collection<JSType> getSubtypesWithProperty(QualifiedName qname) {
     return declaredType.getSubtypesWithProperty(qname);
   }
+
+  public String toString(ToStringContext ctx) {
+    return ctx.forAnnotation()
+        ? this.declaredType.appendTo(new StringBuilder(), ctx).toString()
+        : this.toString();
+  }
 }
diff --git a/src/com/google/javascript/jscomp/newtypes/FunctionType.java b/src/com/google/javascript/jscomp/newtypes/FunctionType.java
@@ -1428,36 +1428,37 @@ public int hashCode() {
 
   @Override
   public String toString() {
-    return appendTo(new StringBuilder()).toString();
+    return appendTo(new StringBuilder(), ToStringContext.TO_STRING).toString();
   }
 
   /**
    * Returns a transformed collection of type parameter names, mapped back
    * to original (pre-uniquified) names.
    */
-  private static Collection<String> getPrettyTypeParams(List<String> typeParams) {
+  private static Collection<String> getPrettyTypeParams(
+      List<String> typeParams, final ToStringContext ctx) {
     return Collections2.transform(
         typeParams,
         new Function<String, String>() {
           @Override
           public String apply(String typeParam) {
-            return UniqueNameGenerator.getOriginalName(typeParam);
+            return ctx.formatTypeVar(typeParam);
           }
         });
   }
 
   @SuppressWarnings("ReferenceEquality")
-  public StringBuilder appendTo(StringBuilder builder) {
+  public StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) {
     if (this == this.commonTypes.LOOSE_TOP_FUNCTION) {
       return builder.append("LOOSE_TOP_FUNCTION");
     } else if (this == this.commonTypes.TOP_FUNCTION) {
       return builder.append("TOP_FUNCTION");
     } else if (isQmarkFunction()) {
-      return builder.append("Function");
+      return builder.append(ctx.forAnnotation() ? "!" : "").append("Function");
     }
     if (!this.typeParameters.isEmpty()) {
       builder.append("<");
-      builder.append(Joiner.on(",").join(getPrettyTypeParams(this.typeParameters)));
+      builder.append(Joiner.on(",").join(getPrettyTypeParams(this.typeParameters, ctx)));
       builder.append(">");
     }
     builder.append("function(");
@@ -1471,16 +1472,16 @@ public StringBuilder appendTo(StringBuilder builder) {
       builder.append(',');
     }
     for (int i = 0; i < requiredFormals.size(); ++i) {
-      requiredFormals.get(i).appendTo(builder);
+      requiredFormals.get(i).appendTo(builder, ctx);
       builder.append(',');
     }
     for (int i = 0; i < optionalFormals.size(); ++i) {
-      optionalFormals.get(i).appendTo(builder);
+      optionalFormals.get(i).appendTo(builder, ctx);
       builder.append("=,");
     }
     if (restFormals != null) {
       builder.append("...");
-      restFormals.appendTo(builder);
+      restFormals.appendTo(builder, ctx);
     }
     // Delete the trailing comma, if present
     if (builder.charAt(builder.length() - 1) == ',') {
@@ -1489,7 +1490,7 @@ public StringBuilder appendTo(StringBuilder builder) {
     builder.append(')');
     if (returnType != null) {
       builder.append(':');
-      returnType.appendTo(builder);
+      returnType.appendTo(builder, ctx);
     }
     if (isLoose()) {
       builder.append(" (loose)");
@@ -1503,7 +1504,7 @@ public StringBuilder appendTo(StringBuilder builder) {
         }
         builder.append(entry.getKey());
         builder.append('=');
-        entry.getValue().appendTo(builder);
+        entry.getValue().appendTo(builder, ctx);
       }
       builder.append('}');
     }

diff --git a/src/com/google/javascript/jscomp/newtypes/JSType.java b/src/com/google/javascript/jscomp/newtypes/JSType.java
@@ -1515,17 +1515,19 @@ public final String toString() {
     if (mockToString) {
       return "";
     }
-    return appendTo(new StringBuilder()).toString();
+    return appendTo(new StringBuilder(), ToStringContext.TO_STRING).toString();
   }
 
-  public final StringBuilder appendTo(StringBuilder builder) {
-    return typeToString(builder);
-  }
-
-  /** For use in {@link #typeToString} */
+  /** For use in {@link #appendTo} */
   private static final Joiner PIPE_JOINER = Joiner.on("|");
 
-  private StringBuilder typeToString(StringBuilder builder) {
+  /**
+   * Appends this type to the `builder`. If `forAnnotations` is true, then
+   * the type will be in a format suitable for type annotations during
+   * code generation.
+   */
+  StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) {
+    // TODO(sdh): Add checkState(!forAnnotations) calls in places where annotations are nonsense.
     switch (getMask()) {
       case BOTTOM_MASK:
         return builder.append("bottom");
@@ -1570,16 +1572,16 @@ private StringBuilder typeToString(StringBuilder builder) {
                 tags &= ~UNDEFINED_MASK;
                 continue;
               case TYPEVAR_MASK:
-                builder.append(UniqueNameGenerator.getOriginalName(getTypeVar()));
+                builder.append(ctx.formatTypeVar(getTypeVar()));
                 tags &= ~TYPEVAR_MASK;
                 continue;
               case NON_SCALAR_MASK: {
                 if (getObjs().size() == 1) {
-                  Iterables.getOnlyElement(getObjs()).appendTo(builder);
+                  Iterables.getOnlyElement(getObjs()).appendTo(builder, ctx);
                 } else {
                   Set<String> strReps = new TreeSet<>();
                   for (ObjectType obj : getObjs()) {
-                    strReps.add(obj.toString());
+                    strReps.add(obj.toString(ctx));
                   }
                   PIPE_JOINER.appendTo(builder, strReps);
                 }
@@ -1588,11 +1590,11 @@ private StringBuilder typeToString(StringBuilder builder) {
               }
               case ENUM_MASK: {
                 if (getEnums().size() == 1) {
-                  builder.append(Iterables.getOnlyElement(getEnums()));
+                  builder.append(Iterables.getOnlyElement(getEnums()).toString(ctx));
                 } else {
                   Set<String> strReps = new TreeSet<>();
                   for (EnumType e : getEnums()) {
-                    strReps.add(e.toString());
+                    strReps.add(e.toString(ctx));
                   }
                   PIPE_JOINER.appendTo(builder, strReps);
                 }

diff --git a/src/com/google/javascript/jscomp/newtypes/NominalType.java b/src/com/google/javascript/jscomp/newtypes/NominalType.java
@@ -676,10 +676,13 @@ static boolean equalRawTypes(NominalType n1, NominalType n2) {
 
   @Override
   public String toString() {
-    return appendTo(new StringBuilder()).toString();
+    return appendTo(new StringBuilder(), ToStringContext.TO_STRING).toString();
   }
 
-  StringBuilder appendTo(StringBuilder builder) {
+  StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) {
+    if (ctx.forAnnotation()) {
+      builder.append("!");
+    }
     if (this.typeMap.isEmpty()) {
       return this.rawType.appendTo(builder);
     }
@@ -695,7 +698,7 @@ StringBuilder appendTo(StringBuilder builder) {
         builder.append(',');
       }
       JSType concrete = this.typeMap.get(typeParam);
-      Preconditions.checkNotNull(concrete).appendTo(builder);
+      Preconditions.checkNotNull(concrete).appendTo(builder, ctx);
     }
     builder.append('>');
     return builder;

diff --git a/src/com/google/javascript/jscomp/newtypes/ObjectType.java b/src/com/google/javascript/jscomp/newtypes/ObjectType.java
@@ -1382,23 +1382,27 @@ boolean isPropDefinedOnSubtype(QualifiedName pname) {
 
   @Override
   public String toString() {
-    return appendTo(new StringBuilder()).toString();
+    return appendTo(new StringBuilder(), ToStringContext.TO_STRING).toString();
   }
 
-  StringBuilder appendTo(StringBuilder builder) {
+  String toString(ToStringContext ctx) {
+    return appendTo(new StringBuilder(), ctx).toString();
+  }
+
+  StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) {
     if (isPrototypeObject()) {
       return builder.append(getOwnerFunction().getThisType()).append(".prototype");
     }
     if (!hasNonPrototypeProperties()) {
       if (fn != null) {
-        return fn.appendTo(builder);
+        return fn.appendTo(builder, ctx);
       }
-      return this.nominalType.appendTo(builder);
+      return this.nominalType.appendTo(builder, ctx);
     }
     if (!nominalType.getName().equals("Function")
         && !nominalType.getName().equals("Object")
         && !nominalType.getName().equals(JSTypes.OBJLIT_CLASS_NAME)) {
-      nominalType.appendTo(builder);
+      nominalType.appendTo(builder, ctx);
     } else if (isStruct()) {
       builder.append("struct");
     } else if (isDict()) {
@@ -1408,7 +1412,7 @@ StringBuilder appendTo(StringBuilder builder) {
     }
     if (this.fn != null) {
       builder.append("<|");
-      fn.appendTo(builder);
+      fn.appendTo(builder, ctx);
       builder.append("|>");
     }
     if (ns == null || !props.isEmpty()) {
@@ -1422,7 +1426,7 @@ StringBuilder appendTo(StringBuilder builder) {
         }
         builder.append(pname);
         builder.append(':');
-        props.get(pname).appendTo(builder);
+        props.get(pname).appendTo(builder, ctx);
       }
       builder.append('}');
     }

diff --git a/src/com/google/javascript/jscomp/newtypes/Property.java b/src/com/google/javascript/jscomp/newtypes/Property.java
@@ -216,17 +216,17 @@ Property substituteGenerics(Map<String, JSType> concreteTypes) {
 
   @Override
   public String toString() {
-    return appendTo(new StringBuilder()).toString();
+    return appendTo(new StringBuilder(), ToStringContext.TO_STRING).toString();
   }
 
-  public StringBuilder appendTo(StringBuilder builder) {
+  public StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) {
     switch (attribute) {
       case CONSTANT:
-        return inferredType.appendTo(builder).append('^');
+        return inferredType.appendTo(builder, ctx).append('^');
       case REQUIRED:
-        return inferredType.appendTo(builder);
+        return inferredType.appendTo(builder, ctx);
       case OPTIONAL:
-        return inferredType.appendTo(builder).append('=');
+        return inferredType.appendTo(builder, ctx).append('=');
       default:
         throw new RuntimeException("Unknown Attribute value " + attribute);
     }

diff --git a/src/com/google/javascript/jscomp/newtypes/ToStringContext.java b/src/com/google/javascript/jscomp/newtypes/ToStringContext.java
@@ -0,0 +1,47 @@
+/*
+ * Copyright 2017 The Closure Compiler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.javascript.jscomp.newtypes;
+
+/**
+ * Configuration options for {@link JSType#toString}.  This object is
+ * passed around to all internal types.
+ */
+public class ToStringContext {
+
+  /** The default context for toString(). */
+  static final ToStringContext TO_STRING = new ToStringContext();
+
+  /** The default context for toAnnotationString(). */
+  static final ToStringContext FOR_ANNOTATION =
+      new ToStringContext() {
+        @Override
+        boolean forAnnotation() {
+          return true;
+        }
+      };
+
+  // Nobody else should be making instances.
+  private ToStringContext() {}
+
+  String formatTypeVar(String typeVar) {
+    return UniqueNameGenerator.getOriginalName(typeVar);
+  }
+
+  boolean forAnnotation() {
+    return false;
+  }
+}