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.

Sign up for GitHub

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

Jump to bottom

Catch variable is [] #8589

Closed
francorbacho opened this issue Nov 9, 2023 · 5 comments
Closed

Catch variable is [] #8589

francorbacho opened this issue Nov 9, 2023 · 5 comments
Labels
good first issue Good for newcomers rule Implementing or modifying a lint rule

Comments

@francorbacho
Copy link

Make F632 catch the following trap:

variable = []
if variable is []:
  print("never runs")

Tested in Arch Linux:

$ pacman -Q ruff 
ruff 0.1.5-1
@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Nov 9, 2023
@charliermarsh
Copy link
Member

It seems reasonable to me.

@zanieb zanieb added the good first issue Good for newcomers label Nov 9, 2023
@francorbacho
Copy link
Author

Although maybe it should be a different rule if we want to also test for variable is [0, 1, 2].

@charliermarsh
Copy link
Member

Seems ok to fold it into the same rule for me, since that will also always be false, right?

@jesse1412
Copy link
Contributor

Could this be expanded to cover any of the mutable initialisers? E.g set, dict, etc?

@jesse1412 jesse1412 mentioned this issue Nov 10, 2023
Merged
charliermarsh pushed a commit that referenced this issue Nov 11, 2023
@jesse1412
Add check for is comparison with mutable initialisers to rule F632 (#…
39728a1 
…8607)

## Summary

Adds an extra check to F632 to check for any `is` comparisons to a
mutable initialisers.
Implements #8589 .

Example:
```Python
named_var = {}
if named_var is {}:  # F632 (fix)
    pass
```
The if condition will always evaluate to False because it checks on
identity and it's impossible to take the same identity as a hard coded
list/set/dict initializer.

## Test Plan

Multiple test cases were added to ensure the rule works + doesn't flag
false positives + the fix works correctly.
@charliermarsh
Copy link
Member

I believe this was closed by #8607 :) Thanks @jesse1412!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers rule Implementing or modifying a lint rule
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@charliermarsh @zanieb @jesse1412 @francorbacho