Fix property validation messaging on interfaces.

Earlier the name of the parent interface was returned instead of the actual one. Replaced the existing class/interface inconsistent algorithm with a BFS search. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=246369763
google · May 3, 2019 · 9317356 · 9317356
1 parent e593e6d
commit 9317356
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 25 deletions.
diff --git a/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java b/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java
@@ -1284,15 +1284,7 @@ String getReadableJSTypeName(Node n, boolean dereference) {
       ObjectType objectType = getJSTypeOrUnknown(n.getFirstChild()).dereference();
       if (objectType != null) {
         String propName = n.getLastChild().getString();
-        if (objectType.getConstructor() != null
-            && objectType.getConstructor().isInterface()) {
-          objectType = objectType.getTopDefiningInterface(propName);
-        } else {
-          // classes
-          while (objectType != null && !objectType.hasOwnProperty(propName)) {
-            objectType = objectType.getImplicitPrototype();
-          }
-        }
+        objectType = objectType.getClosestDefiningType(propName);
 
         // Don't show complex function names or anonymous types.
         // Instead, try to get a human-readable type name.

diff --git a/src/com/google/javascript/rhino/jstype/ObjectType.java b/src/com/google/javascript/rhino/jstype/ObjectType.java
@@ -53,6 +53,7 @@
 import com.google.javascript.rhino.JSDocInfo;
 import com.google.javascript.rhino.Node;
 import java.io.Serializable;
+import java.util.ArrayDeque;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
@@ -329,6 +330,27 @@ public final ObjectType getTopDefiningInterface(String propertyName) {
     return null;
   }
 
+  /** Returns the closest ancestor that defines the property including this type itself. */
+  public final ObjectType getClosestDefiningType(String propertyName) {
+    ArrayDeque<ObjectType> parentsToSeek = new ArrayDeque<>();
+    parentsToSeek.add(this);
+
+    // Make a bread-first search to find the closest ancestor with the property.
+    while (!parentsToSeek.isEmpty()) {
+      ObjectType parent = parentsToSeek.pollFirst();
+      if (parent.hasOwnProperty(propertyName)) {
+        return parent;
+      }
+
+      if (parent.getImplicitPrototype() != null) {
+        parentsToSeek.add(parent.getImplicitPrototype());
+      }
+      Iterables.addAll(parentsToSeek, parent.getCtorExtendedInterfaces());
+    }
+
+    return null;
+  }
+
   /**
    * Gets the implicit prototype (a.k.a. the {@code [[Prototype]]} property).
    */

diff --git a/test/com/google/javascript/jscomp/TypeCheckTest.java b/test/com/google/javascript/jscomp/TypeCheckTest.java
@@ -8225,22 +8225,47 @@ public void testOverriddenParams6() {
   @Test
   public void testOverriddenParams7() {
     testTypes(
-        "/** @constructor\n * @template T */ function Foo() {}" +
-        "/** @param {T} x */" +
-        "Foo.prototype.bar = function(x) { };" +
-        "/**\n" +
-        " * @constructor\n" +
-        " * @extends {Foo<string>}\n" +
-        " */ function SubFoo() {}" +
-        "/**\n" +
-        " * @param {number} x\n" +
-        " * @override\n" +
-        " */" +
-        "SubFoo.prototype.bar = function(x) {};",
-        "mismatch of the bar property type and the type of the " +
-        "property it overrides from superclass Foo\n" +
-        "original: function(this:Foo, string): undefined\n" +
-        "override: function(this:SubFoo, number): undefined");
+        lines(
+            "/** @interface */ function Foo() {}",
+            "/** @param {number} x */",
+            "Foo.prototype.bar = function(x) { };",
+            "/**",
+            " * @interface",
+            " * @extends {Foo}",
+            " */ function SubFoo() {}",
+            "/**",
+            " * @override",
+            " */",
+            "SubFoo.prototype.bar = function() {};",
+            "var subFoo = /** @type {SubFoo} */ (null);",
+            "subFoo.bar(true);"),
+        lines(
+            "actual parameter 1 of SubFoo.prototype.bar does not match formal parameter",
+            "found   : boolean",
+            "required: number"));
+  }
+
+  @Test
+  public void testOverriddenParams8() {
+    testTypes(
+        lines(
+            "/** @constructor\n * @template T */ function Foo() {}",
+            "/** @param {T} x */",
+            "Foo.prototype.bar = function(x) { };",
+            "/**",
+            " * @constructor",
+            " * @extends {Foo<string>}",
+            " */ function SubFoo() {}",
+            "/**",
+            " * @param {number} x",
+            " * @override",
+            " */",
+            "SubFoo.prototype.bar = function(x) {};"),
+        lines(
+            "mismatch of the bar property type and the type of the "
+                + "property it overrides from superclass Foo",
+            "original: function(this:Foo, string): undefined",
+            "override: function(this:SubFoo, number): undefined"));
   }
 
   @Test