From c0b0491703c029d287f29b060127c39678b74efe Mon Sep 17 00:00:00 2001
From: KotlinIsland <65446343+KotlinIsland@users.noreply.github.com>
Date: Fri, 20 Dec 2024 18:33:16 +1000
Subject: [PATCH] Update docs for `eq-without-hash` (#14885)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
## Summary
resolves #14883
This PR removes the known limitation section in the documentation of
`eq-without-hash`. That is not actually a limitation as a subclass
overriding the `__eq__` method would have its `__hash__` set to `None`
implicitly. The user should explicitly inherit the `__hash__` method
from the parent class.
## Test Plan
---------
Co-authored-by: Dhruv Manilawala
---
.../src/rules/pylint/rules/eq_without_hash.rs | 66 +++++++++++++++++--
1 file changed, 60 insertions(+), 6 deletions(-)
diff --git a/crates/ruff_linter/src/rules/pylint/rules/eq_without_hash.rs b/crates/ruff_linter/src/rules/pylint/rules/eq_without_hash.rs
index d69b99a8ff..e87a1b9e30 100644
--- a/crates/ruff_linter/src/rules/pylint/rules/eq_without_hash.rs
+++ b/crates/ruff_linter/src/rules/pylint/rules/eq_without_hash.rs
@@ -11,14 +11,13 @@ use crate::checkers::ast::Checker;
///
/// ## Why is this bad?
/// A class that implements `__eq__` but not `__hash__` will have its hash
-/// method implicitly set to `None`. This will cause the class to be
-/// unhashable, will in turn cause issues when using the class as a key in a
-/// dictionary or a member of a set.
-///
-/// ## Known problems
-/// Does not check for `__hash__` implementations in superclasses.
+/// method implicitly set to `None`, regardless of if a super class defines
+/// `__hash__`. This will cause the class to be unhashable, will in turn
+/// cause issues when using the class as a key in a dictionary or a member
+/// of a set.
///
/// ## Example
+///
/// ```python
/// class Person:
/// def __init__(self):
@@ -29,6 +28,7 @@ use crate::checkers::ast::Checker;
/// ```
///
/// Use instead:
+///
/// ```python
/// class Person:
/// def __init__(self):
@@ -40,6 +40,60 @@ use crate::checkers::ast::Checker;
/// def __hash__(self):
/// return hash(self.name)
/// ```
+///
+/// This issue is particularly tricky with inheritance. Even if a parent class correctly implements
+/// both `__eq__` and `__hash__`, overriding `__eq__` in a child class without also implementing
+/// `__hash__` will make the child class unhashable:
+///
+/// ```python
+/// class Person:
+/// def __init__(self):
+/// self.name = "monty"
+///
+/// def __eq__(self, other):
+/// return isinstance(other, Person) and other.name == self.name
+///
+/// def __hash__(self):
+/// return hash(self.name)
+///
+///
+/// class Developer(Person):
+/// def __init__(self):
+/// super().__init__()
+/// self.language = "python"
+///
+/// def __eq__(self, other):
+/// return (
+/// super().__eq__(other)
+/// and isinstance(other, Developer)
+/// and self.language == other.language
+/// )
+///
+///
+/// hash(Developer()) # TypeError: unhashable type: 'Developer'
+/// ```
+///
+/// One way to fix this is to retain the implementation of `__hash__` from the parent class:
+///
+/// ```python
+/// class Developer(Person):
+/// def __init__(self):
+/// super().__init__()
+/// self.language = "python"
+///
+/// def __eq__(self, other):
+/// return (
+/// super().__eq__(other)
+/// and isinstance(other, Developer)
+/// and self.language == other.language
+/// )
+///
+/// __hash__ = Person.__hash__
+/// ```
+///
+/// ## References
+/// - [Python documentation: `object.__hash__`](https://docs.python.org/3/reference/datamodel.html#object.__hash__)
+/// - [Python glossary: hashable](https://docs.python.org/3/glossary.html#term-hashable)
#[derive(ViolationMetadata)]
pub(crate) struct EqWithoutHash;