Support Forward References for Annotations

[colinc719]

When vulture is run on the code

from collections import Sequence


def temp(arg: "Sequence"):
    assert not arg


if __name__ == '__main__':
    temp([])

produces "unused import 'Sequence'"

I would expect it to see that Sequence is used as a forward reference (https://www.python.org/dev/peps/pep-0484/#forward-references) in temp.

I'm running vulture 1.5.

[jendrikseipp]

Thanks for the report! Vulture currently doesn't parse string forward references, but I guess it would make for a nice addition. In your concrete example, however, couldn't you just use Sequence instead of the string "Sequence"?

[colinc719]

Thanks for the fast response. Yeah, I was just trying to post a simple example. A more realistic use-case would probably use TYPE_CHECKING, which gives the same result:

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from collections import Sequence


def temp(arg: "Sequence"):
    assert not arg


if __name__ == '__main__':
    temp([])

[jendrikseipp]

Thanks for the example!

[jendrikseipp]

#220 added support for parsing type comments, but forward references are still not taken into account by Vulture.

[jingw]

Just FYI, fixing this fully correctly is likely annoying, since the forward references can appear outside a type annotation context:

MyType = Union["LaterType", Whatever]

class LaterType:
    pass

That said, somehow flake8 deals with it, so it could be worth looking at whatever they're doing. Test case:

from foobar import Thing
from typing import Union

Thing2 = Union["Thing", int]

(not planning to work on this, just throwing this out)

Edit: It gets worse. Without following imports, the following code is ambiguous:

from foobar import MyType, MyType2, MyGenericType
from typing import Optional

MyGenericType["MyType"]
Optional["MyType2"]

It is not clear if MyGenericType["MyType"] should be interpreted as an ordinary dictionary or as type creation. Moreover, flake8 doesn't handle this:

test.py:1:1: F401 'foobar.MyType' imported but unused

It could be reasonable to simply remove unused import detection, as flake8 already performs that function. Then again, you could make a similar argument for unused locals, and these things are arguably still in vulture's ballpark. (I'm not the maintainer of this project; this is just my personal opinion.)

A workaround for the case of forward references in type annotations could be to use from __future__ import annotations.

[jendrikseipp]

Thanks for the explanation!

Maybe a solution could be to record all literal strings as used names. (This would render the getattr/hasattr PR obsolete.) My feeling is that this sounds too drastic, but could actually be the right move. On the other hand, maybe we shouldn't worry about this too much, given that there's from __future__ import annotations (thanks for the hint, didn't know about this!). What do you think?

I also thought about dropping unused import detection, but I think Vulture actually has some value in addition to flake8 for detecting unused imports:

# a.py
import sys

# b.py
import a
print(a.sys.argv)

Flake8 reports sys as an unused import, but Vulture doesn't.

[jingw]

I also feel that would be excessive.

Some more fun edge cases that come to mind:

• def f(n: "Union[int, str]"): ... is legal, so you'd also have to ast.parse things. As it turns out you can even nest the quotes: https://mypy-play.net/?mypy=latest&python=3.8&gist=0bfcb7137e8c63c83f12a24a62fe8cff
• dropping unused import detection doesn't suffice when custom types are involved. (i.e. declare MyType, but then only reference it as "MyType")

Fixing this for real looks like quite a can of worms. Declaring this a won't-fix in favor of from __future__ import annotations seems defensible. It doesn't replace stuff like MyType = Optional["OtherType"], but that's probably rare enough to deprioritize.

[jendrikseipp]

I fully agree. Let's not open the can of worms :-) I added a reference to this issue to the README:

Forward references for type annotations

See #216. For example, instead of def foo(arg: "Sequence"): ..., we recommend using

from __future__ import annotations

def foo(arg: Sequence):
    ...

if you're using Python 3.7+.