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

F841 false negative on assignment to multiple variables #8441

Closed
DetachHead opened this issue Nov 2, 2023 · 5 comments · Fixed by #8489
Closed

F841 false negative on assignment to multiple variables #8441

DetachHead opened this issue Nov 2, 2023 · 5 comments · Fixed by #8489
Labels
help wanted Contributions especially welcome rule Implementing or modifying a lint rule

Comments

@DetachHead
Copy link

def foo():
    a = bar() # error: F841
    b, c = bar() # no error

def bar():
    ...

playground

@qdegraaf
Copy link
Contributor

qdegraaf commented Nov 2, 2023

Could this be intentional? From the tests:

def f(tup):
    x, y = tup  # this does NOT trigger F841

Unsure if that was left there as a TODO or consciously omitted. @charliermarsh could you clarify?

@charliermarsh
Copy link
Member

Yeah this is intentional for parity with Flake8. I think it's reasonable. An improvement could be to flag x and y if both are unused.

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Nov 2, 2023
@DetachHead
Copy link
Author

I don't get why it should be treated differently. If it's for the case where you only want to use one of the values from the tulle, you can just prefix it with an _ to mark it as intentionally unused:

a, _b = foo()

@zanieb
Copy link
Member

zanieb commented Nov 2, 2023

Best practice is really

a, _ = foo()

I don't see why we wouldn't raise a violation here, other than flake8 compatibility

@charliermarsh
Copy link
Member

I'm not opposed to changing it. I didn't feel strongly so erred towards compatibility.

@zanieb zanieb added the help wanted Contributions especially welcome label Nov 3, 2023
charliermarsh pushed a commit that referenced this issue Nov 5, 2023
## Summary

Closes #8441 behind preview feature flag.

## Test Plan

`cargo test`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Contributions especially welcome rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants