diff --git a/crates/ruff/resources/test/fixtures/pylint/eq_without_hash.py b/crates/ruff/resources/test/fixtures/pylint/eq_without_hash.py new file mode 100644 index 00000000000000..44a62f3b7aa6a4 --- /dev/null +++ b/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) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 4cc48804c0fbb7..19a74cc65a5f3a 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -694,6 +694,9 @@ where if self.enabled(Rule::DjangoUnorderedBodyContentInModel) { flake8_django::rules::unordered_body_content_in_model(self, bases, body); } + if self.enabled(Rule::EqWithoutHash) { + pylint::rules::object_without_hash_method(self, class_def); + } if !self.is_stub { if self.enabled(Rule::DjangoModelWithoutDunderStr) { flake8_django::rules::model_without_dunder_str(self, class_def); diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index c08c86733a1dd1..2d5d2eecbd1e24 100644 --- a/crates/ruff/src/codes.rs +++ b/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::Unspecified, rules::pylint::rules::EqWithoutHash), (Pylint, "W2901") => (RuleGroup::Unspecified, rules::pylint::rules::RedefinedLoopName), (Pylint, "W3301") => (RuleGroup::Unspecified, rules::pylint::rules::NestedMinMax), diff --git a/crates/ruff/src/rules/pylint/mod.rs b/crates/ruff/src/rules/pylint/mod.rs index af32c98ea55c9b..4e0a8f7e1b4621 100644 --- a/crates/ruff/src/rules/pylint/mod.rs +++ b/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"))] diff --git a/crates/ruff/src/rules/pylint/rules/eq_without_hash.rs b/crates/ruff/src/rules/pylint/rules/eq_without_hash.rs new file mode 100644 index 00000000000000..3a373c8630e242 --- /dev/null +++ b/crates/ruff/src/rules/pylint/rules/eq_without_hash.rs @@ -0,0 +1,75 @@ +use rustpython_parser::ast::{self, Ranged, Stmt}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks that a class that implements a `__eq__` also implements `__hash__`. +/// +/// ## Why is this bad? +/// A class that overrides eq() and does not define hash() will have its hash() +/// implicitly set to None. +/// +/// ## 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 !hash_eq_without_hash(body) { + return; + } + checker + .diagnostics + .push(Diagnostic::new(EqWithoutHash, name.range())); +} + +fn hash_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; + }; + if name == "__hash__" { + has_hash = true; + } else if name == "__eq__" { + has_eq = true; + } + } + has_eq && !has_hash +} diff --git a/crates/ruff/src/rules/pylint/rules/mod.rs b/crates/ruff/src/rules/pylint/rules/mod.rs index 9536ba69096446..1cc36b22937ebe 100644 --- a/crates/ruff/src/rules/pylint/rules/mod.rs +++ b/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; diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW1641_eq_without_hash.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW1641_eq_without_hash.py.snap new file mode 100644 index 00000000000000..83af186c0079b0 --- /dev/null +++ b/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" + | + + diff --git a/ruff.schema.json b/ruff.schema.json index f1c40fa7c085f5..49343659abdd34 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2270,6 +2270,9 @@ "PLW150", "PLW1508", "PLW1509", + "PLW16", + "PLW164", + "PLW1641", "PLW2", "PLW29", "PLW290",