From f7aab5ac69f9505a1776a4bcf20ea2415aa5e442 Mon Sep 17 00:00:00 2001 From: hikaru-kajita <61526956+boolean-light@users.noreply.github.com> Date: Mon, 25 Mar 2024 23:40:01 +0900 Subject: [PATCH] [`pylint`] Fixed false-positive on the rule `PLW1641` (`eq-without-hash`) (#10566) ## Summary Fixed false-positive on the rule `PLW1641`, where the explicit assignment on the `__hash__` method is not counted as an definition of `__hash__`. (Discussed in #10557). Also, added one new testcase. ## Test Plan Checked on `cargo test` in `eq_without_hash.py`. Before the change, for the assignment into `__hash__`, only `__hash__ = None` was counted as an explicit definition of `__hash__` method. Probably any assignment into `__hash__` property could be counted as an explicit definition of hash, so I removed `value.is_none_literal_expr()` check. --- .../test/fixtures/pylint/eq_without_hash.py | 17 +++++++++++++++++ .../src/rules/pylint/rules/eq_without_hash.rs | 11 ++++++----- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/eq_without_hash.py b/crates/ruff_linter/resources/test/fixtures/pylint/eq_without_hash.py index f34c90e4dc3c9..35c2fab7b5a4b 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/eq_without_hash.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/eq_without_hash.py @@ -5,6 +5,7 @@ def __init__(self): def __eq__(self, other): return isinstance(other, Person) and other.name == self.name + # OK class Language: def __init__(self): @@ -16,8 +17,24 @@ def __eq__(self, other): def __hash__(self): return hash(self.name) + class MyClass: def __eq__(self, other): return True __hash__ = None + + +class SingleClass: + def __eq__(self, other): + return True + + def __hash__(self): + return 7 + + +class ChildClass(SingleClass): + def __eq__(self, other): + return True + + __hash__ = SingleClass.__hash__ 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 e8c3b1ad14ab2..23c9999e8ebb1 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 @@ -67,20 +67,21 @@ fn has_eq_without_hash(body: &[Stmt]) -> bool { let mut has_eq = false; for statement in body { match statement { - Stmt::Assign(ast::StmtAssign { targets, value, .. }) => { + Stmt::Assign(ast::StmtAssign { targets, .. }) => { let [Expr::Name(ast::ExprName { id, .. })] = targets.as_slice() else { continue; }; - // Check if `__hash__` was explicitly set to `None`, as in: + // Check if `__hash__` was explicitly set, as in: // ```python - // class Class: + // class Class(SuperClass): // def __eq__(self, other): // return True // - // __hash__ = None + // __hash__ = SuperClass.__hash__ // ``` - if id == "__hash__" && value.is_none_literal_expr() { + + if id == "__hash__" { has_hash = true; } }