Skip to content

Commit

Permalink
[NTI] Handle getters and setters as properties in ObjectType::getProp…
Browse files Browse the repository at this point in the history
…ertyDefsite.

CheckAccessControls benefits from this.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=128633706
  • Loading branch information
aravind-pg authored and blickly committed Jul 28, 2016
1 parent 9cf80e1 commit 3530a82
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 21 deletions.
4 changes: 2 additions & 2 deletions src/com/google/javascript/jscomp/GlobalTypeInfo.java
Expand Up @@ -2212,9 +2212,9 @@ private void mayAddPropToPrototype(
methodScope = visitFunctionLate(initializer, rawType);
methodType = methodScope.getDeclaredFunctionType();
if (defSite.isGetterDef()) {
pname = NewTypeInference.createGetterPropName(pname);
pname = JSType.createGetterPropName(pname);
} else if (defSite.isSetterDef()) {
pname = NewTypeInference.createSetterPropName(pname);
pname = JSType.createSetterPropName(pname);
}
} else if (jsdoc != null && jsdoc.containsFunctionDeclaration()) {
// We're parsing a function declaration without a function initializer
Expand Down
16 changes: 4 additions & 12 deletions src/com/google/javascript/jscomp/NewTypeInference.java
Expand Up @@ -433,14 +433,6 @@ private static void println(Object ... objs) {
}
}

static String createGetterPropName(String originalPropName) {
return "%getter_fun" + originalPropName;
}

static String createSetterPropName(String originalPropName) {
return "%setter_fun" + originalPropName;
}

private TypeEnv getInEnv(DiGraphNode<Node, ControlFlowGraph.Branch> dn) {
List<DiGraphEdge<Node, ControlFlowGraph.Branch>> inEdges = dn.getInEdges();
// True for code considered dead in the CFG
Expand Down Expand Up @@ -2485,10 +2477,10 @@ private EnvTypePair analyzeObjLitFwd(
String specialPropName;
JSType propType;
if (prop.isGetterDef()) {
specialPropName = createGetterPropName(pname);
specialPropName = JSType.createGetterPropName(pname);
propType = funType.getReturnType();
} else {
specialPropName = createSetterPropName(pname);
specialPropName = JSType.createSetterPropName(pname);
propType = pair.type;
}
result = result.withProperty(new QualifiedName(specialPropName), propType);
Expand Down Expand Up @@ -2967,7 +2959,7 @@ private EnvTypePair analyzePropAccessFwd(Node receiver, String pname,
propAccessNode.getParent(), CONST_PROPERTY_DELETED, pname));
}
// Then, analyze the property access.
QualifiedName getterPname = new QualifiedName(createGetterPropName(pname));
QualifiedName getterPname = new QualifiedName(JSType.createGetterPropName(pname));
if (recvType.hasProp(getterPname)) {
return new EnvTypePair(pair.env, recvType.getProp(getterPname));
}
Expand Down Expand Up @@ -3974,7 +3966,7 @@ && mayWarnAboutConstProp(propAccessNode, recvType, pname)) {
mayWarnAboutDictPropAccess(obj, recvType);
}
QualifiedName setterPname =
new QualifiedName(createSetterPropName(pname.getLeftmostName()));
new QualifiedName(JSType.createSetterPropName(pname.getLeftmostName()));
if (recvType.hasProp(setterPname)) {
FunctionType funType = recvType.getProp(setterPname).getFunType();
Preconditions.checkNotNull(funType);
Expand Down
8 changes: 8 additions & 0 deletions src/com/google/javascript/jscomp/newtypes/JSType.java
Expand Up @@ -1214,6 +1214,14 @@ public JSType withFunction(FunctionType ft, NominalType fnNominal) {
getObjTypeIfSingletonObj().withFunction(ft, fnNominal));
}

public static String createGetterPropName(String originalPropName) {
return "%getter_fun" + originalPropName;
}

public static String createSetterPropName(String originalPropName) {
return "%setter_fun" + originalPropName;
}

public boolean isSingletonObj() {
return getMask() == NON_SCALAR_MASK && getObjs().size() == 1;
}
Expand Down
16 changes: 13 additions & 3 deletions src/com/google/javascript/jscomp/newtypes/ObjectType.java
Expand Up @@ -1129,12 +1129,22 @@ private Property getLeftmostPropHelper(QualifiedName qname, boolean ownProp) {
}

Node getPropertyDefSite(String propertyName) {
Property p = getLeftmostProp(new QualifiedName(propertyName));
return p == null ? null : p.getDefSite();
return getPropertyDefSiteHelper(propertyName, false);
}

Node getOwnPropertyDefSite(String propertyName) {
Property p = getLeftmostPropHelper(new QualifiedName(propertyName), /* ownProp */ true);
return getPropertyDefSiteHelper(propertyName, true);
}

Node getPropertyDefSiteHelper(String propertyName, boolean ownProp) {
Property p = getLeftmostPropHelper(new QualifiedName(propertyName), ownProp);
// Try getters and setters specially.
if (p == null) {
p = getLeftmostProp(new QualifiedName(JSType.createGetterPropName(propertyName)));
}
if (p == null) {
p = getLeftmostProp(new QualifiedName(JSType.createSetterPropName(propertyName)));
}
return p == null ? null : p.getDefSite();
}

Expand Down
Expand Up @@ -538,8 +538,6 @@ public void testNoPrivateAccessForProperties10() {
}

public void testNoPrivateAccessForProperties11() {
// TODO(aravindpg): handle looking up getters and setters as properties in NTI
this.mode = TypeInferenceMode.OtiOnly;
test(new String[] {
"/** @constructor */ function Foo() {}"
+ "Foo.prototype = {"
Expand All @@ -550,8 +548,6 @@ public void testNoPrivateAccessForProperties11() {
}

public void testNoPrivateAccessForProperties12() {
// TODO(aravindpg): handle looking up getters and setters as properties in NTI
this.mode = TypeInferenceMode.OtiOnly;
test(new String[] {
"/** @constructor */ function Foo() {}"
+ "Foo.prototype = {"
Expand Down
11 changes: 11 additions & 0 deletions test/com/google/javascript/jscomp/NewTypeInferenceTest.java
Expand Up @@ -17363,4 +17363,15 @@ public void testConstantPropertyDeletion() {
NewTypeInference.NULLABLE_DEREFERENCE,
NewTypeInference.CONST_PROPERTY_DELETED);
}

public void testSetterNotTreatedAsProp() {
typeCheck(LINE_JOINER.join(
"/** @constructor */",
"function Foo() { this.prop = 123; }",
"Foo.prototype = {",
" set a(x) { this.prop = x; }",
"};",
"var y = (new Foo).a;"),
NewTypeInference.INEXISTENT_PROPERTY);
}
}

0 comments on commit 3530a82

Please sign in to comment.