Merge "restore old *args approach for MutableDict.pop()" into main

3 files changed, 59 insertions, 30 deletions

diff --git a/doc/build/changelog/unreleased_20/9380.rst b/doc/build/changelog/unreleased_20/9380.rst
new file mode 100644
index 000000000..6ac454e27
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/9380.rst
@@ -0,0 +1,7 @@
+.. change::
+    :tags: bug, ext, regression
+    :tickets: 9380
+
+    Fixed regression caused by typing added to ``sqlalchemy.ext.mutable`` for
+    :ticket:`8667`, where the semantics of the ``.pop()`` method changed such
+    that the method was non-working. Pull request courtesy Nils Philippsen.
diff --git a/lib/sqlalchemy/ext/mutable.py b/lib/sqlalchemy/ext/mutable.py
index 07f357bd6..940d62b0a 100644
--- a/lib/sqlalchemy/ext/mutable.py
+++ b/lib/sqlalchemy/ext/mutable.py
@@ -369,6 +369,7 @@ from typing import Optional
 from typing import overload
 from typing import Set
 from typing import Tuple
+from typing import TYPE_CHECKING
 from typing import TypeVar
 from typing import Union
 import weakref
@@ -781,27 +782,25 @@ class MutableDict(Mutable, Dict[_KT, _VT]):
         super().__setitem__(key, value)
         self.changed()
 
-    def _exists(self, value: _T | None) -> TypeGuard[_T]:
-        return value is not None
+    if TYPE_CHECKING:
 
-    def _is_none(self, value: _T | None) -> TypeGuard[None]:
-        return value is None
+        @overload
+        def setdefault(self, key: _KT) -> _VT | None:
+            ...
 
-    @overload
-    def setdefault(self, key: _KT) -> _VT | None:
-        ...
+        @overload
+        def setdefault(self, key: _KT, value: _VT) -> _VT:
+            ...
 
-    @overload
-    def setdefault(self, key: _KT, value: _VT) -> _VT:
-        ...
+        def setdefault(self, key: _KT, value: _VT | None = None) -> _VT | None:
+            ...
 
-    def setdefault(self, key: _KT, value: _VT | None = None) -> _VT | None:
-        if self._exists(value):
-            result = super().setdefault(key, value)
-        else:
-            result = super().setdefault(key)  # type: ignore[call-arg]
-        self.changed()
-        return result
+    else:
+
+        def setdefault(self, *arg):  # noqa: F811
+            result = super().setdefault(*arg)
+            self.changed()
+            return result
 
     def __delitem__(self, key: _KT) -> None:
         """Detect dictionary del events and emit change events."""
@@ -812,21 +811,27 @@ class MutableDict(Mutable, Dict[_KT, _VT]):
         super().update(*a, **kw)
         self.changed()
 
-    @overload
-    def pop(self, __key: _KT) -> _VT:
-        ...
+    if TYPE_CHECKING:
 
-    @overload
-    def pop(self, __key: _KT, __default: _VT | _T) -> _VT | _T:
-        ...
+        @overload
+        def pop(self, __key: _KT) -> _VT:
+            ...
 
-    def pop(self, __key: _KT, __default: _VT | _T | None = None) -> _VT | _T:
-        if self._exists(__default):
-            result = super().pop(__key, __default)
-        else:
-            result = super().pop(__key)
-        self.changed()
-        return result
+        @overload
+        def pop(self, __key: _KT, __default: _VT | _T) -> _VT | _T:
+            ...
+
+        def pop(
+            self, __key: _KT, __default: _VT | _T | None = None
+        ) -> _VT | _T:
+            ...
+
+    else:
+
+        def pop(self, *arg):  # noqa: F811
+            result = super().pop(*arg)
+            self.changed()
+            return result
 
     def popitem(self) -> Tuple[_KT, _VT]:
         result = super().popitem()
diff --git a/test/ext/test_mutable.py b/test/ext/test_mutable.py
index 50dc22e35..290518dd6 100644
--- a/test/ext/test_mutable.py
+++ b/test/ext/test_mutable.py
@@ -349,6 +349,19 @@ class _MutableDictTestBase(_MutableDictTestFixture):
 
         eq_(f1.data, {"c": "d"})
 
+    def test_pop_default_none(self):
+        sess = fixture_session()
+
+        f1 = Foo(data={"a": "b", "c": "d"})
+        sess.add(f1)
+        sess.commit()
+
+        eq_(f1.data.pop("a", None), "b")
+        eq_(f1.data.pop("a", None), None)
+        sess.commit()
+
+        eq_(f1.data, {"c": "d"})
+
     def test_popitem(self):
         sess = fixture_session()
 
@@ -386,6 +399,10 @@ class _MutableDictTestBase(_MutableDictTestFixture):
 
         eq_(f1.data, {"a": "b", "c": "d"})
 
+        eq_(f1.data.setdefault("w", None), None)
+        sess.commit()
+        eq_(f1.data, {"a": "b", "c": "d", "w": None})
+
     def test_replace(self):
         sess = fixture_session()
         f1 = Foo(data={"a": "b"})