From 7ab7014048d0dcba71fd90d1a5bca3c7c8c1ba83 Mon Sep 17 00:00:00 2001 From: sdh Date: Mon, 9 Oct 2017 14:13:31 -0700 Subject: [PATCH] [NTI] Call delegate coding conventions during NTI. NEW: ensure delegate proxy types get finalized, and add special handling to automaticall suppress duplicate property errors from implicitly defined delegate properties. Also cleans up some testing around synthetic types, in particular, unifying stringification of synthetic types across OTI and NTI. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=171585245 --- .../jscomp/GlobalTypeInfoCollector.java | 52 +++++++++++++++++++ .../jscomp/newtypes/FunctionType.java | 20 ++++++- .../javascript/jscomp/newtypes/JSType.java | 2 +- .../javascript/jscomp/newtypes/JSTypes.java | 6 +++ .../jscomp/newtypes/NominalType.java | 4 +- .../jscomp/newtypes/ObjectType.java | 2 +- .../jscomp/newtypes/RawNominalType.java | 15 ++++-- .../javascript/rhino/jstype/FunctionType.java | 2 +- .../rhino/jstype/InstanceObjectType.java | 7 ++- .../javascript/rhino/jstype/ObjectType.java | 14 +++-- .../rhino/jstype/PrototypeObjectType.java | 2 +- 11 files changed, 107 insertions(+), 19 deletions(-) diff --git a/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java b/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java index a64ab5e0944..d916367bbe4 100644 --- a/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java +++ b/src/com/google/javascript/jscomp/GlobalTypeInfoCollector.java @@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; +import static com.google.javascript.rhino.jstype.JSTypeNative.U2U_CONSTRUCTOR_TYPE; import com.google.common.base.Preconditions; import com.google.common.collect.HashBasedTable; @@ -30,6 +31,7 @@ import com.google.common.collect.MultimapBuilder; import com.google.javascript.jscomp.AbstractCompiler.MostRecentTypechecker; import com.google.javascript.jscomp.CodingConvention.Bind; +import com.google.javascript.jscomp.CodingConvention.DelegateRelationship; import com.google.javascript.jscomp.CodingConvention.SubclassRelationship; import com.google.javascript.jscomp.NewTypeInference.WarningReporter; import com.google.javascript.jscomp.NodeTraversal.AbstractShallowCallback; @@ -54,11 +56,13 @@ import com.google.javascript.rhino.IR; import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.Node; +import com.google.javascript.rhino.NominalTypeBuilder; import com.google.javascript.rhino.SimpleSourceFile; import com.google.javascript.rhino.Token; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -1443,6 +1447,8 @@ private void visitNewCtorWithoutFunctionLiteral(Node qnameNode) { private class ProcessScope extends AbstractShallowCallback { private final NTIScope currentScope; + private final List delegateProxies = new ArrayList<>(); + private final Map delegateCallingConventions = new HashMap<>(); private Set lendsObjlits = new LinkedHashSet<>(); ProcessScope(NTIScope currentScope) { @@ -1454,6 +1460,8 @@ void finishProcessingScope() { processLendsNode(objlit); } lendsObjlits = null; + convention.defineDelegateProxyPrototypeProperties( + globalTypeInfo, delegateProxies, delegateCallingConventions); } /** @@ -1580,6 +1588,7 @@ public void visit(NodeTraversal t, Node n, Node parent) { if (parent.isExprResult() && n.isQualifiedName()) { visitPropertyDeclaration(n); } + convention.checkForCallingConventionDefinitions(n, delegateCallingConventions); break; case ASSIGN: { Node lvalue = n.getFirstChild(); @@ -1697,6 +1706,10 @@ private void visitCall(Node call) { if (className != null) { applySingletonGetter(className); } + DelegateRelationship delegateRelationship = convention.getDelegateRelationship(call); + if (delegateRelationship != null) { + applyDelegateRelationship(delegateRelationship); + } } private void applySubclassRelationship(SubclassRelationship rel) { @@ -1726,6 +1739,43 @@ private void applySingletonGetter(String className) { } } + private void applyDelegateRelationship(DelegateRelationship rel) { + RawNominalType delegateBase = findInScope(rel.delegateBase); + RawNominalType delegator = findInScope(rel.delegator); + RawNominalType delegateSuper = findInScope(convention.getDelegateSuperclassName()); + if (delegator != null && delegateBase != null && delegateSuper != null) { + // Note: OTI also verified that getConstructor() was non-null on each of these, but since + // they're all coming from RawNominalTypes, there should be no way that can fail here. + JSType findDelegate = + new FunctionTypeBuilder(getCommonTypes()) + .addReqFormal(getCommonTypes().getNativeType(U2U_CONSTRUCTOR_TYPE)) + .addRetType(JSType.join(getCommonTypes().NULL, delegateBase.getInstanceAsJSType())) + .buildType(); + + RawNominalType delegateProxy = RawNominalType.makeClass( + getCommonTypes(), + delegateBase.getDefSite() /* defSite */, + delegateBase.getName() + "(Proxy)" /* name */, + null /* typeParameters */, + ObjectKind.UNRESTRICTED, + false /* isAbstract */); + nominaltypesByNode.put(new Node(null), delegateProxy); + delegateProxy.addSuperClass(delegateBase.getAsNominalType()); + delegateProxy.setCtorFunction( + new FunctionTypeBuilder(getCommonTypes()) + .addRetType(delegateProxy.getInstanceAsJSType()) + .addNominalType(delegateProxy.getInstanceAsJSType()) + .buildFunction()); + convention.applyDelegateRelationship( + new NominalTypeBuilderNti(lateProps, delegateSuper), + new NominalTypeBuilderNti(lateProps, delegateBase), + new NominalTypeBuilderNti(lateProps, delegator), + delegateProxy.getInstanceAsJSType(), + findDelegate); + delegateProxies.add(new NominalTypeBuilderNti(lateProps, delegateProxy)); + } + } + private RawNominalType findInScope(String qname) { return currentScope.getNominalType(QualifiedName.fromQualifiedString(qname)); } @@ -2490,6 +2540,8 @@ private boolean mayWarnAboutExistingProp(RawNominalType classType, JSType previousPropType = classType.getInstancePropDeclaredType(pname); if (classType.mayHaveNonInheritedProp(pname) && previousPropType != null + // TODO(sdh): Remove this special case once all explicit decls are removed b/67509620 + && !previousPropType.toString().endsWith("(Proxy)") && !suppressDupPropWarning(jsdoc, typeInJsdoc, previousPropType)) { warnings.add(JSError.make( propCreationNode, REDECLARED_PROPERTY, pname, "type " + classType)); diff --git a/src/com/google/javascript/jscomp/newtypes/FunctionType.java b/src/com/google/javascript/jscomp/newtypes/FunctionType.java index f5b4f14d5c6..447a9eb003b 100644 --- a/src/com/google/javascript/jscomp/newtypes/FunctionType.java +++ b/src/com/google/javascript/jscomp/newtypes/FunctionType.java @@ -257,6 +257,20 @@ static Map createInitialFunctionTypes(JSTypes commonTypes) false)); functions.put("TOP_FUNCTION", new FunctionType(commonTypes, false)); functions.put("LOOSE_TOP_FUNCTION", new FunctionType(commonTypes, true)); + functions.put( + "U2U_CONSTRUCTOR", + FunctionType.normalized( + commonTypes, + null /* requiredFormals */, + null /* optionalFormals */, + commonTypes.UNKNOWN /* restFormals */, + commonTypes.UNKNOWN /* retType */, + commonTypes.UNKNOWN /* nominalType */, + null /* receiverType */, + null /* outerVars */, + null /* typeParameters */, + true /* isLoose */, + false /* isAbstract */)); return functions; } @@ -1482,12 +1496,14 @@ public String apply(String typeParam) { @SuppressWarnings("ReferenceEquality") public StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) { - if (this == this.commonTypes.LOOSE_TOP_FUNCTION) { + if (isLoose() && ctx.forAnnotation()) { + return builder.append("!Function"); + } else 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(ctx.forAnnotation() ? "!" : "").append("Function"); + return builder.append(ctx.forAnnotation() ? "!Function" : "Function"); } if (!this.typeParameters.isEmpty()) { builder.append("<"); diff --git a/src/com/google/javascript/jscomp/newtypes/JSType.java b/src/com/google/javascript/jscomp/newtypes/JSType.java index f80a009b7dd..c954db2914a 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSType.java +++ b/src/com/google/javascript/jscomp/newtypes/JSType.java @@ -1791,7 +1791,7 @@ public final boolean isVoidType() { } @Override - public final TypeI restrictByNotNullOrUndefined() { + public final JSType restrictByNotNullOrUndefined() { return this.removeType(this.commonTypes.NULL_OR_UNDEFINED); } diff --git a/src/com/google/javascript/jscomp/newtypes/JSTypes.java b/src/com/google/javascript/jscomp/newtypes/JSTypes.java index 4de42683a3f..d0f282f51f3 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSTypes.java +++ b/src/com/google/javascript/jscomp/newtypes/JSTypes.java @@ -119,6 +119,9 @@ public final class JSTypes implements Serializable { final FunctionType TOP_FUNCTION; @SuppressWarnings("ConstantField") final FunctionType LOOSE_TOP_FUNCTION; + // Constructor for unknown type. + @SuppressWarnings("ConstantField") + private final FunctionType U2U_CONSTRUCTOR; @SuppressWarnings("ConstantField") final Map MAP_TO_UNKNOWN; @@ -203,6 +206,7 @@ private JSTypes(boolean inCompatibilityMode) { this.BOTTOM_FUNCTION = checkNotNull(functions.get("BOTTOM_FUNCTION")); this.TOP_FUNCTION = checkNotNull(functions.get("TOP_FUNCTION")); this.LOOSE_TOP_FUNCTION = checkNotNull(functions.get("LOOSE_TOP_FUNCTION")); + this.U2U_CONSTRUCTOR = checkNotNull(functions.get("U2U_CONSTRUCTOR")); this.BOTTOM_PROPERTY_MAP = PersistentMap.of("_", Property.make(this.BOTTOM, this.BOTTOM)); this.allowMethodsAsFunctions = inCompatibilityMode; @@ -516,6 +520,8 @@ public JSType getNativeType(JSTypeNative typeId) { return getIteratorInstance(UNKNOWN); case GENERATOR_TYPE: return getGeneratorInstance(UNKNOWN); + case U2U_CONSTRUCTOR_TYPE: + return fromFunctionType(U2U_CONSTRUCTOR); default: throw new RuntimeException("Native type " + typeId.name() + " not found"); } diff --git a/src/com/google/javascript/jscomp/newtypes/NominalType.java b/src/com/google/javascript/jscomp/newtypes/NominalType.java index 80e6aee760f..f217a259c7a 100644 --- a/src/com/google/javascript/jscomp/newtypes/NominalType.java +++ b/src/com/google/javascript/jscomp/newtypes/NominalType.java @@ -677,10 +677,10 @@ StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) { if (ctx.forAnnotation()) { builder.append("!"); } + this.rawType.appendTo(builder, ctx); if (this.typeMap.isEmpty()) { - return this.rawType.appendTo(builder); + return builder; } - builder.append(this.rawType.name); ImmutableList typeParams = this.rawType.getTypeParameters(); checkState(this.typeMap.keySet().containsAll(typeParams)); boolean firstIteration = true; diff --git a/src/com/google/javascript/jscomp/newtypes/ObjectType.java b/src/com/google/javascript/jscomp/newtypes/ObjectType.java index c36c59a2d64..b83aa607328 100644 --- a/src/com/google/javascript/jscomp/newtypes/ObjectType.java +++ b/src/com/google/javascript/jscomp/newtypes/ObjectType.java @@ -1661,7 +1661,7 @@ String toString(ToStringContext ctx) { StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) { // "Foo.prototype" is a valid type when appropriate. if (isPrototypeObject()) { - return builder.append(getOwnerFunction().getThisType()).append(".prototype"); + return builder.append(getOwnerFunction().getThisType().getDisplayName()).append(".prototype"); } // Annotations need simpler output that can be re-parsed. if (ctx.forAnnotation()) { diff --git a/src/com/google/javascript/jscomp/newtypes/RawNominalType.java b/src/com/google/javascript/jscomp/newtypes/RawNominalType.java index 25d15261ddb..918dbbb1089 100644 --- a/src/com/google/javascript/jscomp/newtypes/RawNominalType.java +++ b/src/com/google/javascript/jscomp/newtypes/RawNominalType.java @@ -728,14 +728,21 @@ public void freeze() { this.isFrozen = true; } - StringBuilder appendTo(StringBuilder builder) { - builder.append(name); - return builder; + StringBuilder appendTo(StringBuilder builder, ToStringContext ctx) { + if (ctx.forAnnotation()) { + // Note: some synthetic nominal types have a parenthesized segment in their name, + // which is not compatible with type annotations, so remove it if found. + int index = name.indexOf('('); + if (index >= 0) { + return builder.append(name, 0, index); + } + } + return builder.append(name); } @Override public String toString() { - return appendTo(new StringBuilder()).toString(); + return appendTo(new StringBuilder(), ToStringContext.TO_STRING).toString(); } @Override diff --git a/src/com/google/javascript/rhino/jstype/FunctionType.java b/src/com/google/javascript/rhino/jstype/FunctionType.java index 2f63d39eb86..68304e685fe 100644 --- a/src/com/google/javascript/rhino/jstype/FunctionType.java +++ b/src/com/google/javascript/rhino/jstype/FunctionType.java @@ -1026,7 +1026,7 @@ public boolean hasEqualCallType(FunctionType otherType) { StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) { if (!isPrettyPrint() || this == registry.getNativeType(JSTypeNative.FUNCTION_INSTANCE_TYPE)) { - return sb.append("Function"); + return sb.append(forAnnotations ? "!Function" : "Function"); } setPrettyPrint(false); diff --git a/src/com/google/javascript/rhino/jstype/InstanceObjectType.java b/src/com/google/javascript/rhino/jstype/InstanceObjectType.java index 821b3e1fa8a..7d37aaa0976 100644 --- a/src/com/google/javascript/rhino/jstype/InstanceObjectType.java +++ b/src/com/google/javascript/rhino/jstype/InstanceObjectType.java @@ -95,8 +95,11 @@ boolean defineProperty(String name, JSType type, boolean inferred, @Override StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) { return constructor.hasReferenceName() - ? sb.append(constructor.getReferenceName()) - : super.appendTo(sb, forAnnotations); + ? sb.append( + forAnnotations + ? constructor.getNormalizedReferenceName() + : constructor.getReferenceName()) + : super.appendTo(sb, forAnnotations); } @Override diff --git a/src/com/google/javascript/rhino/jstype/ObjectType.java b/src/com/google/javascript/rhino/jstype/ObjectType.java index 6ecc374d15b..f08b0ac63dd 100644 --- a/src/com/google/javascript/rhino/jstype/ObjectType.java +++ b/src/com/google/javascript/rhino/jstype/ObjectType.java @@ -218,15 +218,19 @@ final boolean detectInheritanceCycle() { * We construct these types by appending suffixes to the constructor name. * * The normalized reference name does not have these suffixes, and as such, - * recollapses these implicit types back to their real type. + * recollapses these implicit types back to their real type. Note that + * suffixes such as ".prototype" can be added after the delegate + * suffix, so anything after the parentheses must still be retained. */ @Nullable - public String getNormalizedReferenceName() { + public final String getNormalizedReferenceName() { String name = getReferenceName(); if (name != null) { - int pos = name.indexOf('('); - if (pos != -1) { - return name.substring(0, pos); + int start = name.indexOf('('); + if (start != -1) { + int end = name.lastIndexOf(')'); + String prefix = name.substring(0, start); + return end + 1 % name.length() == 0 ? prefix : prefix + name.substring(end + 1); } } return name; diff --git a/src/com/google/javascript/rhino/jstype/PrototypeObjectType.java b/src/com/google/javascript/rhino/jstype/PrototypeObjectType.java index 819fd7796c5..d19f01d9e46 100644 --- a/src/com/google/javascript/rhino/jstype/PrototypeObjectType.java +++ b/src/com/google/javascript/rhino/jstype/PrototypeObjectType.java @@ -279,7 +279,7 @@ public boolean canBeCalled() { @Override StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) { if (hasReferenceName()) { - return sb.append(getReferenceName()); + return sb.append(forAnnotations ? getNormalizedReferenceName() : getReferenceName()); } if (!prettyPrint) { return sb.append(forAnnotations ? "?" : "{...}");