inspect.Signature: Fix discrepancy between __eq__ and __hash__.

Issue #20334. Thanks to Antony Lee for bug report & initial patch.

3 files changed, 39 insertions, 39 deletions

diff --git a/Lib/inspect.py b/Lib/inspect.py
index 4dd9ab1..819bb8d 100644
--- a/Lib/inspect.py
+++ b/Lib/inspect.py
@@ -2239,14 +2239,7 @@ class Parameter:
                                            id(self), self)
 
     def __hash__(self):
-        hash_tuple = (self.name, int(self.kind))
-
-        if self._annotation is not _empty:
-            hash_tuple += (self._annotation,)
-        if self._default is not _empty:
-            hash_tuple += (self._default,)
-
-        return hash(hash_tuple)
+        return hash((self.name, self.kind, self.annotation, self.default))
 
     def __eq__(self, other):
         return (issubclass(other.__class__, Parameter) and
@@ -2541,41 +2534,23 @@ class Signature:
         return type(self)(parameters,
                           return_annotation=return_annotation)
 
-    def __hash__(self):
-        hash_tuple = tuple(self.parameters.values())
-        if self._return_annotation is not _empty:
-            hash_tuple += (self._return_annotation,)
-        return hash(hash_tuple)
+    def _hash_basis(self):
+        params = tuple(param for param in self.parameters.values()
+                             if param.kind != _KEYWORD_ONLY)
 
-    def __eq__(self, other):
-        if (not issubclass(type(other), Signature) or
-                    self.return_annotation != other.return_annotation or
-                    len(self.parameters) != len(other.parameters)):
-            return False
+        kwo_params = {param.name: param for param in self.parameters.values()
+                                        if param.kind == _KEYWORD_ONLY}
 
-        other_positions = {param: idx
-                           for idx, param in enumerate(other.parameters.keys())}
+        return params, kwo_params, self.return_annotation
 
-        for idx, (param_name, param) in enumerate(self.parameters.items()):
-            if param.kind == _KEYWORD_ONLY:
-                try:
-                    other_param = other.parameters[param_name]
-                except KeyError:
-                    return False
-                else:
-                    if param != other_param:
-                        return False
-            else:
-                try:
-                    other_idx = other_positions[param_name]
-                except KeyError:
-                    return False
-                else:
-                    if (idx != other_idx or
-                                    param != other.parameters[param_name]):
-                        return False
+    def __hash__(self):
+        params, kwo_params, return_annotation = self._hash_basis()
+        kwo_params = frozenset(kwo_params.values())
+        return hash((params, kwo_params, return_annotation))
 
-        return True
+    def __eq__(self, other):
+        return (isinstance(other, Signature) and
+                self._hash_basis() == other._hash_basis())
 
     def __ne__(self, other):
         return not self.__eq__(other)
diff --git a/Lib/test/test_inspect.py b/Lib/test/test_inspect.py
index d746636..6da5822 100644
--- a/Lib/test/test_inspect.py
+++ b/Lib/test/test_inspect.py
@@ -2535,43 +2535,67 @@ class TestSignatureObject(unittest.TestCase):
 
         def bar(a, *, b:int) -> float: pass
         self.assertEqual(inspect.signature(foo), inspect.signature(bar))
+        self.assertEqual(
+            hash(inspect.signature(foo)), hash(inspect.signature(bar)))
 
         def bar(a, *, b:int) -> int: pass
         self.assertNotEqual(inspect.signature(foo), inspect.signature(bar))
+        self.assertNotEqual(
+            hash(inspect.signature(foo)), hash(inspect.signature(bar)))
 
         def bar(a, *, b:int): pass
         self.assertNotEqual(inspect.signature(foo), inspect.signature(bar))
+        self.assertNotEqual(
+            hash(inspect.signature(foo)), hash(inspect.signature(bar)))
 
         def bar(a, *, b:int=42) -> float: pass
         self.assertNotEqual(inspect.signature(foo), inspect.signature(bar))
+        self.assertNotEqual(
+            hash(inspect.signature(foo)), hash(inspect.signature(bar)))
 
         def bar(a, *, c) -> float: pass
         self.assertNotEqual(inspect.signature(foo), inspect.signature(bar))
+        self.assertNotEqual(
+            hash(inspect.signature(foo)), hash(inspect.signature(bar)))
 
         def bar(a, b:int) -> float: pass
         self.assertNotEqual(inspect.signature(foo), inspect.signature(bar))
+        self.assertNotEqual(
+            hash(inspect.signature(foo)), hash(inspect.signature(bar)))
         def spam(b:int, a) -> float: pass
         self.assertNotEqual(inspect.signature(spam), inspect.signature(bar))
+        self.assertNotEqual(
+            hash(inspect.signature(spam)), hash(inspect.signature(bar)))
 
         def foo(*, a, b, c): pass
         def bar(*, c, b, a): pass
         self.assertEqual(inspect.signature(foo), inspect.signature(bar))
+        self.assertEqual(
+            hash(inspect.signature(foo)), hash(inspect.signature(bar)))
 
         def foo(*, a=1, b, c): pass
         def bar(*, c, b, a=1): pass
         self.assertEqual(inspect.signature(foo), inspect.signature(bar))
+        self.assertEqual(
+            hash(inspect.signature(foo)), hash(inspect.signature(bar)))
 
         def foo(pos, *, a=1, b, c): pass
         def bar(pos, *, c, b, a=1): pass
         self.assertEqual(inspect.signature(foo), inspect.signature(bar))
+        self.assertEqual(
+            hash(inspect.signature(foo)), hash(inspect.signature(bar)))
 
         def foo(pos, *, a, b, c): pass
         def bar(pos, *, c, b, a=1): pass
         self.assertNotEqual(inspect.signature(foo), inspect.signature(bar))
+        self.assertNotEqual(
+            hash(inspect.signature(foo)), hash(inspect.signature(bar)))
 
         def foo(pos, *args, a=42, b, c, **kwargs:int): pass
         def bar(pos, *args, c, b, a=42, **kwargs:int): pass
         self.assertEqual(inspect.signature(foo), inspect.signature(bar))
+        self.assertEqual(
+            hash(inspect.signature(foo)), hash(inspect.signature(bar)))
 
     def test_signature_hashable(self):
         S = inspect.Signature
diff --git a/Misc/NEWS b/Misc/NEWS
index 9cd20f7..54517b1 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -814,6 +814,7 @@ _ Issue #21597: The separator between the turtledemo text pane and the drawing
   keyword-only.
 
 - Issue #20334: inspect.Signature and inspect.Parameter are now hashable.
+  Thanks to Antony Lee for bug reports and suggestions.
 
 - Issue #15916: doctest.DocTestSuite returns an empty unittest.TestSuite instead
   of raising ValueError if it finds no tests