Skip to content

Commit

Permalink
[NTI] Add the 'constructor' property to prototype objects.
Browse files Browse the repository at this point in the history
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=147877049
  • Loading branch information
dimvar committed Feb 19, 2017
1 parent 511e690 commit ae3bad1
Show file tree
Hide file tree
Showing 8 changed files with 109 additions and 44 deletions.
39 changes: 21 additions & 18 deletions src/com/google/javascript/jscomp/CheckAccessControls.java
Expand Up @@ -252,11 +252,8 @@ private static TypeI normalizeClassType(TypeI type) {
return type;
} else if (type.isConstructor() || type.isInterface()) {
return type.toMaybeFunctionType().getInstanceType();
} else {
ObjectTypeI obj = type.toMaybeObjectType();
if (obj != null) {
return obj.normalizeObjectForCheckAccessControls();
}
} else if (type.isPrototypeObject()) {
return type.toMaybeObjectType().normalizeObjectForCheckAccessControls();
}
return type;
}
Expand Down Expand Up @@ -552,8 +549,7 @@ 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();
Expand All @@ -577,8 +573,7 @@ private void checkConstantProperty(NodeTraversal t,
}

if (isDelete) {
compiler.report(
t.makeError(getprop, CONST_PROPERTY_DELETED, propertyName));
compiler.report(t.makeError(getprop, CONST_PROPERTY_DELETED, propertyName));
return;
}

Expand All @@ -593,18 +588,16 @@ private void checkConstantProperty(NodeTraversal t,

ObjectTypeI oType = objectType;
while (oType != null) {
if (initializedConstantProperties.containsEntry(
oType, propertyName)) {
compiler.report(
t.makeError(getprop, CONST_PROPERTY_REASSIGNED_VALUE,
propertyName));
break;
}
if (initializedConstantProperties.containsEntry(oType, propertyName)
|| initializedConstantProperties.containsEntry(
getCanonicalInstance(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()) {
Expand All @@ -616,6 +609,16 @@ private void checkConstantProperty(NodeTraversal t,
}
}

/**
* 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.
Expand Down
16 changes: 11 additions & 5 deletions src/com/google/javascript/jscomp/ConformanceRules.java
Expand Up @@ -504,9 +504,16 @@ private ConformanceResult checkConformance(NodeTraversal t, Node n, Property pro
Node lhs = n.getFirstChild();
if (typeWithBannedProp != null && lhs.getTypeI() != null) {
TypeI foundType = lhs.getTypeI().restrictByNotNullOrUndefined();
if (foundType.toMaybeObjectType() != null
&& foundType.toMaybeObjectType().isGeneric()) {
foundType = foundType.toMaybeObjectType().getRawType();
ObjectTypeI foundObj = foundType.toMaybeObjectType();
if (foundObj != null) {
if (foundObj.isPrototypeObject()) {
FunctionTypeI ownerFun = foundObj.getOwnerFunction();
if (ownerFun.isConstructor()) {
foundType = ownerFun.getInstanceType();
}
} else if (foundObj.isGeneric()) {
foundType = foundObj.getRawType();
}
}
if (foundType.isSomeUnknownType()
|| foundType.isTypeVariable()
Expand Down Expand Up @@ -534,8 +541,7 @@ 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;
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/com/google/javascript/jscomp/newtypes/JSType.java
Expand Up @@ -1835,10 +1835,7 @@ public Iterable<String> getOwnPropertyNames() {

@Override
public boolean 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;
return isSingletonObj() && getObjTypeIfSingletonObj().isPrototypeObject();
}

@Override
Expand Down Expand Up @@ -2003,7 +2000,10 @@ public ObjectTypeI getTopDefiningInterface(String propName) {

@Override
public FunctionTypeI getOwnerFunction() {
throw new UnsupportedOperationException();
if (isPrototypeObject()) {
return this.commonTypes.fromFunctionType(getObjTypeIfSingletonObj().getOwnerFunction());
}
return null;
}
}

Expand Down
18 changes: 18 additions & 0 deletions src/com/google/javascript/jscomp/newtypes/ObjectType.java
Expand Up @@ -184,6 +184,21 @@ 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")) {
Expand Down Expand Up @@ -1361,6 +1376,9 @@ 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);
Expand Down
19 changes: 11 additions & 8 deletions src/com/google/javascript/jscomp/newtypes/RawNominalType.java
Expand Up @@ -638,21 +638,24 @@ 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, this.protoProps,
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, ctorJstype)),
null, null, false, ObjectKind.UNRESTRICTED));
addCtorProperty("prototype", null, protoObject, false);
this.isFinalized = true;
Expand Down
Expand Up @@ -195,22 +195,14 @@ 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() {
Expand Down
21 changes: 21 additions & 0 deletions test/com/google/javascript/jscomp/CheckConformanceTest.java
Expand Up @@ -453,6 +453,27 @@ public void testReportLooseTypeViolations() {
null);
}

public void testDontCrashOnNonConstructorWithPrototype() {
configuration =
LINE_JOINER.join(
"requirement: {",
" type: BANNED_PROPERTY_WRITE",
" value: 'Bar.prototype.method'",
" error_message: 'asdf'",
" report_loose_type_violations: false",
"}");

testSame(
DEFAULT_EXTERNS + LINE_JOINER.join(
"/** @constructor */",
"function Bar() {}",
"Bar.prototype.method = function() {};"),
LINE_JOINER.join(
"function Foo() {}",
"Foo.prototype.method = function() {};"),
null, null);
}

private void testConformance(String src, DiagnosticType warning) {
testConformance(src, "", warning);
}
Expand Down
22 changes: 22 additions & 0 deletions test/com/google/javascript/jscomp/NewTypeInferenceTest.java
Expand Up @@ -19122,4 +19122,26 @@ 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;",
"}"));
}
}

0 comments on commit ae3bad1

Please sign in to comment.