Avoid infinite recursion in getPropertyTypeMap when two structural in…

…terfaces extends each other. ------------- Created by MOE: http://code.google.com/p/moe-java MOE_MIGRATED_REVID=100063769
google · Aug 7, 2015 · bc84d78 · bc84d78
1 parent fe6e279
commit bc84d78
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 11 deletions.
diff --git a/src/com/google/javascript/rhino/jstype/FunctionType.java b/src/com/google/javascript/rhino/jstype/FunctionType.java
@@ -1471,12 +1471,15 @@ public boolean isStructuralType() {
   @Override
   public Map<String, JSType> getPropertyTypeMap() {
     Map<String, JSType> propTypeMap = new HashMap<String, JSType>();
-    updatePropertyTypeMap(this, propTypeMap);
+    updatePropertyTypeMap(this, propTypeMap, new HashSet<FunctionType>());
     return propTypeMap;
   }
 
+  // cache is added to prevent infinite recursion when retrieving
+  // the super type: see testInterfaceExtendsLoop in TypeCheckTest.java
   private static void updatePropertyTypeMap(
-      FunctionType type, Map<String, JSType> propTypeMap) {
+      FunctionType type, Map<String, JSType> propTypeMap,
+      HashSet<FunctionType> cache) {
     if (type == null) { return; }
     // retrieve all property types on the prototype of this class
     ObjectType prototype = type.getPrototype();
@@ -1493,7 +1496,12 @@ private static void updatePropertyTypeMap(
     Iterable<ObjectType> iterable = type.getExtendedInterfaces();
     if (iterable != null) {
       for (ObjectType interfaceType : iterable) {
-        updatePropertyTypeMap(interfaceType.getConstructor(), propTypeMap);
+        FunctionType superConstructor = interfaceType.getConstructor();
+        if (superConstructor == null
+            || cache.contains(superConstructor)) { continue; }
+        cache.add(superConstructor);
+        updatePropertyTypeMap(superConstructor, propTypeMap, cache);
+        cache.remove(superConstructor);
       }
     }
   }

diff --git a/test/com/google/javascript/jscomp/TypeCheckTest.java b/test/com/google/javascript/jscomp/TypeCheckTest.java
@@ -123,8 +123,8 @@ public void testTypeCheck5() throws Exception {
 
   public void testTypeCheck6() throws Exception {
     testTypes(
-        "/**@return {void}*/function foo(){" +
-        "/** @type {undefined|number} */var a;if (a == foo())return;}");
+        "/**@return {void}*/function foo(){"
+        + "/** @type {undefined|number} */var a;if (a == foo())return;}");
   }
 
   public void testTypeCheck8() throws Exception {
@@ -140,12 +140,12 @@ public void testTypeCheck10() throws Exception {
   }
 
   public void testTypeCheck11() throws Exception {
-    testTypes("/**@type {!Number} */var a;" +
-        "/**@type {!String} */var b;" +
-        "a = b;",
-        "assignment\n" +
-        "found   : String\n" +
-        "required: Number");
+    testTypes("/**@type {!Number} */var a;"
+        + "/**@type {!String} */var b;"
+        + "a = b;",
+        "assignment\n"
+        + "found   : String\n"
+        + "required: Number");
   }
 
   public void testTypeCheck12() throws Exception {
@@ -10194,6 +10194,18 @@ public void testInterfaceExtendsLoop() throws Exception {
         new ArrayList<String>());
   }
 
+  public void testInterfaceExtendsLoop2() throws Exception {
+    // This should emit a warning. This test is still
+    // useful to ensure the compiler doesn't crash.
+    testClosureTypesMultipleWarnings(
+        suppressMissingProperty("foo") +
+            "/** @record \n * @extends {F} */var G = function() {};" +
+            "/** @record \n * @extends {G} */var F = function() {};" +
+            "/** @constructor \n * @implements {F} */var H = function() {};" +
+        "alert((new H).foo);",
+        new ArrayList<String>());
+  }
+
   public void testConversionFromInterfaceToRecursiveConstructor()
       throws Exception {
     testClosureTypesMultipleWarnings(