Repository: snok/flake8-type-checking

Issue:
Type-checking block errors not reported for imports used in `cast()` string annotation
Given the following code:

```python
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:

```ini
[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:

```python
def asdf(a: Callable[..., Any], b: DateTime, c: Foo) -> None:
    ...
```

Discussion:
@sondrelg: This is intentional, since [F401](https://www.flake8rules.com/rules/F401.html) 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 

```diff
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 https://github.com/snok/flake8-type-checking/pull/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 :shrug: 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.

Solution patch:
diff --git a/flake8_type_checking/checker.py b/flake8_type_checking/checker.py
index d866926..39ae83c 100644
--- a/flake8_type_checking/checker.py
+++ b/flake8_type_checking/checker.py
@@ -992,6 +992,31 @@ class ImportAnnotationVisitor(AnnotationVisitor):
         self.import_visitor.in_soft_use_context = previous_context
 
 
+class CastTypeExpressionVisitor(AnnotationVisitor):
+    """Visit a cast type expression and collect all the quoted names."""
+
+    def __init__(self, typing_lookup: SupportsIsTyping) -> None:
+        #: All the quoted_names referenced inside the type expression
+        self.quoted_names: set[str] = set()
+        self._typing_lookup = typing_lookup
+
+    def is_typing(self, node: ast.AST, symbol: str) -> bool:
+        """Check if the given node matches the given typing symbol."""
+        return self._typing_lookup.is_typing(node, symbol)
+
+    def visit_annotation_name(self, node: ast.Name) -> None:
+        """Ignore visited names."""
+        # We could either record them as quoted names pre-emptively or
+        # as uses, but neither seems ideal, let's just skip these names
+        # as we have previously.
+
+    def visit_annotation_string(self, node: ast.Constant) -> None:
+        """Collect all the names referenced inside the forward reference."""
+        visitor = StringAnnotationVisitor(self._typing_lookup)
+        visitor.parse_and_visit_string_annotation(node.value)
+        self.quoted_names.update(visitor.names)
+
+
 class ImportVisitor(
     DunderAllMixin,
     FunctoolsSingledispatchMixin,
@@ -1081,6 +1106,10 @@ class ImportVisitor(
         #: Where typing.cast() is called with an unquoted type.
         self.unquoted_types_in_casts: list[tuple[int, int, str]] = []
 
+        #: All forward referenced names used in cast type expressions
+        # we need to track this in order to avoid false negatives for TC001-003
+        self.quoted_type_names_in_casts: set[str] = set()
+
         #: For tracking which comprehension/IfExp we're currently inside of
         self.active_context: Comprehension | ast.IfExp | None = None
 
@@ -1895,6 +1924,10 @@ class ImportVisitor(
 
         arg = node.args[0]
 
+        visitor = CastTypeExpressionVisitor(self)
+        visitor.visit(arg)
+        self.quoted_type_names_in_casts.update(visitor.quoted_names)
+
         if isinstance(arg, ast.Constant) and isinstance(arg.value, str):
             return  # Type argument is already a string literal.
 
@@ -1999,10 +2032,13 @@ class TypingOnlyImportsChecker:
         unused_imports = all_imports - self.visitor.names - self.visitor.soft_uses
         used_imports = all_imports - unused_imports
         already_imported_modules = [self.visitor.imports[name].module for name in used_imports]
-        annotation_names = (
-            [n for i in self.visitor.wrapped_annotations for n in i.names]
-            + [i.annotation for i in self.visitor.unwrapped_annotations]
-            + [n for i in self.visitor.excess_wrapped_annotations for n in i.names]
+        annotation_names = list(
+            chain(
+                (n for i in self.visitor.wrapped_annotations for n in i.names),
+                (i.annotation for i in self.visitor.unwrapped_annotations),
+                (n for i in self.visitor.excess_wrapped_annotations for n in i.names),
+                self.visitor.quoted_type_names_in_casts,
+            )
         )
 
         for name in unused_imports:


Test patch:
diff --git a/tests/test_tc001_to_tc003.py b/tests/test_tc001_to_tc003.py
index e0dcd57..ec097b1 100644
--- a/tests/test_tc001_to_tc003.py
+++ b/tests/test_tc001_to_tc003.py
@@ -323,6 +323,30 @@ def get_tc_001_to_003_tests(import_: str, ERROR: str) -> L:
             ),
             set(),
         ),
+        # Issue #127
+        (
+            textwrap.dedent(
+                f'''
+                from {import_} import Foo
+                from typing import Any, cast
+
+                a = cast('Foo', 1)
+                '''
+            ),
+            {'2:0 ' + ERROR.format(module=f'{import_}.Foo')},
+        ),
+        # forward reference in sub-expression of cast type
+        (
+            textwrap.dedent(
+                f'''
+                from {import_} import Foo
+                from typing import Any, cast
+
+                a = cast(list['Foo'], 1)
+                '''
+            ),
+            {'2:0 ' + ERROR.format(module=f'{import_}.Foo')},
+        ),
     ]
 
     return [