From 745e9e804b62cf150c0c43ed937d1c8bca24cc68 Mon Sep 17 00:00:00 2001 From: dimvar Date: Thu, 15 Jun 2017 13:56:51 -0700 Subject: [PATCH] When printing enums in type annotations, use the enum name instead of the enumerated type, otherwise we lose type information. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=159149370 --- .../javascript/jscomp/TypedCodeGenerator.java | 2 +- .../javascript/jscomp/newtypes/EnumType.java | 6 ---- .../javascript/jscomp/newtypes/JSType.java | 13 +++----- .../google/javascript/rhino/ObjectTypeI.java | 2 +- .../rhino/jstype/EnumElementType.java | 2 +- .../javascript/rhino/jstype/EnumType.java | 9 ++++-- .../javascript/rhino/jstype/ObjectType.java | 2 +- .../javascript/jscomp/CodePrinterTest.java | 32 ++++++++++++++++++- 8 files changed, 46 insertions(+), 22 deletions(-) diff --git a/src/com/google/javascript/jscomp/TypedCodeGenerator.java b/src/com/google/javascript/jscomp/TypedCodeGenerator.java index 99ba5b1f2af..eccfb624456 100644 --- a/src/com/google/javascript/jscomp/TypedCodeGenerator.java +++ b/src/com/google/javascript/jscomp/TypedCodeGenerator.java @@ -89,7 +89,7 @@ private String getTypeAnnotation(Node node) { return getFunctionAnnotation(node); } else if (type.isEnumObject()) { return "/** @enum {" - + type.toMaybeObjectType().getElementsType().toNonNullAnnotationString() + + type.toMaybeObjectType().getEnumeratedTypeOfEnumObject().toNonNullAnnotationString() + "} */\n"; } else if (!type.isUnknownType() && !type.isBottom() diff --git a/src/com/google/javascript/jscomp/newtypes/EnumType.java b/src/com/google/javascript/jscomp/newtypes/EnumType.java index 0cfb88bc2ec..ca453591258 100644 --- a/src/com/google/javascript/jscomp/newtypes/EnumType.java +++ b/src/com/google/javascript/jscomp/newtypes/EnumType.java @@ -235,10 +235,4 @@ static boolean areSubtypes(JSType t1, JSType t2, SubtypeCache subSuperMap) { public Collection 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/JSType.java b/src/com/google/javascript/jscomp/newtypes/JSType.java index 021e7840599..86a287511e2 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSType.java +++ b/src/com/google/javascript/jscomp/newtypes/JSType.java @@ -503,13 +503,8 @@ public final boolean isEnumObject() { } @Override - public final TypeI getElementsType() { - ObjectType obj = getObjTypeIfSingletonObj(); - if (obj != null) { - EnumType e = obj.getEnumType(); - return e != null ? e.getEnumeratedType() : null; - } - return null; + public final TypeI getEnumeratedTypeOfEnumObject() { + return isEnumObject() ? getObjTypeIfSingletonObj().getEnumType().getEnumeratedType() : null; } public final boolean isUnion() { @@ -1633,11 +1628,11 @@ StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) { } case ENUM_MASK: { if (getEnums().size() == 1) { - builder.append(Iterables.getOnlyElement(getEnums()).toString(ctx)); + builder.append(Iterables.getOnlyElement(getEnums()).toString()); } else { Set strReps = new TreeSet<>(); for (EnumType e : getEnums()) { - strReps.add(e.toString(ctx)); + strReps.add(e.toString()); } PIPE_JOINER.appendTo(builder, strReps); } diff --git a/src/com/google/javascript/rhino/ObjectTypeI.java b/src/com/google/javascript/rhino/ObjectTypeI.java index e2bf87a8807..786e645972f 100644 --- a/src/com/google/javascript/rhino/ObjectTypeI.java +++ b/src/com/google/javascript/rhino/ObjectTypeI.java @@ -157,5 +157,5 @@ public interface ObjectTypeI extends TypeI { * If this type is an enum object, returns the declared type of the elements. * Otherwise returns null. */ - TypeI getElementsType(); + TypeI getEnumeratedTypeOfEnumObject(); } diff --git a/src/com/google/javascript/rhino/jstype/EnumElementType.java b/src/com/google/javascript/rhino/jstype/EnumElementType.java index ec24a7b016b..28211e47269 100644 --- a/src/com/google/javascript/rhino/jstype/EnumElementType.java +++ b/src/com/google/javascript/rhino/jstype/EnumElementType.java @@ -148,7 +148,7 @@ public int hashCode() { @Override StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) { if (forAnnotations) { - return sb.append(this.primitiveType); + return sb.append(getReferenceName()); } return sb.append(getReferenceName()).append("<").append(this.primitiveType).append(">"); } diff --git a/src/com/google/javascript/rhino/jstype/EnumType.java b/src/com/google/javascript/rhino/jstype/EnumType.java index 76d1f2c600a..c6f4bc7be2e 100644 --- a/src/com/google/javascript/rhino/jstype/EnumType.java +++ b/src/com/google/javascript/rhino/jstype/EnumType.java @@ -102,12 +102,17 @@ public boolean defineElement(String name, Node definingNode) { return defineDeclaredProperty(name, elementsType, definingNode); } - /** Gets the elements' type. */ - @Override + /** Gets the elements' type, which is a subtype of the enumerated type. */ public EnumElementType getElementsType() { return elementsType; } + /** Gets the enumerated type. */ + @Override + public JSType getEnumeratedTypeOfEnumObject() { + return elementsType.getPrimitiveType(); + } + @Override public TernaryValue testForEquality(JSType that) { TernaryValue result = super.testForEquality(that); diff --git a/src/com/google/javascript/rhino/jstype/ObjectType.java b/src/com/google/javascript/rhino/jstype/ObjectType.java index 32639258c9c..76163e8da0e 100644 --- a/src/com/google/javascript/rhino/jstype/ObjectType.java +++ b/src/com/google/javascript/rhino/jstype/ObjectType.java @@ -827,7 +827,7 @@ public Map getPropertyTypeMap() { } @Override - public TypeI getElementsType() { + public TypeI getEnumeratedTypeOfEnumObject() { return null; } } diff --git a/test/com/google/javascript/jscomp/CodePrinterTest.java b/test/com/google/javascript/jscomp/CodePrinterTest.java index dd9638399dd..b80ec3c9d66 100644 --- a/test/com/google/javascript/jscomp/CodePrinterTest.java +++ b/test/com/google/javascript/jscomp/CodePrinterTest.java @@ -1389,7 +1389,8 @@ public void testEnumAnnotation2() { LINE_JOINER.join( "/** @const */ var goog = goog || {};", "/** @enum {string} */\ngoog.Enum = {FOO:\"x\", BAR:\"y\"};", - "/** @type {{BAR: string=, FOO: string=}} */\ngoog.Enum2 = goog.x ? {} : goog.Enum;", + "/** @type {{BAR: goog.Enum=, FOO: goog.Enum=}} */", + "goog.Enum2 = goog.x ? {} : goog.Enum;", "")); } @@ -1399,6 +1400,35 @@ public void testEnumAnnotation3() { "/** @enum {!Object} */\nvar Enum = {FOO:{}};\n"); } + public void testEnumAnnotation4() { + assertTypeAnnotations( + LINE_JOINER.join( + "/** @enum {number} */ var E = {A:1, B:2};", + "function f(/** !E */ x) { return x; }"), + LINE_JOINER.join( + "/** @enum {number} */", + "var E = {A:1, B:2};", + "/**", + " * @param {E} x", + " * @return {?}", + " */", + "function f(x) {", + " return x;", + "}", + ""), + LINE_JOINER.join( + "/** @enum {number} */", + "var E = {A:1, B:2};", + "/**", + " * @param {E} x", + " * @return {E}", + " */", + "function f(x) {", + " return x;", + "}", + "")); + } + public void testClosureLibraryTypeAnnotationExamples() { assertTypeAnnotations( LINE_JOINER.join(