Proposed warning: reuse of loop variable in nested loop

[matthewlloyd]

I was just bitten by a bug in production where I accidentally reused an outer loop variable in a nested loop. Example:

# Outer loop.
for i in range(10):
    # Enough code here to hide the outermost loop off screen.
    # ...

    # Developer does not notice "i" is already being used as a loop variable
    # and accidentally overwrites it here. The type is the same, so type checkers
    # don't complain.
    for i in range(10):
        print(f"inner loop {i=}")

    # Enough code here to hide the innermost loop off screen.
    # ...

    # Developer expects "i" to still hold the value of the outer loop.
    # Instead, the value is always 9.
    print(f"outer loop {i=}")

Proposed warning: B???: "Variable 'i' is already declared in 'for' loop or 'with' statement above"

Credit for the wording: this is one of PyCharm's built-in inspections.

[cooperlees]

Hi,

Why have you opened issues for multiple linters? I don't know how ruff works, but does it only implement checks the original linter implements before it impenitents it in rust?

I guess to implement this one could just go through code looking for loops and keep track of each loops variable names and if they are the same in a nested loop error. I feel if this can be made error free/high signal I would accept it. This is pretty well known scope boundaries just being hit and some people might do it on purpose is my only worry (I don't know why) ...

[matthewlloyd]

Why have you opened issues for multiple linters? I don't know how ruff works, but does it only implement checks the original linter implements before it impenitents it in rust?

Just as a courtesy - although I only use Ruff personally, flake8-bugbear is a great project and many rules in Ruff originated here, so I wanted to give you an opportunity to add this feature too.

Ruff has reimplemented many of this project's rules in Rust, but does also have a small set of its own rules. There's no need for flake8-bugbear to implement this check in order for it to be added to Ruff, so don't worry about that, and while this fits more cleanly under the "flake8-bugbear" category in Ruff's list of checks than any other, it could also just be added to the Ruff-specific rules list.

While I only need it for Ruff, I guess ideally, the check would be added to both. At least, that's my hope... I haven't heard back from the Ruff team about their interest level yet.

[cooperlees]

Well, it seems you're going ahead and not using flake8-bugbear codes for RUF and I appreciate the suggestion and consideration here. I do feel any half decent unittest would catch a missus of this, but I am a believer in testing in layers tho so if someone wants to add this, and unittest well I am happy to add it.