Repository: PyCQA/pyflakes
Pull request #517 (2020-03-02 00:08:52)

Be more cautious when identifying typing Literal
I feel that it's better to err on the side of false-negatives here than false-positives -- it's also especially unlikely that a typing-context subscript where the left hand side is "Literal" is anything other than `typing.Literal`

Originally brought up in https://github.com/PyCQA/pyflakes/pull/479#issuecomment-593150973


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..2b6a98f 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,26 +1445,38 @@ 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
 
     def SUBSCRIPT(self, node):
-        if _is_typing(node.value, 'Literal', self.scopeStack):
+        if (
+                (
+                    isinstance(node.value, ast.Name) and
+                    node.value.id == 'Literal'
+                ) or (
+                    isinstance(node.value, ast.Attribute) and
+                    node.value.attr == 'Literal'
+                )
+        ):
             orig, self._in_typing_literal = self._in_typing_literal, True
             try:
                 self.handleChildren(node)
             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 +1605,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 +1830,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..ed28127 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("""
@@ -467,6 +507,19 @@ class TestTypeAnnotations(TestCase):
             return None
         """)
 
+    @skipIf(version_info < (3,), 'new in Python 3')
+    def test_literal_type_some_other_module(self):
+        """err on the side of false-negatives for types named Literal"""
+        self.flakes("""
+        from my_module import compat
+        from my_module.compat import Literal
+
+        def f(x: compat.Literal['some string']) -> None:
+            return None
+        def g(x: Literal['some string']) -> None:
+            return None
+        """)
+
     @skipIf(version_info < (3,), 'new in Python 3')
     def test_literal_union_type_typing(self):
         self.flakes("""