Fixed bug #1620: New @spam.getter property syntax modifies the property in place.

I added also the feature that a @prop.getter decorator does not overwrite the doc string of the property if it was given as an argument to property().

diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py
new file mode 100644 (file)
index 0000000..4b6e20c
--- /dev/null
+++ b/Lib/test/test_property.py
@@ -0,0 +1,98 @@
+# Test case for property
+# more tests are in test_descr
+
+import unittest
+from test.test_support import run_unittest
+
+class PropertyBase(Exception):
+    pass
+
+class PropertyGet(PropertyBase):
+    pass
+
+class PropertySet(PropertyBase):
+    pass
+
+class PropertyDel(PropertyBase):
+    pass
+
+class BaseClass(object):
+    def __init__(self):
+        self._spam = 5
+
+    @property
+    def spam(self):
+        """BaseClass.getter"""
+        return self._spam
+
+    @spam.setter
+    def spam(self, value):
+        self._spam = value
+
+    @spam.deleter
+    def spam(self):
+        del self._spam
+
+class SubClass(BaseClass):
+
+    @BaseClass.spam.getter
+    def spam(self):
+        """SubClass.getter"""
+        raise PropertyGet(self._spam)
+
+    @spam.setter
+    def spam(self, value):
+        raise PropertySet(self._spam)
+
+    @spam.deleter
+    def spam(self):
+        raise PropertyDel(self._spam)
+
+class PropertyDocBase(object):
+    _spam = 1
+    def _get_spam(self):
+        return self._spam
+    spam = property(_get_spam, doc="spam spam spam")
+
+class PropertyDocSub(PropertyDocBase):
+    @PropertyDocBase.spam.getter
+    def spam(self):
+        """The decorator does not use this doc string"""
+        return self._spam
+
+class PropertyTests(unittest.TestCase):
+    def test_property_decorator_baseclass(self):
+        # see #1620
+        base = BaseClass()
+        self.assertEqual(base.spam, 5)
+        self.assertEqual(base._spam, 5)
+        base.spam = 10
+        self.assertEqual(base.spam, 10)
+        self.assertEqual(base._spam, 10)
+        delattr(base, "spam")
+        self.assert_(not hasattr(base, "spam"))
+        self.assert_(not hasattr(base, "_spam"))
+        base.spam = 20
+        self.assertEqual(base.spam, 20)
+        self.assertEqual(base._spam, 20)
+        self.assertEqual(base.__class__.spam.__doc__, "BaseClass.getter")
+
+    def test_property_decorator_subclass(self):
+        # see #1620
+        sub = SubClass()
+        self.assertRaises(PropertyGet, getattr, sub, "spam")
+        self.assertRaises(PropertySet, setattr, sub, "spam", None)
+        self.assertRaises(PropertyDel, delattr, sub, "spam")
+        self.assertEqual(sub.__class__.spam.__doc__, "SubClass.getter")
+
+    def test_property_decorator_doc(self):
+        base = PropertyDocBase()
+        sub = PropertyDocSub()
+        self.assertEqual(base.__class__.spam.__doc__, "spam spam spam")
+        self.assertEqual(sub.__class__.spam.__doc__, "spam spam spam")
+
+def test_main():
+    run_unittest(PropertyTests)
+
+if __name__ == '__main__':
+    test_main()

diff --git a/Misc/NEWS b/Misc/NEWS
index 0d420d0e38501a7ff836d5936aa7cd0bd4b48536..3c95ca53aee28a6534d0335b62842c60262da59b 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,6 +12,9 @@ What's New in Python 2.6 alpha 1?
 Core and builtins
 -----------------
 
+- Issue #1620: New property decorator syntax was modifying the decorator
+  in place instead of creating a new decorator object.
+
 - Issue #1580: New free format floating point representation based on
   "Floating-Point Printer Sample Code", by Robert G. Burger. For example
   repr(11./5) now returns '2.2' instead of '2.2000000000000002'.

diff --git a/Objects/descrobject.c b/Objects/descrobject.c
index 56599ef29d79e60c5593a48e14fae813f8308d9a..3cfdb973489a0a0154ec66d32115e81cae874f8e 100644 (file)
--- a/Objects/descrobject.c
+++ b/Objects/descrobject.c
@@ -1098,8 +1098,12 @@ typedef struct {
        PyObject *prop_set;
        PyObject *prop_del;
        PyObject *prop_doc;
+       int getter_doc;
 } propertyobject;
 
+static PyObject * property_copy(PyObject *, PyObject *, PyObject *,
+                                 PyObject *, PyObject *);
+
 static PyMemberDef property_members[] = {
        {"fget", T_OBJECT, offsetof(propertyobject, prop_get), READONLY},
        {"fset", T_OBJECT, offsetof(propertyobject, prop_set), READONLY},
@@ -1108,53 +1112,37 @@ static PyMemberDef property_members[] = {
        {0}
 };
 
+
 PyDoc_STRVAR(getter_doc,
             "Descriptor to change the getter on a property.");
 
 PyObject *
 property_getter(PyObject *self, PyObject *getter)
 {
-       Py_XDECREF(((propertyobject *)self)->prop_get);
-       if (getter == Py_None)
-               getter = NULL;
-       Py_XINCREF(getter);
-       ((propertyobject *)self)->prop_get = getter;
-       Py_INCREF(self);
-       return self;
+       return property_copy(self, getter, NULL, NULL, NULL);
 }
 
+
 PyDoc_STRVAR(setter_doc,
-            "Descriptor to change the setter on a property.\n");
+            "Descriptor to change the setter on a property.");
 
 PyObject *
 property_setter(PyObject *self, PyObject *setter)
 {
-       Py_XDECREF(((propertyobject *)self)->prop_set);
-       if (setter == Py_None)
-               setter = NULL;
-       Py_XINCREF(setter);
-       ((propertyobject *)self)->prop_set = setter;
-       Py_INCREF(self);
-       return self;
+       return property_copy(self, NULL, setter, NULL, NULL);
 }
 
+
 PyDoc_STRVAR(deleter_doc,
             "Descriptor to change the deleter on a property.");
 
 PyObject *
 property_deleter(PyObject *self, PyObject *deleter)
 {
-       Py_XDECREF(((propertyobject *)self)->prop_del);
-       if (deleter == Py_None)
-               deleter = NULL;
-       Py_XINCREF(deleter);
-       ((propertyobject *)self)->prop_del = deleter;
-       Py_INCREF(self);
-       return self;
+       return property_copy(self, NULL, NULL, deleter, NULL);
 }
 
 
-
 static PyMethodDef property_methods[] = {
        {"getter", property_getter, METH_O, getter_doc},
        {"setter", property_setter, METH_O, setter_doc},
@@ -1219,15 +1207,62 @@ property_descr_set(PyObject *self, PyObject *obj, PyObject *value)
        return 0;
 }
 
+static PyObject *
+property_copy(PyObject *old, PyObject *get, PyObject *set, PyObject *del,
+               PyObject *doc)
+{
+       propertyobject *pold = (propertyobject *)old;
+       propertyobject *pnew = NULL;
+       PyObject *new, *type;
+
+       type = PyObject_Type(old);
+       if (type == NULL)
+               return NULL;
+
+       if (get == NULL || get == Py_None) {
+               Py_XDECREF(get);
+               get = pold->prop_get ? pold->prop_get : Py_None;
+       }
+       if (set == NULL || set == Py_None) {
+               Py_XDECREF(set);
+               set = pold->prop_set ? pold->prop_set : Py_None;
+       }
+       if (del == NULL || del == Py_None) {
+               Py_XDECREF(del);
+               del = pold->prop_del ? pold->prop_del : Py_None;
+       }
+       if (doc == NULL || doc == Py_None) {
+               Py_XDECREF(doc);
+               doc = pold->prop_doc ? pold->prop_doc : Py_None;
+       }
+       
+       new =  PyObject_CallFunction(type, "OOOO", get, set, del, doc);
+       if (new == NULL)
+               return NULL;
+       pnew = (propertyobject *)new;
+       
+       if (pold->getter_doc && get != Py_None) {
+               PyObject *get_doc = PyObject_GetAttrString(get, "__doc__");
+               if (get_doc != NULL) {
+                       Py_XDECREF(pnew->prop_doc);
+                       pnew->prop_doc = get_doc;  /* get_doc already INCREF'd by GetAttr */
+                       pnew->getter_doc = 1;
+               } else {
+                       PyErr_Clear();
+               }
+       }
+       return new;
+}
+
 static int
 property_init(PyObject *self, PyObject *args, PyObject *kwds)
 {
        PyObject *get = NULL, *set = NULL, *del = NULL, *doc = NULL;
        static char *kwlist[] = {"fget", "fset", "fdel", "doc", 0};
-       propertyobject *gs = (propertyobject *)self;
-
+       propertyobject *prop = (propertyobject *)self;
+       
        if (!PyArg_ParseTupleAndKeywords(args, kwds, "|OOOO:property",
-                                        kwlist, &get, &set, &del, &doc))
+                                        kwlist, &get, &set, &del, &doc))
                return -1;
 
        if (get == Py_None)
@@ -1242,22 +1277,24 @@ property_init(PyObject *self, PyObject *args, PyObject *kwds)
        Py_XINCREF(del);
        Py_XINCREF(doc);
 
+       prop->prop_get = get;
+       prop->prop_set = set;
+       prop->prop_del = del;
+       prop->prop_doc = doc;
+       prop->getter_doc = 0;
+
        /* if no docstring given and the getter has one, use that one */
        if ((doc == NULL || doc == Py_None) && get != NULL) {
                PyObject *get_doc = PyObject_GetAttrString(get, "__doc__");
                if (get_doc != NULL) {
-                       Py_XDECREF(doc);
-                       doc = get_doc;  /* get_doc already INCREF'd by GetAttr */
+                       Py_XDECREF(prop->prop_doc);
+                       prop->prop_doc = get_doc;  /* get_doc already INCREF'd by GetAttr */
+                       prop->getter_doc = 1;
                } else {
                        PyErr_Clear();
                }
        }
 
-       gs->prop_get = get;
-       gs->prop_set = set;
-       gs->prop_del = del;
-       gs->prop_doc = doc;
-
        return 0;
 }