Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False-positive PLW1641 when setting __hash__ = <ParentClass>.__hash__ #10557

Closed
janosh opened this issue Mar 25, 2024 · 1 comment
Closed

False-positive PLW1641 when setting __hash__ = <ParentClass>.__hash__ #10557

janosh opened this issue Mar 25, 2024 · 1 comment
Labels
bug Something isn't working help wanted Contributions especially welcome rule Implementing or modifying a lint rule

Comments

@janosh
Copy link

janosh commented Mar 25, 2024

with v0.3.4, ruff check t.py --select PLW1641 on

class A:
    def __eq__(self, other):
        return False

    def __hash__(self):
        return 7

class B(A):
    def __eq__(self, other):
        return True

    __hash__ = A.__hash__

issues

PLW1641 Object does not implement `__hash__` method
   |
12 | class B(A):
   |       ^ PLW1641
13 |     def __eq__(self, other):
14 |         return True

even though the docs explicitly recommend this:

If a class that overrides eq() needs to retain the implementation of hash() from a parent class, the interpreter must be told this explicitly by setting __hash__ = <ParentClass>.__hash__.

@MichaReiser MichaReiser added bug Something isn't working rule Implementing or modifying a lint rule help wanted Contributions especially welcome labels Mar 25, 2024
charliermarsh pushed a commit that referenced this issue Mar 25, 2024
…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.
@charliermarsh
Copy link
Member

This is now fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Contributions especially welcome rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

3 participants