From 3530a822bc43d05a349539ba6e7c17e9a8ba813b Mon Sep 17 00:00:00 2001 From: aravindpg Date: Wed, 27 Jul 2016 15:04:41 -0700 Subject: [PATCH] [NTI] Handle getters and setters as properties in ObjectType::getPropertyDefsite. CheckAccessControls benefits from this. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=128633706 --- .../google/javascript/jscomp/GlobalTypeInfo.java | 4 ++-- .../javascript/jscomp/NewTypeInference.java | 16 ++++------------ .../javascript/jscomp/newtypes/JSType.java | 8 ++++++++ .../javascript/jscomp/newtypes/ObjectType.java | 16 +++++++++++++--- .../jscomp/CheckAccessControlsTest.java | 4 ---- .../javascript/jscomp/NewTypeInferenceTest.java | 11 +++++++++++ 6 files changed, 38 insertions(+), 21 deletions(-) diff --git a/src/com/google/javascript/jscomp/GlobalTypeInfo.java b/src/com/google/javascript/jscomp/GlobalTypeInfo.java index c97f665ac1a..7221e0ba8ec 100644 --- a/src/com/google/javascript/jscomp/GlobalTypeInfo.java +++ b/src/com/google/javascript/jscomp/GlobalTypeInfo.java @@ -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 diff --git a/src/com/google/javascript/jscomp/NewTypeInference.java b/src/com/google/javascript/jscomp/NewTypeInference.java index 3a38fb12ef1..33c61a83ba8 100644 --- a/src/com/google/javascript/jscomp/NewTypeInference.java +++ b/src/com/google/javascript/jscomp/NewTypeInference.java @@ -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 dn) { List> inEdges = dn.getInEdges(); // True for code considered dead in the CFG @@ -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); @@ -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)); } @@ -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); diff --git a/src/com/google/javascript/jscomp/newtypes/JSType.java b/src/com/google/javascript/jscomp/newtypes/JSType.java index 22a69e90221..1f0db5c0cc0 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSType.java +++ b/src/com/google/javascript/jscomp/newtypes/JSType.java @@ -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; } diff --git a/src/com/google/javascript/jscomp/newtypes/ObjectType.java b/src/com/google/javascript/jscomp/newtypes/ObjectType.java index f168107aef1..7f8bdbba724 100644 --- a/src/com/google/javascript/jscomp/newtypes/ObjectType.java +++ b/src/com/google/javascript/jscomp/newtypes/ObjectType.java @@ -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(); } diff --git a/test/com/google/javascript/jscomp/CheckAccessControlsTest.java b/test/com/google/javascript/jscomp/CheckAccessControlsTest.java index 375de929c6e..24c2e7a0999 100644 --- a/test/com/google/javascript/jscomp/CheckAccessControlsTest.java +++ b/test/com/google/javascript/jscomp/CheckAccessControlsTest.java @@ -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 = {" @@ -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 = {" diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceTest.java b/test/com/google/javascript/jscomp/NewTypeInferenceTest.java index a97352676b7..e2309e699f0 100644 --- a/test/com/google/javascript/jscomp/NewTypeInferenceTest.java +++ b/test/com/google/javascript/jscomp/NewTypeInferenceTest.java @@ -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); + } }