From 7cf5873fbcf4d64504b0253dc47db32297dfd163 Mon Sep 17 00:00:00 2001 From: dimvar Date: Thu, 10 Nov 2016 12:46:50 -0800 Subject: [PATCH] [NTI] Simplify the representation of object types, by requiring all object types to have a non-null nominal type. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=138792860 --- .../javascript/jscomp/GlobalTypeInfo.java | 4 +- .../javascript/jscomp/NewTypeInference.java | 13 +- .../javascript/jscomp/newtypes/JSType.java | 2 +- .../newtypes/JSTypeCreatorFromJSDoc.java | 4 +- .../javascript/jscomp/newtypes/JSTypes.java | 68 ++++-- .../jscomp/newtypes/ObjectType.java | 208 ++++++++---------- .../jscomp/newtypes/RawNominalType.java | 12 +- .../jscomp/CheckAccessControlsTest.java | 2 +- .../jscomp/CheckConformanceTest.java | 15 ++ .../jscomp/NewTypeInferenceTest.java | 1 + 10 files changed, 173 insertions(+), 156 deletions(-) diff --git a/src/com/google/javascript/jscomp/GlobalTypeInfo.java b/src/com/google/javascript/jscomp/GlobalTypeInfo.java index b5450c579fb..fdaea3350d6 100644 --- a/src/com/google/javascript/jscomp/GlobalTypeInfo.java +++ b/src/com/google/javascript/jscomp/GlobalTypeInfo.java @@ -460,7 +460,7 @@ public void process(Node externs, Node root) { globalThisType = win.getInstanceAsJSType(); } else { // Type the global THIS as a loose object - globalThisType = this.commonTypes.TOP_OBJECT.withLoose(); + globalThisType = this.commonTypes.getTopObject().withLoose(); } this.globalScope.setDeclaredType( (new FunctionTypeBuilder(this.commonTypes)). @@ -2030,7 +2030,7 @@ private JSType simpleInferExprType(Node n) { return simpleInferDeclaration( this.currentScope.getDeclaration(n.getString(), false)); case OBJECTLIT: { - JSType objLitType = commonTypes.TOP_OBJECT; + JSType objLitType = commonTypes.getEmptyObjectLiteral(); for (Node prop : n.children()) { JSType propType = simpleInferExprType(prop.getFirstChild()); if (propType == null) { diff --git a/src/com/google/javascript/jscomp/NewTypeInference.java b/src/com/google/javascript/jscomp/NewTypeInference.java index 4565e016c1d..439ff0e9f37 100644 --- a/src/com/google/javascript/jscomp/NewTypeInference.java +++ b/src/com/google/javascript/jscomp/NewTypeInference.java @@ -411,9 +411,7 @@ void add(JSError warning) { private JSType NUMBER_OR_STRING; private JSType STRING; private JSType TOP; - private JSType TOP_DICT; private JSType TOP_OBJECT; - private JSType TOP_STRUCT; private JSType TRUE_TYPE; private JSType TRUTHY; private JSType UNDEFINED; @@ -466,9 +464,7 @@ public void process(Node externs, Node root) { this.NUMBER_OR_STRING = this.commonTypes.NUMBER_OR_STRING; this.STRING = this.commonTypes.STRING; this.TOP = this.commonTypes.TOP; - this.TOP_DICT = this.commonTypes.TOP_DICT; - this.TOP_OBJECT = this.commonTypes.TOP_OBJECT; - this.TOP_STRUCT = this.commonTypes.TOP_STRUCT; + this.TOP_OBJECT = this.commonTypes.getTopObject(); this.TRUE_TYPE = this.commonTypes.TRUE_TYPE; this.TRUTHY = this.commonTypes.TRUTHY; this.UNDEFINED = this.commonTypes.UNDEFINED; @@ -2770,8 +2766,7 @@ private JSType predicateTransformType( ? arrayType : beforeType.removeType(arrayType); } case "isArrayLike": - return TOP_OBJECT.withProperty( - new QualifiedName("length"), NUMBER); + return TOP_OBJECT.withProperty(new QualifiedName("length"), NUMBER); case "boolean": case "isBoolean": return booleanContext.isTrueOrTruthy() @@ -4289,10 +4284,10 @@ private JSType pickReqObjType(Node expr) { case OBJECTLIT: { JSDocInfo jsdoc = expr.getJSDocInfo(); if (jsdoc != null && jsdoc.makesStructs()) { - return TOP_STRUCT; + return this.commonTypes.getTopStruct(); } if (jsdoc != null && jsdoc.makesDicts()) { - return TOP_DICT; + return this.commonTypes.getTopDict(); } return this.commonTypes.getEmptyObjectLiteral(); } diff --git a/src/com/google/javascript/jscomp/newtypes/JSType.java b/src/com/google/javascript/jscomp/newtypes/JSType.java index 45e0a6df024..1920e336931 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSType.java +++ b/src/com/google/javascript/jscomp/newtypes/JSType.java @@ -1194,7 +1194,7 @@ public JSType removeType(JSType other) { commonTypes, TRUE_MASK | FALSE_MASK | NUMBER_MASK | STRING_MASK | NULL_MASK | UNDEFINED_MASK | NON_SCALAR_MASK, - ImmutableSet.of(this.commonTypes.TOP_OBJECTTYPE), null, NO_ENUMS); + ImmutableSet.of(this.commonTypes.getTopObjectType()), null, NO_ENUMS); return almostTop.removeType(other); } int newMask = getMask() & ~otherMask; diff --git a/src/com/google/javascript/jscomp/newtypes/JSTypeCreatorFromJSDoc.java b/src/com/google/javascript/jscomp/newtypes/JSTypeCreatorFromJSDoc.java index 44169632161..b5db7e9eea0 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSTypeCreatorFromJSDoc.java +++ b/src/com/google/javascript/jscomp/newtypes/JSTypeCreatorFromJSDoc.java @@ -419,7 +419,7 @@ private JSType getNamedTypeHelper( // at least not warn about inexistent properties on it, so we type it // as @dict. return maybeMakeNullable(n.hasChildren() - ? this.commonTypes.TOP_DICT : this.commonTypes.TOP_OBJECT); + ? this.commonTypes.getTopDict() : this.commonTypes.getTopObject()); default: return lookupTypeByName(typeName, n, registry, outerTypeParameters); } @@ -598,7 +598,7 @@ private void fillInFunTypeBuilder( } else if (child.getToken() == Token.NEW) { Node newTypeNode = child.getFirstChild(); JSType t = getThisOrNewType(newTypeNode, registry, typeParameters); - if (!t.isSubtypeOf(this.commonTypes.TOP_OBJECT) + if (!t.isSubtypeOf(this.commonTypes.getTopObject()) && (!t.hasTypeVariable() || t.hasScalar())) { warnings.add(JSError.make( newTypeNode, NEW_EXPECTS_OBJECT_OR_TYPEVAR, t.toString())); diff --git a/src/com/google/javascript/jscomp/newtypes/JSTypes.java b/src/com/google/javascript/jscomp/newtypes/JSTypes.java index eea050f739b..53921eb9d9b 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSTypes.java +++ b/src/com/google/javascript/jscomp/newtypes/JSTypes.java @@ -75,12 +75,13 @@ public final class JSTypes { public final JSType UNDEFINED; public final JSType UNKNOWN; - final ObjectType TOP_OBJECTTYPE; + private ObjectType topObjectType; final PersistentMap BOTTOM_PROPERTY_MAP; - final ObjectType BOTTOM_OBJECT; - public final JSType TOP_OBJECT; - public final JSType TOP_STRUCT; - public final JSType TOP_DICT; + private JSType topObject; + private ObjectType looseTopObject; + private JSType topStruct; + private JSType topDict; + private ObjectType bottomObject; // Corresponds to Function, which is a subtype and supertype of all functions. final FunctionType QMARK_FUNCTION; @@ -164,16 +165,7 @@ private JSTypes(boolean inCompatibilityMode) { this.BOTTOM_FUNCTION = Preconditions.checkNotNull(functions.get("BOTTOM_FUNCTION")); this.TOP_FUNCTION = Preconditions.checkNotNull(functions.get("TOP_FUNCTION")); this.LOOSE_TOP_FUNCTION = Preconditions.checkNotNull(functions.get("LOOSE_TOP_FUNCTION")); - this.BOTTOM_PROPERTY_MAP = PersistentMap.of("_", Property.make(this.BOTTOM, this.BOTTOM)); - Map objects = ObjectType.createInitialObjectTypes(this); - this.TOP_OBJECTTYPE = Preconditions.checkNotNull(objects.get("TOP_OBJECTTYPE")); - this.TOP_OBJECT = JSType.fromObjectType(this.TOP_OBJECTTYPE); - this.TOP_STRUCT = JSType.fromObjectType( - Preconditions.checkNotNull(objects.get("TOP_STRUCT"))); - this.TOP_DICT = JSType.fromObjectType( - Preconditions.checkNotNull(objects.get("TOP_DICT"))); - this.BOTTOM_OBJECT = Preconditions.checkNotNull(objects.get("BOTTOM_OBJECT")); this.allowMethodsAsFunctions = inCompatibilityMode; this.looseSubtypingForLooseObjects = inCompatibilityMode; @@ -288,6 +280,14 @@ public NominalType getObjectType() { return this.builtinObject == null ? null : this.builtinObject.getAsNominalType(); } + ObjectType getTopObjectType() { + return this.topObjectType; + } + + ObjectType getLooseTopObjectType() { + return this.looseTopObject; + } + public NominalType getLiteralObjNominalType() { return this.literalObject == null ? null : this.literalObject.getAsNominalType(); } @@ -296,6 +296,22 @@ public JSType getEmptyObjectLiteral() { return this.literalObject == null ? null : this.literalObject.getInstanceAsJSType(); } + public JSType getTopObject() { + return topObject; + } + + public JSType getTopStruct() { + return this.topStruct; + } + + public JSType getTopDict() { + return this.topDict; + } + + ObjectType getBottomObject() { + return this.bottomObject; + } + public NominalType getIObjectType() { return this.iObject == null ? null : this.iObject.getAsNominalType(); } @@ -345,18 +361,15 @@ public JSType getStringInstance() { } ObjectType getNumberInstanceObjType() { - return numberInstanceObjtype != null - ? numberInstanceObjtype : this.TOP_OBJECTTYPE; + return numberInstanceObjtype != null ? numberInstanceObjtype : this.topObjectType; } ObjectType getBooleanInstanceObjType() { - return booleanInstanceObjtype != null - ? booleanInstanceObjtype : this.TOP_OBJECTTYPE; + return booleanInstanceObjtype != null ? booleanInstanceObjtype : this.topObjectType; } ObjectType getStringInstanceObjType() { - return stringInstanceObjtype != null - ? stringInstanceObjtype : this.TOP_OBJECTTYPE; + return stringInstanceObjtype != null ? stringInstanceObjtype : this.topObjectType; } public JSType getArgumentsArrayType() { @@ -389,7 +402,7 @@ public JSType getNativeType(JSTypeNative typeId) { case ARRAY_TYPE: return getArrayInstance(); case OBJECT_TYPE: - return TOP_OBJECT; + return getTopObject(); case TRUTHY: return TRUTHY; default: @@ -406,7 +419,20 @@ public void setFunctionType(RawNominalType builtinFunction) { } public void setObjectType(RawNominalType builtinObject) { + NominalType builtinObjectNT = builtinObject.getAsNominalType(); this.builtinObject = builtinObject; + this.topObjectType = builtinObject.getInstanceAsJSType().getObjTypeIfSingletonObj(); + this.looseTopObject = ObjectType.makeObjectType( + this, builtinObjectNT, PersistentMap.create(), + null, null, true, ObjectKind.UNRESTRICTED); + this.topObject = JSType.fromObjectType(this.topObjectType); + this.topStruct = JSType.fromObjectType(ObjectType.makeObjectType( + this, builtinObjectNT, PersistentMap.create(), + null, null, false, ObjectKind.STRUCT)); + this.topDict = JSType.fromObjectType(ObjectType.makeObjectType( + this, builtinObjectNT, PersistentMap.create(), + null, null, false, ObjectKind.DICT)); + this.bottomObject = ObjectType.createBottomObject(this); } public void setLiteralObjNominalType(RawNominalType literalObject) { diff --git a/src/com/google/javascript/jscomp/newtypes/ObjectType.java b/src/com/google/javascript/jscomp/newtypes/ObjectType.java index 6ae02128a5c..529601378cf 100644 --- a/src/com/google/javascript/jscomp/newtypes/ObjectType.java +++ b/src/com/google/javascript/jscomp/newtypes/ObjectType.java @@ -24,7 +24,6 @@ import com.google.javascript.jscomp.newtypes.ObjectsBuilder.ResolveConflictsBy; import com.google.javascript.rhino.Node; import java.util.Arrays; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -48,50 +47,32 @@ final class ObjectType implements TypeWithProperties { private final JSTypes commonTypes; - static Map createInitialObjectTypes(JSTypes commonTypes) { - LinkedHashMap objects = new LinkedHashMap<>(); - - objects.put( - "TOP_OBJECTTYPE", - makeObjectType(commonTypes, null, null, null, null, false, ObjectKind.UNRESTRICTED)); - objects.put( - "TOP_STRUCT", - ObjectType.makeObjectType(commonTypes, null, null, null, null, false, ObjectKind.STRUCT)); - objects.put( - "TOP_DICT", - ObjectType.makeObjectType(commonTypes, null, null, null, null, false, ObjectKind.DICT)); - objects.put( - "BOTTOM_OBJECT", - new ObjectType(commonTypes, null, - Preconditions.checkNotNull(commonTypes.BOTTOM_PROPERTY_MAP), - null, null, false, ObjectKind.UNRESTRICTED)); - - return objects; + static ObjectType createBottomObject(JSTypes commonTypes) { + return new ObjectType(commonTypes, commonTypes.getObjectType(), + Preconditions.checkNotNull(commonTypes.BOTTOM_PROPERTY_MAP), + null, null, false, ObjectKind.UNRESTRICTED); } private ObjectType(JSTypes commonTypes, NominalType nominalType, PersistentMap props, FunctionType fn, Namespace ns, boolean isLoose, ObjectKind objectKind) { Preconditions.checkNotNull(commonTypes); + Preconditions.checkNotNull(nominalType); Preconditions.checkArgument( fn == null || fn.isQmarkFunction() || fn.isLoose() == isLoose, "isLoose: %s, fn: %s", isLoose, fn); Preconditions.checkArgument(FunctionType.isInhabitable(fn)); - Preconditions.checkArgument(fn == null || nominalType != null, - "Cannot create function %s without nominal type", fn); - if (ns != null && nominalType != null) { + if (ns != null) { String name = nominalType.getName(); Preconditions.checkArgument(name.equals(JSTypes.OBJLIT_CLASS_NAME) || name.equals("Function") || name.equals("Window"), "Can't create namespace with nominal type %s", name); } - if (nominalType != null) { - Preconditions.checkArgument(!nominalType.isClassy() || !isLoose, - "Cannot create loose objectType with nominal type %s", nominalType); - Preconditions.checkArgument(fn == null || nominalType.isFunction(), - "Cannot create objectType of nominal type %s with function (%s)", - nominalType, fn); - } + Preconditions.checkArgument(!nominalType.isClassy() || !isLoose, + "Cannot create loose objectType with nominal type %s", nominalType); + Preconditions.checkArgument(fn == null || nominalType.isFunction(), + "Cannot create objectType of nominal type %s with function (%s)", + nominalType, fn); this.commonTypes = commonTypes; this.nominalType = nominalType; this.props = isLoose ? loosenProps(props) : props; @@ -112,7 +93,7 @@ private static PersistentMap loosenProps( JSType propType = entry.getValue().getType(); ObjectType objType = propType.getObjTypeIfSingletonObj(); if (objType != null - && !objType.getNominalType().isClassy() && !objType.isLoose()) { + && !objType.nominalType.isClassy() && !objType.isLoose()) { newProps = newProps.with( entry.getKey(), Property.make(propType.withLoose(), null)); @@ -124,10 +105,11 @@ private static PersistentMap loosenProps( static ObjectType makeObjectType(JSTypes commonTypes, NominalType nominalType, PersistentMap props, FunctionType fn, Namespace ns, boolean isLoose, ObjectKind ok) { + Preconditions.checkNotNull(nominalType); if (props == null) { props = PersistentMap.create(); } else if (containsBottomProp(props) || !FunctionType.isInhabitable(fn)) { - return commonTypes.BOTTOM_OBJECT; + return commonTypes.getBottomObject(); } if (fn != null && !props.containsKey("prototype") && (ns == null || ns.getNsProp("prototype") == null)) { @@ -151,11 +133,12 @@ static ObjectType fromProperties(JSTypes commonTypes, Map oldP for (Map.Entry entry : oldProps.entrySet()) { Property prop = entry.getValue(); if (prop.getDeclaredType().isBottom()) { - return commonTypes.BOTTOM_OBJECT; + return commonTypes.getBottomObject(); } newProps = newProps.with(entry.getKey(), prop); } - return new ObjectType(commonTypes, null, newProps, null, null, false, ObjectKind.UNRESTRICTED); + return new ObjectType(commonTypes, commonTypes.getObjectType(), newProps, + null, null, false, ObjectKind.UNRESTRICTED); } JSTypes getCommonTypes() { @@ -163,7 +146,7 @@ JSTypes getCommonTypes() { } boolean isInhabitable() { - return this != this.commonTypes.BOTTOM_OBJECT; + return this != this.commonTypes.getBottomObject(); } static boolean containsBottomProp(PersistentMap props) { @@ -234,7 +217,7 @@ private static boolean hasOnlyBuiltinProps(ObjectType obj, ObjectType someBuilti static JSType mayTurnLooseObjectToScalar(JSType t, JSTypes commonTypes) { ObjectType obj = t.getObjTypeIfSingletonObj(); if (obj == null || !obj.isLoose() || obj.props.isEmpty() || obj.fn != null - || hasOnlyBuiltinProps(obj, t.getCommonTypes().TOP_OBJECTTYPE) + || hasOnlyBuiltinProps(obj, t.getCommonTypes().getTopObjectType()) || hasOnlyBuiltinProps( obj, commonTypes.getArrayInstance().getObjTypeIfSingletonObj())) { return t; @@ -264,6 +247,9 @@ static JSType mayTurnLooseObjectToScalar(JSType t, JSTypes commonTypes) { // but mostly it catches real bugs. private ObjectType withLoose() { + if (isTopObject()) { + return this.commonTypes.getLooseTopObjectType(); + } if (isLoose() // Don't loosen nominal types || this.nominalType != null && this.nominalType.isClassy() @@ -422,11 +408,7 @@ private static PersistentMap meetPropsHelper( boolean specializeProps1, NominalType resultNominalType, PersistentMap props1, PersistentMap props2) { - if (resultNominalType == null) { - // If props1 or props2 contains a property that also exists on Object, - // we must take the inherited property type into account. - resultNominalType = commonTypes.getObjectType(); - } + Preconditions.checkNotNull(resultNominalType); PersistentMap newProps = props1; for (Map.Entry propsEntry : props1.entrySet()) { String pname = propsEntry.getKey(); @@ -608,7 +590,7 @@ static void whyNotSubtypeOf( */ private boolean isSubtypeOfHelper(boolean keepLoosenessOfThis, ObjectType other, SubtypeCache subSuperMap, MismatchInfo[] boxedInfo) { - if (other == this.commonTypes.TOP_OBJECTTYPE) { + if (other.isTopObject()) { return true; } @@ -616,8 +598,8 @@ private boolean isSubtypeOfHelper(boolean keepLoosenessOfThis, return this.isLooseSubtypeOf(other, subSuperMap); } - NominalType thisNt = getNominalType(); - NominalType otherNt = other.getNominalType(); + NominalType thisNt = this.nominalType; + NominalType otherNt = other.nominalType; boolean checkOnlyLocalProps = true; if (otherNt.isStructuralInterface()) { if (otherNt.equals(subSuperMap.get(thisNt))) { @@ -784,7 +766,7 @@ private static boolean getPropMismatchInfo(String pname, Property prop1, // and we don't warn about possibly inexistent properties. boolean isLooseSubtypeOf(ObjectType other, SubtypeCache subSuperMap) { Preconditions.checkState(isLoose || other.isLoose); - if (other == this.commonTypes.TOP_OBJECTTYPE) { + if (other.isTopObject()) { return true; } @@ -815,7 +797,7 @@ boolean isLooseSubtypeOf(ObjectType other, SubtypeCache subSuperMap) { if (other.fn == null) { return this.fn == null - || other.getNominalType().isBuiltinObject() || other.isLoose(); + || other.nominalType.isBuiltinObject() || other.isLoose(); } else if (this.fn == null) { return isLoose; } @@ -825,38 +807,37 @@ boolean isLooseSubtypeOf(ObjectType other, SubtypeCache subSuperMap) { ObjectType specialize(ObjectType other) { Preconditions.checkState( areRelatedNominalTypes(this.nominalType, other.nominalType)); - if (this == this.commonTypes.TOP_OBJECTTYPE && other.objectKind.isUnrestricted()) { + if (isTopObject() && other.objectKind.isUnrestricted()) { return other; } if (this.ns != null) { return specializeNamespace(other); } - NominalType resultNomType = - NominalType.pickSubclass(this.nominalType, other.nominalType); - if (resultNomType != null && resultNomType.isClassy()) { + NominalType resultNomType = NominalType.pickSubclass(this.nominalType, other.nominalType); + if (resultNomType.isClassy()) { 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) { - return this.commonTypes.BOTTOM_OBJECT; + return this.commonTypes.getBottomObject(); } return new ObjectType( this.commonTypes, resultNomType, newProps, null, this.ns, false, this.objectKind); } FunctionType thisFn = this.fn; boolean isLoose = this.isLoose; - if (resultNomType != null && resultNomType.isFunction() && this.fn == null) { + if (resultNomType.isFunction() && this.fn == null) { thisFn = other.fn; isLoose = other.fn.isLoose(); } PersistentMap newProps = meetPropsHelper(this.commonTypes, true, resultNomType, this.props, other.props); if (newProps == this.commonTypes.BOTTOM_PROPERTY_MAP) { - return this.commonTypes.BOTTOM_OBJECT; + return this.commonTypes.getBottomObject(); } FunctionType newFn = thisFn == null ? null : thisFn.specialize(other.fn); if (!FunctionType.isInhabitable(newFn)) { - return this.commonTypes.BOTTOM_OBJECT; + return this.commonTypes.getBottomObject(); } return new ObjectType( this.commonTypes, resultNomType, newProps, newFn, this.ns, isLoose, this.objectKind); @@ -892,7 +873,7 @@ ObjectType specializeNamespace(ObjectType other) { Preconditions.checkNotNull(this.ns); if (this == other || other.ns != null - || !other.getNominalType().equals(this.commonTypes.getObjectType())) { + || !other.nominalType.equals(this.commonTypes.getObjectType())) { return this; } QualifiedName propPath = getPropertyPath(other); @@ -915,25 +896,32 @@ ObjectType specializeNamespace(ObjectType other) { } private boolean isTopObject() { - return this == this.commonTypes.TOP_OBJECTTYPE; + // Reference equality because we want to make sure that we only ever create + // one top object type. + return this == this.commonTypes.getTopObjectType(); + } + + 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(); } static ObjectType meet(ObjectType obj1, ObjectType obj2) { Preconditions.checkState(areRelatedNominalTypes(obj1.nominalType, obj2.nominalType)); - if (obj1.isTopObject()) { + if (obj1.isTopObject() || obj2.isBottomObject()) { return obj2; - } else if (obj2.isTopObject()) { + } else if (obj2.isTopObject() || obj1.isBottomObject()) { return obj1; } JSTypes commonTypes = obj1.commonTypes; - NominalType resultNomType = - NominalType.pickSubclass(obj1.nominalType, obj2.nominalType); + NominalType resultNomType = NominalType.pickSubclass(obj1.nominalType, obj2.nominalType); FunctionType fn = FunctionType.meet(obj1.fn, obj2.fn); if (!FunctionType.isInhabitable(fn)) { - return commonTypes.BOTTOM_OBJECT; + return commonTypes.getBottomObject(); } boolean isLoose = obj1.isLoose && obj2.isLoose || fn != null && fn.isLoose(); - if (resultNomType != null && resultNomType.isFunction() && fn == null) { + if (resultNomType.isFunction() && fn == null) { fn = obj1.fn == null ? obj2.fn : obj1.fn; isLoose = fn.isLoose(); } @@ -944,7 +932,7 @@ static ObjectType meet(ObjectType obj1, ObjectType obj2) { props = meetPropsHelper(commonTypes, false, resultNomType, obj1.props, obj2.props); } if (props == commonTypes.BOTTOM_PROPERTY_MAP) { - return commonTypes.BOTTOM_OBJECT; + return commonTypes.getBottomObject(); } ObjectKind ok = ObjectKind.meet(obj1.objectKind, obj2.objectKind); Namespace resultNs = Objects.equals(obj1.ns, obj2.ns) ? obj1.ns : null; @@ -953,16 +941,14 @@ static ObjectType meet(ObjectType obj1, ObjectType obj2) { static ObjectType join(ObjectType obj1, ObjectType obj2) { if (obj1.isTopObject() || obj2.isTopObject()) { - return obj1.commonTypes.TOP_OBJECTTYPE; + return obj1.commonTypes.getTopObjectType(); } - NominalType nom1 = obj1.nominalType; - NominalType nom2 = obj2.nominalType; - Preconditions.checkState(nom1 == null || nom2 == null - || nom1.isRawSubtypeOf(nom2) || nom2.isRawSubtypeOf(nom1)); - if (obj1.equals(obj2)) { return obj1; } + NominalType nt1 = obj1.nominalType; + NominalType nt2 = obj2.nominalType; + Preconditions.checkState(nt1.isRawSubtypeOf(nt2) || nt2.isRawSubtypeOf(nt1)); JSTypes commonTypes = obj1.commonTypes; boolean isLoose = obj1.isLoose || obj2.isLoose; FunctionType fn = FunctionType.join(obj1.fn, obj2.fn); @@ -971,10 +957,10 @@ static ObjectType join(ObjectType obj1, ObjectType obj2) { fn = fn == null ? null : fn.withLoose(); props = joinPropsLoosely(commonTypes, obj1.props, obj2.props); } else { - props = joinProps(obj1.props, obj2.props, nom1, nom2); + props = joinProps(obj1.props, obj2.props, nt1, nt2); } - NominalType nominal = NominalType.pickSuperclass(nom1, nom2); - if (nominal == null && fn != null) { + NominalType nominal = NominalType.pickSuperclass(nt1, nt2); + if (nominal.isBuiltinObject() && fn != null) { if (isLoose) { nominal = obj1.commonTypes.getFunctionType(); } else { @@ -1002,13 +988,13 @@ static ImmutableSet joinSets( boolean addedObj2 = false; for (int i = 0; i < objs1Arr.length; i++) { ObjectType obj1 = objs1Arr[i]; - NominalType nominalType1 = obj1.nominalType; - NominalType nominalType2 = obj2.nominalType; - if (areRelatedNominalTypes(nominalType1, nominalType2)) { - if (nominalType2 == null && nominalType1 != null - && !obj1.isSubtypeOf(obj2, SubtypeCache.create()) - || nominalType1 == null && nominalType2 != null - && !obj2.isSubtypeOf(obj1, SubtypeCache.create())) { + NominalType nt1 = obj1.nominalType; + NominalType nt2 = obj2.nominalType; + if (areRelatedNominalTypes(nt1, nt2)) { + if ((nt2.isBuiltinObject() && nt1 != null + && !obj1.isSubtypeOf(obj2, SubtypeCache.create())) + || (nt1.isBuiltinObject() && nt2 != null + && !obj2.isSubtypeOf(obj1, SubtypeCache.create()))) { // Don't merge other classes with record types break; } @@ -1038,9 +1024,6 @@ static ImmutableSet joinSets( } private static boolean areRelatedNominalTypes(NominalType c1, NominalType c2) { - if (c1 == null || c2 == null) { - return true; - } return c1.isNominalSubtypeOf(c2) || c2.isNominalSubtypeOf(c1); } @@ -1065,10 +1048,10 @@ static ImmutableSet meetSetsHelper( newObj = meet(obj1, obj2); } newObjs.add(newObj); - } else if (obj1.getNominalType().isStructuralInterface() + } else if (obj1.nominalType.isStructuralInterface() && obj2.isSubtypeOf(obj1, SubtypeCache.create())) { newObjs.add(obj2); - } else if (obj2.getNominalType().isStructuralInterface() + } else if (obj2.nominalType.isStructuralInterface() && obj1.isSubtypeOf(obj2, SubtypeCache.create())) { newObjs.add(obj1); } @@ -1092,7 +1075,7 @@ FunctionType getFunType() { } NominalType getNominalType() { - return this.nominalType == null ? this.commonTypes.getObjectType() : this.nominalType; + return this.nominalType; } @Override @@ -1147,8 +1130,7 @@ private Property getLeftmostOwnProp(QualifiedName qname) { Property p = props.get(pname); // Only return the extra/specialized prop p if we know that we don't have this property // on our nominal type. - if (p != null - && (this.nominalType == null || !this.nominalType.mayHaveProp(pname))) { + if (p != null && !this.nominalType.mayHaveProp(pname)) { return p; } if (this.ns != null) { @@ -1157,10 +1139,7 @@ private Property getLeftmostOwnProp(QualifiedName qname) { return p; } } - if (this.nominalType != null) { - return this.nominalType.getOwnProp(pname); - } - return null; + return this.nominalType.getOwnProp(pname); } Node getPropertyDefSite(String propertyName) { @@ -1226,18 +1205,18 @@ static ObjectType unifyUnknowns(ObjectType t1, ObjectType t2) { if (!Objects.equals(t1.ns, t2.ns)) { return null; } - NominalType nt1 = t1.nominalType; - NominalType nt2 = t2.nominalType; - NominalType nt; - if (nt1 == null && nt2 == null) { - nt = null; - } else if (nt1 == null || nt2 == null) { + if (t1.isTopObject()) { + return t2.isTopObject() ? t1 : null; + } else if (t2.isTopObject()) { + return null; + } else if (t1.isBottomObject()) { + return t2.isBottomObject() ? t1 : null; + } else if (t2.isBottomObject()) { + return null; + } + NominalType nt = NominalType.unifyUnknowns(t1.nominalType, t2.nominalType); + if (nt == null) { return null; - } else { - nt = NominalType.unifyUnknowns(nt1, nt2); - if (nt == null) { - return null; - } } FunctionType newFn = null; if (t1.fn != null || t2.fn != null) { @@ -1279,7 +1258,7 @@ boolean unifyWithSubtype(ObjectType other, List typeParameters, } NominalType thisNt = this.nominalType; NominalType otherNt = other.nominalType; - if (thisNt != null && otherNt != null) { + if (!thisNt.isBuiltinObject() && !otherNt.isBuiltinObject()) { if (thisNt.unifyWithSubtype( otherNt, typeParameters, typeMultimap, subSuperMap)) { return true; @@ -1294,13 +1273,13 @@ boolean unifyWithSubtype(ObjectType other, List typeParameters, subSuperMap = subSuperMap.with(otherNt, thisNt); } } - if (thisNt != null && !thisNt.isStructuralInterface() && otherNt == null) { + if (!thisNt.isBuiltinObject() && !thisNt.isStructuralInterface() + && otherNt.isBuiltinObject()) { return false; } - Set thisProps = thisNt != null && thisNt.isStructuralInterface() + Set thisProps = !thisNt.isBuiltinObject() && thisNt.isStructuralInterface() ? thisNt.getAllPropsOfInterface() : this.props.keySet(); - return unifyPropsWithSubtype( - other, thisProps, typeParameters, typeMultimap, subSuperMap); + return unifyPropsWithSubtype(other, thisProps, typeParameters, typeMultimap, subSuperMap); } private boolean unifyPropsWithSubtype(ObjectType other, @@ -1326,7 +1305,7 @@ private boolean unifyPropsWithSubtype(ObjectType other, } ObjectType substituteGenerics(Map concreteTypes) { - if (concreteTypes.isEmpty()) { + if (isTopObject() || concreteTypes.isEmpty()) { return this; } PersistentMap newProps = PersistentMap.create(); @@ -1339,7 +1318,6 @@ ObjectType substituteGenerics(Map concreteTypes) { FunctionType newFn = fn == null ? null : fn.substituteGenerics(concreteTypes); return makeObjectType( this.commonTypes, - this.nominalType == null ? null : this.nominalType.instantiateGenerics(concreteTypes), newProps, newFn, @@ -1350,8 +1328,7 @@ ObjectType substituteGenerics(Map concreteTypes) { boolean isPropDefinedOnSubtype(QualifiedName pname) { Preconditions.checkArgument(pname.isIdentifier()); - NominalType nt = getNominalType(); - return nt.isBuiltinObject() || nt.isPropDefinedOnSubtype(pname); + return this.nominalType.isBuiltinObject() || this.nominalType.isPropDefinedOnSubtype(pname); } @Override @@ -1363,11 +1340,12 @@ StringBuilder appendTo(StringBuilder builder) { if (!hasNonPrototypeProperties()) { if (fn != null) { return fn.appendTo(builder); - } else if (getNominalType() != null) { - return getNominalType().appendTo(builder); + } else if (this.nominalType != null) { + return this.nominalType.appendTo(builder); } } - if (nominalType != null && !nominalType.getName().equals("Function") + if (!nominalType.getName().equals("Function") + && !nominalType.getName().equals("Object") && !nominalType.getName().equals(JSTypes.OBJLIT_CLASS_NAME)) { nominalType.appendTo(builder); } else if (isStruct()) { @@ -1382,7 +1360,7 @@ StringBuilder appendTo(StringBuilder builder) { fn.appendTo(builder); builder.append("|>"); } - if (nominalType == null && ns == null || !props.isEmpty()) { + if (ns == null || !props.isEmpty()) { builder.append('{'); boolean firstIteration = true; for (String pname : new TreeSet<>(props.keySet())) { diff --git a/src/com/google/javascript/jscomp/newtypes/RawNominalType.java b/src/com/google/javascript/jscomp/newtypes/RawNominalType.java index 42f5000bb21..e4218fe9322 100644 --- a/src/com/google/javascript/jscomp/newtypes/RawNominalType.java +++ b/src/com/google/javascript/jscomp/newtypes/RawNominalType.java @@ -101,10 +101,6 @@ private RawNominalType( if (isBuiltinHelper(name, "Function", defSite)) { objInstance = ObjectType.fromFunction(this.commonTypes.TOP_FUNCTION, this.wrappedAsNominal); - } else if (isBuiltinHelper(name, "Object", defSite)) { - // We do this to avoid having two instances of ObjectType that both - // represent the top JS object. - objInstance = this.commonTypes.TOP_OBJECTTYPE; } else { objInstance = ObjectType.fromNominalType(this.wrappedAsNominal); } @@ -634,8 +630,14 @@ public void finalize() { // at the "constructor" property. // If in future we decide that it's important to model this property, // we'll have to address the subtyping issues. + NominalType protoNT = this.superclass; + if (protoNT == null) { + NominalType builtinObj = Preconditions.checkNotNull(this.commonTypes.getObjectType(), + "Missing externs for the builtin Object type"); + protoNT = builtinObj; + } JSType protoObject = JSType.fromObjectType(ObjectType.makeObjectType( - this.commonTypes, this.superclass, this.protoProps, + this.commonTypes, protoNT, this.protoProps, null, null, false, ObjectKind.UNRESTRICTED)); addCtorProperty("prototype", null, protoObject, false); this.isFinalized = true; diff --git a/test/com/google/javascript/jscomp/CheckAccessControlsTest.java b/test/com/google/javascript/jscomp/CheckAccessControlsTest.java index dc6a1c45ccd..bd0bc65d6cf 100644 --- a/test/com/google/javascript/jscomp/CheckAccessControlsTest.java +++ b/test/com/google/javascript/jscomp/CheckAccessControlsTest.java @@ -209,7 +209,7 @@ public void testWarningForPrototypeProperty() { this.mode = TypeInferenceMode.NTI_ONLY; testDepProp( js, - "Property bar of type Object{bar:?, baz:function(this:Foo):?} has been deprecated:" + "Property bar of type {bar:?, baz:function(this:Foo):?} has been deprecated:" + " It is now in production, use that model..."); } diff --git a/test/com/google/javascript/jscomp/CheckConformanceTest.java b/test/com/google/javascript/jscomp/CheckConformanceTest.java index d79c033dc0f..74a0c6459d6 100644 --- a/test/com/google/javascript/jscomp/CheckConformanceTest.java +++ b/test/com/google/javascript/jscomp/CheckConformanceTest.java @@ -1286,6 +1286,21 @@ public void testCustomBanUnknownProp6() { "Violation: My rule message\nThe property \"prop\" on type \"module$contents$example_f\""); } + public void testCustomBanUnknownProp7() { + configuration = + config(rule("BanUnknownTypedClassPropsReferences"), "My rule message", value("String")); + + testNoWarning(LINE_JOINER.join( + "/** @constructor */", + "function Foo() {", + " /** @type {!Object} */", + " this.prop;", + "}", + "function f(/** !Foo */ x) {", + " return x.prop[1] + 123;", + "}")); + } + public void testCustomBanUnknownInterfaceProp1() { configuration = config(rule("BanUnknownTypedClassPropsReferences"), "My rule message", value("String")); diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceTest.java b/test/com/google/javascript/jscomp/NewTypeInferenceTest.java index 856f6897b37..a12353d6a3d 100644 --- a/test/com/google/javascript/jscomp/NewTypeInferenceTest.java +++ b/test/com/google/javascript/jscomp/NewTypeInferenceTest.java @@ -15450,6 +15450,7 @@ public void testDontCrashOnMistypedWindow() { // deliberately not adding the default externs here, to avoid the definition // of window. typeCheckCustomExterns(LINE_JOINER.join( + "/** @constructor */ function Object() {}", "/**", " * @constructor", " * @param {...*} var_args",