[pylint] Implement eq-without-hash rule (PLW1641) (#5955)

Implement
https://pylint.pycqa.org/en/latest/user_guide/messages/warning/eq-without-hash.html
Issue #970

It's not enabled by default in pylint, so I guess it shouldn't in Ruff
either?

crates/ruff/resources/test/fixtures/pylint/eq_without_hash.py

@@ -0,0 +1,16 @@
+class Person:
+    def __init__(self):
+        self.name = "monty"
+
+    def __eq__(self, other):
+        return isinstance(other, Person) and other.name == self.name
+
+class Language:
+    def __init__(self):
+        self.name = "python"
+
+    def __eq__(self, other):
+        return isinstance(other, Language) and other.name == self.name
+
+    def __hash__(self):
+        return hash(self.name)

crates/ruff/src/checkers/ast/analyze/statement.rs

@@ -396,6 +396,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
                     flake8_django::rules::model_without_dunder_str(checker, class_def);
                 }
             }
+            if checker.enabled(Rule::EqWithoutHash) {
+                pylint::rules::object_without_hash_method(checker, class_def);
+            }
             if checker.enabled(Rule::GlobalStatement) {
                 pylint::rules::global_statement(checker, name);
             }

crates/ruff/src/codes.rs

@@ -223,6 +223,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Pylint, "W0711") => (RuleGroup::Unspecified, rules::pylint::rules::BinaryOpException),
         (Pylint, "W1508") => (RuleGroup::Unspecified, rules::pylint::rules::InvalidEnvvarDefault),
         (Pylint, "W1509") => (RuleGroup::Unspecified, rules::pylint::rules::SubprocessPopenPreexecFn),
+        (Pylint, "W1641") => (RuleGroup::Nursery, rules::pylint::rules::EqWithoutHash),
         (Pylint, "W2901") => (RuleGroup::Unspecified, rules::pylint::rules::RedefinedLoopName),
         (Pylint, "W3301") => (RuleGroup::Unspecified, rules::pylint::rules::NestedMinMax),
 

crates/ruff/src/rules/pylint/mod.rs

@@ -32,6 +32,7 @@ mod tests {
         Path::new("repeated_isinstance_calls.py")
     )]
     #[test_case(Rule::ComparisonWithItself, Path::new("comparison_with_itself.py"))]
+    #[test_case(Rule::EqWithoutHash, Path::new("eq_without_hash.py"))]
     #[test_case(Rule::ManualFromImport, Path::new("import_aliasing.py"))]
     #[test_case(Rule::SingleStringSlots, Path::new("single_string_slots.py"))]
     #[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_0.py"))]

crates/ruff/src/rules/pylint/rules/eq_without_hash.rs

@@ -0,0 +1,78 @@
+use ruff_python_ast::{self as ast, Ranged, Stmt};
+
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, violation};
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for classes that implement `__eq__` but not `__hash__`.
+///
+/// ## 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.
+///
+/// ## Example
+/// ```python
+/// class Person:
+///     def __init__(self):
+///         self.name = "monty"
+///
+///     def __eq__(self, other):
+///         return isinstance(other, Person) and other.name == self.name
+/// ```
+///
+/// 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)
+/// ```
+#[violation]
+pub struct EqWithoutHash;
+
+impl Violation for EqWithoutHash {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Object does not implement `__hash__` method")
+    }
+}
+
+/// W1641
+pub(crate) fn object_without_hash_method(
+    checker: &mut Checker,
+    ast::StmtClassDef { name, body, .. }: &ast::StmtClassDef,
+) {
+    if has_eq_without_hash(body) {
+        checker
+            .diagnostics
+            .push(Diagnostic::new(EqWithoutHash, name.range()));
+    }
+}
+
+fn has_eq_without_hash(body: &[Stmt]) -> bool {
+    let mut has_hash = false;
+    let mut has_eq = false;
+    for statement in body {
+        let Stmt::FunctionDef(ast::StmtFunctionDef { name, .. }) = statement else {
+            continue;
+        };
+        match name.as_str() {
+            "__hash__" => has_hash = true,
+            "__eq__" => has_eq = true,
+            _ => {}
+        }
+    }
+    has_eq && !has_hash
+}

crates/ruff/src/rules/pylint/rules/mod.rs

@@ -10,6 +10,7 @@ pub(crate) use comparison_of_constant::*;
 pub(crate) use comparison_with_itself::*;
 pub(crate) use continue_in_finally::*;
 pub(crate) use duplicate_bases::*;
+pub(crate) use eq_without_hash::*;
 pub(crate) use global_statement::*;
 pub(crate) use global_variable_not_assigned::*;
 pub(crate) use import_self::*;
@@ -63,6 +64,7 @@ mod comparison_of_constant;
 mod comparison_with_itself;
 mod continue_in_finally;
 mod duplicate_bases;
+mod eq_without_hash;
 mod global_statement;
 mod global_variable_not_assigned;
 mod import_self;

crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW1641_eq_without_hash.py.snap

@@ -0,0 +1,12 @@
+---
+source: crates/ruff/src/rules/pylint/mod.rs
+---
+eq_without_hash.py:1:7: PLW1641 Object does not implement `__hash__` method
+  |
+1 | class Person:
+  |       ^^^^^^ PLW1641
+2 |     def __init__(self):
+3 |         self.name = "monty"
+  |
+
+