Skip to content

Commit

Permalink
[pylint] Fixed false-positive on the rule PLW1641 (`eq-without-ha…
Browse files Browse the repository at this point in the history
…sh`) (#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.
  • Loading branch information
boolean-light authored Mar 25, 2024
1 parent 59ac3f4 commit f7aab5a
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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__
11 changes: 6 additions & 5 deletions crates/ruff_linter/src/rules/pylint/rules/eq_without_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down

0 comments on commit f7aab5a

Please sign in to comment.