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

Consider extending PLR2004 to cover iterables with magic values #12155

Open
kgleba opened this issue Jul 2, 2024 · 0 comments
Open

Consider extending PLR2004 to cover iterables with magic values #12155

kgleba opened this issue Jul 2, 2024 · 0 comments
Labels
rule Implementing or modifying a lint rule

Comments

@kgleba
Copy link

kgleba commented Jul 2, 2024

Keywords: PLR2004, magic-value-comparison

Here's a suggestion regarding PLR2004, although I don't quite understand the final optimal form it should take (I've found a comment that Ruff tries to be a "faithful implementation of the Pylint rule" in this case, so I do not insist on changing the behavior of the original rule).

Imagine that a developer wants to check if the status_code of some request is in a redirect group (e.g., 301, 302).

A naive approach is to simply check for equality:

if status_code == 301 or status_code == 302:
    ...

The developer immediately receives 3 errors: PLR1714 (repeated-equality-comparison) and 2 instances of PLR2004.

More advanced Python developers will skip the step above and just write:

if status_code in (301, 302):
    ...

And all 3 errors will disappear, even though the problem with magic value comparison still remains.

The code above is also the default suggestion from PLR1714, and copy-pasting the solution (which mysteriously removed 2 more warnings) can be misleading for inexperienced developers.

As I mentioned above, I'm not sure how to properly address this issue and I'm obviously unaware of some implementational pitfalls, but I believe that forcing this check even for homogeneous iterables (such as list[int] or tuple[int]) would be a pretty good step in the right direction.

Hoping to get some feedback! :)

@MichaReiser MichaReiser added the rule Implementing or modifying a lint rule label Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

2 participants