PyCQA / pyflakes

A simple program which checks Python source files for errors
https://pypi.org/project/pyflakes
MIT License
1.36k stars 178 forks source link

Spurious unsued import warning for type annotation used in quoted cast #510

Closed PeterJCLaw closed 4 years ago

PeterJCLaw commented 4 years ago

Given

# demo.py 
from typing import Union, cast

x = cast('Union[str, int]', 42)
reveal_type(x)

Then I would hope that pyflakes would be able to see the usage of Union in the quoted cast, but it can't.

$ pyflakes demo.py 
demo.py:1: 'typing.Union' imported but unused
demo.py:4: undefined name 'reveal_type'

(reveal_type being undefined is correct here).

mypy does seem happy with this usage though.

$ mypy demo.py 
demo.py:4: error: Revealed type is 'Union[builtins.str, builtins.int]'

I realise this example is somewhat contrived (why would you quote this), however in the case that one of the union members is a circular import for example, you need to quote at least some part of the type and it's often cleaner to quote the whole name rather than parts of it.

asottile commented 4 years ago

does #479 fix this (is this a duplicate?) and/or could you review the changes there?

PeterJCLaw commented 4 years ago

Unfortunately #479 doesn't seem to resolve this issue.

asottile commented 4 years ago

ah this is the "cast argument 1 is actually a typing context" -- probably will need the code in #479 to solve it though

PeterJCLaw commented 4 years ago

There's another case, which I think is a variant of this:

from queue import Queue
from typing import Optional

Thing = Optional['Queue[int]']  # pyflakes can't see this usage of `Queue`

def foo(queue: Thing = None) -> None:
    print(queue)

# Revealed type is 'def (queue: Union[queue.Queue[builtins.int], None] =)'
reveal_type(foo)
asottile commented 4 years ago

could you review and/or approve #479 (we have double sign-off) -- then I can build on that to implement the fixes for those two cases

PeterJCLaw commented 4 years ago

Ah, I'd not realised about double sign-off, sorry, will try to look this evening.

jnsnow commented 4 years ago

Since the issue I filed was closed as a duplicate, I want to relay some of the information I wrote there that I think is pertinent relating to MyPy's usage of types like os.PathLike.

From the now-closed #515:

First, note that PathLike[str] is a case where MyPy uses a Generic type stub, but that the runtime object here is not subscriptable. In these cases, such types must be string quoted, even when using delayed annotations support from PEP 563.

Might be the same root cause for pyflakes, but the workarounds for the use case here and in 515 are going to wind up being different. At the very least, it's another test case :)

Thanks all! --js

asottile commented 4 years ago

yep yep! Queue[int] above is the same sort of "typing-time-only subscriptable"s

jnsnow commented 4 years ago

AH MY MISTAKE. I'm drowning in CQA errors and it's hard to keep everything fully coherent. Thanks for being patient @asottile

PeterJCLaw commented 4 years ago

@jnsnow I've got a fix for this up at #516 -- any chance you'd be able to test it out and review that PR?

eli-schwartz commented 3 years ago

If I'm reading the fix correctly, this only solves the case of

import typing as T
from typing import SomeType

T.cast('SomeType', var)

where SomeType comes from the typing module. But not if SomeType is imported from your own codebase?

I could not find an open issue for the latter case. Is this tracked/on the roadmap? Should I open a new issue?

Example where I am hitting this:

asottile commented 3 years ago

If I'm reading the fix correctly

fortunately you're not reading it correctly -- it's checking if cast comes from the typing module

eli-schwartz commented 3 years ago

Ok.

So, one way or another I still seem to be seeing spurious unused import warnings HERE.

$ pyflakes --version
2.3.1 Python 3.9.7 on Linux
$ pyflakes mesonbuild/modules/__init__.py
mesonbuild/modules/__init__.py:25:5 '..interpreter.MachineHolder' imported but unused

And it is consumed by a T.cast(), and nowhere else.

asottile commented 3 years ago

pyflakes 2.3.1 does not understand aliasing the typing module

eli-schwartz commented 3 years ago

Ah indeed, an additional import typing and using typing.cast made it all work.

Apologies for getting confused and posting nonsense to a random bug. (I had no clue what to look for, so tried searching frantically for something mentioning quoted casts.)

I'll go follow #632 instead. Thanks for clearing that up!