Spurious F811 with conditional definition

[oscarbenjamin]

With pyflakes 3.1.0 (but not 3.0.1) the following produces F811:

from module import function

A = None

if function():
    class A:
        pass

This gives:

$ flake8 t.py 
t.py:6:5: F811 redefinition of unused 'A' from line 3

Here A is defined conditionally with None as a default so this is not really a "redefinition".

This was previously PyCQA/flake8#1855

Possibly the cause of the change is gh-761.

[asottile]

you've already got a # type: ignore on that line for the same reason -- you can also add a # noqa: F811 -- or write it with the assignment in an else: (which imo is clearer anyway)

I think this is working as intended

[oscarbenjamin]

Fair enough.

I've added the noqa but personally I consider that neither of these should be needed. The type: ignore is there because of deficiencies in mypy which won't accept any variant of the code that preserves the class name. And now the noqa is also there because of a deficiency in flake8.

You might think that putting it in the else is clearer. I'm not sure I agree but I don't feel strongly about it. Mainly what I don't want is to go round making pointless changes like that because whatever change I choose here is not going to improve anything in any meaningful way.

We get a lot of value from flake8 and I like the emphasis on minimising "false positives" but that's exactly what I would call this: a false positive.