Improve attrs hashability detection (#16556)

Tinche · pre-commit-ci[bot] · web-flow · commit 91be28552e06 · 2023-12-12T23:08:14.000+03:00
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>
diff --git a/mypy/plugins/attrs.py b/mypy/plugins/attrs.py
@@ -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
@@ -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)
@@ -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
 
@@ -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.
 
diff --git a/mypy/plugins/common.py b/mypy/plugins/common.py
@@ -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.
@@ -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]
diff --git a/test-data/unit/check-plugin-attrs.test b/test-data/unit/check-plugin-attrs.test
@@ -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]
diff --git a/test-data/unit/fixtures/plugin_attrs.pyi b/test-data/unit/fixtures/plugin_attrs.pyi
@@ -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
diff --git a/test-data/unit/lib-stub/attr/__init__.pyi b/test-data/unit/lib-stub/attr/__init__.pyi
@@ -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 = ...,
@@ -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 = ...,
diff --git a/test-data/unit/lib-stub/attrs/__init__.pyi b/test-data/unit/lib-stub/attrs/__init__.pyi
@@ -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 = ...,
@@ -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 = ...,
