Skip to content
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

Update docs for eq-without-hash #14885

Merged
merged 2 commits into from
Dec 20, 2024
Merged
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
66 changes: 60 additions & 6 deletions crates/ruff_linter/src/rules/pylint/rules/eq_without_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
KotlinIsland marked this conversation as resolved.
Show resolved Hide resolved
///
/// ## Example
///
/// ```python
/// class Person:
/// def __init__(self):
Expand All @@ -29,6 +28,23 @@ use crate::checkers::ast::Checker;
/// ```
///
/// Use instead:
///
/// ```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)
/// ```
///
/// 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):
Expand All @@ -39,7 +55,45 @@ use crate::checkers::ast::Checker;
///
/// 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;

Expand Down
Loading