Close issue 17482: don't overwrite __wrapped__

5 files changed, 34 insertions, 6 deletions

diff --git a/Doc/library/functools.rst b/Doc/library/functools.rst
index 3d70955..2f6d9af 100644
--- a/Doc/library/functools.rst
+++ b/Doc/library/functools.rst
@@ -306,8 +306,8 @@ The :mod:`functools` module defines the following functions:
 
    To allow access to the original function for introspection and other purposes
    (e.g. bypassing a caching decorator such as :func:`lru_cache`), this function
-   automatically adds a __wrapped__ attribute to the wrapper that refers to
-   the original function.
+   automatically adds a ``__wrapped__`` attribute to the wrapper that refers to
+   the function being wrapped.
 
    The main intended use for this function is in :term:`decorator` functions which
    wrap the decorated function and return the wrapper. If the wrapper function is
@@ -330,6 +330,11 @@ The :mod:`functools` module defines the following functions:
    .. versionchanged:: 3.2
       Missing attributes no longer trigger an :exc:`AttributeError`.
 
+   .. versionchanged:: 3.4
+      The ``__wrapped__`` attribute now always refers to the wrapped
+      function, even if that function defined a ``__wrapped__`` attribute.
+      (see :issue:`17482`)
+
 
 .. decorator:: wraps(wrapped, assigned=WRAPPER_ASSIGNMENTS, updated=WRAPPER_UPDATES)
 
diff --git a/Doc/whatsnew/3.4.rst b/Doc/whatsnew/3.4.rst
index 4068947..2b114f1 100644
--- a/Doc/whatsnew/3.4.rst
+++ b/Doc/whatsnew/3.4.rst
@@ -315,3 +315,12 @@ that may require changes to your code.
   found but improperly structured. If you were catching ImportError before and
   wish to continue to ignore syntax or decoding issues, catch all three
   exceptions now.
+
+* :func:`functools.update_wrapper` and :func:`functools.wraps` now correctly
+  set the ``__wrapped__`` attribute even if the wrapped function had a
+  wrapped attribute set. This means ``__wrapped__`` attributes now correctly
+  link a stack of decorated functions rather than every ``__wrapped__``
+  attribute in the chain referring to the innermost function. Introspection
+  libraries that assumed the previous behaviour was intentional will need to
+  be updated to walk the chain of ``__wrapped__`` attributes to find the
+  innermost function.
diff --git a/Lib/functools.py b/Lib/functools.py
index 6aa13a2..19f88c7 100644
--- a/Lib/functools.py
+++ b/Lib/functools.py
@@ -55,7 +55,6 @@ def update_wrapper(wrapper,
        are updated with the corresponding attribute from the wrapped
        function (defaults to functools.WRAPPER_UPDATES)
     """
-    wrapper.__wrapped__ = wrapped
     for attr in assigned:
         try:
             value = getattr(wrapped, attr)
@@ -65,6 +64,9 @@ def update_wrapper(wrapper,
             setattr(wrapper, attr, value)
     for attr in updated:
         getattr(wrapper, attr).update(getattr(wrapped, attr, {}))
+    # Issue #17482: set __wrapped__ last so we don't inadvertently copy it
+    # from the wrapped function when updating __dict__
+    wrapper.__wrapped__ = wrapped
     # Return the wrapper so this can be used as a decorator via partial()
     return wrapper
 
diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py
index 99dccb0..ab76efb 100644
--- a/Lib/test/test_functools.py
+++ b/Lib/test/test_functools.py
@@ -224,19 +224,26 @@ class TestUpdateWrapper(unittest.TestCase):
                       updated=functools.WRAPPER_UPDATES):
         # Check attributes were assigned
         for name in assigned:
-            self.assertTrue(getattr(wrapper, name) is getattr(wrapped, name))
+            self.assertIs(getattr(wrapper, name), getattr(wrapped, name))
         # Check attributes were updated
         for name in updated:
             wrapper_attr = getattr(wrapper, name)
             wrapped_attr = getattr(wrapped, name)
             for key in wrapped_attr:
-                self.assertTrue(wrapped_attr[key] is wrapper_attr[key])
+                if name == "__dict__" and key == "__wrapped__":
+                    # __wrapped__ is overwritten by the update code
+                    continue
+                self.assertIs(wrapped_attr[key], wrapper_attr[key])
+        # Check __wrapped__
+        self.assertIs(wrapper.__wrapped__, wrapped)
+
 
     def _default_update(self):
         def f(a:'This is a new annotation'):
             """This is a test"""
             pass
         f.attr = 'This is also a test'
+        f.__wrapped__ = "This is a bald faced lie"
         def wrapper(b:'This is the prior annotation'):
             pass
         functools.update_wrapper(wrapper, f)
@@ -331,14 +338,15 @@ class TestWraps(TestUpdateWrapper):
             """This is a test"""
             pass
         f.attr = 'This is also a test'
+        f.__wrapped__ = "This is still a bald faced lie"
         @functools.wraps(f)
         def wrapper():
             pass
-        self.check_wrapper(wrapper, f)
         return wrapper, f
 
     def test_default_update(self):
         wrapper, f = self._default_update()
+        self.check_wrapper(wrapper, f)
         self.assertEqual(wrapper.__name__, 'f')
         self.assertEqual(wrapper.__qualname__, f.__qualname__)
         self.assertEqual(wrapper.attr, 'This is also a test')
diff --git a/Misc/NEWS b/Misc/NEWS
index 8e175b3..78d55ea 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -154,6 +154,10 @@ Core and Builtins
 Library
 -------
 
+- Issue #17482: functools.update_wrapper (and functools.wraps) now set the
+  __wrapped__ attribute correctly even if the underlying function has a
+  __wrapped__ attribute set.
+
 - Issue #18431: The new email header parser now decodes RFC2047 encoded words
   in structured headers.