From 1f7eaf46dc5cbe5b63fec1bad46fab6b2a0099fc Mon Sep 17 00:00:00 2001 From: sdh Date: Thu, 25 May 2017 19:24:05 -0700 Subject: [PATCH] [NTI] Clean up lint errors and add JavaDoc in ObjectType. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=157182146 --- .../javascript/jscomp/newtypes/JSType.java | 37 +- .../javascript/jscomp/newtypes/JSTypes.java | 5 + .../jscomp/newtypes/ObjectType.java | 451 ++++++++++++------ .../jscomp/newtypes/PersistentMap.java | 1 - .../jscomp/newtypes/TypeWithProperties.java | 29 +- 5 files changed, 366 insertions(+), 157 deletions(-) diff --git a/src/com/google/javascript/jscomp/newtypes/JSType.java b/src/com/google/javascript/jscomp/newtypes/JSType.java index 097626a4c87..b64e432269a 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSType.java +++ b/src/com/google/javascript/jscomp/newtypes/JSType.java @@ -1365,8 +1365,14 @@ public final JSType withLoose() { Preconditions.checkState(!getEnums().isEmpty()); return this; } - return makeType(this.commonTypes, getMask(), - ObjectType.withLooseObjects(getObjs()), getTypeVar(), getEnums()); + // TODO(dimvar): here, a lot of the time the set of objects will only contain objects for which + // withLoose is a no-op. Worth it to detect it and return `this` in that case, to avoid + // unnecessary creation of types? + ImmutableSet.Builder looseObjs = ImmutableSet.builder(); + for (ObjectType obj : getObjs()) { + looseObjs.add(obj.withLoose()); + } + return makeType(this.commonTypes, getMask(), looseObjs.build(), getTypeVar(), getEnums()); } public final JSType getProp(QualifiedName qname) { @@ -1437,8 +1443,11 @@ public final JSType withProperty(QualifiedName qname, JSType type) { if (isUnknown() || isBottom() || getObjs().isEmpty()) { return this; } - return makeType(this.commonTypes, getMask(), - ObjectType.withProperty(getObjs(), qname, type), getTypeVar(), getEnums()); + ImmutableSet.Builder newObjs = ImmutableSet.builder(); + for (ObjectType obj : getObjs()) { + newObjs.add(obj.withProperty(qname, type)); + } + return makeType(this.commonTypes, getMask(), newObjs.build(), getTypeVar(), getEnums()); } public final JSType withDeclaredProperty( @@ -1447,16 +1456,22 @@ public final JSType withDeclaredProperty( if (type == null && isConstant) { type = this.commonTypes.UNKNOWN; } - return makeType(this.commonTypes, - getMask(), - ObjectType.withDeclaredProperty(getObjs(), qname, type, isConstant), getTypeVar(), getEnums()); + ImmutableSet.Builder newObjs = ImmutableSet.builder(); + for (ObjectType obj : getObjs()) { + newObjs.add(obj.withDeclaredProperty(qname, type, isConstant)); + } + return makeType(this.commonTypes, getMask(), newObjs.build(), getTypeVar(), getEnums()); } public final JSType withPropertyRequired(String pname) { - return (isUnknown() || getObjs().isEmpty()) ? - this : - makeType(this.commonTypes, getMask(), - ObjectType.withPropertyRequired(getObjs(), pname), getTypeVar(), getEnums()); + if (isUnknown() || getObjs().isEmpty()) { + return this; + } + ImmutableSet.Builder newObjs = ImmutableSet.builder(); + for (ObjectType obj : getObjs()) { + newObjs.add(obj.withPropertyRequired(pname)); + } + return makeType(this.commonTypes, getMask(), newObjs.build(), getTypeVar(), getEnums()); } // For a type A, this method tries to return the greatest subtype of A that diff --git a/src/com/google/javascript/jscomp/newtypes/JSTypes.java b/src/com/google/javascript/jscomp/newtypes/JSTypes.java index 34595c913e2..938d2d93468 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSTypes.java +++ b/src/com/google/javascript/jscomp/newtypes/JSTypes.java @@ -546,4 +546,9 @@ public boolean isNumStrScalarOrObj(JSType t) { } return t.isSubtypeOf(anyNumOrStr); } + + @SuppressWarnings("ReferenceEquality") + boolean isBottomPropertyMap(PersistentMap map) { + return map == BOTTOM_PROPERTY_MAP; + } } diff --git a/src/com/google/javascript/jscomp/newtypes/ObjectType.java b/src/com/google/javascript/jscomp/newtypes/ObjectType.java index 58e9ab5dacd..d1ec72f645a 100644 --- a/src/com/google/javascript/jscomp/newtypes/ObjectType.java +++ b/src/com/google/javascript/jscomp/newtypes/ObjectType.java @@ -32,6 +32,10 @@ import java.util.TreeSet; /** + * {@link JSType}s include a possibly-empty set of ObjectType instances, + * representing the non-primitive components of a union type. An ObjectType + * instance includes information about the object: nominal type, function + * signature, and extra properties. * * @author blickly@google.com (Ben Lickly) * @author dimvar@google.com (Dimitris Vardoulakis) @@ -48,6 +52,7 @@ final class ObjectType implements TypeWithProperties { private final JSTypes commonTypes; + /** Creates a new "bottom" object. Should only be called in JSTypes. */ static ObjectType createBottomObject(JSTypes commonTypes) { return new ObjectType(commonTypes, commonTypes.getObjectType(), Preconditions.checkNotNull(commonTypes.BOTTOM_PROPERTY_MAP), @@ -148,11 +153,18 @@ JSTypes getCommonTypes() { return this.commonTypes; } + @SuppressWarnings("ReferenceEquality") boolean isInhabitable() { return this != this.commonTypes.getBottomObject(); } - static boolean containsBottomProp(PersistentMap props) { + /** + * Returns true if the property map contains bottom. To guarantee + * that no non-bottom objects have any bottom-typed properties, + * the caller should typically short-circuit a return of the bottom + * object if this occurs. + */ + private static boolean containsBottomProp(PersistentMap props) { for (Property p : props.values()) { if (p.getType().isBottom()) { return true; @@ -189,6 +201,10 @@ boolean isPrototypeObject() { return getOwnerFunction() != null; } + /** + * If this a prototype object (e.g. Foo.prototype), returns the + * associated constructor (e.g. Foo). Otherwise returns null. + */ FunctionType getOwnerFunction() { JSType t = getProp(new QualifiedName("constructor")); if (t != null && t.isFunctionType()) { @@ -209,14 +225,10 @@ private boolean hasNonPrototypeProperties() { return this.ns != null; } - static ImmutableSet withLooseObjects(Set objs) { - ImmutableSet.Builder newObjs = ImmutableSet.builder(); - for (ObjectType obj : objs) { - newObjs.add(obj.withLoose()); - } - return newObjs.build(); - } - + /** + * Returns true if the properties of obj are a subset of the + * properties of someBuiltinObj. + */ private static boolean hasOnlyBuiltinProps(ObjectType obj, ObjectType someBuiltinObj) { for (String pname : obj.props.keySet()) { if (!someBuiltinObj.mayHaveProp(new QualifiedName(pname))) { @@ -226,12 +238,13 @@ private static boolean hasOnlyBuiltinProps(ObjectType obj, ObjectType someBuilti return true; } - // Crude heuristic to decide whether a loose object is actually a scalar type - // and methods have been called on it. - // Does not apply to too-common properties such as toString (and for this - // reason it doesn't apply to booleans). - // Only uses property names; change it to use types if precision isn't - // satisfactory. + /** + * Applies a crude heuristic to decide whether a loose object is actually a + * scalar type (specifically number or string) based on methods that have been + * called on it. Does not apply to too-common properties such as toString (and + * for this reason it doesn't apply to booleans). Only uses property names, + * though it could be changed to use types if precision isn't satisfactory. + */ static JSType mayTurnLooseObjectToScalar(JSType t, JSTypes commonTypes) { ObjectType obj = t.getObjTypeIfSingletonObj(); if (obj == null || !obj.isLoose() || obj.props.isEmpty() || obj.fn != null @@ -249,22 +262,24 @@ static JSType mayTurnLooseObjectToScalar(JSType t, JSTypes commonTypes) { return t; } - // Trade-offs about property behavior on loose object types: - // We never mark properties as optional on loose objects. The reason is that - // we cannot know for sure when a property is optional or not. - // Eg, when we see an assignment to a loose obj - // obj.p1 = 123; - // we cannot know if obj already has p1, or if this is a property creation. - // If the assignment is inside an IF branch, we should not say after the IF - // that p1 is optional. But as a consequence, this means that any property we - // see on a loose object might be optional. That's why we don't warn about - // possibly-inexistent properties on loose objects. - // Last, say we infer a loose object type with a property p1 for a formal - // parameter of a function f. If we pass a non-loose object to f that does not - // have a p1, we warn. This may create spurious warnings, if p1 is optional, - // but mostly it catches real bugs. - - private ObjectType withLoose() { + /** + * Returns a "loose" version of this object type. + * + * Trade-offs about property behavior on loose object types: + * (1) We never mark properties as optional on loose objects. The reason is + * that we cannot know for sure when a property is optional or not + * (e.g, when we see an assignment to a loose object `obj.p1 = 123`, + * we cannot know if obj already has p1, or if this is a property creation). + * (2) If the assignment is inside an IF branch, we should not say after the IF + * that p1 is optional. But as a consequence, this means that any property we + * see on a loose object might be optional. That's why we don't warn about + * possibly-inexistent properties on loose objects. + * (3) Last, say we infer a loose object type with a property p1 for a formal + * parameter of a function f. If we pass a non-loose object to f that does not + * have a p1, we warn. This may create spurious warnings, if p1 is optional, + * but mostly it catches real bugs. + */ + ObjectType withLoose() { if (isTopObject()) { return this.commonTypes.getLooseTopObjectType(); } @@ -288,6 +303,11 @@ private ObjectType withLoose() { this.commonTypes, this.nominalType, newProps, fn, null, true, this.objectKind); } + /** + * Returns a version of this object with the given function signature. + * This only makes sense if (1) this is a namespace, and (2) the function + * is not loose (with the exception of the question-mark function). + */ ObjectType withFunction(FunctionType ft, NominalType fnNominal) { Preconditions.checkState(this.isNamespace()); Preconditions.checkState(!ft.isLoose() || ft.isQmarkFunction()); @@ -297,8 +317,8 @@ ObjectType withFunction(FunctionType ft, NominalType fnNominal) { return obj; } - static ImmutableSet withoutProperty( - Set objs, QualifiedName qname) { + /** Returns a transformed set of object types without the given property. */ + static ImmutableSet withoutProperty(Set objs, QualifiedName qname) { ImmutableSet.Builder newObjs = ImmutableSet.builder(); for (ObjectType obj : objs) { newObjs.add(obj.withProperty(qname, null)); @@ -306,11 +326,15 @@ static ImmutableSet withoutProperty( return newObjs.build(); } - // When type is null, this method removes the property. - // If the property is already declared, but isDeclared is false, be careful - // to not un-declare it. - // If the property is already constant, but isConstant is false, be careful - // to not un-const it. + /** + * Helper method to return a new object with the property added, updated, + * or removed. If the type is null, the property is removed. Declared + * and constant are "one-way" switches: a declared property will not be + * made un-declared, nor a const property made non-constant. If a + * property is already inferred with the correct type, it will not be + * explicitly added. + */ + @SuppressWarnings("ReferenceEquality") private ObjectType withPropertyHelper(QualifiedName qname, JSType type, boolean isDeclared, boolean isConstant) { // TODO(dimvar): We do some short-circuiting based on the declared type, @@ -381,30 +405,28 @@ private ObjectType withPropertyHelper(QualifiedName qname, JSType type, this.fn, this.ns, this.isLoose, this.objectKind); } - // When type is null, this method removes the property. + /** + * Returns a new object type with the given property added or + * (if type is null) removed. + */ ObjectType withProperty(QualifiedName qname, JSType type) { return withPropertyHelper(qname, type, false, false); } - static ImmutableSet withProperty( - Set objs, QualifiedName qname, JSType type) { - ImmutableSet.Builder newObjs = ImmutableSet.builder(); - for (ObjectType obj : objs) { - newObjs.add(obj.withProperty(qname, type)); - } - return newObjs.build(); - } - - static ImmutableSet withDeclaredProperty(Set objs, - QualifiedName qname, JSType type, boolean isConstant) { - ImmutableSet.Builder newObjs = ImmutableSet.builder(); - for (ObjectType obj : objs) { - newObjs.add(obj.withPropertyHelper(qname, type, true, isConstant)); - } - return newObjs.build(); + /** + * Returns a new object type with the given declared property added. + * Passing null for the type will mark an inferred property as + * declared, rather than removing it. + */ + ObjectType withDeclaredProperty(QualifiedName qname, JSType type, boolean isConstant) { + return withPropertyHelper(qname, type, true, isConstant); } - private ObjectType withPropertyRequired(String pname) { + /** + * Returns a new object type with the named property marked as required. + * If the property is not present, it will be typed as unknown. + */ + ObjectType withPropertyRequired(String pname) { Property oldProp = this.props.get(pname); Property newProp = oldProp == null ? Property.make(this.commonTypes.UNKNOWN, null) @@ -413,15 +435,15 @@ private ObjectType withPropertyRequired(String pname) { this.fn, this.ns, this.isLoose, this.objectKind); } - static ImmutableSet withPropertyRequired( - Set objs, String pname) { - ImmutableSet.Builder newObjs = ImmutableSet.builder(); - for (ObjectType obj : objs) { - newObjs.add(obj.withPropertyRequired(pname)); - } - return newObjs.build(); - } - + /** + * Meets the given property maps. The returned map will be a union of + * the input maps (since the set of properties is contravariant). When + * a property is present in both, the result will be either the meet + * of the two types, or if specializeProps1 is given, it will be the + * type from props1 specialized by the type from props2. The + * resultNominalType should be the nominal type that belongs with the + * returned property map (i.e. of the met or specialized type). + */ private static PersistentMap meetPropsHelper( JSTypes commonTypes, boolean specializeProps1, NominalType resultNominalType, @@ -435,7 +457,7 @@ private static PersistentMap meetPropsHelper( if (otherProp != null) { newProps = addOrRemoveProp( specializeProps1, newProps, pname, otherProp, propsEntry.getValue()); - if (newProps == commonTypes.BOTTOM_PROPERTY_MAP) { + if (commonTypes.isBottomPropertyMap(newProps)) { return commonTypes.BOTTOM_PROPERTY_MAP; } } @@ -458,7 +480,7 @@ private static PersistentMap meetPropsHelper( Property otherProp = resultNominalType.getProp(pname); if (otherProp != null) { newProps = addOrRemoveProp(specializeProps1, newProps, pname, otherProp, newProp); - if (newProps == commonTypes.BOTTOM_PROPERTY_MAP) { + if (commonTypes.isBottomPropertyMap(newProps)) { return commonTypes.BOTTOM_PROPERTY_MAP; } } else { @@ -471,6 +493,12 @@ private static PersistentMap meetPropsHelper( return newProps; } + /** + * Utility method used by meetPropsHelper. Adds the property if the result + * type is a known proper subtype of the nominal type's property, otherwise + * removes (since there is nothing new beyond what's already stored in the + * nominal type). Returns bottom map if the result type would be bottom. + */ private static PersistentMap addOrRemoveProp( boolean specializeProps1, PersistentMap props, String pname, Property nomProp, Property objProp) { @@ -490,6 +518,10 @@ private static PersistentMap addOrRemoveProp( return props.without(pname); } + /** + * Looks up the property from the property map, falling back on the given + * nominal type if it's not in the map. + */ private static Property getProp(Map props, NominalType nom, String pname) { if (props.containsKey(pname)) { return props.get(pname); @@ -499,10 +531,12 @@ private static Property getProp(Map props, NominalType nom, St return null; } - // This method needs the nominal types because otherwise a property may become - // optional by mistake after the join. - // joinPropsLoosely doesn't need that, because we don't create optional props - // on loose types. + /** + * Joins two property maps. The nominal types are required because otherwise + * a property may become optional by mistake after the join. This is not + * necessary for {@link #joinPropsLoosely}, because we don't create optional + * props on loose types. + */ private static PersistentMap joinProps( Map props1, Map props2, NominalType nom1, NominalType nom2) { @@ -523,18 +557,22 @@ private static PersistentMap joinProps( return newProps; } + /** + * Loosely joins two property maps. Properties that are only present in + * one or the other are added as "required" properties to the result. + * Properties in both maps are joined as normal. + */ private static PersistentMap joinPropsLoosely( - JSTypes commonTypes, - Map props1, Map props2) { + JSTypes commonTypes, Map props1, Map props2) { + // Note: If ever newProps == BOTTOM_PROPERTY_MAP, it could be returned early, + // but as long as it only ever comes from with(), that is impossible. We may + // want to bail out early if either props1 or props2 is bottom. PersistentMap newProps = PersistentMap.create(); for (Map.Entry propsEntry : props1.entrySet()) { String pname = propsEntry.getKey(); if (!props2.containsKey(pname)) { newProps = newProps.with(pname, propsEntry.getValue().withRequired()); } - if (newProps == commonTypes.BOTTOM_PROPERTY_MAP) { - return commonTypes.BOTTOM_PROPERTY_MAP; - } } for (Map.Entry propsEntry : props2.entrySet()) { String pname = propsEntry.getKey(); @@ -545,25 +583,34 @@ private static PersistentMap joinPropsLoosely( } else { newProps = newProps.with(pname, prop2.withRequired()); } - if (newProps == commonTypes.BOTTOM_PROPERTY_MAP) { - return commonTypes.BOTTOM_PROPERTY_MAP; - } } return newProps; } + /** + * Returns true if the union objs1 is a subtype of the union objs2. + * This means that every member of objs1 has at least one supertype + * in objs2. If keepLoosenessOfThis is false, the looseness of + * members of objs1 will be disregarded. + */ static boolean isUnionSubtype(boolean keepLoosenessOfThis, Set objs1, Set objs2, SubtypeCache subSuperMap) { return isUnionSubtypeHelper(keepLoosenessOfThis, objs1, objs2, subSuperMap, null); } + /** + * Fills boxedInfo (a single-element "output" array) with the reason + * why objs1 is not a subtype of objs2, for the purpose of building + * informative error messages. + * @throws IllegalArgumentException if objs1 actually is a subtype of objs2. + */ static void whyNotUnionSubtypes(boolean keepLoosenessOfThis, Set objs1, Set objs2, SubtypeCache subSuperMap, MismatchInfo[] boxedInfo) { Preconditions.checkArgument(boxedInfo.length == 1); boolean areSubtypes = isUnionSubtypeHelper( keepLoosenessOfThis, objs1, objs2, subSuperMap, boxedInfo); - Preconditions.checkState(!areSubtypes); + Preconditions.checkArgument(!areSubtypes); } private static boolean isUnionSubtypeHelper(boolean keepLoosenessOfThis, @@ -588,10 +635,17 @@ private static boolean isUnionSubtypeHelper(boolean keepLoosenessOfThis, return true; } + /** Returns true if this is a subtype of obj2. */ boolean isSubtypeOf(ObjectType obj2, SubtypeCache subSuperMap) { return isSubtypeOfHelper(true, obj2, subSuperMap, null); } + /** + * Fills boxedInfo (a single-element "output" array) with the reason + * why obj1 is not a subtype of obj2, for the purpose of building + * informative error messages. + * @throws IllegalArgumentException if obj1 actually is a subtype of obj2. + */ static void whyNotSubtypeOf(ObjectType obj1, ObjectType obj2, MismatchInfo[] boxedInfo) { Preconditions.checkArgument(boxedInfo.length == 1); boolean areSubtypes = obj1.isSubtypeOfHelper(true, obj2, SubtypeCache.create(), boxedInfo); @@ -602,7 +656,7 @@ static void whyNotSubtypeOf(ObjectType obj1, ObjectType obj2, MismatchInfo[] box * Required properties are acceptable where an optional is required, * but not vice versa. * Optional properties create cycles in the type lattice, eg, - * { } \le { p: num= } and also { p: num= } \le { }. + * { } ≤ { p: num= } and also { p: num= } ≤ { }. */ private boolean isSubtypeOfHelper(boolean keepLoosenessOfThis, ObjectType other, SubtypeCache subSuperMap, MismatchInfo[] boxedInfo) { @@ -667,41 +721,52 @@ private boolean isSubtypeOfHelper(boolean keepLoosenessOfThis, return areFunsSubtypes; } - // NOTE(dimvar): it's not ideal that the unquoted properties of the - // object literal are checked as part of the IObject type. We want - // a property to either always be accessed with dot or with brackets, - // and checking the unquoted properties against IObject gives the - // impression that we support both kinds of accesses for the same - // property. The alternatives are (none is very satisfactory): - // 1) Don't check any object-literal properties against IObject - // 2) Check all object-literal properties against IObject (what we're currently doing) - // 3) Check only the quoted object-literal properties against IObject. - // This is not great because NTI also checks quoted properties individually - // if the name is known. - // 4) Remember in the property map whether a property name was declared as - // quoted or not. This will likely involve a lot of extra plumbing. - private boolean compareRecordTypeToIObject( - NominalType otherNt, SubtypeCache subSuperMap) { - JSType keyType = otherNt.getIndexType(); - JSType valueType = otherNt.getIndexedType(); - for (Map.Entry entry : this.props.entrySet()) { - String pname = entry.getKey(); - JSType ptype = entry.getValue().getType(); - if (keyType.isNumber() && Ints.tryParse(pname) == null) { - return false; - } - if (!keyType.isNumber() && !keyType.isString() && !keyType.isUnknown()) { - return false; - } - // Bracket accesses on the IObject (or on an Array) can generally return undefined - // and we don't warn about that; so ignore undefined for the object literal as well. - if (!ptype.removeType(this.commonTypes.UNDEFINED).isSubtypeOf(valueType, subSuperMap)) { - return false; - } - } - return true; - } + /** + * Special case of isSubtypeOf for testing a builtin or literal object as a + * subtype of IObject⟨KEY, VALUE⟩. Specifically, checks that all properties + * have the correct key and value type. + * + * NOTE(dimvar): it's not ideal that the unquoted properties of the + * object literal are checked as part of the IObject type. We want + * a property to either always be accessed with dot or with brackets, + * and checking the unquoted properties against IObject gives the + * impression that we support both kinds of accesses for the same + * property. The alternatives are (none is very satisfactory): + * (1) Don't check any object-literal properties against IObject + * (2) Check all object-literal properties against IObject (what we're currently doing) + * (3) Check only the quoted object-literal properties against IObject. + * This is not great because NTI also checks quoted properties individually + * if the name is known. + * (4) Remember in the property map whether a property name was declared as + * quoted or not. This will likely involve a lot of extra plumbing. + */ + private boolean compareRecordTypeToIObject(NominalType otherNt, SubtypeCache subSuperMap) { + JSType keyType = otherNt.getIndexType(); + if (!keyType.isNumber() && !keyType.isString() && !keyType.isUnknown()) { + return this.props.isEmpty(); + } + JSType valueType = otherNt.getIndexedType(); + for (Map.Entry entry : this.props.entrySet()) { + String pname = entry.getKey(); + JSType ptype = entry.getValue().getType(); + if (keyType.isNumber() && Ints.tryParse(pname) == null) { + return false; + } + // TODO(sdh): support string-enum keys. + // Bracket accesses on the IObject (or on an Array) can generally return undefined + // and we don't warn about that; so ignore undefined for the object literal as well. + if (!ptype.removeType(this.commonTypes.UNDEFINED).isSubtypeOf(valueType, subSuperMap)) { + return false; + } + } + return true; + } + /** + * Iterates over the specified properties of the other object and tests that + * all corresponding properties in this object are subtypes of other's + * properties. Fills in boxedInfo if non-null. + */ private boolean arePropertiesSubtypes(ObjectType other, Set otherPropNames, SubtypeCache subSuperMap, MismatchInfo[] boxedInfo) { @@ -728,6 +793,10 @@ private boolean arePropertiesSubtypes(ObjectType other, return true; } + /** + * Returns true if prop1 (which may be null, representing an absent property) + * is a subtype of prop2. Fills in boxedInfo if non-null. + */ private static boolean isPropertySubtype(String pname, Property prop1, Property prop2, SubtypeCache subSuperMap, MismatchInfo[] boxedInfo) { return boxedInfo != null @@ -778,8 +847,15 @@ private static boolean getPropMismatchInfo(String pname, Property prop1, return true; } - // We never infer properties as optional on loose objects, - // and we don't warn about possibly inexistent properties. + /** + * Returns true if this is a "loose subtype" of other. This only makes + * sense if either this or other is a loose object. This takes into + * account whether this object is a struct, and if all the common + * properties are in a correct subtype relationship. + * + * Note that we never infer properties as optional on loose objects, + * but also don't warn about possibly-inexistent properties. + */ boolean isLooseSubtypeOf(ObjectType other, SubtypeCache subSuperMap) { Preconditions.checkState(isLoose || other.isLoose); if (other.isTopObject()) { @@ -820,6 +896,10 @@ boolean isLooseSubtypeOf(ObjectType other, SubtypeCache subSuperMap) { return fn.isLooseSubtypeOf(other.fn); } + /** + * Specializes this object with the {@code other} (related) object type. + * TODO(sdh): explain a bit more concretely how this is different from meet. + */ ObjectType specialize(ObjectType other) { Preconditions.checkState(areRelatedNominalTypes(this.nominalType, other.nominalType)); if (isTopObject() && other.objectKind.isUnrestricted()) { @@ -840,7 +920,7 @@ ObjectType specialize(ObjectType other) { Preconditions.checkState(this.fn == null && other.fn == null); PersistentMap newProps = meetPropsHelper(this.commonTypes, true, resultNomType, this.props, other.props); - if (newProps == this.commonTypes.BOTTOM_PROPERTY_MAP) { + if (this.commonTypes.isBottomPropertyMap(newProps)) { return this.commonTypes.getBottomObject(); } return new ObjectType( @@ -857,7 +937,7 @@ ObjectType specialize(ObjectType other) { } PersistentMap newProps = meetPropsHelper(this.commonTypes, true, resultNomType, this.props, other.props); - if (newProps == this.commonTypes.BOTTOM_PROPERTY_MAP) { + if (this.commonTypes.isBottomPropertyMap(newProps)) { return this.commonTypes.getBottomObject(); } FunctionType newFn = thisFn == null ? null : thisFn.specialize(other.fn); @@ -868,8 +948,12 @@ ObjectType specialize(ObjectType other) { this.commonTypes, resultNomType, newProps, newFn, this.ns, isLoose, this.objectKind); } - // If obj represents a type of the form {p1: p2: {... {p_n: A}}} - // then return the path p1,p2,...,p_n. Otherwise, return null. + /** + * Determines if the given object type is of the form {@code + * {p1: {p2: { ... {p_n: A}}}} + * }. If so, returns the full qualified name "p1.p2.....p_n". + * Otherwise, returns null. + */ private static QualifiedName getPropertyPath(ObjectType obj) { if (obj.props.size() != 1) { return null; @@ -888,12 +972,20 @@ private static QualifiedName getPropertyPath(ObjectType obj) { return QualifiedName.join(leftmostPname, restPath); } - // Specializing namespace types is very expensive; not just the operation - // itself, but also the fact that you create a large type that you flow around - // later and many other expensive type operations happen on it. - // Therefore, we only specialize namespace types in a very specific case: to - // narrow down a mutable namespace field that has a union type, eg, - // if (goog.bar.baz !== null) { ... } + /** + * Handle the specific case of specializing a namespace (note that other + * is not necessarily a namespace). Because specializing namespaces is + * very expensive (not just the operation itself, but also the fact that + * you create a large type that you flow around later and many other + * expensive type operations happen on it), we only specialize namespace + * types in a very specific case: to narrow down a mutable namespace field + * that has a union type, e.g., {@code + * if (goog.bar.baz !== null) { ... } + * }. In this case, other's nominal type is Object, and other has a single + * "property path". In all other cases, we just return this namespace + * object unchanged. + */ + @SuppressWarnings("ReferenceEquality") ObjectType specializeNamespace(ObjectType other) { Preconditions.checkNotNull(this.ns); if (this == other @@ -920,18 +1012,24 @@ ObjectType specializeNamespace(ObjectType other) { return this; } + @SuppressWarnings("ReferenceEquality") private boolean isTopObject() { // Reference equality because we want to make sure that we only ever create // one top object type. return this == this.commonTypes.getTopObjectType(); } + @SuppressWarnings("ReferenceEquality") private boolean isBottomObject() { // Reference equality because we want to make sure that we only ever create // one bottom object type. return this == this.commonTypes.getBottomObject(); } + /** + * Returns true if this is the type of an enum namespace (as opposed to the + * type of the individual elements). + */ boolean isEnumObject() { if (!this.nominalType.isLiteralObject()) { return false; @@ -947,6 +1045,16 @@ boolean isEnumObject() { return e != null && this.equals(e.toJSType().getObjTypeIfSingletonObj()); } + /** + * Computes the intersection type between two object types. Generally + * this involves picking the subclass between the (related) nominal + * types, a property map with properties from both object types (with + * meets when properties overlap), and meeting any functions. If the + * result is loose (either because both arguments are loose, or because + * it is a loose function) then common properties are joined, instead. + * Namespaces are not handled: a common namespace is preserved, but + * otherwise will be dropped from the result. + */ static ObjectType meet(ObjectType obj1, ObjectType obj2) { NominalType nt1 = obj1.nominalType; NominalType nt2 = obj2.nominalType; @@ -963,18 +1071,19 @@ static ObjectType meet(ObjectType obj1, ObjectType obj2) { if (!FunctionType.isInhabitable(fn)) { return commonTypes.getBottomObject(); } - boolean isLoose = (obj1.isLoose && obj2.isLoose) || fn != null && fn.isLoose(); + boolean isLoose = (obj1.isLoose && obj2.isLoose) || (fn != null && fn.isLoose()); if (resultNomType.isFunction() && fn == null) { fn = obj1.fn == null ? obj2.fn : obj1.fn; isLoose = fn.isLoose(); } PersistentMap props; if (isLoose) { + // Do a simple union of the maps props = joinPropsLoosely(commonTypes, obj1.props, obj2.props); } else { props = meetPropsHelper(commonTypes, false, resultNomType, obj1.props, obj2.props); } - if (props == commonTypes.BOTTOM_PROPERTY_MAP) { + if (commonTypes.isBottomPropertyMap(props)) { return commonTypes.getBottomObject(); } ObjectKind ok = ObjectKind.meet(obj1.objectKind, obj2.objectKind); @@ -982,6 +1091,13 @@ static ObjectType meet(ObjectType obj1, ObjectType obj2) { return new ObjectType(commonTypes, resultNomType, props, fn, resultNs, isLoose, ok); } + /** + * Computes the union type between two object types. Generally this + * involves picking the superclass between the (related) nominal types, + * the union of the property maps (joining on overlaps), and joining + * any functions. Namespaces are not handled: a common namespace is + * preserved, but otherwise will be dropped from the result. + */ static ObjectType join(ObjectType obj1, ObjectType obj2) { if (obj1.isTopObject() || obj2.isTopObject()) { return obj1.commonTypes.getTopObjectType(); @@ -1023,6 +1139,12 @@ static ObjectType join(ObjectType obj1, ObjectType obj2) { ObjectKind.join(obj1.objectKind, obj2.objectKind)); } + /** + * Joins two sets of object types. This is somewhat complicated + * because individual pairs of ObjectTypes may only be joined if + * their nominal types are related. All such pairs are joined + * recursively. + */ static ImmutableSet joinSets( ImmutableSet objs1, ImmutableSet objs2) { if (objs1.isEmpty()) { @@ -1067,14 +1189,20 @@ static ImmutableSet joinSets( return builder.build(); } + /** Returns true if either nominal type is a subtype of the other. */ private static boolean areRelatedNominalTypes(NominalType c1, NominalType c2) { return c1.isNominalSubtypeOf(c2) || c2.isNominalSubtypeOf(c1); } - // TODO(dimvar): handle greatest lower bound of interface types. - // If we do that, we need to normalize the output, otherwise it could contain - // two object types that are in a subtype relation, eg, see - // NewTypeInferenceTest#testDifficultObjectSpecialization. + /** + * Utility method extracting common functionality between + * {@link #meetSets} and {@link #specializeSet}. + * + * TODO(dimvar): handle greatest lower bound of interface types. + * If we do that, we need to normalize the output, otherwise it could + * contain two object types that are in a subtype relation, e.g., see + * {@link NewTypeInferenceTest#testDifficultObjectSpecialization}. + */ static ImmutableSet meetSetsHelper( boolean specializeObjs1, Set objs1, Set objs2) { ObjectsBuilder newObjs = new ObjectsBuilder(); @@ -1103,10 +1231,23 @@ static ImmutableSet meetSetsHelper( return newObjs.build(); } + /** + * Meets two sets of object types by distributing the meet over + * the union: (A|B)∧(C|D) = (A∧C)|(A∧D)|(B∧C)|(B∧D), though an + * individual meet will only show up in the result if the two + * types are related - that is, one is a subclass of the other, + * either nominally (for classes and interfaces) or structurally + * (for records). All other cases meet to bottom and are therefore + * excluded from the result. + */ static ImmutableSet meetSets(Set objs1, Set objs2) { return meetSetsHelper(false, objs1, objs2); } + /** + * Similar to {@link #meetSets}, but related nominal types are + * specalized instead of met. + */ static ImmutableSet specializeSet(Set objs1, Set objs2) { return meetSetsHelper(true, objs1, objs2); } @@ -1141,6 +1282,12 @@ public JSType getDeclaredProp(QualifiedName qname) { return p.getType().getDeclaredProp(qname.getAllButLeftmost()); } + /** + * Returns the property corresponding to the left-most component of the + * qname. All other components of qname are ignored. First checks the + * property map, then the namespace, and finally the nominal type. + * Returns null if no property is found. + */ private Property getLeftmostProp(QualifiedName qname) { String pname = qname.getLeftmostName(); Property p = props.get(pname); @@ -1156,9 +1303,13 @@ private Property getLeftmostProp(QualifiedName qname) { return this.nominalType.getProp(pname); } - // NOTE(aravindpg): This method deliberately does not return the more specialized version of - // a property if it is already present on the nominal type. This may be unsuitable from a - // typing point of view. Revisit if needed. + /** + * Similar to {@link #getLeftmostProp}, but does not return the specialized + * versions of any properties already present on the nominal type. + * + * TODO(aravindpg): This may be unsuitable from a typing point of view. + * Revisit if needed. + */ private Property getLeftmostOwnProp(QualifiedName qname) { String pname = qname.getLeftmostName(); Property p = props.get(pname); @@ -1176,15 +1327,24 @@ private Property getLeftmostOwnProp(QualifiedName qname) { return this.nominalType.getOwnProp(pname); } + /** + * Returns the node that defines the given property, or null if the + * property does not exist. May return definitions on supertypes. + */ Node getPropertyDefSite(String propertyName) { return getPropertyDefSiteHelper(propertyName, false); } + /** + * Returns the node that defines the given property on this exact + * object, or null if the property does not exist (or only exists + * on supertypes). + */ Node getOwnPropertyDefSite(String propertyName) { return getPropertyDefSiteHelper(propertyName, true); } - Node getPropertyDefSiteHelper(String propertyName, boolean ownProp) { + private Node getPropertyDefSiteHelper(String propertyName, boolean ownProp) { QualifiedName qname = new QualifiedName(propertyName); Property p = ownProp ? getLeftmostOwnProp(qname) : getLeftmostProp(qname); // Try getters and setters specially. @@ -1212,6 +1372,10 @@ public boolean hasProp(QualifiedName qname) { return p != null; } + /** + * Similar to {@link #hasProp}, but disregards properties that are + * only defined on supertypes. + */ boolean hasOwnProperty(QualifiedName qname) { Preconditions.checkArgument(qname.isIdentifier()); Property p = getLeftmostOwnProp(qname); @@ -1353,6 +1517,12 @@ private boolean unifyPropsWithSubtype(ObjectType other, return true; } + /** + * Takes a map M from type variables to concrete types. For each *free* + * type variable in this type, if the type variable is in the domain of + * M, then it gets deeply replaced with the corresponding concrete type + * in the returned ObjectType. + */ ObjectType substituteGenerics(Map concreteTypes) { if (isTopObject() || concreteTypes.isEmpty()) { return this; @@ -1371,10 +1541,15 @@ ObjectType substituteGenerics(Map concreteTypes) { newProps, newFn, this.ns, - newFn != null && newFn.isQmarkFunction() || isLoose, + (newFn != null && newFn.isQmarkFunction()) || isLoose, this.objectKind); } + /** + * Looks for the given property name (which must be a non-qualified + * identifier) on any nominal subtypes. The built-in object type + * always returns true. + */ boolean isPropDefinedOnSubtype(QualifiedName pname) { Preconditions.checkArgument(pname.isIdentifier()); return this.nominalType.isBuiltinObject() || this.nominalType.isPropDefinedOnSubtype(pname); diff --git a/src/com/google/javascript/jscomp/newtypes/PersistentMap.java b/src/com/google/javascript/jscomp/newtypes/PersistentMap.java index add0829c852..55c7aa7b2d3 100644 --- a/src/com/google/javascript/jscomp/newtypes/PersistentMap.java +++ b/src/com/google/javascript/jscomp/newtypes/PersistentMap.java @@ -49,5 +49,4 @@ public static PersistentMap create() { public static PersistentMap of(K key, V value) { return PersistentMap.create().with(key, value); } - } diff --git a/src/com/google/javascript/jscomp/newtypes/TypeWithProperties.java b/src/com/google/javascript/jscomp/newtypes/TypeWithProperties.java index 1ecfb3a55d8..d5b42b81edc 100644 --- a/src/com/google/javascript/jscomp/newtypes/TypeWithProperties.java +++ b/src/com/google/javascript/jscomp/newtypes/TypeWithProperties.java @@ -20,23 +20,38 @@ import java.util.Collection; /** - * A type that can contain properties, - * such as an ObjectType, NominalType, or a Namespace. + * A type that can contain properties, such as an ObjectType, and + * EnumType (a separate, special case, of ObjectType). */ interface TypeWithProperties extends Serializable { - /** Get the inferred type of the given property */ + /** + * Get the inferred type of the given property. Returns the undefined + * type if the named property is not found. + */ JSType getProp(QualifiedName qname); - /** Get the declared type of the given property */ + /** + * Get the declared type of the given property, or null if the named + * property is not declared. + */ JSType getDeclaredProp(QualifiedName qname); - /** Return whether this type contains any form of property */ + /** + * Return true if this type contains any form of the given property + * (this may include declared, inferred, required, or optional properties). + */ boolean mayHaveProp(QualifiedName qname); - /** Return whether this type contains a required property */ + /** + * Return true if this type contains the given required property. + * The qname must be an identifier, not a general qualified name. + */ boolean hasProp(QualifiedName qname); - /** Return whether this type contains a constant property */ + /** + * Return true if this type contains the given property and it is constant. + * The qname must be an identifier, not a general qualified name. + */ boolean hasConstantProp(QualifiedName qname); /**