do distinguish between dict . and [] operators

Author: philippedistributive

include/JSObjectProxy.hh

@@ -48,14 +48,23 @@ public:
   static Py_ssize_t JSObjectProxy_length(JSObjectProxy *self);
 
   /**
-   * @brief Getter method (.mp_subscript), returns a value from the JSObjectProxy given a key, used by several built-in python methods as well as the [] operator
+   * @brief Getter method, returns a value from the JSObjectProxy given a key, used by several built-in python methods as well as the . operator
    *
    * @param self - The JSObjectProxy
    * @param key - The key for the value in the JSObjectProxy
    * @return PyObject* NULL on exception, the corresponding value otherwise
    */
   static PyObject *JSObjectProxy_get(JSObjectProxy *self, PyObject *key);
 
+  /**
+   * @brief Getter method (.mp_subscript), returns a value from the JSObjectProxy given a key, used by the [] operator
+   *
+   * @param self - The JSObjectProxy
+   * @param key - The key for the value in the JSObjectProxy
+   * @return PyObject* NULL on exception, the corresponding value otherwise
+   */
+  static PyObject *JSObjectProxy_get_subscript(JSObjectProxy *self, PyObject *key);
+
   /**
    * @brief Test method (.sq_contains), returns whether a key exists, used by the in operator
    *
@@ -289,7 +298,7 @@ PyDoc_STRVAR(dict_values__doc__,
  */
 static PyMappingMethods JSObjectProxy_mapping_methods = {
   .mp_length = (lenfunc)JSObjectProxyMethodDefinitions::JSObjectProxy_length,
-  .mp_subscript = (binaryfunc)JSObjectProxyMethodDefinitions::JSObjectProxy_get,
+  .mp_subscript = (binaryfunc)JSObjectProxyMethodDefinitions::JSObjectProxy_get_subscript,
   .mp_ass_subscript = (objobjargproc)JSObjectProxyMethodDefinitions::JSObjectProxy_assign
 };
 

src/JSObjectProxy.cc

@@ -64,7 +64,7 @@ Py_ssize_t JSObjectProxyMethodDefinitions::JSObjectProxy_length(JSObjectProxy *s
   return props.length();
 }
 
-static inline PyObject *getKey(JSObjectProxy *self, PyObject *key, JS::HandleId id) {
+static inline PyObject *getKey(JSObjectProxy *self, PyObject *key, JS::HandleId id, bool checkPropertyShadowsMethod) {
   // look through the methods for dispatch
   for (size_t index = 0;; index++) {
     const char *methodName = JSObjectProxyType.tp_methods[index].ml_name;
@@ -111,14 +111,16 @@ static inline PyObject *getKey(JSObjectProxy *self, PyObject *key, JS::HandleId
     }
     else {
       if (strcmp(methodName, PyUnicode_AsUTF8(key)) == 0) {
-        // just make sure no property is shadowing a method by name
-        JS::RootedValue value(GLOBAL_CX);
-        JS_GetPropertyById(GLOBAL_CX, *(self->jsObject), id, &value);
-        if (!value.isUndefined()) {
-          return pyTypeFactory(GLOBAL_CX, value)->getPyObject();
-        } else {
-          return PyObject_GenericGetAttr((PyObject *)self, key);
+        if (checkPropertyShadowsMethod) {
+          // just make sure no property is shadowing a method by name
+          JS::RootedValue value(GLOBAL_CX);
+          JS_GetPropertyById(GLOBAL_CX, *(self->jsObject), id, &value);
+          if (!value.isUndefined()) {
+            return pyTypeFactory(GLOBAL_CX, value)->getPyObject();
+          }
         }
+
+        return PyObject_GenericGetAttr((PyObject *)self, key);
       }
     }
   }
@@ -132,7 +134,18 @@ PyObject *JSObjectProxyMethodDefinitions::JSObjectProxy_get(JSObjectProxy *self,
     return NULL;
   }
 
-  return getKey(self, key, id);
+  return getKey(self, key, id, false);
+}
+
+PyObject *JSObjectProxyMethodDefinitions::JSObjectProxy_get_subscript(JSObjectProxy *self, PyObject *key)
+{
+  JS::RootedId id(GLOBAL_CX);
+  if (!keyToId(key, &id)) {
+    PyErr_SetString(PyExc_AttributeError, "JSObjectProxy property name must be of type str or int");
+    return NULL;
+  }
+
+  return getKey(self, key, id, true);
 }
 
 int JSObjectProxyMethodDefinitions::JSObjectProxy_contains(JSObjectProxy *self, PyObject *key)
@@ -589,7 +602,7 @@ PyObject *JSObjectProxyMethodDefinitions::JSObjectProxy_setdefault_method(JSObje
     return NULL;
   }
 
-  PyObject *value = getKey(self, key, id);
+  PyObject *value = getKey(self, key, id, true);
   if (value == Py_None) {
     assignKeyValue(self, key, id, default_value);
     Py_XINCREF(default_value);

tests/python/test_dict_methods.py

@@ -55,7 +55,12 @@ def test_setdefault_no_params():
         assert (False)
     except Exception as e:    
         assert str(type(e)) == "<class 'TypeError'>"
-        assert str(e) == "setdefault expected at least 1 argument, got 0"      
+        assert str(e) == "setdefault expected at least 1 argument, got 0"    
+
+def test_setdefault_with_shadowing():
+    jsObj = pm.eval("({get: 'value'})")
+    a = jsObj.setdefault("get", "val")
+    assert a == 'value'
 
 #pop
 def test_pop_found():
@@ -453,5 +458,5 @@ def test_get_method():
 #get method shadowing
 def test_method_shadowing():
     jsObj = pm.eval("({get: 'value'})")
-    assert jsObj.get == 'value'
+    assert repr(jsObj.get).__contains__("<built-in method get of dict object at")
     assert jsObj['get'] == 'value'