From 90e10e79ea1be00489fd68e10e67911dda0567c9 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Fri, 14 Dec 2007 02:35:23 +0000 Subject: [PATCH] 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(). --- Lib/test/test_property.py | 98 ++++++++++++++++++++++++++++++++++++ Misc/NEWS | 3 ++ Objects/descrobject.c | 103 ++++++++++++++++++++++++++------------ 3 files changed, 171 insertions(+), 33 deletions(-) create mode 100644 Lib/test/test_property.py diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py new file mode 100644 index 00000000000..4b6e20caa39 --- /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 0d420d0e385..3c95ca53aee 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 56599ef29d7..3cfdb973489 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; }