From 25d10b1b6e3a77d9f2b4962d73bdd2f866ceee39 Mon Sep 17 00:00:00 2001 From: STerliakov Date: Thu, 29 May 2025 18:13:22 +0200 Subject: [PATCH 1/8] Make behaviour of `__hash__ = None` more intuitive: can be rolled back in subclasses, only allowed to override `object.__hash__` --- mypy/checker.py | 12 +++++++++++ mypy/semanal.py | 9 ++++++++ test-data/unit/check-classes.test | 29 +++++++++++++++++++++----- test-data/unit/fixtures/primitives.pyi | 1 + 4 files changed, 46 insertions(+), 5 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 9c389cccd95f..b5bf8f18ee1f 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -2215,6 +2215,10 @@ def check_method_override_for_base_with_name( # Will always fail to typecheck below, since we know the node is a method original_type = NoneType() + if name == "__hash__" and isinstance(original_type, NoneType): + # Allow defining `__hash__` even if parent class was explicitly made unhashable + return False + always_allow_covariant = False if is_settable_property(defn) and ( is_settable_property(original_node) or isinstance(original_node, Var) @@ -3444,6 +3448,14 @@ def check_compatibility_all_supers(self, lvalue: RefExpr, rvalue: Expression) -> lvalue_node.name == "__slots__" and base.fullname == "builtins.object" ): continue + if ( + lvalue_node.name == "__hash__" + and base.fullname == "builtins.object" + and isinstance(get_proper_type(lvalue_type), NoneType) + ): + # allow `__hash__ = None` if the overridden `__hash__` comes from object + # This isn't LSP-compliant, but too common in real code. + continue if is_private(lvalue_node.name): continue diff --git a/mypy/semanal.py b/mypy/semanal.py index c5f4443588f8..3db328d7bff4 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -2030,6 +2030,15 @@ def analyze_class_body_common(self, defn: ClassDef) -> None: self.setup_self_type() defn.defs.accept(self) self.apply_class_plugin_hooks(defn) + + if "__eq__" in defn.info.names and "__hash__" not in defn.info.names: + # If a class defines `__eq__` without `__hash__`, it's no longer hashable. + hash_none = Var("__hash__", NoneType()) + hash_none.info = defn.info + hash_none.set_line(defn) + hash_none.is_classvar = True + self.add_symbol("__hash__", hash_none, defn) + self.leave_class() def analyze_typeddict_classdef(self, defn: ClassDef) -> bool: diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index e0ea00aee361..0fc093b1f0fd 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -116,11 +116,7 @@ class Base: __hash__: None = None class Derived(Base): - def __hash__(self) -> int: # E: Signature of "__hash__" incompatible with supertype "Base" \ - # N: Superclass: \ - # N: None \ - # N: Subclass: \ - # N: def __hash__(self) -> int + def __hash__(self) -> int: pass # Correct: @@ -147,6 +143,29 @@ class Base: class Derived(Base): __hash__ = 1 # E: Incompatible types in assignment (expression has type "int", base class "Base" defined the type as "Callable[[], int]") +[case testEqWithoutHash] +class A: + def __eq__(self, other) -> bool: ... + +reveal_type(A.__hash__) # N: Revealed type is "None" +[builtins fixtures/primitives.pyi] + +[case testHashNoneOverride] +class A: + __hash__ = None + +class B(A): + def __hash__(self) -> int: return 0 +[builtins fixtures/primitives.pyi] + +[case testHashNoneBadOverride] +class A: + def __hash__(self) -> int: return 0 + +class B(A): + __hash__ = None # E: Incompatible types in assignment (expression has type "None", base class "A" defined the type as "Callable[[], int]") +[builtins fixtures/primitives.pyi] + [case testOverridePartialAttributeWithMethod] # This was crashing: https://github.com/python/mypy/issues/11686. class Base: diff --git a/test-data/unit/fixtures/primitives.pyi b/test-data/unit/fixtures/primitives.pyi index 2f8623c79b9f..7860b3e3d826 100644 --- a/test-data/unit/fixtures/primitives.pyi +++ b/test-data/unit/fixtures/primitives.pyi @@ -10,6 +10,7 @@ class object: def __str__(self) -> str: pass def __eq__(self, other: object) -> bool: pass def __ne__(self, other: object) -> bool: pass + def __hash__(self) -> int: ... class type: def __init__(self, x: object) -> None: pass From a17a313592fd82baaf3c8dde8745f9fce87c8a5d Mon Sep 17 00:00:00 2001 From: STerliakov Date: Thu, 29 May 2025 18:38:35 +0200 Subject: [PATCH 2/8] Missing get_proper_type --- mypy/checker.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index b5bf8f18ee1f..e4d08d474553 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -2215,10 +2215,6 @@ def check_method_override_for_base_with_name( # Will always fail to typecheck below, since we know the node is a method original_type = NoneType() - if name == "__hash__" and isinstance(original_type, NoneType): - # Allow defining `__hash__` even if parent class was explicitly made unhashable - return False - always_allow_covariant = False if is_settable_property(defn) and ( is_settable_property(original_node) or isinstance(original_node, Var) @@ -2242,6 +2238,10 @@ def check_method_override_for_base_with_name( typ = get_proper_type(typ) original_type = get_proper_type(original_type) + if name == "__hash__" and isinstance(original_type, NoneType): + # Allow defining `__hash__` even if parent class was explicitly made unhashable + return False + if ( is_property(defn) and isinstance(original_node, Var) From 31a5c2a77f1372aeba40fad66ef6da7e6714f956 Mon Sep 17 00:00:00 2001 From: STerliakov Date: Sun, 1 Jun 2025 16:31:56 +0200 Subject: [PATCH 3/8] Allow any such overrides --- mypy/semanal.py | 6 ++++++ test-data/unit/check-classes.test | 12 ++++++++++++ 2 files changed, 18 insertions(+) diff --git a/mypy/semanal.py b/mypy/semanal.py index 3db328d7bff4..a5f0fe201c63 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -2037,6 +2037,12 @@ def analyze_class_body_common(self, defn: ClassDef) -> None: hash_none.info = defn.info hash_none.set_line(defn) hash_none.is_classvar = True + # Making a class hashable is allowed even if its parents weren't. + # The only possible consequence of this LSP violation would be + # `assert child.__hash__ is None` no longer passing, probably nobody + # cares about that - it's impossible to statically restrict to non-hashable + # anyway. + hash_none.allow_incompatible_override = True self.add_symbol("__hash__", hash_none, defn) self.leave_class() diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index 0fc093b1f0fd..b7d33d041c99 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -166,6 +166,18 @@ class B(A): __hash__ = None # E: Incompatible types in assignment (expression has type "None", base class "A" defined the type as "Callable[[], int]") [builtins fixtures/primitives.pyi] +[case testHashOverrideInMultipleInheritance] +class Foo: + def __eq__(self, other: "Foo") -> bool: ... + +class Bar: + def __hash__(self) -> int: ... + +class BadChild(Foo, Bar): ... # E: Definition of "__hash__" in base class "Foo" is incompatible with definition in base class "Bar" +class GoodChild(Bar, Foo): ... +[builtins fixtures/tuple.pyi] + + [case testOverridePartialAttributeWithMethod] # This was crashing: https://github.com/python/mypy/issues/11686. class Base: From 22a2729a9ed0ff8fa77ae39c3ce0e31a6dd7288b Mon Sep 17 00:00:00 2001 From: STerliakov Date: Sun, 1 Jun 2025 16:32:33 +0200 Subject: [PATCH 4/8] Fix stub for mypyc test --- mypyc/test-data/fixtures/ir.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mypyc/test-data/fixtures/ir.py b/mypyc/test-data/fixtures/ir.py index 1b92590a5fd4..5a5c65900bf4 100644 --- a/mypyc/test-data/fixtures/ir.py +++ b/mypyc/test-data/fixtures/ir.py @@ -90,6 +90,7 @@ def __init__(self, x: object) -> None: pass def __add__(self, x: str) -> str: pass def __mul__(self, x: int) -> str: pass def __rmul__(self, x: int) -> str: pass + def __hash__(self) -> int: pass def __eq__(self, x: object) -> bool: pass def __ne__(self, x: object) -> bool: pass def __lt__(self, x: str) -> bool: ... From eccc70231d8c4d5c3d4347f46e29655fbd6512c3 Mon Sep 17 00:00:00 2001 From: STerliakov Date: Sun, 1 Jun 2025 16:36:36 +0200 Subject: [PATCH 5/8] Adjust remaining test, replace ellipsis with `pass` for consistency --- test-data/unit/fine-grained-inspect.test | 4 ++-- test-data/unit/fixtures/primitives.pyi | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test-data/unit/fine-grained-inspect.test b/test-data/unit/fine-grained-inspect.test index 0e05769370a2..6bee5cd875f7 100644 --- a/test-data/unit/fine-grained-inspect.test +++ b/test-data/unit/fine-grained-inspect.test @@ -122,10 +122,10 @@ None [builtins fixtures/args.pyi] [out] == -{"C": ["x"], "object": ["__eq__", "__init__", "__ne__"]} +{"C": ["x"], "object": ["__eq__", "__hash__", "__init__", "__ne__"]} {"Iterable": ["__iter__"]} NameExpr -> {} -NameExpr -> {"object": ["__eq__", "__init__", "__ne__"]} +NameExpr -> {"object": ["__eq__", "__hash__", "__init__", "__ne__"]} [case testInspectTypeVarBoundAttrs] # inspect2: --show=attrs tmp/foo.py:8:13 diff --git a/test-data/unit/fixtures/primitives.pyi b/test-data/unit/fixtures/primitives.pyi index 7860b3e3d826..22e1b2bb8302 100644 --- a/test-data/unit/fixtures/primitives.pyi +++ b/test-data/unit/fixtures/primitives.pyi @@ -10,7 +10,7 @@ class object: def __str__(self) -> str: pass def __eq__(self, other: object) -> bool: pass def __ne__(self, other: object) -> bool: pass - def __hash__(self) -> int: ... + def __hash__(self) -> int: pass class type: def __init__(self, x: object) -> None: pass From fad22f2890377a6fbff0de2b4f9ba67da0bb3c80 Mon Sep 17 00:00:00 2001 From: STerliakov Date: Sun, 1 Jun 2025 18:19:10 +0200 Subject: [PATCH 6/8] Do not add implicit `__hash__ = None` to protocols --- mypy/semanal.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/mypy/semanal.py b/mypy/semanal.py index a5f0fe201c63..89197c5707de 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -2031,8 +2031,14 @@ def analyze_class_body_common(self, defn: ClassDef) -> None: defn.defs.accept(self) self.apply_class_plugin_hooks(defn) - if "__eq__" in defn.info.names and "__hash__" not in defn.info.names: + if ( + "__eq__" in defn.info.names + and "__hash__" not in defn.info.names + and not defn.info.is_protocol + ): # If a class defines `__eq__` without `__hash__`, it's no longer hashable. + # Excludes Protocol from consideration as we don't want to enforce unhashability + # of their instances. hash_none = Var("__hash__", NoneType()) hash_none.info = defn.info hash_none.set_line(defn) From bd953b43a82edaa07afd46e28d946bc6b6dc084d Mon Sep 17 00:00:00 2001 From: STerliakov Date: Tue, 3 Jun 2025 21:53:06 +0200 Subject: [PATCH 7/8] Support in assignments too --- mypy/checker.py | 11 +++++++++-- test-data/unit/check-classes.test | 25 ++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 5e805986fcdd..49c810819525 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -2241,7 +2241,11 @@ def check_method_override_for_base_with_name( if name == "__hash__" and isinstance(original_type, NoneType): # Allow defining `__hash__` even if parent class was explicitly made unhashable - return False + if base.fullname == "builtins.object": + # This is only for test stubs to avoid adding object.__hash__ + # to all of them. + return True + return self.check_method_override_for_base_with_name(defn, name, base.mro[-1]) if ( is_property(defn) @@ -3493,7 +3497,7 @@ def check_compatibility_all_supers(self, lvalue: RefExpr, rvalue: Expression) -> lvalue, lvalue_type, base_type, base ) return - if base is last_immediate_base: + if base is last_immediate_base and base_node.name != "__hash__": # At this point, the attribute was found to be compatible with all # immediate parents. break @@ -3507,6 +3511,9 @@ def check_compatibility_super( base_node: Node, always_allow_covariant: bool, ) -> bool: + if base_node.name == "__hash__" and isinstance(base_type, NoneType): + # Allow defining `__hash__` even if parent class was explicitly made unhashable + return True # TODO: check __set__() type override for custom descriptors. # TODO: for descriptors check also class object access override. ok = self.check_subtype( diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index 1ed65c487283..52dc71190117 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -155,7 +155,30 @@ class A: __hash__ = None class B(A): - def __hash__(self) -> int: return 0 + def __hash__(self) -> int: + return 0 + +class C(A): + __hash__ = object.__hash__ + +class D(A): + def __hash__(self, x: int) -> str: # E: Signature of "__hash__" incompatible with supertype "object" \ + # N: Superclass: \ + # N: def __hash__(self) -> int \ + # N: Subclass: \ + # N: def __hash__(self, x: int) -> str \ + # N: Superclass: \ + # N: def __hash__(self) -> int \ + # N: Subclass: \ + # N: def __hash__(self, x: int) -> str + return '' + +def bad_hash(x: E, y: str) -> str: + return y + +class E(A): + __hash__ = bad_hash # E: Incompatible types in assignment (expression has type "Callable[[str], str]", base class "object" defined the type as "Callable[[], int]") + [builtins fixtures/primitives.pyi] [case testHashNoneBadOverride] From 18815c7473eeef8edf7ef5da7e7233f983780018 Mon Sep 17 00:00:00 2001 From: STerliakov Date: Tue, 3 Jun 2025 22:11:11 +0200 Subject: [PATCH 8/8] Handle `allow_incompatible_override` consistenly instead --- mypy/checker.py | 34 ++++++++++++++----------------- mypy/semanal.py | 16 +++++++++++++++ test-data/unit/check-classes.test | 5 ----- 3 files changed, 31 insertions(+), 24 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index 49c810819525..709afdaa2765 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -2216,6 +2216,9 @@ def check_method_override_for_base_with_name( # Will always fail to typecheck below, since we know the node is a method original_type = NoneType() + if isinstance(original_node, Var) and original_node.allow_incompatible_override: + return False + always_allow_covariant = False if is_settable_property(defn) and ( is_settable_property(original_node) or isinstance(original_node, Var) @@ -2239,14 +2242,6 @@ def check_method_override_for_base_with_name( typ = get_proper_type(typ) original_type = get_proper_type(original_type) - if name == "__hash__" and isinstance(original_type, NoneType): - # Allow defining `__hash__` even if parent class was explicitly made unhashable - if base.fullname == "builtins.object": - # This is only for test stubs to avoid adding object.__hash__ - # to all of them. - return True - return self.check_method_override_for_base_with_name(defn, name, base.mro[-1]) - if ( is_property(defn) and isinstance(original_node, Var) @@ -3448,13 +3443,6 @@ def check_compatibility_all_supers(self, lvalue: RefExpr, rvalue: Expression) -> return for base in lvalue_node.info.mro[1:]: - # The type of "__slots__" and some other attributes usually doesn't need to - # be compatible with a base class. We'll still check the type of "__slots__" - # against "object" as an exception. - if lvalue_node.allow_incompatible_override and not ( - lvalue_node.name == "__slots__" and base.fullname == "builtins.object" - ): - continue if ( lvalue_node.name == "__hash__" and base.fullname == "builtins.object" @@ -3468,6 +3456,17 @@ def check_compatibility_all_supers(self, lvalue: RefExpr, rvalue: Expression) -> continue base_type, base_node = self.node_type_from_base(lvalue_node.name, base, lvalue) + # The type of "__slots__" and some other attributes usually doesn't need to + # be compatible with a base class. We'll still check the type of "__slots__" + # against "object" as an exception. + if ( + isinstance(base_node, Var) + and base_node.allow_incompatible_override + and not ( + lvalue_node.name == "__slots__" and base.fullname == "builtins.object" + ) + ): + continue custom_setter = is_custom_settable_property(base_node) if isinstance(base_type, PartialType): base_type = None @@ -3497,7 +3496,7 @@ def check_compatibility_all_supers(self, lvalue: RefExpr, rvalue: Expression) -> lvalue, lvalue_type, base_type, base ) return - if base is last_immediate_base and base_node.name != "__hash__": + if base is last_immediate_base: # At this point, the attribute was found to be compatible with all # immediate parents. break @@ -3511,9 +3510,6 @@ def check_compatibility_super( base_node: Node, always_allow_covariant: bool, ) -> bool: - if base_node.name == "__hash__" and isinstance(base_type, NoneType): - # Allow defining `__hash__` even if parent class was explicitly made unhashable - return True # TODO: check __set__() type override for custom descriptors. # TODO: for descriptors check also class object access override. ok = self.check_subtype( diff --git a/mypy/semanal.py b/mypy/semanal.py index 85ba5a3d11d1..0251dea64764 100644 --- a/mypy/semanal.py +++ b/mypy/semanal.py @@ -3257,6 +3257,22 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None: self.process__all__(s) self.process__deletable__(s) self.process__slots__(s) + self.process__hash__(s) + + def process__hash__(self, s: AssignmentStmt) -> None: + # Allow overriding `__hash__ = None` in subclasses. + if ( + isinstance(self.type, TypeInfo) + and len(s.lvalues) == 1 + and isinstance(s.lvalues[0], NameExpr) + and s.lvalues[0].name == "__hash__" + and s.lvalues[0].kind == MDEF + and isinstance(s.rvalue, NameExpr) + and s.rvalue.name == "None" + ): + var = s.lvalues[0].node + if isinstance(var, Var): + var.allow_incompatible_override = True def analyze_identity_global_assignment(self, s: AssignmentStmt) -> bool: """Special case 'X = X' in global scope. diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index 52dc71190117..7f0f9a6d2b40 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -163,10 +163,6 @@ class C(A): class D(A): def __hash__(self, x: int) -> str: # E: Signature of "__hash__" incompatible with supertype "object" \ - # N: Superclass: \ - # N: def __hash__(self) -> int \ - # N: Subclass: \ - # N: def __hash__(self, x: int) -> str \ # N: Superclass: \ # N: def __hash__(self) -> int \ # N: Subclass: \ @@ -178,7 +174,6 @@ def bad_hash(x: E, y: str) -> str: class E(A): __hash__ = bad_hash # E: Incompatible types in assignment (expression has type "Callable[[str], str]", base class "object" defined the type as "Callable[[], int]") - [builtins fixtures/primitives.pyi] [case testHashNoneBadOverride]