[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
google · Oct 10, 2017 · 7ab7014 · 7ab7014
1 parent 9b02a32
commit 7ab7014
Show file tree

Hide file tree

Showing 11 changed files with 107 additions and 19 deletions.
diff --git 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<NominalTypeBuilder> delegateProxies = new ArrayList<>();
+    private final Map<String, String> delegateCallingConventions = new HashMap<>();
     private Set<Node> 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
@@ -257,6 +257,20 @@ static Map<String, FunctionType> 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
@@ -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
@@ -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<String, JSType> 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
@@ -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<String> 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 <i>after</i> 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
@@ -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 ? "?" : "{...}");