Repository: PyCQA/pyflakes

Issue:
Spurious unsued import warning for type annotation used in quoted cast
Given
``` python
# 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.
``` shell
$ 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.
``` shell
$ 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.

Discussion:
PeterJCLaw: @asottile thanks for the review. I can fix the issues you raised, though it would be good to get input on the approach for the subscript case as I think that's going to determine the best way to slice these changes between PRs -- do you have any suggestions on that?
asottile: I *think* the only case that won't be caught for user-defined subscripted types is a subscript-at-typing-time-only type inside a user-defined subscripted type in a type alias -- and I can't really think of a heuristic to notice that :/

```python
class C(Generic[T]): ...

MyAlias = C['Queue[int]']
```

it's probably fine to ignore that edge case for now and we can circle back on it later, most of the common cases are caught because you can't have an alias that's purely a string (and commonly you'll be aliasing to some `typing`-module thing) and things inside variable annotations / function parameter annotations / return value annotations are well-known to be in a typing context
PeterJCLaw: I realise I've not broken out the refactors yet; happy to do that once we're happy with the approach.
PeterJCLaw: Rebased to squash together the refactors and separate them better from functional changes. Also took the opportunity to remove some of the dead changes (e.g: `ExitStack`).
PeterJCLaw: Test case I've just thought of that I should add:
```
Func = Callable[
    ['Queue[int]', ...],
    None,
]
```
I _think_ the current implementation will catch this, but would be good to be sure.
jnsnow: Hi, testing this on my local code fixes all the places where I had been seeing issues :)

tyvm!

Patch:
diff --git a/pyflakes/api.py b/pyflakes/api.py
index 50e8ad8..ec3ef5a 100644
--- a/pyflakes/api.py
+++ b/pyflakes/api.py
@@ -193,14 +193,18 @@ def _get_version():
 
 def main(prog=None, args=None):
     """Entry point for the script "pyflakes"."""
-    import optparse
+    import argparse
 
     # Handle "Keyboard Interrupt" and "Broken pipe" gracefully
     _exitOnSignal('SIGINT', '... stopped')
     _exitOnSignal('SIGPIPE', 1)
 
-    parser = optparse.OptionParser(prog=prog, version=_get_version())
-    (__, args) = parser.parse_args(args=args)
+    parser = argparse.ArgumentParser(prog=prog,
+                                     description='Check Python source files for errors')
+    parser.add_argument('-V', '--version', action='version', version=_get_version())
+    parser.add_argument('path', nargs='*',
+                        help='Path(s) of Python file(s) to check. STDIN if not given.')
+    args = parser.parse_args(args=args).path
     reporter = modReporter._makeDefaultReporter()
     if args:
         warnings = checkRecursive(args, reporter)
diff --git a/pyflakes/checker.py b/pyflakes/checker.py
index 351ab12..c239950 100644
--- a/pyflakes/checker.py
+++ b/pyflakes/checker.py
@@ -8,6 +8,7 @@ import __future__
 import ast
 import bisect
 import collections
+import contextlib
 import doctest
 import functools
 import os
@@ -663,17 +664,26 @@ def getNodeName(node):
         return node.name
 
 
-def _is_typing(node, typing_attr, scope_stack):
+TYPING_MODULES = frozenset(('typing', 'typing_extensions'))
+
+
+def _is_typing_helper(node, is_name_match_fn, scope_stack):
+    """
+    Internal helper to determine whether or not something is a member of a
+    typing module. This is used as part of working out whether we are within a
+    type annotation context.
+
+    Note: you probably don't want to use this function directly. Instead see the
+    utils below which wrap it (`_is_typing` and `_is_any_typing_member`).
+    """
+
     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 expected_typing_names
+                    scope[name].module in TYPING_MODULES and
+                    is_name_match_fn(scope[name].real_name)
                 )
 
         return False
@@ -685,12 +695,33 @@ def _is_typing(node, typing_attr, scope_stack):
         ) or (
             isinstance(node, ast.Attribute) and
             isinstance(node.value, ast.Name) and
-            node.value.id in {'typing', 'typing_extensions'} and
-            node.attr == typing_attr
+            node.value.id in TYPING_MODULES and
+            is_name_match_fn(node.attr)
         )
     )
 
 
+def _is_typing(node, typing_attr, scope_stack):
+    """
+    Determine whether `node` represents the member of a typing module specified
+    by `typing_attr`.
+
+    This is used as part of working out whether we are within a type annotation
+    context.
+    """
+    return _is_typing_helper(node, lambda x: x == typing_attr, scope_stack)
+
+
+def _is_any_typing_member(node, scope_stack):
+    """
+    Determine whether `node` represents any member of a typing module.
+
+    This is used as part of working out whether we are within a type annotation
+    context.
+    """
+    return _is_typing_helper(node, lambda x: True, scope_stack)
+
+
 def is_typing_overload(value, scope_stack):
     return (
         isinstance(value.source, FUNCTION_TYPES) and
@@ -704,11 +735,8 @@ def is_typing_overload(value, scope_stack):
 def in_annotation(func):
     @functools.wraps(func)
     def in_annotation_func(self, *args, **kwargs):
-        orig, self._in_annotation = self._in_annotation, True
-        try:
+        with self._enter_annotation():
             return func(self, *args, **kwargs)
-        finally:
-            self._in_annotation = orig
     return in_annotation_func
 
 
@@ -793,7 +821,7 @@ class Checker(object):
         ast.DictComp: GeneratorScope,
     }
     if PY35_PLUS:
-        _ast_node_scope[ast.AsyncFunctionDef] = FunctionScope,
+        _ast_node_scope[ast.AsyncFunctionDef] = FunctionScope
 
     nodeDepth = 0
     offset = None
@@ -1236,6 +1264,14 @@ class Checker(object):
             except KeyError:
                 self.report(messages.UndefinedName, node, name)
 
+    @contextlib.contextmanager
+    def _enter_annotation(self):
+        orig, self._in_annotation = self._in_annotation, True
+        try:
+            yield
+        finally:
+            self._in_annotation = orig
+
     def _handle_type_comments(self, node):
         for (lineno, col_offset), comment in self._type_comments.get(node, ()):
             comment = comment.split(':', 1)[1].strip()
@@ -1409,14 +1445,14 @@ class Checker(object):
         pass
 
     # "stmt" type nodes
-    DELETE = PRINT = FOR = ASYNCFOR = WHILE = IF = WITH = WITHITEM = \
+    DELETE = PRINT = FOR = ASYNCFOR = WHILE = WITH = WITHITEM = \
         ASYNCWITH = ASYNCWITHITEM = TRYFINALLY = EXEC = \
         EXPR = ASSIGN = handleChildren
 
     PASS = ignore
 
     # "expr" type nodes
-    BOOLOP = UNARYOP = IFEXP = SET = \
+    BOOLOP = UNARYOP = SET = \
         REPR = ATTRIBUTE = \
         STARRED = NAMECONSTANT = NAMEDEXPR = handleChildren
 
@@ -1428,7 +1464,11 @@ class Checker(object):
             finally:
                 self._in_typing_literal = orig
         else:
-            self.handleChildren(node)
+            if _is_any_typing_member(node.value, self.scopeStack):
+                with self._enter_annotation():
+                    self.handleChildren(node)
+            else:
+                self.handleChildren(node)
 
     def _handle_string_dot_format(self, node):
         try:
@@ -1557,6 +1597,15 @@ class Checker(object):
                 node.func.attr == 'format'
         ):
             self._handle_string_dot_format(node)
+
+        if (
+            _is_typing(node.func, 'cast', self.scopeStack) and
+            len(node.args) >= 1 and
+            isinstance(node.args[0], ast.Str)
+        ):
+            with self._enter_annotation():
+                self.handleNode(node.args[0], node)
+
         self.handleChildren(node)
 
     def _handle_percent_format(self, node):
@@ -1773,6 +1822,13 @@ class Checker(object):
                         )
         self.handleChildren(node)
 
+    def IF(self, node):
+        if isinstance(node.test, ast.Tuple) and node.test.elts != []:
+            self.report(messages.IfTuple, node)
+        self.handleChildren(node)
+
+    IFEXP = IF
+
     def ASSERT(self, node):
         if isinstance(node.test, ast.Tuple) and node.test.elts != []:
             self.report(messages.AssertTuple, node)
diff --git a/pyflakes/messages.py b/pyflakes/messages.py
index e93a493..1bb4ab0 100644
--- a/pyflakes/messages.py
+++ b/pyflakes/messages.py
@@ -233,9 +233,16 @@ class TooManyExpressionsInStarredAssignment(Message):
     message = 'too many expressions in star-unpacking assignment'
 
 
+class IfTuple(Message):
+    """
+    Conditional test is a non-empty tuple literal, which are always True.
+    """
+    message = '\'if tuple literal\' is always true, perhaps remove accidental comma?'
+
+
 class AssertTuple(Message):
     """
-    Assertion test is a tuple, which are always True.
+    Assertion test is a non-empty tuple literal, which are always True.
     """
     message = 'assertion is always true, perhaps remove parentheses?'
 


Test patch:
diff --git a/pyflakes/test/test_code_segment.py b/pyflakes/test/test_code_segment.py
index 5e53f8f..131a74d 100644
--- a/pyflakes/test/test_code_segment.py
+++ b/pyflakes/test/test_code_segment.py
@@ -1,7 +1,9 @@
+from sys import version_info
+
 from pyflakes import messages as m
 from pyflakes.checker import (FunctionScope, ClassScope, ModuleScope,
                               Argument, FunctionDefinition, Assignment)
-from pyflakes.test.harness import TestCase
+from pyflakes.test.harness import TestCase, skipIf
 
 
 class TestCodeSegments(TestCase):
@@ -124,3 +126,7 @@ class TestCodeSegments(TestCase):
         self.assertIsInstance(function_scope_bar['g'], Argument)
         self.assertIsInstance(function_scope_bar['h'], Argument)
         self.assertIsInstance(function_scope_bar['i'], Argument)
+
+    @skipIf(version_info < (3, 5), 'new in Python 3.5')
+    def test_scope_async_function(self):
+        self.flakes('async def foo(): pass', is_segment=True)
diff --git a/pyflakes/test/test_other.py b/pyflakes/test/test_other.py
index 282accb..7a02468 100644
--- a/pyflakes/test/test_other.py
+++ b/pyflakes/test/test_other.py
@@ -1433,6 +1433,29 @@ class TestUnusedAssignment(TestCase):
         self.flakes("a = foo if True else 'oink'", m.UndefinedName)
         self.flakes("a = 'moo' if True else bar", m.UndefinedName)
 
+    def test_if_tuple(self):
+        """
+        Test C{if (foo,)} conditions.
+        """
+        self.flakes("""if (): pass""")
+        self.flakes("""
+        if (
+            True
+        ):
+            pass
+        """)
+        self.flakes("""
+        if (
+            True,
+        ):
+            pass
+        """, m.IfTuple)
+        self.flakes("""
+        x = 1 if (
+            True,
+        ) else 2
+        """, m.IfTuple)
+
     def test_withStatementNoNames(self):
         """
         No warnings are emitted for using inside or after a nameless C{with}
diff --git a/pyflakes/test/test_type_annotations.py b/pyflakes/test/test_type_annotations.py
index 15c658b..4804dda 100644
--- a/pyflakes/test/test_type_annotations.py
+++ b/pyflakes/test/test_type_annotations.py
@@ -449,6 +449,46 @@ class TestTypeAnnotations(TestCase):
             return None
         """)
 
+    def test_partially_quoted_type_assignment(self):
+        self.flakes("""
+        from queue import Queue
+        from typing import Optional
+
+        MaybeQueue = Optional['Queue[str]']
+        """)
+
+    def test_nested_partially_quoted_type_assignment(self):
+        self.flakes("""
+        from queue import Queue
+        from typing import Callable
+
+        Func = Callable[['Queue[str]'], None]
+        """)
+
+    def test_quoted_type_cast(self):
+        self.flakes("""
+        from typing import cast, Optional
+
+        maybe_int = cast('Optional[int]', 42)
+        """)
+
+    def test_type_cast_literal_str_to_str(self):
+        # Checks that our handling of quoted type annotations in the first
+        # argument to `cast` doesn't cause issues when (only) the _second_
+        # argument is a literal str which looks a bit like a type annoation.
+        self.flakes("""
+        from typing import cast
+
+        a_string = cast(str, 'Optional[int]')
+        """)
+
+    def test_quoted_type_cast_renamed_import(self):
+        self.flakes("""
+        from typing import cast as tsac, Optional as Maybe
+
+        maybe_int = tsac('Maybe[int]', 42)
+        """)
+
     @skipIf(version_info < (3,), 'new in Python 3')
     def test_literal_type_typing(self):
         self.flakes("""