From 2cdc02065b5eafecf598a883ee45397bc7657626 Mon Sep 17 00:00:00 2001 From: dimvar Date: Wed, 20 Jul 2016 15:16:23 -0700 Subject: [PATCH] Fix bug in DisambiguateProperties involving generic interfaces and interface inheritance. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=127994567 --- .../javascript/rhino/jstype/PropertyMap.java | 23 +++++-- .../rhino/jstype/ProxyObjectType.java | 8 ++- .../jscomp/DisambiguatePropertiesTest.java | 65 ++++++++++++++++++- .../javascript/jscomp/TypeCheckTest.java | 9 +-- 4 files changed, 92 insertions(+), 13 deletions(-) diff --git a/src/com/google/javascript/rhino/jstype/PropertyMap.java b/src/com/google/javascript/rhino/jstype/PropertyMap.java index d3a4301811e..f2fcfa68bbc 100644 --- a/src/com/google/javascript/rhino/jstype/PropertyMap.java +++ b/src/com/google/javascript/rhino/jstype/PropertyMap.java @@ -44,9 +44,11 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; -import java.util.HashSet; - +import com.google.common.collect.Sets; import java.io.Serializable; +import java.util.Collections; +import java.util.HashSet; +import java.util.IdentityHashMap; import java.util.Map; import java.util.Set; @@ -163,16 +165,29 @@ Set getOwnPropertyNames() { } void collectPropertyNames(Set props) { + Set identitySet = Sets.newIdentityHashSet(); + collectPropertyNamesHelper(props, identitySet); + } + + // The interface inheritance chain can have cycles. + // Use cache to avoid stack overflow. + private void collectPropertyNamesHelper( + Set props, Set cache) { + if (cache.contains(this)) { + return; + } + cache.add(this); props.addAll(properties.keySet()); PropertyMap primaryParent = getPrimaryParent(); if (primaryParent != null) { - primaryParent.collectPropertyNames(props); + primaryParent.collectPropertyNamesHelper(props, cache); } for (PropertyMap p : getSecondaryParents()) { if (p != null) { - p.collectPropertyNames(props); + p.collectPropertyNamesHelper(props, cache); } } + cache.remove(this); } boolean removeProperty(String name) { diff --git a/src/com/google/javascript/rhino/jstype/ProxyObjectType.java b/src/com/google/javascript/rhino/jstype/ProxyObjectType.java index df25ccff9c9..aa826c5ec19 100644 --- a/src/com/google/javascript/rhino/jstype/ProxyObjectType.java +++ b/src/com/google/javascript/rhino/jstype/ProxyObjectType.java @@ -44,7 +44,6 @@ import com.google.javascript.rhino.ErrorReporter; import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.Node; - import java.util.Collections; /** @@ -259,6 +258,13 @@ public Iterable getCtorImplementedInterfaces() { referencedObjType.getCtorImplementedInterfaces(); } + @Override + public Iterable getCtorExtendedInterfaces() { + return this.referencedObjType == null + ? Collections.emptyList() + : this.referencedObjType.getCtorExtendedInterfaces(); + } + @Override public int hashCode() { return referencedType.hashCode(); diff --git a/test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java b/test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java index af91dc007cd..5f970117a9f 100644 --- a/test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java +++ b/test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java @@ -23,7 +23,6 @@ import com.google.javascript.rhino.jstype.JSTypeRegistry; import com.google.javascript.rhino.testing.BaseJSTypeTestCase; import com.google.javascript.rhino.testing.TestErrorReporter; - import java.util.Collection; import java.util.HashMap; import java.util.Map; @@ -1830,6 +1829,70 @@ public void testStructuralInterfacesInExterns() { testSets(externs, js, js, "{}"); } + public void testPropInParentInterface1() { + String js = LINE_JOINER.join( + "/** @interface */", + "function MyIterable() {}", + "MyIterable.prototype.iterator = function() {};", + "/**", + " * @interface", + " * @extends {MyIterable}", + " * @template T", + " */", + "function MyCollection() {}", + "/**", + " * @constructor", + " * @implements {MyCollection}", + " */", + "function MyAbstractCollection() {}", + "/** @override */", + "MyAbstractCollection.prototype.iterator = function() {};"); + + testSets(js, "{iterator=[[MyAbstractCollection.prototype, MyIterable.prototype]]}"); + } + + public void testPropInParentInterface2() { + String js = LINE_JOINER.join( + "/** @interface */", + "function MyIterable() {}", + "MyIterable.prototype.iterator = function() {};", + "/**", + " * @interface", + " * @extends {MyIterable}", + " */", + "function MyCollection() {}", + "/**", + " * @constructor", + " * @implements {MyCollection}", + " */", + "function MyAbstractCollection() {}", + "/** @override */", + "MyAbstractCollection.prototype.iterator = function() {};"); + + testSets(js, "{iterator=[[MyAbstractCollection.prototype, MyIterable.prototype]]}"); + } + + public void testPropInParentInterface3() { + String js = LINE_JOINER.join( + "/** @interface */", + "function MyIterable() {}", + "MyIterable.prototype.iterator = function() {};", + "/**", + " * @interface", + " * @extends {MyIterable}", + " */", + "function MyCollection() {}", + "/**", + " * @constructor", + " * @implements {MyCollection}", + " */", + "function MyAbstractCollection() {}", + "/** @override */", + "MyAbstractCollection.prototype.iterator = function() {};"); + + testSets(js, js, "{iterator=[[MyAbstractCollection.prototype, MyIterable.prototype]]}"); + } + public void testErrorOnProtectedProperty() { testError("function addSingletonGetter(foo) { foo.foobar = 'a'; };", DisambiguateProperties.Warnings.INVALIDATION); diff --git a/test/com/google/javascript/jscomp/TypeCheckTest.java b/test/com/google/javascript/jscomp/TypeCheckTest.java index 87356ab8c77..730ec512987 100644 --- a/test/com/google/javascript/jscomp/TypeCheckTest.java +++ b/test/com/google/javascript/jscomp/TypeCheckTest.java @@ -31,7 +31,6 @@ import com.google.javascript.rhino.jstype.JSType; import com.google.javascript.rhino.jstype.JSTypeNative; import com.google.javascript.rhino.jstype.ObjectType; - import java.util.Arrays; import java.util.HashSet; import java.util.List; @@ -10146,17 +10145,13 @@ public void testInterfaceExtendsLoop() { } public void testInterfaceExtendsLoop2() { - testClosureTypesMultipleWarnings( + testClosureTypes( 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);", - ImmutableList.of( - "extends loop involving F, " - + "loop: F -> G -> F", - "extends loop involving G, " - + "loop: G -> F -> G")); + "Parse error. Cycle detected in inheritance chain of type F"); } public void testConversionFromInterfaceToRecursiveConstructor() {