Repository: PyCQA/pyflakes (python)

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.

Pull request:
Fix quoted type annotations in unusual contexts
This aims to fix #510.

There is a case which this doesn't fix, which might need a slightly different approach -- that of a quoted annotation within a generic subscript for a user-defined type (which is inherently not part of the `typing` module).
For this reason I'm tempted to break out the subscript case from the `cast` case. Currently this would be fairly easy to do (the `cast` case is handled by the first two commits here, the latter commits relate to the subscript case).

Do we want to solve the general generics case now, or is this useful enough as it is?
~If we do want to solve this now, suggestions on how to approach it would be useful. I can see a couple of routes:~
- ~try to walk the parents of the type we find to see if this subscript should be read as an annotation (likely somewhat involved because I don't think `pyflakes` has anything approaching this at the moment?)~
- ~try to parse the subscript _both_ as an annotation and otherwise; this feels like it could result in some false results, but might be easier to get working. If this approach is preferred, do we have some examples of this sort of thing?~

Conclusion: we'll ignore this case for now.

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("""