`F841` ignores redefinitions of local assignments

[mertcangokgoz]

While developing complex code in Django, I realised that F841 was not working properly.

I am leaving a small code example. under normal conditions flake8 says that the "teams" in this code are not used, but I do not encounter any warning in ruff.

Or even if it detects it, it shows the last one found, it must show all three

def test():
    if user.amin_user:
        teams = user.teams.all()
        if user.teams.filter(name="admin").exists():
            print("admin")
        else:
            print("not admin")

    if user.related_user:
        teams = user.teams.all()
        if user.teams.filter(name="related").exists():
            print("related")
        else:
            print("not related")

    if user.test_user:
        teams = user.teams.all()
        if user.teams.filter(name="test").exists():
            print("test")
        else:
            print("tests")

    if user.related_user:
        teams = user.teams.all()
        if user.teams.filter(name="related").exists():
            print("related")
        else:
            print("not related")

Ruff version: 0.0.285

[charliermarsh]

I see the same output here for Ruff and Flake8.

Ruff:

❯ ruff check foo.py -n --select F841 --isolated
foo.py:24:9: F841 [*] Local variable `teams` is assigned to but never used

Flake8:

❯ flake8 foo.py --select F841
foo.py:24:9: F841 local variable 'teams' is assigned to but never used

What am I missing here? :)

[mertcangokgoz]

Sorry, my example is wrong, the trouble starts exactly as follows.

def test():
    if user.admin_user:
        teams = user.teams.all()
        if user.teams.filter(name="admin").exists():
            print("admin")
        else:
            print("not admin")

    if user.related_user:
        if user.teams.filter(name="related").exists():
            print("related")
        else:
            print("not related")

    if user.test_user:
        if user.teams.filter(name="test").exists():
            print("test")
        else:
            print("tests")

    if user.related_user:
        if user.teams.filter(name="related").exists():
            print("related")
        else:
            print("not related")

    if user.org_user:
        teams = user.teams.all()
        if teams.filter(booking=True).exists():
            return self.filter(models.Q(booking__isnull=False))
        return self.filter()

    return self.none()

[charliermarsh]

Thanks, though I still don't see F841 errors for Ruff or Flake8 with that code:

❯ flake8 foo.py --select F841
❯ ruff check foo.py -n --select F841

[mertcangokgoz]

Yes, but normally you should see it, because the teams in line 3 have never been used :S

[charliermarsh]

There are a bunch of Pyflakes issues about this like https://github.com/PyCQA/pyflakes/issues/498 or the parent issue https://github.com/PyCQA/pyflakes/issues/715. It's not trivial because we need to handle cases like:

a = 1
if ...:
  a = 2
print(a)

Or, above:

def test():
    if user.admin_user:
        teams = user.teams.all()
        if user.teams.filter(name="admin").exists():
            print("admin")
        else:
            print("not admin")

    if user.org_user:
        # Redefined `teams`, unused.
        teams = user.teams.all()

    # But it _might_ be used here.
    if teams.filter(booking=True).exists():
        return self.filter(models.Q(booking__isnull=False))

    return self.filter()

Undecided on whether to keep this open as it does have parity with Pyflakes and this kind of branch analysis is probably a bigger project beyond this issue.

[mertcangokgoz]

It makes it very difficult for us to find the definitions that the developer overlooked in the ci/cd process. Thank you for the information and tagging. <3

[MichaReiser]

I think Ruff should support this but it requires a branch sensitive analysis which Ruff doesn't support today