Type-checking block errors not reported for imports used in cast() string annotation

[bryanforbes]

Given the following code:

from __future__ import annotations

from collections.abc import Callable
from typing import Any, cast

from pendulum.datetime import DateTime

from .foo import Foo

a = cast('Callable[..., Any]', {})
b = cast('DateTime', {})
c = cast('Foo', {})

And the following config:

[flake8]
select = TC,TC1

No errors are reported at line 3 (TC003), line 6 (TC002), and line 8 (TC001).

If the following function is added in the code, the errors are triggered:

def asdf(a: Callable[..., Any], b: DateTime, c: Foo) -> None:
    ...

[sondrelg]

This is intentional, since F401 which comes built-into flake8 already flags unused imports. Do you have a use case where F401 is not flagged, and we still don't flag anything?

[bryanforbes]

If I change the config to be select = F,TC,TC1, no errors are triggered (including F401).

[sondrelg]

Huh yeah that's strange. @Goldziher know what we're missing here?

[Goldziher]

I would suspect flake8 parses the values as strings. If you remove the strings from cast, what happens?

[bryanforbes]

Removing the strings (changing them to ex. cast(DateTime, {})) raises TC006 but no other errors since all of those symbols are referenced in running code now

[sondrelg]

yep, looks like

from collections.abc import Callable
from typing import Any, cast

from pendulum.datetime import DateTime

from .foo import Foo

a = cast('Callable[..., Any]', {})
- b = cast('DateTime', {})
c = cast('Foo', {})

Triggers the F401:

test.py:4:1: F401 'pendulum.datetime.DateTime' imported but unused

[sondrelg]

I suppose we should try to parse string values as well, and still raise TC errors when something's present. If that's not possible, I think we'll need to revert to always raising.

[sondrelg]

After looking at this for a bit, the problem is harder than just noting down the string value of every cast and crosschecking imports against it. In the example string literal "Callable[..., Any]" Callable is contained in the string, but to match it to an import we either need to fully parse all the parts of complex annotations (which sounds like it will add a ton of complexity to the plugin for very little value), or do partial matches (import in annotation) which seems error prone, and a poor solution in general.

A second option would be to revert #125, which means we would start raising TC[1,2,3] errors when imports are not used at all again.

And a third option would be to do nothing. To accept that casts contain a blind spot. Unused imports are still caught by a lot of other linters and IDEs, so this seems like it might be reasonable. Seems like it would warrant a note in the readme though.

I hope I'm wrong and that there's a fourth elegant solution which doesn't require a trade-off, but AFAICT there isn't 🤷 Suggestions are very welcome.

I'm leaning towards option 2 or 3, but don't have a strong preference. What do you think?

[bryanforbes]

Thanks for the explanation! If it's going to add a bunch of complexity to the plugin, I vote for the third option and closing this.

[sondrelg]

I don't love any of these solutions. In some ways I prefer flagging unused imports since a false positive seems better than a false negative. The only downside is there's a lot more false positives. Maybe we can keep this open for now in the hope someone has a suggestion for a fourth option.