Repository: PyCQA/pyflakes
Pull request #499 (2020-01-01 22:48:32)

Deprecate optparse in favor of argparse
According to [the official docs](https://docs.python.org/3.8/library/optparse.html), the optparse standard library has been deprecated since Python 3.2, in favor of the argparse standard library.

This pull request includes a commit that removes usage of optparse from pyflakes and uses argparse in its place. It also adds a couple of short descriptive strings for pyflakes usage via the command-line: a very brief one-liner describing pyflakes (taken from the README) and some information about the (optional) command-line arguments. Feel free to improve them, of course.

Note: this is my first fork, my first pull request, my first real usage of GitHub. I'm also relatively new to git and Python. Let me know if anything in my code or in the way I'm collaborating/etc. should be changed. I'm always trying to improve. (And happy new year!)

sim-d: And it looks like pull requests are based off of branches, not commits... Adding my recent commit (for a separate issue--#505) to the deprecate_optparse branch seemed to update #499. Sorry if I should have created a new branch for #505! If needed (e.g. if #505 should not be supported), how would I go about "removing" that recent commit? I know I could make a new commit which essentially un-does the update for #505, but I'm not sure if that's the best practice.
(Still learning.)
asottile: ah, if you want to remove a commit you could do the following:

```bash
# make sure you're on the `deprecate_optparse` branch
git checkout deprecate_optparse
# *reset* back to the old commit
git reset --hard HEAD^  # here HEAD^ refers to the commit before the current revision, you could also explciitly use the exact hash
# then push this branch up -- note you need `--force` since you're erasing history
git push origin HEAD -f
```

in this case I think it's fine to keep the two commits together -- not sure how you'd do the second one without the first one (since it requires `argparse` features)
sim-d: Thanks! Yeah, the recent commit is certainly dependent upon the prior, though I probably should have created a new branch for it so as not to interfere with this PR. I'll leave them together for now, but would be happy to remove the recent commit if the change proposed by #505 (and now included in this PR) is not desired.

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 c8ccf56..f0b7c37 100644
--- a/pyflakes/checker.py
+++ b/pyflakes/checker.py
@@ -78,6 +78,51 @@ else:
     LOOP_TYPES = (ast.While, ast.For)
     FUNCTION_TYPES = (ast.FunctionDef,)
 
+
+if PY38_PLUS:
+    def _is_singleton(node):  # type: (ast.AST) -> bool
+        return (
+            isinstance(node, ast.Constant) and
+            isinstance(node.value, (bool, type(Ellipsis), type(None)))
+        )
+elif not PY2:
+    def _is_singleton(node):  # type: (ast.AST) -> bool
+        return isinstance(node, (ast.NameConstant, ast.Ellipsis))
+else:
+    def _is_singleton(node):  # type: (ast.AST) -> bool
+        return (
+            isinstance(node, ast.Name) and
+            node.id in {'True', 'False', 'Ellipsis', 'None'}
+        )
+
+
+def _is_tuple_constant(node):  # type: (ast.AST) -> bool
+    return (
+        isinstance(node, ast.Tuple) and
+        all(_is_constant(elt) for elt in node.elts)
+    )
+
+
+if PY38_PLUS:
+    def _is_constant(node):
+        return isinstance(node, ast.Constant) or _is_tuple_constant(node)
+else:
+    _const_tps = (ast.Str, ast.Num)
+    if not PY2:
+        _const_tps += (ast.Bytes,)
+
+    def _is_constant(node):
+        return (
+            isinstance(node, _const_tps) or
+            _is_singleton(node) or
+            _is_tuple_constant(node)
+        )
+
+
+def _is_const_non_singleton(node):  # type: (ast.AST) -> bool
+    return _is_constant(node) and not _is_singleton(node)
+
+
 # https://github.com/python/typed_ast/blob/1.4.0/ast27/Parser/tokenizer.c#L102-L104
 TYPE_COMMENT_RE = re.compile(r'^#\s*type:\s*')
 # https://github.com/python/typed_ast/blob/1.4.0/ast27/Parser/tokenizer.c#L1408-L1413
@@ -618,40 +663,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):
@@ -733,11 +793,14 @@ 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
     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 +832,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 +1073,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 +1363,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 +1387,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 +1400,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)
 
@@ -1324,17 +1409,27 @@ 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 = \
-        REPR = ATTRIBUTE = SUBSCRIPT = \
+    BOOLOP = UNARYOP = SET = \
+        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 +1670,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
@@ -1658,6 +1773,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)
@@ -1799,6 +1921,10 @@ class Checker(object):
             addArgs(node.args.args)
             defaults = node.args.defaults
         else:
+            if PY38_PLUS:
+                for arg in node.args.posonlyargs:
+                    args.append(arg.arg)
+                    annotations.append(arg.annotation)
             for arg in node.args.args + node.args.kwonlyargs:
                 args.append(arg.arg)
                 annotations.append(arg.annotation)
@@ -2050,14 +2176,14 @@ class Checker(object):
             self.handleNode(node.value, node)
 
     def COMPARE(self, node):
-        literals = (ast.Str, ast.Num)
-        if not PY2:
-            literals += (ast.Bytes,)
-
         left = node.left
         for op, right in zip(node.ops, node.comparators):
-            if (isinstance(op, (ast.Is, ast.IsNot)) and
-                    (isinstance(left, literals) or isinstance(right, literals))):
+            if (
+                    isinstance(op, (ast.Is, ast.IsNot)) and (
+                        _is_const_non_singleton(left) or
+                        _is_const_non_singleton(right)
+                    )
+            ):
                 self.report(messages.IsLiteral, node)
             left = right
 
diff --git a/pyflakes/messages.py b/pyflakes/messages.py
index cf67cf8..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?'
 
@@ -265,7 +272,7 @@ class InvalidPrintSyntax(Message):
 
 
 class IsLiteral(Message):
-    message = 'use ==/!= to compare str, bytes, and int literals'
+    message = 'use ==/!= to compare constant literals (str, bytes, int, float, tuple)'
 
 
 class FStringMissingPlaceholders(Message):
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_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_is_literal.py b/pyflakes/test/test_is_literal.py
index 9280cda..fbbb205 100644
--- a/pyflakes/test/test_is_literal.py
+++ b/pyflakes/test/test_is_literal.py
@@ -198,3 +198,25 @@ class Test(TestCase):
         if 4 < x is 'foo':
             pass
         """, IsLiteral)
+
+    def test_is_tuple_constant(self):
+        self.flakes('''\
+            x = 5
+            if x is ():
+                pass
+        ''', IsLiteral)
+
+    def test_is_tuple_constant_containing_constants(self):
+        self.flakes('''\
+            x = 5
+            if x is (1, '2', True, (1.5, ())):
+                pass
+        ''', IsLiteral)
+
+    def test_is_tuple_containing_variables_ok(self):
+        # a bit nonsensical, but does not trigger a SyntaxWarning
+        self.flakes('''\
+            x = 5
+            if x is (x,):
+                pass
+        ''')
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 289535d..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')
@@ -418,3 +430,72 @@ class TestTypeAnnotations(TestCase):
                 Y = 2
                 return Y
         """, m.UndefinedName)
+
+    @skipIf(version_info < (3, 8), 'new in Python 3.8')
+    def test_positional_only_argument_annotations(self):
+        self.flakes("""
+        from x import C
+
+        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
+        """)