From f530eaf58f34c03fc4a8dd79dd04e1257f99620e Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 3 Nov 2022 14:21:16 +0100 Subject: [PATCH 1/3] gh-98963: Add a note to the error for property subclasses without __doc__ Subclasses created using the C API don't get __dict__, which means their __doc__ can't be set for each property separately. The __doc__ descriptor from `property` itself is shadowed by the subclass's docstring. This edge case isn't really worth mentioning in the docs. This adds a note that should guide anyone encountering the issue to the right fix. --- Lib/test/test_descr.py | 11 ++++++++++ ...2-11-03-13-54-54.gh-issue-98963.Z3c6M5.rst | 3 +++ Objects/descrobject.c | 22 ++++++++++++++++++- 3 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-11-03-13-54-54.gh-issue-98963.Z3c6M5.rst diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py index 40cf81ff0b33f5..536f454460e517 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -2359,6 +2359,17 @@ def test_testcapi_no_segfault(self): class X(object): p = property(_testcapi.test_with_docstring) + def test_subclass_with_nonwritable_doc(self): + # gh-98963: subclasses must have writable __doc__. A note is added to + # the exception in this surprising edge case. + class BadProperty(property): + __doc__ = property(lambda: None) + with self.assertRaises(AttributeError) as cm: + p = BadProperty(lambda: 123) + notes = cm.exception.__notes__ + wanted = "subclasses of 'property' need to provide a writable __doc__" + self.assertTrue(any(note.startswith(wanted) for note in notes), notes) + def test_properties_plus(self): class C(object): foo = property(doc="hello") diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-11-03-13-54-54.gh-issue-98963.Z3c6M5.rst b/Misc/NEWS.d/next/Core and Builtins/2022-11-03-13-54-54.gh-issue-98963.Z3c6M5.rst new file mode 100644 index 00000000000000..3a6d4ed5313bb5 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-11-03-13-54-54.gh-issue-98963.Z3c6M5.rst @@ -0,0 +1,3 @@ +Add a note to the ``AttributeError`` raised when instantiating a subclass of +:py:type:`property` that does not have a writable ``__doc__`` attribute. +(Creating such a subclass is possible via the C API.) diff --git a/Objects/descrobject.c b/Objects/descrobject.c index a2974f91aaaec3..ea57029b8963e0 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -1826,8 +1826,28 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset, int err = PyObject_SetAttr( (PyObject *)self, &_Py_ID(__doc__), prop_doc); Py_XDECREF(prop_doc); - if (err < 0) + if (err < 0) { + if (PyErr_ExceptionMatches(PyExc_AttributeError)) { + // gh-98963: the subclass doesn't have a __dict__ + // (probably it was subclassed using C API). + // Add a note to prevent surprises. + PyObject *type, *value, *traceback; + PyErr_Fetch(&type, &value, &traceback); + PyErr_NormalizeException(&type, &value, &traceback); + if (value) { + PyObject_CallMethod( + value, "add_note", "s", + "subclasses of 'property' need to provide a writable " + "__doc__ attribute (e.g. a __doc__ member or a " + "__dict__) to avoid per-property docstrings being " + "shadowed by the subclass docstring"); + // ignore errors while setting the note + PyErr_Clear(); + } + PyErr_Restore(type, value, traceback); + } return -1; + } } return 0; From 2d7945467d4895b286567da4351ee572c049aeb4 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 3 Nov 2022 17:15:38 +0100 Subject: [PATCH 2/3] Fix ReST syntax --- .../2022-11-03-13-54-54.gh-issue-98963.Z3c6M5.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-11-03-13-54-54.gh-issue-98963.Z3c6M5.rst b/Misc/NEWS.d/next/Core and Builtins/2022-11-03-13-54-54.gh-issue-98963.Z3c6M5.rst index 3a6d4ed5313bb5..9045b46a9d35d3 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2022-11-03-13-54-54.gh-issue-98963.Z3c6M5.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2022-11-03-13-54-54.gh-issue-98963.Z3c6M5.rst @@ -1,3 +1,3 @@ Add a note to the ``AttributeError`` raised when instantiating a subclass of -:py:type:`property` that does not have a writable ``__doc__`` attribute. +:py:class:`property` that does not have a writable ``__doc__`` attribute. (Creating such a subclass is possible via the C API.) From 66428bc68973e2edee5358170d707a57ae1330df Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 3 Nov 2022 17:17:41 +0100 Subject: [PATCH 3/3] Move the note check to an existing test --- Lib/test/test_descr.py | 11 ----------- Lib/test/test_property.py | 12 +++++++----- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py index 536f454460e517..40cf81ff0b33f5 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -2359,17 +2359,6 @@ def test_testcapi_no_segfault(self): class X(object): p = property(_testcapi.test_with_docstring) - def test_subclass_with_nonwritable_doc(self): - # gh-98963: subclasses must have writable __doc__. A note is added to - # the exception in this surprising edge case. - class BadProperty(property): - __doc__ = property(lambda: None) - with self.assertRaises(AttributeError) as cm: - p = BadProperty(lambda: 123) - notes = cm.exception.__notes__ - wanted = "subclasses of 'property' need to provide a writable __doc__" - self.assertTrue(any(note.startswith(wanted) for note in notes), notes) - def test_properties_plus(self): class C(object): foo = property(doc="hello") diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py index d07b8632aa8722..7fe676c1ecc698 100644 --- a/Lib/test/test_property.py +++ b/Lib/test/test_property.py @@ -229,16 +229,18 @@ class PropertySubSlots(property): class PropertySubclassTests(unittest.TestCase): def test_slots_docstring_copy_exception(self): - try: + with self.assertRaises(AttributeError) as cm: class Foo(object): @PropertySubSlots def spam(self): """Trying to copy this docstring will raise an exception""" return 1 - except AttributeError: - pass - else: - raise Exception("AttributeError not raised") + # gh-98963: A note is added to the exception when we don't have + # a writable __doc__. + notes = cm.exception.__notes__ + wanted = "subclasses of 'property' need to provide a writable __doc__" + self.assertTrue(any(note.startswith(wanted) for note in notes), notes) + @unittest.skipIf(sys.flags.optimize >= 2, "Docstrings are omitted with -O2 and above")