Issue #7990: dir() on ElementTree.Element now lists properties: "tag",

"text", "tail" and "attrib".  Original patch by Santoso Wijaya.

3 files changed, 103 insertions, 80 deletions

diff --git a/Lib/test/test_xml_etree.py b/Lib/test/test_xml_etree.py
index 57d8e4d2de..029320153e 100644
--- a/Lib/test/test_xml_etree.py
+++ b/Lib/test/test_xml_etree.py
@@ -182,10 +182,12 @@ class ElementTreeTest(unittest.TestCase):
 
         def check_element(element):
             self.assertTrue(ET.iselement(element), msg="not an element")
-            self.assertTrue(hasattr(element, "tag"), msg="no tag member")
-            self.assertTrue(hasattr(element, "attrib"), msg="no attrib member")
-            self.assertTrue(hasattr(element, "text"), msg="no text member")
-            self.assertTrue(hasattr(element, "tail"), msg="no tail member")
+            direlem = dir(element)
+            for attr in 'tag', 'attrib', 'text', 'tail':
+                self.assertTrue(hasattr(element, attr),
+                        msg='no %s member' % attr)
+                self.assertIn(attr, direlem,
+                        msg='no %s visible by dir' % attr)
 
             check_string(element.tag)
             check_mapping(element.attrib)
diff --git a/Misc/NEWS b/Misc/NEWS
index ac1744bb60..56a4d2d841 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -95,6 +95,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #7990: dir() on ElementTree.Element now lists properties: "tag",
+  "text", "tail" and "attrib".  Original patch by Santoso Wijaya.
+
 - Issue #25725: Fixed a reference leak in pickle.loads() when unpickling
   invalid data including tuple instructions.
 
diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c
index 744e8338de..f03e136ef2 100644
--- a/Modules/_elementtree.c
+++ b/Modules/_elementtree.c
@@ -1870,94 +1870,92 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value)
 }
 
 static PyObject*
-element_getattro(ElementObject* self, PyObject* nameobj)
+element_tag_getter(ElementObject *self, void *closure)
 {
-    PyObject* res;
-    char *name = "";
+    PyObject *res = self->tag;
+    Py_INCREF(res);
+    return res;
+}
 
-    if (PyUnicode_Check(nameobj))
-        name = _PyUnicode_AsString(nameobj);
+static PyObject*
+element_text_getter(ElementObject *self, void *closure)
+{
+    PyObject *res = element_get_text(self);
+    Py_XINCREF(res);
+    return res;
+}
 
-    if (name == NULL)
-        return NULL;
+static PyObject*
+element_tail_getter(ElementObject *self, void *closure)
+{
+    PyObject *res = element_get_tail(self);
+    Py_XINCREF(res);
+    return res;
+}
 
-    /* handle common attributes first */
-    if (strcmp(name, "tag") == 0) {
-        res = self->tag;
-        Py_INCREF(res);
-        return res;
-    } else if (strcmp(name, "text") == 0) {
-        res = element_get_text(self);
-        Py_XINCREF(res);
-        return res;
+static PyObject*
+element_attrib_getter(ElementObject *self, void *closure)
+{
+    PyObject *res;
+    if (!self->extra) {
+        if (create_extra(self, NULL) < 0)
+            return NULL;
     }
+    res = element_get_attrib(self);
+    Py_XINCREF(res);
+    return res;
+}
 
-    /* methods */
-    res = PyObject_GenericGetAttr((PyObject*) self, nameobj);
-    if (res)
-        return res;
-
-    /* less common attributes */
-    if (strcmp(name, "tail") == 0) {
-        PyErr_Clear();
-        res = element_get_tail(self);
-    } else if (strcmp(name, "attrib") == 0) {
-        PyErr_Clear();
-        if (!self->extra) {
-            if (create_extra(self, NULL) < 0)
-                return NULL;
-        }
-        res = element_get_attrib(self);
+/* macro for setter validation */
+#define _VALIDATE_ATTR_VALUE(V)                     \
+    if ((V) == NULL) {                              \
+        PyErr_SetString(                            \
+            PyExc_AttributeError,                   \
+            "can't delete element attribute");      \
+        return -1;                                  \
     }
 
-    if (!res)
-        return NULL;
-
-    Py_INCREF(res);
-    return res;
+static int
+element_tag_setter(ElementObject *self, PyObject *value, void *closure)
+{
+    _VALIDATE_ATTR_VALUE(value);
+    Py_INCREF(value);
+    Py_DECREF(self->tag);
+    self->tag = value;
+    return 0;
 }
 
 static int
-element_setattro(ElementObject* self, PyObject* nameobj, PyObject* value)
+element_text_setter(ElementObject *self, PyObject *value, void *closure)
 {
-    char *name = "";
+    _VALIDATE_ATTR_VALUE(value);
+    Py_INCREF(value);
+    Py_DECREF(JOIN_OBJ(self->text));
+    self->text = value;
+    return 0;
+}
 
-    if (value == NULL) {
-        PyErr_SetString(PyExc_AttributeError,
-            "can't delete attribute");
-        return -1;
-    }
-    if (PyUnicode_Check(nameobj))
-        name = _PyUnicode_AsString(nameobj);
-    if (name == NULL)
-        return -1;
+static int
+element_tail_setter(ElementObject *self, PyObject *value, void *closure)
+{
+    _VALIDATE_ATTR_VALUE(value);
+    Py_INCREF(value);
+    Py_DECREF(JOIN_OBJ(self->tail));
+    self->tail = value;
+    return 0;
+}
 
-    if (strcmp(name, "tag") == 0) {
-        Py_DECREF(self->tag);
-        self->tag = value;
-        Py_INCREF(self->tag);
-    } else if (strcmp(name, "text") == 0) {
-        Py_DECREF(JOIN_OBJ(self->text));
-        self->text = value;
-        Py_INCREF(self->text);
-    } else if (strcmp(name, "tail") == 0) {
-        Py_DECREF(JOIN_OBJ(self->tail));
-        self->tail = value;
-        Py_INCREF(self->tail);
-    } else if (strcmp(name, "attrib") == 0) {
-        if (!self->extra) {
-            if (create_extra(self, NULL) < 0)
-                return -1;
-        }
-        Py_DECREF(self->extra->attrib);
-        self->extra->attrib = value;
-        Py_INCREF(self->extra->attrib);
-    } else {
-        PyErr_SetString(PyExc_AttributeError,
-            "Can't set arbitrary attributes on Element");
-        return -1;
+static int
+element_attrib_setter(ElementObject *self, PyObject *value, void *closure)
+{
+    _VALIDATE_ATTR_VALUE(value);
+    if (!self->extra) {
+        if (create_extra(self, NULL) < 0)
+            return -1;
     }
-
+    Py_INCREF(value);
+    Py_DECREF(self->extra->attrib);
+    self->extra->attrib = value;
     return 0;
 }
 
@@ -3770,6 +3768,26 @@ static PyMappingMethods element_as_mapping = {
     (objobjargproc) element_ass_subscr,
 };
 
+static PyGetSetDef element_getsetlist[] = {
+    {"tag",
+        (getter)element_tag_getter,
+        (setter)element_tag_setter,
+        "A string identifying what kind of data this element represents"},
+    {"text",
+        (getter)element_text_getter,
+        (setter)element_text_setter,
+        "A string of text directly after the start tag, or None"},
+    {"tail",
+        (getter)element_tail_getter,
+        (setter)element_tail_setter,
+        "A string of text directly after the end tag, or None"},
+    {"attrib",
+        (getter)element_attrib_getter,
+        (setter)element_attrib_setter,
+        "A dictionary containing the element's attributes"},
+    {NULL},
+};
+
 static PyTypeObject Element_Type = {
     PyVarObject_HEAD_INIT(NULL, 0)
     "xml.etree.ElementTree.Element", sizeof(ElementObject), 0,
@@ -3786,8 +3804,8 @@ static PyTypeObject Element_Type = {
     0,                                              /* tp_hash */
     0,                                              /* tp_call */
     0,                                              /* tp_str */
-    (getattrofunc)element_getattro,                 /* tp_getattro */
-    (setattrofunc)element_setattro,                 /* tp_setattro */
+    PyObject_GenericGetAttr,                        /* tp_getattro */
+    0,                                              /* tp_setattro */
     0,                                              /* tp_as_buffer */
     Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC,
                                                     /* tp_flags */
@@ -3800,7 +3818,7 @@ static PyTypeObject Element_Type = {
     0,                                              /* tp_iternext */
     element_methods,                                /* tp_methods */
     0,                                              /* tp_members */
-    0,                                              /* tp_getset */
+    element_getsetlist,                             /* tp_getset */
     0,                                              /* tp_base */
     0,                                              /* tp_dict */
     0,                                              /* tp_descr_get */