Repository: PyCQA/pyflakes (python)

Issue:
Spurious import warning for type used in nested quoted annotation
This is similar to #290 and #313, but seems to still be an issue.

If you import a type and then use it only within a type annotation which includes a nested quoted part, then `pyflakes` reports the import as unused.

``` python
from queue import Queue  # F401: 'queue.Queue' imported but unused
from typing import Optional

def foo(queue: Optional['Queue[int]'] = None) -> None:
    print(queue)

# Revealed type is 'def (queue: Union[queue.Queue[builtins.int], None] =)'
reveal_type(foo)
```

This can be worked around by quoting the whole of the type annotation:
``` python
def foo(queue: 'Optional[Queue[int]]' = None) -> None:
    print(queue)
```

I'm using pyflakes 2.1.1 on Python 3.5.2 on Linux.

Pull request:
Fix false positive with partially quoted annotations
Resolves #447

There's a couple cases that needed handling here:
- partially quoted annotations (as reported in the original issue)
- annotations which are doubly forward annotations (I don't have a word for this or a document that explains them, but `mypy` handles them fine -- things like `-> "foo['bar']"`
- quoted annotations while using `from __future__ import annotations`

Patch:
diff --git a/pyflakes/checker.py b/pyflakes/checker.py
index d157008..c89d6fb 100644
--- a/pyflakes/checker.py
+++ b/pyflakes/checker.py
@@ -618,40 +618,55 @@ def getNodeName(node):
         return node.name
 
 
-def is_typing_overload(value, scope_stack):
-    def name_is_typing_overload(name):  # type: (str) -> bool
+def _is_typing(node, typing_attr, scope_stack):
+    def _bare_name_is_attr(name):
+        expected_typing_names = {
+            'typing.{}'.format(typing_attr),
+            'typing_extensions.{}'.format(typing_attr),
+        }
         for scope in reversed(scope_stack):
             if name in scope:
                 return (
                     isinstance(scope[name], ImportationFrom) and
-                    scope[name].fullName in (
-                        'typing.overload', 'typing_extensions.overload',
-                    )
+                    scope[name].fullName in expected_typing_names
                 )
 
         return False
 
-    def is_typing_overload_decorator(node):
-        return (
-            (
-                isinstance(node, ast.Name) and name_is_typing_overload(node.id)
-            ) or (
-                isinstance(node, ast.Attribute) and
-                isinstance(node.value, ast.Name) and
-                node.value.id == 'typing' and
-                node.attr == 'overload'
-            )
+    return (
+        (
+            isinstance(node, ast.Name) and
+            _bare_name_is_attr(node.id)
+        ) or (
+            isinstance(node, ast.Attribute) and
+            isinstance(node.value, ast.Name) and
+            node.value.id in {'typing', 'typing_extensions'} and
+            node.attr == typing_attr
         )
+    )
 
+
+def is_typing_overload(value, scope_stack):
     return (
         isinstance(value.source, FUNCTION_TYPES) and
         any(
-            is_typing_overload_decorator(dec)
+            _is_typing(dec, 'overload', scope_stack)
             for dec in value.source.decorator_list
         )
     )
 
 
+def in_annotation(func):
+    @functools.wraps(func)
+    def in_annotation_func(self, *args, **kwargs):
+        orig, self._in_annotation = self._in_annotation, True
+        try:
+            return func(self, *args, **kwargs)
+        finally:
+            self._in_annotation = orig
+    return in_annotation_func
+
+
 def make_tokens(code):
     # PY3: tokenize.tokenize requires readline of bytes
     if not isinstance(code, bytes):
@@ -738,6 +753,9 @@ class Checker(object):
     nodeDepth = 0
     offset = None
     traceTree = False
+    _in_annotation = False
+    _in_typing_literal = False
+    _in_deferred = False
 
     builtIns = set(builtin_vars).union(_MAGIC_GLOBALS)
     _customBuiltIns = os.environ.get('PYFLAKES_BUILTINS')
@@ -769,6 +787,7 @@ class Checker(object):
         for builtin in self.builtIns:
             self.addBinding(None, Builtin(builtin))
         self.handleChildren(tree)
+        self._in_deferred = True
         self.runDeferred(self._deferredFunctions)
         # Set _deferredFunctions to None so that deferFunction will fail
         # noisily if called after we've run through the deferred functions.
@@ -1009,12 +1028,30 @@ class Checker(object):
 
         self.scope[value.name] = value
 
+    def _unknown_handler(self, node):
+        # this environment variable configures whether to error on unknown
+        # ast types.
+        #
+        # this is silent by default but the error is enabled for the pyflakes
+        # testsuite.
+        #
+        # this allows new syntax to be added to python without *requiring*
+        # changes from the pyflakes side.  but will still produce an error
+        # in the pyflakes testsuite (so more specific handling can be added if
+        # needed).
+        if os.environ.get('PYFLAKES_ERROR_UNKNOWN'):
+            raise NotImplementedError('Unexpected type: {}'.format(type(node)))
+        else:
+            self.handleChildren(node)
+
     def getNodeHandler(self, node_class):
         try:
             return self._nodeHandlers[node_class]
         except KeyError:
             nodeType = getNodeType(node_class)
-        self._nodeHandlers[node_class] = handler = getattr(self, nodeType)
+        self._nodeHandlers[node_class] = handler = getattr(
+            self, nodeType, self._unknown_handler,
+        )
         return handler
 
     def handleNodeLoad(self, node):
@@ -1281,6 +1318,7 @@ class Checker(object):
         self.popScope()
         self.scopeStack = saved_stack
 
+    @in_annotation
     def handleStringAnnotation(self, s, node, ref_lineno, ref_col_offset, err):
         try:
             tree = ast.parse(s)
@@ -1304,6 +1342,7 @@ class Checker(object):
 
         self.handleNode(parsed_annotation, node)
 
+    @in_annotation
     def handleAnnotation(self, annotation, node):
         if isinstance(annotation, ast.Str):
             # Defer handling forward annotation.
@@ -1316,7 +1355,8 @@ class Checker(object):
                 messages.ForwardAnnotationSyntaxError,
             ))
         elif self.annotationsFutureEnabled:
-            self.deferFunction(lambda: self.handleNode(annotation, node))
+            fn = in_annotation(Checker.handleNode)
+            self.deferFunction(lambda: fn(self, annotation, node))
         else:
             self.handleNode(annotation, node)
 
@@ -1332,9 +1372,19 @@ class Checker(object):
 
     # "expr" type nodes
     BOOLOP = UNARYOP = IFEXP = SET = \
-        REPR = ATTRIBUTE = SUBSCRIPT = \
+        REPR = ATTRIBUTE = \
         STARRED = NAMECONSTANT = NAMEDEXPR = handleChildren
 
+    def SUBSCRIPT(self, node):
+        if _is_typing(node.value, 'Literal', self.scopeStack):
+            orig, self._in_typing_literal = self._in_typing_literal, True
+            try:
+                self.handleChildren(node)
+            finally:
+                self._in_typing_literal = orig
+        else:
+            self.handleChildren(node)
+
     def _handle_string_dot_format(self, node):
         try:
             placeholders = tuple(parse_format_string(node.func.value.s))
@@ -1575,7 +1625,27 @@ class Checker(object):
             self._handle_percent_format(node)
         self.handleChildren(node)
 
-    NUM = STR = BYTES = ELLIPSIS = CONSTANT = ignore
+    def STR(self, node):
+        if self._in_annotation and not self._in_typing_literal:
+            fn = functools.partial(
+                self.handleStringAnnotation,
+                node.s,
+                node,
+                node.lineno,
+                node.col_offset,
+                messages.ForwardAnnotationSyntaxError,
+            )
+            if self._in_deferred:
+                fn()
+            else:
+                self.deferFunction(fn)
+
+    if PY38_PLUS:
+        def CONSTANT(self, node):
+            if isinstance(node.value, str):
+                return self.STR(node)
+    else:
+        NUM = BYTES = ELLIPSIS = CONSTANT = ignore
 
     # "slice" type nodes
     SLICE = EXTSLICE = INDEX = handleChildren
diff --git a/tox.ini b/tox.ini
index 5821483..f2256c8 100644
--- a/tox.ini
+++ b/tox.ini
@@ -5,6 +5,7 @@ envlist =
 
 [testenv]
 deps = flake8==3.6.0
+setenv = PYFLAKES_ERROR_UNKNOWN=1
 commands =
     python -m unittest discover pyflakes
     flake8 pyflakes setup.py


Test patch:
diff --git a/pyflakes/test/test_type_annotations.py b/pyflakes/test/test_type_annotations.py
index 1fa4f5e..15c658b 100644
--- a/pyflakes/test/test_type_annotations.py
+++ b/pyflakes/test/test_type_annotations.py
@@ -42,6 +42,7 @@ class TestTypeAnnotations(TestCase):
     def test_typingExtensionsOverload(self):
         """Allow intentional redefinitions via @typing_extensions.overload"""
         self.flakes("""
+        import typing_extensions
         from typing_extensions import overload
 
         @overload
@@ -54,6 +55,17 @@ class TestTypeAnnotations(TestCase):
 
         def f(s):
             return s
+
+        @typing_extensions.overload
+        def g(s):  # type: (None) -> None
+            pass
+
+        @typing_extensions.overload
+        def g(s):  # type: (int) -> int
+            pass
+
+        def g(s):
+            return s
         """)
 
     @skipIf(version_info < (3, 5), 'new in Python 3.5')
@@ -426,3 +438,64 @@ class TestTypeAnnotations(TestCase):
 
         def f(c: C, /): ...
         """)
+
+    @skipIf(version_info < (3,), 'new in Python 3')
+    def test_partially_quoted_type_annotation(self):
+        self.flakes("""
+        from queue import Queue
+        from typing import Optional
+
+        def f() -> Optional['Queue[str]']:
+            return None
+        """)
+
+    @skipIf(version_info < (3,), 'new in Python 3')
+    def test_literal_type_typing(self):
+        self.flakes("""
+        from typing import Literal
+
+        def f(x: Literal['some string']) -> None:
+            return None
+        """)
+
+    @skipIf(version_info < (3,), 'new in Python 3')
+    def test_literal_type_typing_extensions(self):
+        self.flakes("""
+        from typing_extensions import Literal
+
+        def f(x: Literal['some string']) -> None:
+            return None
+        """)
+
+    @skipIf(version_info < (3,), 'new in Python 3')
+    def test_literal_union_type_typing(self):
+        self.flakes("""
+        from typing import Literal
+
+        def f(x: Literal['some string', 'foo bar']) -> None:
+            return None
+        """)
+
+    @skipIf(version_info < (3,), 'new in Python 3')
+    def test_deferred_twice_annotation(self):
+        self.flakes("""
+            from queue import Queue
+            from typing import Optional
+
+
+            def f() -> "Optional['Queue[str]']":
+                return None
+        """)
+
+    @skipIf(version_info < (3, 7), 'new in Python 3.7')
+    def test_partial_string_annotations_with_future_annotations(self):
+        self.flakes("""
+            from __future__ import annotations
+
+            from queue import Queue
+            from typing import Optional
+
+
+            def f() -> Optional['Queue[str]']:
+                return None
+        """)