[3.12] gh-98963: Restore the ability to have a dict-less property. (GH-105262) (#105297)

miss-islington · gpshead · web-flow · commit 3e7ddc29a3f3 · 2023-06-05T03:45:12.000Z
gh-98963: Restore the ability to have a dict-less property. (GH-105262) Ignore doc string assignment failures in `property` as has been the behavior of all past Python releases. (the docstring is discarded) (cherry picked from commit 418befd) This fixes a behavior regression in 3.12beta1 where an AttributeError was being raised in a situation it has never been in the past. It keeps the existing unusual single situation where AttributeError does get raised. Existing widely deployed projects depend on this not raising an exception. Co-authored-by: Gregory P. Smith <greg@krypto.org>
diff --git a/Lib/test/test_property.py b/Lib/test/test_property.py
@@ -246,16 +246,67 @@ class PropertySubSlots(property):
 class PropertySubclassTests(unittest.TestCase):
 
     def test_slots_docstring_copy_exception(self):
-        try:
+        # A special case error that we preserve despite the GH-98963 behavior
+        # that would otherwise silently ignore this error.
+        # This came from commit b18500d39d791c879e9904ebac293402b4a7cd34
+        # as part of https://bugs.python.org/issue5890 which allowed docs to
+        # be set via property subclasses in the first place.
+        with self.assertRaises(AttributeError):
             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")
+
+    def test_property_with_slots_no_docstring(self):
+        # https://github.com/python/cpython/issues/98963#issuecomment-1574413319
+        class slotted_prop(property):
+            __slots__ = ("foo",)
+
+        p = slotted_prop()  # no AttributeError
+        self.assertIsNone(getattr(p, "__doc__", None))
+
+        def undocumented_getter():
+            return 4
+
+        p = slotted_prop(undocumented_getter)  # New in 3.12: no AttributeError
+        self.assertIsNone(getattr(p, "__doc__", None))
+
+    @unittest.skipIf(sys.flags.optimize >= 2,
+                     "Docstrings are omitted with -O2 and above")
+    def test_property_with_slots_docstring_silently_dropped(self):
+        # https://github.com/python/cpython/issues/98963#issuecomment-1574413319
+        class slotted_prop(property):
+            __slots__ = ("foo",)
+
+        p = slotted_prop(doc="what's up")  # no AttributeError
+        self.assertIsNone(p.__doc__)
+
+        def documented_getter():
+            """getter doc."""
+            return 4
+
+        # Historical behavior: A docstring from a getter always raises.
+        # (matches test_slots_docstring_copy_exception above).
+        with self.assertRaises(AttributeError):
+            p = slotted_prop(documented_getter)
+
+    @unittest.skipIf(sys.flags.optimize >= 2,
+                     "Docstrings are omitted with -O2 and above")
+    def test_property_with_slots_and_doc_slot_docstring_present(self):
+        # https://github.com/python/cpython/issues/98963#issuecomment-1574413319
+        class slotted_prop(property):
+            __slots__ = ("foo", "__doc__")
+
+        p = slotted_prop(doc="what's up")
+        self.assertEqual("what's up", p.__doc__)  # new in 3.12: This gets set.
+
+        def documented_getter():
+            """what's up getter doc?"""
+            return 4
+
+        p = slotted_prop(documented_getter)
+        self.assertEqual("what's up getter doc?", p.__doc__)
 
     @unittest.skipIf(sys.flags.optimize >= 2,
                      "Docstrings are omitted with -O2 and above")
diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-06-02-17-39-19.gh-issue-98963.J4wJgk.rst b/Misc/NEWS.d/next/Core and Builtins/2023-06-02-17-39-19.gh-issue-98963.J4wJgk.rst
@@ -0,0 +1,4 @@
+Restore the ability for a subclass of :class:`property` to define ``__slots__``
+or otherwise be dict-less by ignoring failures to set a docstring on such a
+class.  This behavior had regressed in 3.12beta1.  An :exc:`AttributeError`
+where there had not previously been one was disruptive to existing code.
diff --git a/Objects/descrobject.c b/Objects/descrobject.c
@@ -1485,7 +1485,10 @@ class property(object):
         self.__get = fget
         self.__set = fset
         self.__del = fdel
-        self.__doc__ = doc
+        try:
+            self.__doc__ = doc
+        except AttributeError:  # read-only or dict-less class
+            pass
 
     def __get__(self, inst, type=None):
         if inst is None:
@@ -1791,6 +1794,19 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset,
         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);
@@ -1806,19 +1822,32 @@ property_init_impl(propertyobject *self, PyObject *fget, PyObject *fset,
     if (Py_IS_TYPE(self, &PyProperty_Type)) {
         Py_XSETREF(self->prop_doc, prop_doc);
     } else {
-        /* If this is a property subclass, put __doc__
-           in dict of the subclass instance instead,
-           otherwise it gets shadowed by __doc__ in the
-           class's dict. */
+        /* If this is a property subclass, put __doc__ in the dict
+           or designated slot of the subclass instance instead, otherwise
+           it gets shadowed by __doc__ in the class's dict. */
 
         if (prop_doc == NULL) {
             prop_doc = Py_NewRef(Py_None);
         }
         int err = PyObject_SetAttr(
                     (PyObject *)self, &_Py_ID(__doc__), prop_doc);
-        Py_XDECREF(prop_doc);
-        if (err < 0)
-            return -1;
+        Py_DECREF(prop_doc);
+        if (err < 0) {
+            assert(PyErr_Occurred());
+            if (PyErr_ExceptionMatches(PyExc_AttributeError)) {
+                PyErr_Clear();
+                // https://github.com/python/cpython/issues/98963#issuecomment-1574413319
+                // Python silently dropped this doc assignment through 3.11.
+                // We preserve that behavior for backwards compatibility.
+                //
+                // If we ever want to deprecate this behavior, only raise a
+                // warning or error when proc_doc is not None so that
+                // property without a specific doc= still works.
+                return 0;
+            } else {
+                return -1;
+            }
+        }
     }
 
     return 0;
