Skip to content

Commit

Permalink
Improve attrs hashability detection (#16556)
Browse files Browse the repository at this point in the history
Fixes #16550

Improve hashability detection for attrs classes.

I added a new parameter to `add_attribute_to_class`,
`overwrite_existing`, since I needed it.

Frozen classes remain hashable, non-frozen default to no. This can be
overriden by passing in `hash=True` or `unsafe_hash=True` (new
parameter, added to stubs) to `define()`.

Inheritance-wise I think we're correct, if a non-hashable class inherits
from a hashable one, it's still unhashable at runtime.

Accompanying tests.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
Tinche and pre-commit-ci[bot] committed Dec 12, 2023
1 parent 5fa9569 commit 91be285
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 1 deletion.
15 changes: 15 additions & 0 deletions mypy/plugins/attrs.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,8 @@ def attr_class_maker_callback(
it will add an __init__ or all the compare methods.
For frozen=True it will turn the attrs into properties.
Hashability will be set according to https://www.attrs.org/en/stable/hashing.html.
See https://www.attrs.org/en/stable/how-does-it-work.html for information on how attrs works.
If this returns False, some required metadata was not ready yet and we need another
Expand All @@ -321,6 +323,9 @@ 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 @@ -359,10 +364,13 @@ def attr_class_maker_callback(
adder = MethodAdder(ctx)
# If __init__ is not being generated, attrs still generates it as __attrs_init__ instead.
_add_init(ctx, attributes, adder, "__init__" if init else ATTRS_INIT_NAME)

if order:
_add_order(ctx, adder)
if frozen:
_make_frozen(ctx, attributes)
elif not hashable:
_remove_hashability(ctx)

return True

Expand Down Expand Up @@ -943,6 +951,13 @@ def _add_match_args(ctx: mypy.plugin.ClassDefContext, attributes: list[Attribute
add_attribute_to_class(api=ctx.api, cls=ctx.cls, name="__match_args__", typ=match_args)


def _remove_hashability(ctx: mypy.plugin.ClassDefContext) -> None:
"""Remove hashability from a class."""
add_attribute_to_class(
ctx.api, ctx.cls, "__hash__", NoneType(), is_classvar=True, overwrite_existing=True
)


class MethodAdder:
"""Helper to add methods to a TypeInfo.
Expand Down
3 changes: 2 additions & 1 deletion mypy/plugins/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,7 @@ def add_attribute_to_class(
override_allow_incompatible: bool = False,
fullname: str | None = None,
is_classvar: bool = False,
overwrite_existing: bool = False,
) -> Var:
"""
Adds a new attribute to a class definition.
Expand All @@ -408,7 +409,7 @@ def add_attribute_to_class(

# NOTE: we would like the plugin generated node to dominate, but we still
# need to keep any existing definitions so they get semantically analyzed.
if name in info.names:
if name in info.names and not overwrite_existing:
# Get a nice unique name instead.
r_name = get_unique_redefinition_name(name, info.names)
info.names[r_name] = info.names[name]
Expand Down
59 changes: 59 additions & 0 deletions test-data/unit/check-plugin-attrs.test
Original file line number Diff line number Diff line change
Expand Up @@ -2321,3 +2321,62 @@ reveal_type(b.x) # N: Revealed type is "builtins.int"
reveal_type(b.y) # N: Revealed type is "builtins.int"

[builtins fixtures/plugin_attrs.pyi]

[case testDefaultHashability]
from attrs import define

@define
class A:
a: int

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

[builtins fixtures/plugin_attrs.pyi]

[case testFrozenHashability]
from attrs import frozen

@frozen
class A:
a: int

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

[builtins fixtures/plugin_attrs.pyi]

[case testManualHashHashability]
from attrs import define

@define(hash=True)
class A:
a: int

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

[builtins fixtures/plugin_attrs.pyi]

[case testManualUnsafeHashHashability]
from attrs import define

@define(unsafe_hash=True)
class A:
a: int

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

[builtins fixtures/plugin_attrs.pyi]

[case testSubclassingHashability]
from attrs import define

@define(unsafe_hash=True)
class A:
a: int

@define
class B(A):
pass

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

[builtins fixtures/plugin_attrs.pyi]
1 change: 1 addition & 0 deletions test-data/unit/fixtures/plugin_attrs.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ class object:
def __init__(self) -> None: pass
def __eq__(self, o: object) -> bool: pass
def __ne__(self, o: object) -> bool: pass
def __hash__(self) -> int: ...

class type: pass
class bytes: pass
Expand Down
2 changes: 2 additions & 0 deletions test-data/unit/lib-stub/attr/__init__.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ def define(
*,
these: Optional[Mapping[str, Any]] = ...,
repr: bool = ...,
unsafe_hash: Optional[bool]=None,
hash: Optional[bool] = ...,
init: bool = ...,
slots: bool = ...,
Expand All @@ -153,6 +154,7 @@ def define(
*,
these: Optional[Mapping[str, Any]] = ...,
repr: bool = ...,
unsafe_hash: Optional[bool]=None,
hash: Optional[bool] = ...,
init: bool = ...,
slots: bool = ...,
Expand Down
2 changes: 2 additions & 0 deletions test-data/unit/lib-stub/attrs/__init__.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ def define(
*,
these: Optional[Mapping[str, Any]] = ...,
repr: bool = ...,
unsafe_hash: Optional[bool]=None,
hash: Optional[bool] = ...,
init: bool = ...,
slots: bool = ...,
Expand All @@ -44,6 +45,7 @@ def define(
*,
these: Optional[Mapping[str, Any]] = ...,
repr: bool = ...,
unsafe_hash: Optional[bool]=None,
hash: Optional[bool] = ...,
init: bool = ...,
slots: bool = ...,
Expand Down

0 comments on commit 91be285

Please sign in to comment.