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: B031 #3829

Closed
AA-Turner opened this issue Mar 31, 2023 · 5 comments
Closed

False positive: B031 #3829

AA-Turner opened this issue Mar 31, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@AA-Turner
Copy link
Contributor

AA-Turner commented Mar 31, 2023

Ruff 0.0.260

Reproducer:

(sphinx) PS I:\Development\sphinx> ruff -V                                                
ruff 0.0.260
(sphinx) PS I:\Development\sphinx> type bug_b031.py
from itertools import groupby


def f():
    for w, g in groupby('string', len):
        if w == 1:
            return (''.join(g)).split(" ")
        else:
            return list(g)  # Ruff indicates B031 here


def g():
    for w, g in groupby('string', len):
        tmp = [*g]  # by introducing an interstitial variable, we avoid B031
        if w == 1:
            return (''.join(tmp)).split(" ")
        else:
            return list(tmp)
(sphinx) PS I:\Development\sphinx> ruff --isolated --show-source --select B031 bug_b031.py
bug_b031.py:9:25: B031 Using the generator returned from `itertools.groupby()` more than once will do nothing on the second usage
  |
9 |             return list(g)  # Ruff indicates B031 here
  |                         ^ B031
  |

Found 1 error.
(sphinx) PS I:\Development\sphinx> 
bug_b031.py
from itertools import groupby


def f():
    for w, g in groupby('string', len):
        if w == 1:
            return (''.join(g)).split(" ")
        else:
            return list(g)  # Ruff indicates B031 here


def g():
    for w, g in groupby('string', len):
        tmp = [*g]  # by introducing an interstitial variable, we avoid B031
        if w == 1:
            return (''.join(tmp)).split(" ")
        else:
            return list(tmp)

Sorry for the rather contrived example, this was reduced from some code in Sphinx. Broadly I think that the error comes from the heuristic checking any re-use, whereas here as the symbol g is used twice, but only once per branch of an if/elif/else chain.

Please would it be possible to add some heuristic for tracking usages like this, in if/elif/else chains?

Thanks,
Adam

@charliermarsh
Copy link
Member

👍 I think this is the same as #3801. flake8-bugbear also has this problem. I think we should be able to avoid it though.

@charliermarsh charliermarsh added the bug Something isn't working label Mar 31, 2023
@AA-Turner
Copy link
Contributor Author

Ahh, sorry--should have had a quick look before submitting this!

Thanks,
Adam

@charliermarsh
Copy link
Member

All good, it's useful to know that others have hit this too and to see another example!

@dhruvmanila
Copy link
Member

I'll try to look into it over the weekend but I guess it would be better to track it in one issue. We could move the example in the other one.

@charliermarsh
Copy link
Member

@dhruvmanila - Sounds good, I'm going to mark as a duplicate.

@charliermarsh charliermarsh closed this as not planned Won't fix, can't repro, duplicate, stale Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants