[NTI] Exclude stray properties when checking for implemented abstract…

… or interface methods.

Sadly, we still include stray properties in all subtyping checks. But possibly the only way to improve on that is to stop supporting stray property definitions altogether. (Unsure if feasible.)

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

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

@@ -532,7 +532,7 @@ private void checkAndFreezeNominalType(RawNominalType rawType) {
         if (superClass.isAbstractClass()
             && superClass.hasAbstractMethod(pname)
             && !rawType.isAbstractClass()
-            && !rawType.mayHaveOwnProp(pname)) {
+            && !rawType.mayHaveOwnNonStrayProp(pname)) {
           warnings.add(JSError.make(
               rawType.getDefSite(), ABSTRACT_METHOD_NOT_IMPLEMENTED_IN_CONCRETE_CLASS,
               pname, superClass.getName()));
@@ -681,8 +681,8 @@ private void checkSuperProperty(
       }
     } else {
       PropertyDef propdef = getPropDefFromClass(superType, pname);
-      // TODO(dimvar): fix look-ups of stray properties in a follow-up change, and add a
-      // precondition here that propdef should not be null.
+      // This can happen if superType is a class created by a mixin application. The class may
+      // have prototype properties but we can't see where these properties are defined.
       if (propdef == null) {
         return;
       }
@@ -691,7 +691,7 @@ private void checkSuperProperty(
     if (superType.isInterface()
         && current.isClass()
         && !isCtorDefinedByCall(current)
-        && !current.mayHaveProp(pname)) {
+        && !current.mayHaveNonStrayProp(pname)) {
       warnings.add(JSError.make(
           inheritedPropDefs.iterator().next().defSite,
           INTERFACE_METHOD_NOT_IMPLEMENTED,

src/com/google/javascript/jscomp/newtypes/Namespace.java

@@ -220,7 +220,7 @@ final Property getNsProp(String pname) {
     if (this instanceof NamespaceLit) {
       NominalType maybeWin = ((NamespaceLit) this).getWindowType();
       if (maybeWin != null) {
-        return maybeWin.getProp(pname, PropAccess.INCLUDE_STRAY_PROPS);
+        return maybeWin.getProp(pname, PropAccess.EXCLUDE_STRAY_PROPS);
       }
     }
     return null;

src/com/google/javascript/jscomp/newtypes/NominalType.java

@@ -437,7 +437,7 @@ Property getOwnProp(String pname) {
   }
 
   public boolean hasConstantProp(String pname) {
-    Property p = this.rawType.getProp(pname, PropAccess.INCLUDE_STRAY_PROPS);
+    Property p = this.rawType.getProp(pname, PropAccess.EXCLUDE_STRAY_PROPS);
     return p != null && p.isConstant();
   }
 
@@ -449,29 +449,6 @@ public boolean hasAbstractMethod(String pname) {
     return this.rawType.hasAbstractMethod(pname);
   }
 
-  boolean isSubtypeOf(NominalType other, SubtypeCache subSuperMap) {
-    return isNominalSubtypeOf(other)
-        || (other.isStructuralInterface() && isStructuralSubtypeOf(other, subSuperMap));
-  }
-
-  private boolean isStructuralSubtypeOf(NominalType other, SubtypeCache subSuperMap) {
-    checkArgument(other.isStructuralInterface());
-    for (String pname : other.getAllPropsOfInterface()) {
-      Property prop2 = other.getProp(pname, PropAccess.INCLUDE_STRAY_PROPS);
-      Property prop1 = this.getProp(pname, PropAccess.INCLUDE_STRAY_PROPS);
-      if (prop2.isOptional()) {
-        if (prop1 != null
-            && !prop1.getType().isSubtypeOf(prop2.getType(), subSuperMap)) {
-          return false;
-        }
-      } else if (prop1 == null || prop1.isOptional()
-          || !prop1.getType().isSubtypeOf(prop2.getType(), subSuperMap)) {
-        return false;
-      }
-    }
-    return true;
-  }
-
   // Checks for subtyping without taking generics into account
   boolean isRawSubtypeOf(NominalType other) {
     return this.rawType.isSubtypeOf(other.rawType);

src/com/google/javascript/jscomp/newtypes/RawNominalType.java

@@ -411,8 +411,8 @@ public JSType getProtoPropDeclaredType(String pname) {
     return null;
   }
 
-  public boolean hasAbstractMethod(String pname) {
-    Property p = getPropFromClass(pname, PropAccess.INCLUDE_STRAY_PROPS);
+  boolean hasAbstractMethod(String pname) {
+    Property p = getPropFromClass(pname, PropAccess.EXCLUDE_STRAY_PROPS);
     JSType ptype = p == null ? null : p.getType();
     return ptype != null && ptype.isFunctionType() && ptype.getFunType().isAbstract();
   }
@@ -460,10 +460,18 @@ public boolean mayHaveOwnProp(String pname) {
     return getOwnProp(pname, PropAccess.INCLUDE_STRAY_PROPS) != null;
   }
 
+  public boolean mayHaveOwnNonStrayProp(String pname) {
+    return getOwnProp(pname, PropAccess.EXCLUDE_STRAY_PROPS) != null;
+  }
+
   public boolean mayHaveProp(String pname) {
     return getProp(pname, PropAccess.INCLUDE_STRAY_PROPS) != null;
   }
 
+  public boolean mayHaveNonStrayProp(String pname) {
+    return getProp(pname, PropAccess.EXCLUDE_STRAY_PROPS) != null;
+  }
+
   public JSType getInstancePropDeclaredType(String pname) {
     Property p = getProp(pname, PropAccess.INCLUDE_STRAY_PROPS);
     if (p == null) {

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

@@ -15812,6 +15812,14 @@ public void testWindowAsNamespace() {
             "  var /** string */ s = x.prop;",
             "}"),
         NewTypeInference.MISTYPED_ASSIGN_RHS);
+
+    typeCheck(LINE_JOINER.join(
+        "function f(/** !Window */ w) {",
+        "  /** @type {number} */",
+        "  w.myprop = 123;",
+        "}",
+        "var /** string */ s = window.myprop;"),
+        NewTypeInference.MISTYPED_ASSIGN_RHS);
   }
 
   public void testInstantiateToTheSuperType() {
@@ -21012,8 +21020,7 @@ public void testRegisterPropertyOfPrototypeLiteral() {
         "}"));
   }
 
-  public void testInheritanceAndStrayProperties() {
-    // TODO(dimvar): warn here about GlobalTypeInfoCollector.INTERFACE_METHOD_NOT_IMPLEMENTED
+  public void testStrayProperties() {
     typeCheck(LINE_JOINER.join(
         "/** @record */",
         "function Foo() {}",
@@ -21030,7 +21037,8 @@ public void testInheritanceAndStrayProperties() {
         " * @constructor",
         " * @extends {Bar}",
         " */",
-        "function Baz() {}"));
+        "function Baz() {}"),
+        GlobalTypeInfoCollector.INTERFACE_METHOD_NOT_IMPLEMENTED);
 
     typeCheck(LINE_JOINER.join(
         "/** @record */",
@@ -21055,5 +21063,30 @@ public void testInheritanceAndStrayProperties() {
         " * @extends {Bar}",
         " */",
         "function Baz() {}"));
+
+    typeCheck(LINE_JOINER.join(
+        "/** @constructor @abstract */",
+        "function Foo() {}",
+        "/** @abstract */",
+        "Foo.prototype.mymethod = function() {};",
+        "/** @constructor @extends {Foo} */",
+        "function Bar() {}",
+        "function f(/** !Bar */ x) {",
+        "  x.mymethod = function() {};",
+        "}"),
+        GlobalTypeInfoCollector.ABSTRACT_METHOD_NOT_IMPLEMENTED_IN_CONCRETE_CLASS);
+
+    // It would be nice to give a mistyped-assign-rhs here. But the stray property appears on
+    // on all instances of Bar, so we don't catch the issue.
+    typeCheck(LINE_JOINER.join(
+        "/** @record */",
+        "function Foo() {}",
+        "Foo.prototype.a;",
+        "/** @constructor */",
+        "function Bar() {}",
+        "function f(/** !Bar */ x) {",
+        "  x.a = 123;",
+        "}",
+        "var /** !Foo */ x = new Bar;"));
   }
 }