From 37f4920e1e1b7d0931434cc3e306e52eefd90785 Mon Sep 17 00:00:00 2001 From: Victor Hugo Gomes Date: Mon, 21 Aug 2023 20:51:21 -0300 Subject: [PATCH] Don't trigger `eq-without-hash` when `__hash__` is explicitly set to `None` (#6739) --- .../test/fixtures/pylint/eq_without_hash.py | 9 ++++- .../src/rules/pylint/rules/eq_without_hash.rs | 33 ++++++++++++++----- ...nt__tests__PLW1641_eq_without_hash.py.snap | 2 +- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pylint/eq_without_hash.py b/crates/ruff/resources/test/fixtures/pylint/eq_without_hash.py index 44a62f3b7a..f34c90e4dc 100644 --- a/crates/ruff/resources/test/fixtures/pylint/eq_without_hash.py +++ b/crates/ruff/resources/test/fixtures/pylint/eq_without_hash.py @@ -1,10 +1,11 @@ -class Person: +class Person: # [eq-without-hash] def __init__(self): self.name = "monty" def __eq__(self, other): return isinstance(other, Person) and other.name == self.name +# OK class Language: def __init__(self): self.name = "python" @@ -14,3 +15,9 @@ class Language: def __hash__(self): return hash(self.name) + +class MyClass: + def __eq__(self, other): + return True + + __hash__ = None diff --git a/crates/ruff/src/rules/pylint/rules/eq_without_hash.rs b/crates/ruff/src/rules/pylint/rules/eq_without_hash.rs index 3cd0f5ce5e..134538534e 100644 --- a/crates/ruff/src/rules/pylint/rules/eq_without_hash.rs +++ b/crates/ruff/src/rules/pylint/rules/eq_without_hash.rs @@ -1,7 +1,7 @@ -use ruff_python_ast::{self as ast, Ranged, Stmt}; - use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::is_const_none; +use ruff_python_ast::{self as ast, Expr, Ranged, Stmt}; use crate::checkers::ast::Checker; @@ -65,12 +65,29 @@ 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, + match statement { + Stmt::Assign(ast::StmtAssign { targets, value, .. }) => { + let [Expr::Name(ast::ExprName { id, .. })] = targets.as_slice() else { + continue; + }; + + // Check if `__hash__` was explicitly set to `None`, as in: + // ```python + // class Class: + // def __eq__(self, other): + // return True + // + // __hash__ = None + // ``` + if id == "__hash__" && is_const_none(value) { + has_hash = true; + } + } + Stmt::FunctionDef(ast::StmtFunctionDef { name, .. }) => match name.as_str() { + "__hash__" => has_hash = true, + "__eq__" => has_eq = true, + _ => {} + }, _ => {} } } 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 index 83af186c00..5be273e060 100644 --- 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 @@ -3,7 +3,7 @@ source: crates/ruff/src/rules/pylint/mod.rs --- eq_without_hash.py:1:7: PLW1641 Object does not implement `__hash__` method | -1 | class Person: +1 | class Person: # [eq-without-hash] | ^^^^^^ PLW1641 2 | def __init__(self): 3 | self.name = "monty"