Correct property (dis)ambiguation for dynamically created classes.

This change is necessary for type checking and property (dis)ambiguation to
work correctly when class transpilation is moved after type checking.

The core of the change is to expand the logic in ObjectType.isAmbiguousObject()
to return true for cases where the implementation of the object's class or one
of its superclasses isn't fully known.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=215136357

src/com/google/javascript/jscomp/FunctionTypeBuilder.java

@@ -408,24 +408,46 @@ private boolean maybeUseNativeClassTemplateNames(JSDocInfo info) {
     return false;
   }
 
-  /** Infer any supertypes from the JSDocInfo or the passed-in base type. */
-  FunctionTypeBuilder inferInheritance(@Nullable JSDocInfo info, @Nullable ObjectType baseType) {
+  /**
+   * Infer any supertypes from the JSDocInfo or the passed-in base type.
+   *
+   * @param info JSDoc info that is attached to the type declaration, if any
+   * @param classExtendsType The type of the extends clause in `class C extends SuperClass {}`, if
+   *     present.
+   * @return this object
+   */
+  FunctionTypeBuilder inferInheritance(
+      @Nullable JSDocInfo info, @Nullable ObjectType classExtendsType) {
 
-    // base type
     if (info != null && info.hasBaseType()) {
       if (isConstructor) {
         ObjectType infoBaseType =
             info.getBaseType().evaluate(templateScope, typeRegistry).toMaybeObjectType();
         // TODO(sdh): ensure JSDoc's baseType and AST's baseType are compatible if both are set
-        baseType = infoBaseType;
+        if (infoBaseType.setValidator(new ExtendedTypeValidator())) {
+          baseType = infoBaseType;
+        }
       } else {
         reportWarning(EXTENDS_WITHOUT_TYPEDEF, formatFnName());
       }
-    }
-    if (baseType != null && isConstructor) {
-      if (baseType.setValidator(new ExtendedTypeValidator())) {
-        this.baseType = baseType;
-      }
+    } else if (classExtendsType != null && isConstructor) {
+      // This case is:
+      // // no JSDoc here
+      // class extends astBaseType {...}
+      //
+      // It may well be that astBaseType is something dynamically created, like a value passed into
+      // a function. A common pattern is:
+      //
+      // function mixinX(superClass) {
+      //   return class extends superClass {
+      //     ...
+      //   };
+      // }
+      // The ExtendedTypeValidator() used in the JSDocInfo case above will report errors for these
+      // cases, and we don't want that.
+      // Since astBaseType is an actual value in code rather than an annotation, we can
+      // rely on validation elsewhere to ensure it is actually defined.
+      baseType = classExtendsType;
     }
 
     // Implemented interfaces (for constructors only).
@@ -471,13 +493,20 @@ FunctionTypeBuilder inferInheritance(@Nullable JSDocInfo info, @Nullable ObjectT
             extendedInterfaces.add((ObjectType) maybeInterfaceType);
           }
           // de-dupe baseType (from extends keyword) if it's also in @extends jsdoc.
-          if (baseType != null && maybeInterfaceType.isSubtypeOf(baseType)) {
-            baseType = null;
+          if (classExtendsType != null && maybeInterfaceType.isSubtypeOf(classExtendsType)) {
+            classExtendsType = null;
           }
         }
       }
-      if (baseType != null && baseType.setValidator(new ExtendedTypeValidator())) {
-        extendedInterfaces.add(baseType);
+      if (classExtendsType != null && classExtendsType.setValidator(new ExtendedTypeValidator())) {
+        // case is:
+        // /**
+        //  * @interface
+        //  * @extends {OtherInterface}
+        //  */
+        // class SomeInterface extends astBaseType {}
+        // Add the explicit extends type to the extended interfaces listed in JSDoc.
+        extendedInterfaces.add(classExtendsType);
       }
     }
 

src/com/google/javascript/jscomp/TypedScopeCreator.java

@@ -1187,8 +1187,19 @@ private ObjectType findSuperClassFromNodes(Node extendsNode, @Nullable JSDocInfo
         }
       }
 
-      if (ctorType != null && (ctorType.isConstructor() || ctorType.isInterface())) {
-        return ctorType.toMaybeFunctionType().getInstanceType();
+      if (ctorType != null) {
+        if (ctorType.isConstructor() || ctorType.isInterface()) {
+          return ctorType.toMaybeFunctionType().getInstanceType();
+        } else if (ctorType.isUnknownType()) {
+          // The constructor could have an unknown type for cases where it is dynamically
+          // created or passed in from elsewhere.
+          // e.g. with a mixin pattern
+          // function mixinSomething(ctor) {
+          //   return class extends ctor { ... };
+          // }
+          // In that case consider the super class instance type to be unknown.
+          return ctorType.toMaybeObjectType();
+        }
       }
 
       // We couldn't determine the type, so for TypedScope creation purposes we will treat it as if

src/com/google/javascript/rhino/jstype/FunctionType.java

@@ -89,6 +89,14 @@ private enum PropAccess {
     DICT
   }
 
+  enum ConstructorAmbiguity {
+    UNKNOWN,
+    CONSTRUCTS_AMBIGUOUS_OBJECTS,
+    CONSTRUCTS_UNAMBIGUOUS_OBJECTS
+  }
+
+  private ConstructorAmbiguity constructorAmbiguity = ConstructorAmbiguity.UNKNOWN;
+
   /** {@code [[Call]]} property. */
   private ArrowType call;
 
@@ -1447,4 +1455,62 @@ public final ImmutableList<TemplateType> getConstructorOnlyTemplateParameters()
     }
     return ctorKeys.build();
   }
+
+  boolean createsAmbiguousObjects() {
+    if (this.constructorAmbiguity == ConstructorAmbiguity.UNKNOWN) {
+      constructorAmbiguity = calculateConstructorAmbiguity();
+    }
+    return constructorAmbiguity == ConstructorAmbiguity.CONSTRUCTS_AMBIGUOUS_OBJECTS;
+  }
+
+  private ConstructorAmbiguity calculateConstructorAmbiguity() {
+    final ConstructorAmbiguity constructorAmbiguity;
+    if (isUnknownType()) {
+      constructorAmbiguity = ConstructorAmbiguity.CONSTRUCTS_AMBIGUOUS_OBJECTS;
+    } else if (isNativeObjectType()) {
+      // native types other than unknown are never ambiguous
+      constructorAmbiguity = ConstructorAmbiguity.CONSTRUCTS_UNAMBIGUOUS_OBJECTS;
+    } else {
+      FunctionType superConstructor = getSuperClassConstructor();
+      if (superConstructor == null) {
+        // TODO(bradfordcsmith): Why is superConstructor ever null here?
+        constructorAmbiguity = ConstructorAmbiguity.CONSTRUCTS_AMBIGUOUS_OBJECTS;
+      } else if (superConstructor.createsAmbiguousObjects()) {
+        // Subclasses of ambiguous objects are also ambiguous
+        constructorAmbiguity = ConstructorAmbiguity.CONSTRUCTS_AMBIGUOUS_OBJECTS;
+      } else if (source != null) {
+        // We can see the definition of the class, so we know all properties it directly declares
+        // or references.
+        // The same is true for its superclass (previous condition).
+        constructorAmbiguity = ConstructorAmbiguity.CONSTRUCTS_UNAMBIGUOUS_OBJECTS;
+      } else if (isDelegateProxy()) {
+        // Type was created by the compiler as a proxy that inherits from the real type that was in
+        // the code.
+        // Since we've made it this far, we know the real type creates unambiguous objects.
+        // Therefore, the proxy does, too.
+        constructorAmbiguity = ConstructorAmbiguity.CONSTRUCTS_UNAMBIGUOUS_OBJECTS;
+      } else {
+        // Type was created directly from JSDoc without a function or class literal.
+        // e.g.
+        // /**
+        //  * @constructor
+        //  * @param {string} x
+        //  * @implements {SomeInterface}
+        //  */
+        // const MyImpl = createMyImpl();
+        // The actual properties on this class are hidden from us, so we must consider it ambiguous.
+        constructorAmbiguity = ConstructorAmbiguity.CONSTRUCTS_AMBIGUOUS_OBJECTS;
+      }
+    }
+    return constructorAmbiguity;
+  }
+
+  // See also TypedScopeCreator.DELEGATE_PROXY_SUFFIX
+  // Unfortunately we cannot use that constant here.
+  private static final String DELEGATE_SUFFIX = ObjectType.createDelegateSuffix("Proxy");
+
+  private boolean isDelegateProxy() {
+    // TODO(bradfordcsmith): There should be a better way to determine that we have a proxy type.
+    return hasReferenceName() && getReferenceName().endsWith(DELEGATE_SUFFIX);
+  }
 }

src/com/google/javascript/rhino/jstype/InstanceObjectType.java

@@ -188,6 +188,11 @@ public Iterable<ObjectType> getCtorExtendedInterfaces() {
     return getConstructor().getExtendedInterfaces();
   }
 
+  @Override
+  public boolean isAmbiguousObject() {
+    return getConstructor().createsAmbiguousObjects();
+  }
+
   // The owner will always be a resolved type, so there's no need to set
   // the constructor in resolveInternal.
   // (it would lead to infinite loops if we did).

src/com/google/javascript/rhino/jstype/ObjectType.java

@@ -252,7 +252,7 @@ public static String createDelegateSuffix(String suffix) {
     return "(" + suffix + ")";
   }
 
-  public final boolean isAmbiguousObject() {
+  public boolean isAmbiguousObject() {
     return !hasReferenceName();
   }
 

test/com/google/javascript/jscomp/Es6RewriteClassTest.java

@@ -835,7 +835,6 @@ public void testExtendsInExterns() {
 
   @Test
   public void testExtendForwardDeclaredClass() {
-    ignoreWarnings(TypeCheck.POSSIBLE_INEXISTENT_PROPERTY);
     enableClosurePass();
     test(
         srcs(
@@ -851,7 +850,7 @@ public void testExtendForwardDeclaredClass() {
                 "  return ns.D.apply(this, arguments) || this;",
                 "};",
                 "$jscomp.inherits(C, ns.D);")),
-        warning(FunctionTypeBuilder.RESOLVED_TAG_EMPTY));
+        warning(TypeCheck.POSSIBLE_INEXISTENT_PROPERTY));
   }
 
   @Test