Skip to content

Commit

Permalink
Fix attrs hashability detection
Browse files Browse the repository at this point in the history
This commit fixes a couple regressions in 1.9.0 from 91be285.

Attrs' logic for hashability is slightly complex:

* https://www.attrs.org/en/stable/hashing.html
* https://github.com/python-attrs/attrs/blob/9e443b18527dc96b194e92805fa751cbf8434ba9/src/attr/_make.py#L1660-L1689

Mypy now properly emulates attrs' logic so that custom `__hash__`
implementations are preserved, `@frozen` subclasses are always hashable,
and classes are only made unhashable based on the values of eq and
`unsafe_hash`.

Fixes python#17015
Fixes python#16556 (comment)

Based on a patch in python#17012
Co-Authored-By: Tin Tvrtkovic <[email protected]>
  • Loading branch information
Hnasar committed Mar 11, 2024
1 parent ea49e1f commit 00c0ef7
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 14 deletions.
23 changes: 19 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,25 @@ 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.
hash = _get_decorator_bool_argument(
ctx, "hash", True
) and _get_decorator_bool_argument(ctx, "unsafe_hash", True)
else:
hash = _get_decorator_optional_bool_argument(ctx, "unsafe_hash")
if hash is None:
hash = _get_decorator_optional_bool_argument(ctx, "hash")

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

do_nothing = has_own_hash or (hash is None and eq is False)
if do_nothing:
pass
elif hash:
# 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
104 changes: 95 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 @@ -2342,6 +2343,12 @@ class A:

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

@frozen(hash=False)
class B:
b: int

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

[builtins fixtures/plugin_attrs.pyi]

[case testManualHashHashability]
Expand Down Expand Up @@ -2380,3 +2387,82 @@ 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]
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

0 comments on commit 00c0ef7

Please sign in to comment.