In DisambiguateProperties, stop conflating ctor types with the inte…

…rface types they declared their instance types to implement.

We want conflation between instance types and their implemented interfaces, but ctors types don't have any connection to those interfaces. The mistake was that when getting the ctor type of a ctor type, which is where interfaces would be stored, we were returning the input ctor type; the ctor of a ctor is not itself.

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

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

@@ -205,10 +205,8 @@ void addType(JSType type, JSType relatedType) {
       } else {
         getTypes().union(top, relatedType);
       }
-      FunctionType constructor = getConstructor(type);
-      if (constructor != null && recordInterfacesCache.add(type)) {
-        recordInterfaces(constructor, top, this);
-      }
+
+      recordInterfaces(type, top);
     }
 
     /** Records the given type as one to skip for this property. */
@@ -346,6 +344,38 @@ private JSType processProperty(JSType type, @Nullable JSType relatedType) {
         return topType;
       }
     }
+
+    /**
+     * Records that this property could be referenced from any interface that this type inherits
+     * from.
+     *
+     * <p>If the property p is defined only on a subtype of constructor, then this method has no
+     * effect. But we tried modifying getTypeWithProperty to tell us when the returned type is a
+     * subtype, and then skip those calls to recordInterface, and there was no speed-up. And it made
+     * the code harder to understand, so we don't do it.
+     */
+    private void recordInterfaces(JSType type, JSType relatedType) {
+      @Nullable FunctionType constructor = getConstructor(type);
+      if (constructor == null || !recordInterfacesCache.add(type)) {
+        return;
+      }
+
+      Iterable<ObjectType> interfaces = ancestorInterfaces.get(constructor);
+      if (interfaces == null) {
+        interfaces = constructor.getAncestorInterfaces();
+        ancestorInterfaces.put(constructor, interfaces);
+      }
+      for (ObjectType itype : interfaces) {
+        JSType top = getTypeWithProperty(name, itype);
+        if (top != null) {
+          addType(itype, relatedType);
+        }
+        // If this interface invalidated this property, return now.
+        if (skipRenaming) {
+          return;
+        }
+      }
+    }
   }
 
   private final Map<String, Property> properties = new LinkedHashMap<>();
@@ -1069,44 +1099,25 @@ private static JSType getInstanceIfPrototype(JSType maybePrototype) {
   }
 
   /**
-   * Records that this property could be referenced from any interface that this type inherits from.
-   *
-   * <p>If the property p is defined only on a subtype of constructor, then this method has no
-   * effect. But we tried modifying getTypeWithProperty to tell us when the returned type is a
-   * subtype, and then skip those calls to recordInterface, and there was no speed-up. And it made
-   * the code harder to understand, so we don't do it.
+   * Return the constructor type of {@code type}, which holds {@code type}'s implemented interfaces.
    */
-  private void recordInterfaces(FunctionType constructor, JSType relatedType, Property p) {
-    Iterable<ObjectType> interfaces = ancestorInterfaces.get(constructor);
-    if (interfaces == null) {
-      interfaces = constructor.getAncestorInterfaces();
-      ancestorInterfaces.put(constructor, interfaces);
-    }
-    for (ObjectType itype : interfaces) {
-      JSType top = getTypeWithProperty(p.name, itype);
-      if (top != null) {
-        p.addType(itype, relatedType);
-      }
-      // If this interface invalidated this property, return now.
-      if (p.skipRenaming) {
-        return;
-      }
-    }
-  }
-
   private FunctionType getConstructor(JSType type) {
     ObjectType objType = type.toMaybeObjectType();
     if (objType == null) {
       return null;
     }
-    FunctionType constructor = null;
+
     if (objType.isFunctionType()) {
-      constructor = objType.toMaybeFunctionType();
+      // Recall that the constructor of a constructor `A`, is not `A` itself, but rather `Function`.
+      // If `Function` implemented some interface, that interface would be conflated with all other
+      // ctors, however that currently isn't the case, and would be correct.
+      return (FunctionType) registry.getNativeType(JSTypeNative.FUNCTION_FUNCTION_TYPE);
     } else if (objType.isFunctionPrototypeType()) {
-      constructor = objType.getOwnerFunction();
+      // For the purposes of disambiguation, pretend that prototypes implement the interfaces of
+      // their instance type.
+      return objType.getOwnerFunction();
     } else {
-      constructor = objType.getConstructor();
+      return objType.getConstructor();
     }
-    return constructor;
   }
 }

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

@@ -3165,6 +3165,53 @@ public void testEs6ClassSideInheritedMethods_referencedWithSuper_areDisambiguate
             "Bar._typeof_Bar_$method();"));
   }
 
+  @Test
+  public void testCtors_areNotConflatedWithOtherTypes_viaTheirInstanceInterfaces() {
+    testSets(
+        lines(
+            // The property isn't on the interface; the interface just creates edges in the type
+            // lattice.
+            "/** @interface */",
+            "class Iface {}",
+            "",
+            "/** @implements {Iface} */",
+            "class InstanceFoo {",
+            "  method() {}",
+            "}",
+            "/** @implements {Iface} */",
+            "class StaticFoo {",
+            "  static method() {}",
+            "}",
+            "",
+            "new InstanceFoo().method();",
+            "StaticFoo.method();"),
+        "{method=[[(typeof StaticFoo)], [InstanceFoo.prototype]]}");
+
+    testSets(
+        lines(
+            // But also try it when the interface does include the property, just to be sure.
+            "/** @interface */",
+            "class Iface {",
+            "  method() { }",
+            "}",
+            "",
+            "/** @implements {Iface} */",
+            "class InstanceFoo {",
+            "  method() {}",
+            "}",
+            "/** @implements {Iface} */",
+            "class StaticFoo {",
+            "  static method() {}",
+            "",
+            "  method() {}", // For typechecking.
+            "}",
+            "",
+            "new InstanceFoo().method();",
+            "StaticFoo.method();"),
+        "{method=[[(typeof StaticFoo)], [Iface.prototype, InstanceFoo.prototype,"
+            + " StaticFoo.prototype]]}");
+  }
+
   @Test
   public void testDoNotDisambiguateEs6ClassConstructor() {
     testSets("class Foo { constructor() {} } class Bar { constructor() {} }", "{}");