Skip to content

attrs: Fix emulating hash method logic #17016

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions mypy/plugins/attrs.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,9 +325,6 @@ def attr_class_maker_callback(
frozen = _get_frozen(ctx, frozen_default)
order = _determine_eq_order(ctx)
slots = _get_decorator_bool_argument(ctx, "slots", slots_default)
hashable = _get_decorator_bool_argument(ctx, "hash", False) or _get_decorator_bool_argument(
ctx, "unsafe_hash", False
)

auto_attribs = _get_decorator_optional_bool_argument(ctx, "auto_attribs", auto_attribs_default)
kw_only = _get_decorator_bool_argument(ctx, "kw_only", False)
Expand Down Expand Up @@ -371,7 +368,24 @@ def attr_class_maker_callback(
_add_order(ctx, adder)
if frozen:
_make_frozen(ctx, attributes)
elif not hashable:
# Frozen classes are hashable by default, even if inheriting from non-frozen ones.
hashable: bool | None = _get_decorator_bool_argument(
ctx, "hash", True
) and _get_decorator_bool_argument(ctx, "unsafe_hash", True)
else:
hashable = _get_decorator_optional_bool_argument(ctx, "unsafe_hash")
if hashable is None: # unspecified
hashable = _get_decorator_optional_bool_argument(ctx, "hash")

eq = _get_decorator_optional_bool_argument(ctx, "eq")
has_own_hash = "__hash__" in ctx.cls.info.names

if has_own_hash or (hashable is None and eq is False):
pass # Do nothing.
elif hashable:
# We copy the `__hash__` signature from `object` to make them hashable.
ctx.cls.info.names["__hash__"] = ctx.cls.info.mro[-1].names["__hash__"]
else:
_remove_hashability(ctx)

return True
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-incremental.test
Original file line number Diff line number Diff line change
Expand Up @@ -3015,7 +3015,7 @@ class NoInit:
class NoCmp:
x: int

[builtins fixtures/list.pyi]
[builtins fixtures/plugin_attrs.pyi]
[rechecked]
[stale]
[out1]
Expand Down
113 changes: 104 additions & 9 deletions test-data/unit/check-plugin-attrs.test
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,8 @@ class A:

a = A(5)
a.a = 16 # E: Property "a" defined in "A" is read-only
[builtins fixtures/bool.pyi]
[builtins fixtures/plugin_attrs.pyi]

[case testAttrsNextGenFrozen]
from attr import frozen, field

Expand All @@ -370,7 +371,7 @@ class A:

a = A(5)
a.a = 16 # E: Property "a" defined in "A" is read-only
[builtins fixtures/bool.pyi]
[builtins fixtures/plugin_attrs.pyi]

[case testAttrsNextGenDetect]
from attr import define, field
Expand Down Expand Up @@ -420,7 +421,7 @@ reveal_type(A) # N: Revealed type is "def (a: builtins.int, b: builtins.bool) -
reveal_type(B) # N: Revealed type is "def (a: builtins.bool, b: builtins.int) -> __main__.B"
reveal_type(C) # N: Revealed type is "def (a: builtins.int) -> __main__.C"

[builtins fixtures/bool.pyi]
[builtins fixtures/plugin_attrs.pyi]

[case testAttrsDataClass]
import attr
Expand Down Expand Up @@ -1155,7 +1156,7 @@ c = NonFrozenFrozen(1, 2)
c.a = 17 # E: Property "a" defined in "NonFrozenFrozen" is read-only
c.b = 17 # E: Property "b" defined in "NonFrozenFrozen" is read-only

[builtins fixtures/bool.pyi]
[builtins fixtures/plugin_attrs.pyi]
[case testAttrsCallableAttributes]
from typing import Callable
import attr
Expand All @@ -1178,7 +1179,7 @@ class G:
class FFrozen(F):
def bar(self) -> bool:
return self._cb(5, 6)
[builtins fixtures/callable.pyi]
[builtins fixtures/plugin_attrs.pyi]

[case testAttrsWithFactory]
from typing import List
Expand Down Expand Up @@ -1450,7 +1451,7 @@ class C:
total = attr.ib(type=Bad) # E: Name "Bad" is not defined

C(0).total = 1 # E: Property "total" defined in "C" is read-only
[builtins fixtures/bool.pyi]
[builtins fixtures/plugin_attrs.pyi]

[case testTypeInAttrDeferredStar]
import lib
Expand Down Expand Up @@ -1941,7 +1942,7 @@ class C:
default=None, converter=default_if_none(factory=dict) \
# E: Unsupported converter, only named functions, types and lambdas are currently supported
)
[builtins fixtures/dict.pyi]
[builtins fixtures/plugin_attrs.pyi]

[case testAttrsUnannotatedConverter]
import attr
Expand Down Expand Up @@ -2012,7 +2013,7 @@ class Sub(Base):

@property
def name(self) -> str: ...
[builtins fixtures/property.pyi]
[builtins fixtures/plugin_attrs.pyi]

[case testOverrideWithPropertyInFrozenClassChecked]
from attrs import frozen
Expand All @@ -2035,7 +2036,7 @@ class Sub(Base):

# This matches runtime semantics
reveal_type(Sub) # N: Revealed type is "def (*, name: builtins.str, first_name: builtins.str, last_name: builtins.str) -> __main__.Sub"
[builtins fixtures/property.pyi]
[builtins fixtures/plugin_attrs.pyi]

[case testFinalInstanceAttribute]
from attrs import define
Expand Down Expand Up @@ -2380,3 +2381,97 @@ class B(A):
reveal_type(B.__hash__) # N: Revealed type is "None"

[builtins fixtures/plugin_attrs.pyi]

[case testManualOwnHashability]
from attrs import define, frozen

@define
class A:
a: int
def __hash__(self) -> int:
...

reveal_type(A.__hash__) # N: Revealed type is "def (self: __main__.A) -> builtins.int"

[builtins fixtures/plugin_attrs.pyi]

[case testSubclassDefaultLosesHashability]
from attrs import define, frozen

@define
class A:
a: int
def __hash__(self) -> int:
...

@define
class B(A):
pass

reveal_type(B.__hash__) # N: Revealed type is "None"

[builtins fixtures/plugin_attrs.pyi]

[case testSubclassEqFalseKeepsHashability]
from attrs import define, frozen

@define
class A:
a: int
def __hash__(self) -> int:
...

@define(eq=False)
class B(A):
pass

reveal_type(B.__hash__) # N: Revealed type is "def (self: __main__.A) -> builtins.int"

[builtins fixtures/plugin_attrs.pyi]

[case testSubclassingFrozenHashability]
from attrs import define, frozen

@define
class A:
a: int

@frozen
class B(A):
pass

reveal_type(B.__hash__) # N: Revealed type is "def (self: builtins.object) -> builtins.int"

[builtins fixtures/plugin_attrs.pyi]

[case testSubclassingFrozenHashOffHashability]
from attrs import define, frozen

@define
class A:
a: int
def __hash__(self) -> int:
...

@frozen(unsafe_hash=False)
class B(A):
pass

reveal_type(B.__hash__) # N: Revealed type is "None"

[builtins fixtures/plugin_attrs.pyi]

[case testUnsafeHashPrecedence]
from attrs import define, frozen

@define(unsafe_hash=True, hash=False)
class A:
pass
reveal_type(A.__hash__) # N: Revealed type is "def (self: builtins.object) -> builtins.int"

@define(unsafe_hash=False, hash=True)
class B:
pass
reveal_type(B.__hash__) # N: Revealed type is "None"

[builtins fixtures/plugin_attrs.pyi]
2 changes: 2 additions & 0 deletions test-data/unit/fixtures/plugin_attrs.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,5 @@ class tuple(Sequence[Tco], Generic[Tco]):
def __iter__(self) -> Iterator[Tco]: pass
def __contains__(self, item: object) -> bool: pass
def __getitem__(self, x: int) -> Tco: pass

property = object() # Dummy definition