diff --git a/src/com/google/javascript/jscomp/CheckAccessControls.java b/src/com/google/javascript/jscomp/CheckAccessControls.java index 6e38ddd6a0a..8030689a921 100644 --- a/src/com/google/javascript/jscomp/CheckAccessControls.java +++ b/src/com/google/javascript/jscomp/CheckAccessControls.java @@ -252,8 +252,11 @@ private static TypeI normalizeClassType(TypeI type) { return type; } else if (type.isConstructor() || type.isInterface()) { return type.toMaybeFunctionType().getInstanceType(); - } else if (type.isPrototypeObject()) { - return type.toMaybeObjectType().normalizeObjectForCheckAccessControls(); + } else { + ObjectTypeI obj = type.toMaybeObjectType(); + if (obj != null) { + return obj.normalizeObjectForCheckAccessControls(); + } } return type; } @@ -549,7 +552,8 @@ private void checkFinalClassOverrides(NodeTraversal t, Node fn, Node parent) { * @param t The current traversal. * @param getprop The getprop node. */ - private void checkConstantProperty(NodeTraversal t, Node getprop) { + private void checkConstantProperty(NodeTraversal t, + Node getprop) { // Check whether the property is modified Node parent = getprop.getParent(); boolean isDelete = parent.isDelProp(); @@ -573,7 +577,8 @@ private void checkConstantProperty(NodeTraversal t, Node getprop) { } if (isDelete) { - compiler.report(t.makeError(getprop, CONST_PROPERTY_DELETED, propertyName)); + compiler.report( + t.makeError(getprop, CONST_PROPERTY_DELETED, propertyName)); return; } @@ -588,16 +593,18 @@ private void checkConstantProperty(NodeTraversal t, Node getprop) { ObjectTypeI oType = objectType; while (oType != null) { - if (initializedConstantProperties.containsEntry(oType, propertyName) - || initializedConstantProperties.containsEntry( - getCanonicalInstance(oType), propertyName)) { - compiler.report(t.makeError(getprop, CONST_PROPERTY_REASSIGNED_VALUE, propertyName)); - break; - } + if (initializedConstantProperties.containsEntry( + oType, propertyName)) { + compiler.report( + t.makeError(getprop, CONST_PROPERTY_REASSIGNED_VALUE, + propertyName)); + break; + } oType = oType.getPrototypeObject(); } - initializedConstantProperties.put(objectType, propertyName); + initializedConstantProperties.put(objectType, + propertyName); // Add the prototype when we're looking at an instance object if (objectType.isInstanceType()) { @@ -609,16 +616,6 @@ private void checkConstantProperty(NodeTraversal t, Node getprop) { } } - /** - * Return an object with the same nominal type as obj, - * but without any possible extra properties that exist on obj. - */ - static ObjectTypeI getCanonicalInstance(ObjectTypeI obj) { - FunctionTypeI ctor = obj.getConstructor(); - // In NTI ctor is never null, but it might be in OTI. - return ctor == null ? obj : ctor.getInstanceType(); - } - /** * Reports an error if the given property is not visible in the current * context. diff --git a/src/com/google/javascript/jscomp/ConformanceRules.java b/src/com/google/javascript/jscomp/ConformanceRules.java index 50a02d4e718..527ff025b82 100644 --- a/src/com/google/javascript/jscomp/ConformanceRules.java +++ b/src/com/google/javascript/jscomp/ConformanceRules.java @@ -504,13 +504,9 @@ private ConformanceResult checkConformance(NodeTraversal t, Node n, Property pro Node lhs = n.getFirstChild(); if (typeWithBannedProp != null && lhs.getTypeI() != null) { TypeI foundType = lhs.getTypeI().restrictByNotNullOrUndefined(); - ObjectTypeI foundObj = foundType.toMaybeObjectType(); - if (foundObj != null) { - if (foundObj.isPrototypeObject()) { - foundType = foundObj.getOwnerFunction().getInstanceType(); - } else if (foundObj.isGeneric()) { - foundType = foundObj.getRawType(); - } + if (foundType.toMaybeObjectType() != null + && foundType.toMaybeObjectType().isGeneric()) { + foundType = foundType.toMaybeObjectType().getRawType(); } if (foundType.isSomeUnknownType() || foundType.isTypeVariable() @@ -538,7 +534,8 @@ private ConformanceResult checkConformance(NodeTraversal t, Node n, Property pro private boolean matchesPrototype(TypeI type, TypeI maybePrototype) { ObjectTypeI methodClassObjectType = type.toMaybeObjectType(); if (methodClassObjectType != null) { - if (methodClassObjectType.getPrototypeObject().isEquivalentTo(maybePrototype)) { + if (methodClassObjectType.getPrototypeObject().isEquivalentTo( + maybePrototype)) { return true; } } diff --git a/src/com/google/javascript/jscomp/newtypes/JSType.java b/src/com/google/javascript/jscomp/newtypes/JSType.java index 307dfccdc80..4ae2b9dad0f 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSType.java +++ b/src/com/google/javascript/jscomp/newtypes/JSType.java @@ -1835,7 +1835,10 @@ public Iterable getOwnPropertyNames() { @Override public boolean isPrototypeObject() { - return isSingletonObj() && getObjTypeIfSingletonObj().isPrototypeObject(); + // TODO(dimvar): DisambiguateProperties needs a proper implementation here, not a stub. + // Either add the 'constructor' property to prototype objects, or change DisambiguateProperties + // to not require this method. + return false; } @Override @@ -2000,10 +2003,7 @@ public ObjectTypeI getTopDefiningInterface(String propName) { @Override public FunctionTypeI getOwnerFunction() { - if (isPrototypeObject()) { - return this.commonTypes.fromFunctionType(getObjTypeIfSingletonObj().getOwnerFunction()); - } - return null; + throw new UnsupportedOperationException(); } } diff --git a/src/com/google/javascript/jscomp/newtypes/ObjectType.java b/src/com/google/javascript/jscomp/newtypes/ObjectType.java index 83b1c30b4fe..73ef3e45f95 100644 --- a/src/com/google/javascript/jscomp/newtypes/ObjectType.java +++ b/src/com/google/javascript/jscomp/newtypes/ObjectType.java @@ -184,21 +184,6 @@ boolean isNamespace() { return this.ns != null; } - boolean isPrototypeObject() { - return getOwnerFunction() != null; - } - - FunctionType getOwnerFunction() { - JSType t = getProp(new QualifiedName("constructor")); - if (t != null && t.isFunctionType()) { - FunctionType maybeCtor = t.getFunTypeIfSingletonObj(); - if (maybeCtor.isSomeConstructorOrInterface()) { - return maybeCtor; - } - } - return null; - } - private boolean hasNonPrototypeProperties() { for (String pname : this.props.keySet()) { if (!pname.equals("prototype")) { @@ -1376,9 +1361,6 @@ public String toString() { } StringBuilder appendTo(StringBuilder builder) { - if (isPrototypeObject()) { - return builder.append(getOwnerFunction().getInstanceTypeOfCtor()).append(".prototype"); - } if (!hasNonPrototypeProperties()) { if (fn != null) { return fn.appendTo(builder); diff --git a/src/com/google/javascript/jscomp/newtypes/RawNominalType.java b/src/com/google/javascript/jscomp/newtypes/RawNominalType.java index 1d9fbf3c7d0..d4986c7c3c9 100644 --- a/src/com/google/javascript/jscomp/newtypes/RawNominalType.java +++ b/src/com/google/javascript/jscomp/newtypes/RawNominalType.java @@ -638,24 +638,21 @@ public void finalize() { } } } + // NOTE(dimvar): We currently don't add the "constructor" property to the + // prototype object. A tricky issue with it is that it needs to be ignored + // during subtyping, eg, when you are comparing a @record Foo with an + // object literal that has the same properties, they would still differ + // 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 ctorJstype = this.commonTypes.fromFunctionType(ctorFn); JSType protoObject = JSType.fromObjectType(ObjectType.makeObjectType( - this.commonTypes, protoNT, - // NOTE(dimvar): We add the "constructor" property to the prototype object, but we - // don't update the this.protoProps map. As a result, for a class Foo, - // Foo.prototype.constructor has a more precise type than (new Foo).constructor, - // which points back to the definition in Object.prototype.constructor. - // This handling is a bit imprecise, but still more precise than the old type checker. - // We do it to work around some tricky type checking issues. - // For example, when passing an object literal to a context that expects some - // record Bar, you don't want to include the "constructor" property in the comparison. - this.protoProps.with("constructor", Property.make(ctorJstype, null)), + 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 7d8aaa99769..4d445fa0051 100644 --- a/test/com/google/javascript/jscomp/CheckAccessControlsTest.java +++ b/test/com/google/javascript/jscomp/CheckAccessControlsTest.java @@ -195,14 +195,22 @@ public void testWarningForDeprecatedSuperClass2() { } public void testWarningForPrototypeProperty() { + // TODO(aravindpg): in NTI the string representation of prototype object types is less than + // ideal due to the way NTI represents them. Fix if possible. String js = "/** @constructor */ function Foo() {}" + "/** @deprecated It is now in production, use that model... */ Foo.prototype.bar = 3;" + "Foo.prototype.baz = function() { alert(Foo.prototype.bar); };"; + this.mode = TypeInferenceMode.OTI_ONLY; testDepProp( js, "Property bar of type Foo.prototype has been deprecated:" + " It is now in production, use that model..."); + this.mode = TypeInferenceMode.NTI_ONLY; + testDepProp( + js, + "Property bar of type {bar:?, baz:function(this:Foo):?} has been deprecated:" + + " It is now in production, use that model..."); } public void testNoWarningForNumbers() { diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceTest.java b/test/com/google/javascript/jscomp/NewTypeInferenceTest.java index ed307913bcb..5555342a99e 100644 --- a/test/com/google/javascript/jscomp/NewTypeInferenceTest.java +++ b/test/com/google/javascript/jscomp/NewTypeInferenceTest.java @@ -19122,26 +19122,4 @@ public void testMixedClassInheritance() { "Foo.prototype.foo = function() { return ''; };"), GlobalTypeInfo.INVALID_PROP_OVERRIDE); } - - public void testConstructorProperty() { - typeCheck(LINE_JOINER.join( - "/** @constructor */", - "function Foo() {}", - "var /** !Foo */ x = new Foo.prototype.constructor;")); - - typeCheck(LINE_JOINER.join( - "/** @constructor */", - "function Foo() {}", - "var /** null */ x = new Foo.prototype.constructor;"), - NewTypeInference.MISTYPED_ASSIGN_RHS); - - // Resolves to the Object.prototype.constructor property, not to Foo.prototype.constructor - typeCheck(LINE_JOINER.join( - "/** @constructor */", - "function Foo() {}", - "var ctor = (new Foo).constructor;", - "if (ctor != null) {", - " var /** null */ n = new ctor;", - "}")); - } }