From aa5874513f85b1731b0749cde561141b7dd2bcf8 Mon Sep 17 00:00:00 2001 From: sdh Date: Wed, 21 Jun 2017 18:16:27 -0700 Subject: [PATCH] [NTI] Implement JSType#getSubTypes ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=159773350 --- .../jscomp/AmbiguateProperties.java | 13 +++------- .../jscomp/DisambiguateProperties.java | 9 ++++--- .../javascript/jscomp/newtypes/JSType.java | 25 ++++++++----------- .../javascript/rhino/FunctionTypeI.java | 9 ++++--- .../google/javascript/rhino/ObjectTypeI.java | 3 --- .../javascript/rhino/jstype/FunctionType.java | 21 ++++++++-------- .../rhino/jstype/JSTypeRegistry.java | 5 ++-- .../javascript/rhino/jstype/ObjectType.java | 8 +----- .../jscomp/AmbiguatePropertiesTest.java | 16 ++++++++++++ .../jscomp/DisambiguatePropertiesTest.java | 11 ++++++++ .../jscomp/FunctionTypeBuilderTest.java | 10 +++++--- .../javascript/rhino/jstype/JSTypeTest.java | 7 +++--- 12 files changed, 77 insertions(+), 60 deletions(-) diff --git a/src/com/google/javascript/jscomp/AmbiguateProperties.java b/src/com/google/javascript/jscomp/AmbiguateProperties.java index 3192ba0ac84..2450d240ad2 100644 --- a/src/com/google/javascript/jscomp/AmbiguateProperties.java +++ b/src/com/google/javascript/jscomp/AmbiguateProperties.java @@ -30,6 +30,7 @@ import com.google.javascript.jscomp.graph.GraphColoring.GreedyGraphColoring; import com.google.javascript.jscomp.graph.GraphNode; import com.google.javascript.jscomp.graph.SubGraph; +import com.google.javascript.rhino.FunctionTypeI; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.TypeI; import com.google.javascript.rhino.jstype.FunctionType; @@ -337,17 +338,11 @@ private void computeRelatedTypes(JSType type) { // An instance is related to its subclasses. FunctionType constructor = type.toObjectType().getConstructor(); - if (constructor != null && constructor.getSubTypes() != null) { - for (FunctionType subType : constructor.getSubTypes()) { - addRelatedInstance(subType, related); + if (constructor != null) { + for (FunctionTypeI subType : constructor.getSubTypes()) { + addRelatedInstance((FunctionType) subType, related); } } - - // An interface is related to its implementors. - for (FunctionType implementor : compiler.getTypeRegistry() - .getDirectImplementors(type.toObjectType())) { - addRelatedInstance(implementor, related); - } } /** diff --git a/src/com/google/javascript/jscomp/DisambiguateProperties.java b/src/com/google/javascript/jscomp/DisambiguateProperties.java index 8b75550461a..c275cfadb94 100644 --- a/src/com/google/javascript/jscomp/DisambiguateProperties.java +++ b/src/com/google/javascript/jscomp/DisambiguateProperties.java @@ -17,6 +17,7 @@ import static com.google.common.base.Preconditions.checkState; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableSet; @@ -779,6 +780,7 @@ private Map buildPropNames(Property prop) { } /** Returns a map from field name to types for which it will be renamed. */ + @VisibleForTesting Multimap> getRenamedTypesForTesting() { Multimap> ret = HashMultimap.create(); for (Map.Entry entry : properties.entrySet()) { @@ -886,11 +888,10 @@ private Iterable getTypeAlternatives(TypeI type) { return type.getUnionMembers(); } else { ObjectTypeI objType = type.toMaybeObjectType(); - if (objType != null - && objType.getConstructor() != null - && objType.getConstructor().isInterface()) { + FunctionTypeI constructor = objType != null ? objType.getConstructor() : null; + if (constructor != null && constructor.isInterface()) { List list = new ArrayList<>(); - for (FunctionTypeI impl : objType.getDirectImplementors()) { + for (FunctionTypeI impl : constructor.getSubTypes()) { list.add(impl.getInstanceType()); } return list.isEmpty() ? null : list; diff --git a/src/com/google/javascript/jscomp/newtypes/JSType.java b/src/com/google/javascript/jscomp/newtypes/JSType.java index 08b34bf14db..59174f423f6 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSType.java +++ b/src/com/google/javascript/jscomp/newtypes/JSType.java @@ -1856,8 +1856,17 @@ public final Node getSource() { } @Override - public final List getSubTypes() { - throw new UnsupportedOperationException("getSubTypes not implemented yet"); + public final Collection getSubTypes() { + Preconditions.checkState(this.isConstructor() || this.isInterface()); + ImmutableList.Builder result = ImmutableList.builder(); + NominalType nt = + getFunTypeIfSingletonObj().getInstanceTypeOfCtor().getNominalTypeIfSingletonObj(); + if (nt != null) { + for (RawNominalType rawType : nt.getSubtypes()) { + result.add(this.commonTypes.fromFunctionType(rawType.getConstructorFunction())); + } + } + return result.build(); } @Override @@ -2182,18 +2191,6 @@ public final TypeI getGreatestSubtypeWithProperty(String pname) { return joinManyTypes(this.commonTypes, getSubtypesWithProperty(new QualifiedName(pname))); } - @Override - public final Collection getDirectImplementors() { - Set result = new HashSet<>(); - NominalType nt = getNominalTypeIfSingletonObj(); - if (nt != null) { - for (RawNominalType rawType : nt.getSubtypes()) { - result.add(this.commonTypes.fromFunctionType(rawType.getConstructorFunction())); - } - } - return result; - } - @Override public final ObjectTypeI getPrototypeProperty() { return getProp(new QualifiedName("prototype")); diff --git a/src/com/google/javascript/rhino/FunctionTypeI.java b/src/com/google/javascript/rhino/FunctionTypeI.java index fd6d363daa8..8161168936e 100644 --- a/src/com/google/javascript/rhino/FunctionTypeI.java +++ b/src/com/google/javascript/rhino/FunctionTypeI.java @@ -76,11 +76,12 @@ public interface FunctionTypeI extends TypeI { Node getSource(); /** - * Returns a list of types that are subtypes of this type. This is only - * valid for constructor functions, and may be null. This allows a downward - * traversal of the subtype graph. + * Returns an iterable of direct types that are subtypes of this type. + * This is only valid for constructors and interfaces, and will not be + * null. This allows a downward traversal of the subtype graph. */ - List getSubTypes(); + // TODO(sdh): change the name to getDirectSubTypes() + Iterable getSubTypes(); /** Gets the type of {@code this} in this function. */ TypeI getTypeOfThis(); diff --git a/src/com/google/javascript/rhino/ObjectTypeI.java b/src/com/google/javascript/rhino/ObjectTypeI.java index 786e645972f..744ad9100e0 100644 --- a/src/com/google/javascript/rhino/ObjectTypeI.java +++ b/src/com/google/javascript/rhino/ObjectTypeI.java @@ -40,7 +40,6 @@ package com.google.javascript.rhino; import com.google.common.collect.ImmutableList; -import java.util.Collection; /** * @author blickly@google.com (Ben Lickly) @@ -133,8 +132,6 @@ public interface ObjectTypeI extends TypeI { */ TypeI getLegacyResolvedType(); - Collection getDirectImplementors(); - /** * Given an interface and a property, finds a top-most super interface * that has the property defined (including this interface). diff --git a/src/com/google/javascript/rhino/jstype/FunctionType.java b/src/com/google/javascript/rhino/jstype/FunctionType.java index b4f50461bc7..43d5de2fb6e 100644 --- a/src/com/google/javascript/rhino/jstype/FunctionType.java +++ b/src/com/google/javascript/rhino/jstype/FunctionType.java @@ -48,6 +48,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.javascript.rhino.ErrorReporter; import com.google.javascript.rhino.FunctionTypeI; import com.google.javascript.rhino.Node; @@ -150,7 +151,7 @@ private enum PropAccess { ANY, STRUCT, DICT } * The types which are subtypes of this function. It is only relevant for * constructors and may be {@code null}. */ - private List subTypes; + private List subTypes; /** Creates an instance for a function that might be a constructor. */ FunctionType( @@ -1246,8 +1247,8 @@ public void clearCachedValues() { super.clearCachedValues(); if (subTypes != null) { - for (FunctionType subType : subTypes) { - subType.clearCachedValues(); + for (FunctionTypeI subType : subTypes) { + ((FunctionType) subType).clearCachedValues(); } } @@ -1262,14 +1263,11 @@ public void clearCachedValues() { } } - /** - * Returns a list of types that are subtypes of this type. This is only valid - * for constructor functions, and may be null. This allows a downward - * traversal of the subtype graph. - */ @Override - public List getSubTypes() { - return subTypes; + public Iterable getSubTypes() { + return Iterables.concat( + subTypes != null ? subTypes : ImmutableList.of(), + this.registry.getDirectImplementors(this)); } @Override @@ -1318,8 +1316,9 @@ JSType resolveInternal(ErrorReporter t, StaticTypedScope scope) { if (subTypes != null) { for (int i = 0; i < subTypes.size(); i++) { + FunctionType subType = (FunctionType) subTypes.get(i); subTypes.set( - i, JSType.toMaybeFunctionType(subTypes.get(i).resolve(t, scope))); + i, JSType.toMaybeFunctionType(subType.resolve(t, scope))); } } diff --git a/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java b/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java index 063284c4c7a..11b02e20de5 100644 --- a/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java +++ b/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java @@ -56,6 +56,7 @@ import com.google.common.collect.LinkedHashMultimap; import com.google.common.collect.Multimap; import com.google.javascript.rhino.ErrorReporter; +import com.google.javascript.rhino.FunctionTypeI; import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.JSTypeExpression; import com.google.javascript.rhino.Node; @@ -184,7 +185,7 @@ public class JSTypeRegistry implements TypeIRegistry { new HashMap<>(); // A map from interface name to types that implement it. - private final Multimap interfaceToImplementors = + private final Multimap interfaceToImplementors = LinkedHashMultimap.create(); // All the unresolved named types. @@ -954,7 +955,7 @@ void registerTypeImplementingInterface( * be returned. {@code interfaceInstance} must be an ObjectType for the * instance of the interface. */ - public Collection getDirectImplementors(ObjectType interfaceInstance) { + public Collection getDirectImplementors(ObjectType interfaceInstance) { return interfaceToImplementors.get(interfaceInstance.getReferenceName()); } diff --git a/src/com/google/javascript/rhino/jstype/ObjectType.java b/src/com/google/javascript/rhino/jstype/ObjectType.java index 17f66c4369a..3286465bb30 100644 --- a/src/com/google/javascript/rhino/jstype/ObjectType.java +++ b/src/com/google/javascript/rhino/jstype/ObjectType.java @@ -53,7 +53,6 @@ import com.google.javascript.rhino.Node; import com.google.javascript.rhino.ObjectTypeI; import com.google.javascript.rhino.TypeI; -import java.util.Collection; import java.util.Map; import java.util.Set; import java.util.TreeSet; @@ -253,7 +252,7 @@ public boolean hasReferenceName() { } @Override - public boolean isUnknownObject() { + public final boolean isUnknownObject() { return !hasReferenceName(); } @@ -316,11 +315,6 @@ public ObjectType getTopDefiningInterface(String propertyName) { return foundType; } - @Override - public Collection getDirectImplementors() { - return this.registry.getDirectImplementors(this); - } - /** * Gets the implicit prototype (a.k.a. the {@code [[Prototype]]} property). */ diff --git a/test/com/google/javascript/jscomp/AmbiguatePropertiesTest.java b/test/com/google/javascript/jscomp/AmbiguatePropertiesTest.java index 6e833c6eaab..9e5651cfe24 100644 --- a/test/com/google/javascript/jscomp/AmbiguatePropertiesTest.java +++ b/test/com/google/javascript/jscomp/AmbiguatePropertiesTest.java @@ -687,6 +687,22 @@ public void testExtendsInterface() { test(js, output); } + public void testInterfaceWithSubInterfaceAndDirectImplementors() { + test( + LINE_JOINER.join( + "/** @interface */ function Foo(){};", + "Foo.prototype.foo = function(){};", + "/** @interface @extends {Foo} */ function Bar(){};", + "/** @suppress {checkTypes}\n @constructor @implements {Foo} */ function Baz(){};", + "Baz.prototype.baz = function(){};"), + LINE_JOINER.join( + "/** @interface */ function Foo(){};", + "Foo.prototype.b = function(){};", + "/** @interface @extends {Foo} */ function Bar(){};", + "/** @suppress {checkTypes}\n @constructor @implements {Foo} */ function Baz(){};", + "Baz.prototype.a = function(){};")); + } + public void testFunctionSubType() { String js = LINE_JOINER.join( "Function.prototype.a = 1;", diff --git a/test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java b/test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java index cc4a2d5b584..64df4096226 100644 --- a/test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java +++ b/test/com/google/javascript/jscomp/DisambiguatePropertiesTest.java @@ -1297,6 +1297,17 @@ public void testInterface_noDirectImplementors() { + " b=[[Foo.prototype, I.prototype], [Z.prototype]]}"); } + public void testInterface_subInterfaceAndDirectImplementors() { + String js = LINE_JOINER.join( + "/** @interface */ function I() {};", + "I.prototype.a;", + "/** @constructor @implements {I} */ function Foo() {};", + "Foo.prototype.a;", + "/** @interface @extends {I} */ function Bar() {};", + "Bar.prototype.a;"); + testSets(js, "{a=[[Bar.prototype, Foo.prototype, I.prototype]]}"); + } + public void testInterfaceOfSuperclass() { String js = "" + "/** @interface */ function I() {};\n" diff --git a/test/com/google/javascript/jscomp/FunctionTypeBuilderTest.java b/test/com/google/javascript/jscomp/FunctionTypeBuilderTest.java index 6bf1690564b..226395427ff 100644 --- a/test/com/google/javascript/jscomp/FunctionTypeBuilderTest.java +++ b/test/com/google/javascript/jscomp/FunctionTypeBuilderTest.java @@ -18,6 +18,8 @@ import static com.google.javascript.rhino.testing.BaseJSTypeTestCase.ALL_NATIVE_EXTERN_TYPES; +import com.google.common.collect.ImmutableList; +import com.google.javascript.rhino.FunctionTypeI; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.jstype.FunctionType; import com.google.javascript.rhino.jstype.ObjectType; @@ -139,9 +141,11 @@ public void testInlineJsDoc2() throws Exception { public void testExternSubTypes() throws Exception { testSame(ALL_NATIVE_EXTERN_TYPES, ""); - List subtypes = ((ObjectType) getLastCompiler() - .getTypeRegistry().getType("Error")).getConstructor().getSubTypes(); - for (FunctionType type : subtypes) { + List subtypes = + ImmutableList.copyOf( + ((ObjectType) getLastCompiler().getTypeRegistry().getType("Error")) + .getConstructor().getSubTypes()); + for (FunctionTypeI type : subtypes) { String typeName = type.getInstanceType().toString(); FunctionType typeInRegistry = ((ObjectType) getLastCompiler() .getTypeRegistry().getType(typeName)).getConstructor(); diff --git a/test/com/google/javascript/rhino/jstype/JSTypeTest.java b/test/com/google/javascript/rhino/jstype/JSTypeTest.java index a752acd894c..107a2f11d3f 100644 --- a/test/com/google/javascript/rhino/jstype/JSTypeTest.java +++ b/test/com/google/javascript/rhino/jstype/JSTypeTest.java @@ -51,6 +51,7 @@ import com.google.javascript.rhino.Node; import com.google.javascript.rhino.SimpleErrorReporter; import com.google.javascript.rhino.Token; +import com.google.javascript.rhino.TypeI; import com.google.javascript.rhino.jstype.JSType.TypePair; import com.google.javascript.rhino.testing.AbstractStaticScope; import com.google.javascript.rhino.testing.Asserts; @@ -6365,9 +6366,9 @@ public void testCanCastTo() { } private static boolean containsType( - Iterable types, JSType type) { - for (JSType alt : types) { - if (alt.isEquivalentTo(type)) { + Iterable types, JSType type) { + for (TypeI alt : types) { + if (alt.equals(type)) { return true; } }