From 4829522b8d3e1a28930f1cccfcc9635e035a0eb4 Mon Sep 17 00:00:00 2001 From: "E. M. Bray" Date: Mon, 10 Jun 2024 10:55:49 +0200 Subject: [PATCH] bpo-24766: doc= argument to subclasses of property not handled correctly (GH-2487) Co-authored-by: Serhiy Storchaka --- Lib/test/test_property.py | 34 +++++++++++++++++++ .../2018-10-09-15-14-53.bpo-24766.c_C1Wc.rst | 1 + Objects/descrobject.c | 19 +++-------- 3 files changed, 39 insertions(+), 15 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-10-09-15-14-53.bpo-24766.c_C1Wc.rst diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py index 408e64f5314..b7a2219b961 100644 --- a/Lib/test/test_property.py +++ b/Lib/test/test_property.py @@ -463,6 +463,40 @@ class PropertySubclassTests(unittest.TestCase): self.assertEqual(p.__doc__, "user") self.assertEqual(p2.__doc__, "user") + @unittest.skipIf(sys.flags.optimize >= 2, + "Docstrings are omitted with -O2 and above") + def test_prefer_explicit_doc(self): + # Issue 25757: subclasses of property lose docstring + self.assertEqual(property(doc="explicit doc").__doc__, "explicit doc") + self.assertEqual(PropertySub(doc="explicit doc").__doc__, "explicit doc") + + class Foo: + spam = PropertySub(doc="spam explicit doc") + + @spam.getter + def spam(self): + """ignored as doc already set""" + return 1 + + def _stuff_getter(self): + """ignored as doc set directly""" + stuff = PropertySub(doc="stuff doc argument", fget=_stuff_getter) + + #self.assertEqual(Foo.spam.__doc__, "spam explicit doc") + self.assertEqual(Foo.stuff.__doc__, "stuff doc argument") + + def test_property_no_doc_on_getter(self): + # If a property's getter has no __doc__ then the property's doc should + # be None; test that this is consistent with subclasses as well; see + # GH-2487 + class NoDoc: + @property + def __doc__(self): + raise AttributeError + + self.assertEqual(property(NoDoc()).__doc__, None) + self.assertEqual(PropertySub(NoDoc()).__doc__, None) + @unittest.skipIf(sys.flags.optimize >= 2, "Docstrings are omitted with -O2 and above") def test_property_setter_copies_getter_docstring(self): diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-10-09-15-14-53.bpo-24766.c_C1Wc.rst b/Misc/NEWS.d/next/Core and Builtins/2018-10-09-15-14-53.bpo-24766.c_C1Wc.rst new file mode 100644 index 00000000000..93a8562efe6 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-10-09-15-14-53.bpo-24766.c_C1Wc.rst @@ -0,0 +1 @@ +Fix handling of ``doc`` argument to subclasses of ``property``. diff --git a/Objects/descrobject.c b/Objects/descrobject.c index 1b7e2fde3ce..4eccd1704eb 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1859,22 +1859,9 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset, /* if no docstring given and the getter has one, use that one */ else if (fget != NULL) { int rc = PyObject_GetOptionalAttr(fget, &_Py_ID(__doc__), &prop_doc); - if (rc <= 0) { + if (rc < 0) { return rc; } - if (!Py_IS_TYPE(self, &PyProperty_Type) && - prop_doc != NULL && prop_doc != Py_None) { - // This oddity preserves the long existing behavior of surfacing - // an AttributeError when using a dict-less (__slots__) property - // subclass as a decorator on a getter method with a docstring. - // See PropertySubclassTest.test_slots_docstring_copy_exception. - int err = PyObject_SetAttr( - (PyObject *)self, &_Py_ID(__doc__), prop_doc); - if (err < 0) { - Py_DECREF(prop_doc); // release our new reference. - return -1; - } - } if (prop_doc == Py_None) { prop_doc = NULL; Py_DECREF(Py_None); @@ -1902,7 +1889,9 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset, Py_DECREF(prop_doc); if (err < 0) { assert(PyErr_Occurred()); - if (PyErr_ExceptionMatches(PyExc_AttributeError)) { + if (!self->getter_doc && + PyErr_ExceptionMatches(PyExc_AttributeError)) + { PyErr_Clear(); // https://github.com/python/cpython/issues/98963#issuecomment-1574413319 // Python silently dropped this doc assignment through 3.11.