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().

3 files changed, 171 insertions, 33 deletions

diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py
new file mode 100644
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 0d420d0..3c95ca5 100644
--- 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 56599ef..3cfdb97 100644
--- 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;
 }