diff --git a/src/com/google/javascript/jscomp/TypeValidator.java b/src/com/google/javascript/jscomp/TypeValidator.java index 5441cd5dca8..bf78d5a28ce 100644 --- a/src/com/google/javascript/jscomp/TypeValidator.java +++ b/src/com/google/javascript/jscomp/TypeValidator.java @@ -34,6 +34,7 @@ import com.google.javascript.jscomp.parsing.parser.util.format.SimpleFormat; import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.Node; +import com.google.javascript.rhino.TypeI.Nullability; import com.google.javascript.rhino.jstype.FunctionType; import com.google.javascript.rhino.jstype.JSType; import com.google.javascript.rhino.jstype.JSType.SubtypingMode; @@ -836,8 +837,8 @@ private static String formatFoundRequired( String foundStr = found.toString(); String requiredStr = required.toString(); if (foundStr.equals(requiredStr)) { - foundStr = found.toAnnotationString(); - requiredStr = required.toAnnotationString(); + foundStr = found.toAnnotationString(Nullability.IMPLICIT); + requiredStr = required.toAnnotationString(Nullability.IMPLICIT); } String missingStr = ""; String mismatchStr = ""; diff --git a/src/com/google/javascript/jscomp/TypedCodeGenerator.java b/src/com/google/javascript/jscomp/TypedCodeGenerator.java index 89ada12993e..4a8c9c5c7fa 100644 --- a/src/com/google/javascript/jscomp/TypedCodeGenerator.java +++ b/src/com/google/javascript/jscomp/TypedCodeGenerator.java @@ -26,6 +26,7 @@ import com.google.javascript.rhino.Node; import com.google.javascript.rhino.ObjectTypeI; import com.google.javascript.rhino.TypeI; +import com.google.javascript.rhino.TypeI.Nullability; import com.google.javascript.rhino.TypeIRegistry; import com.google.javascript.rhino.jstype.JSTypeNative; import java.util.Collection; @@ -91,13 +92,15 @@ private String getTypeAnnotation(Node node) { return getFunctionAnnotation(node); } else if (type.isEnumObject()) { return "/** @enum {" - + type.toMaybeObjectType().getEnumeratedTypeOfEnumObject().toNonNullAnnotationString() + + type.toMaybeObjectType() + .getEnumeratedTypeOfEnumObject() + .toAnnotationString(Nullability.EXPLICIT) + "} */\n"; } else if (!type.isUnknownType() && !type.isBottom() && !type.isVoidType() && !type.isPrototypeObject()) { - return "/** @type {" + node.getTypeI().toNonNullAnnotationString() + "} */\n"; + return "/** @type {" + node.getTypeI().toAnnotationString(Nullability.EXPLICIT) + "} */\n"; } else { return ""; } @@ -153,7 +156,7 @@ private String getFunctionAnnotation(Node fnNode) { && !funType.isInterface() // Interfaces never return a value. && !(funType.isConstructor() && retType.isVoidType())) { sb.append(" * "); - appendAnnotation(sb, "return", retType.toNonNullAnnotationString()); + appendAnnotation(sb, "return", retType.toAnnotationString(Nullability.EXPLICIT)); sb.append("\n"); } @@ -166,7 +169,7 @@ private String getFunctionAnnotation(Node fnNode) { if (thisType != null && !thisType.isUnknownType() && !thisType.isVoidType()) { if (fnNode == null || !thisType.equals(findMethodOwner(fnNode))) { sb.append(" * "); - appendAnnotation(sb, "this", thisType.toNonNullAnnotationString()); + appendAnnotation(sb, "this", thisType.toAnnotationString(Nullability.EXPLICIT)); sb.append("\n"); } } @@ -191,14 +194,14 @@ private void appendConstructorAnnotations(StringBuilder sb, FunctionTypeI funTyp ObjectTypeI superInstance = superConstructor.getInstanceType(); if (!superInstance.toString().equals("Object")) { sb.append(" * "); - appendAnnotation(sb, "extends", superInstance.toAnnotationString()); + appendAnnotation(sb, "extends", superInstance.toAnnotationString(Nullability.IMPLICIT)); sb.append("\n"); } } // Avoid duplicates, add implemented type to a set first Set interfaces = new TreeSet<>(); for (ObjectTypeI interfaze : funType.getAncestorInterfaces()) { - interfaces.add(interfaze.toAnnotationString()); + interfaces.add(interfaze.toAnnotationString(Nullability.IMPLICIT)); } for (String interfaze : interfaces) { sb.append(" * "); @@ -211,7 +214,7 @@ private void appendConstructorAnnotations(StringBuilder sb, FunctionTypeI funTyp private void appendInterfaceAnnotations(StringBuilder sb, FunctionTypeI funType) { Set interfaces = new TreeSet<>(); for (ObjectTypeI interfaceType : funType.getAncestorInterfaces()) { - interfaces.add(interfaceType.toAnnotationString()); + interfaces.add(interfaceType.toAnnotationString(Nullability.IMPLICIT)); } for (String interfaze : interfaces) { sb.append(" * "); @@ -258,13 +261,13 @@ private static void appendAnnotation(StringBuilder sb, String name, String type) private String getParameterJSDocType(List types, int index, int minArgs, int maxArgs) { TypeI type = types.get(index); if (index < minArgs) { - return type.toNonNullAnnotationString(); + return type.toAnnotationString(Nullability.EXPLICIT); } boolean isRestArgument = maxArgs == Integer.MAX_VALUE && index == types.size() - 1; if (isRestArgument) { - return "..." + restrictByUndefined(type).toNonNullAnnotationString(); + return "..." + restrictByUndefined(type).toAnnotationString(Nullability.EXPLICIT); } - return restrictByUndefined(type).toNonNullAnnotationString() + "="; + return restrictByUndefined(type).toAnnotationString(Nullability.EXPLICIT) + "="; } /** Removes undefined from a union type. */ diff --git a/src/com/google/javascript/jscomp/newtypes/JSType.java b/src/com/google/javascript/jscomp/newtypes/JSType.java index 567808d2476..59989beba67 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSType.java +++ b/src/com/google/javascript/jscomp/newtypes/JSType.java @@ -1702,14 +1702,9 @@ StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) { } @Override - public final String toNonNullAnnotationString() { - return appendTo(new StringBuilder(), ToStringContext.FOR_ANNOTATION).toString(); - } - - @Override - public final String toAnnotationString() { - String s = toNonNullAnnotationString(); - return s.startsWith("!") ? s.substring(1) : s; + public final String toAnnotationString(Nullability nullability) { + String result = appendTo(new StringBuilder(), ToStringContext.FOR_ANNOTATION).toString(); + return nullability == Nullability.IMPLICIT ? result.replaceAll("^!", "") : result; } @Override diff --git a/src/com/google/javascript/rhino/TypeI.java b/src/com/google/javascript/rhino/TypeI.java index adb655bc222..4702155c054 100644 --- a/src/com/google/javascript/rhino/TypeI.java +++ b/src/com/google/javascript/rhino/TypeI.java @@ -204,23 +204,29 @@ public interface TypeI extends Serializable { */ Collection getTypeParameters(); - // TODO(sdh): Replace calls of toAnnotationString with toNonNullAnnotationString and - // then substring off any leading '!' if necessary. Then delete toAnnotationString - // and consider renaming toNonNullAnnotationString as simply toAnnotationString. /** * Returns a string representation of this type, suitable for printing - * in type annotations at code generation time. In particular, explicit - * non-null modifiers will be added to implicitly nullable types (except - * the outermost type, which is expected to be a reference type). - */ - String toAnnotationString(); - - /** - * Returns a string representation of this type, suitable for printing - * in type annotations at code generation time. In particular, explicit - * non-null modifiers will be added to implicitly nullable types. - */ - String toNonNullAnnotationString(); + * in type annotations at code generation time. + */ + String toAnnotationString(Nullability nullability); + + /** + * Specifies how to express nullability of reference types in annotation strings and error + * messages. Note that this only applies to the outer-most type. Nullability of generic type + * arguments is always explicit. + */ + enum Nullability { + /** + * Include an explicit '!' for non-nullable reference types. This is suitable for use + * in most type contexts (particularly 'type', 'param', and 'return' annotations). + */ + EXPLICIT, + /** + * Omit the explicit '!' from the outermost non-nullable reference type. This is suitable for + * use in cases where a single reference type is expected (e.g. 'extends' and 'implements'). + */ + IMPLICIT, + } /** * Returns the type inference of this object. Useful for debugging. diff --git a/src/com/google/javascript/rhino/jstype/JSType.java b/src/com/google/javascript/rhino/jstype/JSType.java index d2f49d141af..5a047c2a29f 100644 --- a/src/com/google/javascript/rhino/jstype/JSType.java +++ b/src/com/google/javascript/rhino/jstype/JSType.java @@ -1672,21 +1672,12 @@ public String toDebugHashCodeString() { return "{" + hashCode() + "}"; } - /** - * 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. - */ - @Override - public final String toAnnotationString() { - return appendTo(new StringBuilder(), true).toString(); - } - // Don't call from this package; use appendAsNonNull instead. @Override - public final String toNonNullAnnotationString() { - return appendAsNonNull(new StringBuilder(), true).toString(); + public final String toAnnotationString(Nullability nullability) { + return nullability == Nullability.EXPLICIT + ? appendAsNonNull(new StringBuilder(), true).toString() + : appendTo(new StringBuilder(), true).toString(); } final StringBuilder appendAsNonNull(StringBuilder sb, boolean forAnnotations) { diff --git a/test/com/google/javascript/rhino/jstype/RecordTypeTest.java b/test/com/google/javascript/rhino/jstype/RecordTypeTest.java index c2108e75b6a..bb72956e8ae 100644 --- a/test/com/google/javascript/rhino/jstype/RecordTypeTest.java +++ b/test/com/google/javascript/rhino/jstype/RecordTypeTest.java @@ -38,6 +38,7 @@ package com.google.javascript.rhino.jstype; +import com.google.javascript.rhino.TypeI.Nullability; import com.google.javascript.rhino.testing.Asserts; import com.google.javascript.rhino.testing.BaseJSTypeTestCase; @@ -57,7 +58,7 @@ public void testRecursiveRecord() { assertEquals("{\n loop: {...},\n number: number,\n string: string\n}", record.toString()); assertEquals("{loop: ?, number: number, string: string}", - record.toAnnotationString()); + record.toAnnotationString(Nullability.EXPLICIT)); Asserts.assertEquivalenceOperations(record, loop); } @@ -94,7 +95,7 @@ public void testLongToString() { assertEquals( "{a01: number, a02: number, a03: number, a04: number, a05: number, a06: number," + " a07: number, a08: number, a09: number, a10: number, a11: number}", - record.toAnnotationString()); + record.toAnnotationString(Nullability.EXPLICIT)); } public void testSupAndInf() {