Flatten handlers

pyflakes has traditionally recursed with a handler for every
level of the ast.  The ast depth can become very large, especially
for an expression containing many binary operators.

Python has a maximum recursion limit, defaulting to a low number
like 1000, which resulted in a RuntimeError for the ast of:

    x = 1 + 2 + 3 + ... + 1001

This change avoids recursing for nodes that do not have a specific
handler.

Checker.nodeDepth and node.depth changes from always being the
ast depth, which varied between Python version due to ast differences,
to being the number of nested handlers within pyflakes.

Author: jayvdb

pyflakes/checker.py

@@ -820,9 +820,42 @@ def on_conditional_branch():
                 self.report(messages.UndefinedName, node, name)
 
     def handleChildren(self, tree, omit=None):
+        """Handle all children recursively, but may be flattened."""
         for node in iter_child_nodes(tree, omit=omit):
             self.handleNode(node, tree)
 
+    def handleChildrenNested(self, node):
+        """Handle all children recursively."""
+        self.handleChildren(node)
+
+    def _iter_flattened(self, tree, omit, _fields_order=_FieldsOrder()):
+        """
+        Yield child nodes of *node* and their children, with handler.
+
+        The value yielded is a tuple of the node, its parent and its handler.
+        The handler may be False to indicate that no handler and no recursion
+        is required as the node is part of a flattened list.
+        """
+        _may_flatten = (self.handleChildren,
+                        self.handleChildrenFlattened)
+
+        nodes = [(tree, None)]
+        for node, parent in nodes:
+            # Skip the root of the tree, which has parent None
+            handler = self.getNodeHandler(node.__class__) if parent else False
+            if handler and handler not in _may_flatten:
+                yield node, parent, handler
+            else:
+                nodes[:] += ((child, node)
+                             for child in iter_child_nodes(node,
+                                                           omit,
+                                                           _fields_order))
+
+    def handleChildrenFlattened(self, tree, omit=None):
+        """Handle all children recursively as a flat list where possible."""
+        for node, parent, handler in self._iter_flattened(tree, omit=omit):
+            self.handleNode(node, parent, handler)
+
     def isLiteralTupleUnpacking(self, node):
         if isinstance(node, ast.Assign):
             for child in node.targets + [node.value]:
@@ -852,7 +885,12 @@ def getDocstring(self, node):
 
         return (node.s, doctest_lineno)
 
-    def handleNode(self, node, parent):
+    def handleNode(self, node, parent, handler=None):
+        """
+        Handle a single node, invoking its handler, which may recurse.
+
+        If handler is None, the default handler is used.
+        """
         if node is None:
             return
         if self.offset and getattr(node, 'lineno', None) is not None:
@@ -863,11 +901,18 @@ def handleNode(self, node, parent):
         if self.futuresAllowed and not (isinstance(node, ast.ImportFrom) or
                                         self.isDocstring(node)):
             self.futuresAllowed = False
-        self.nodeDepth += 1
-        node.depth = self.nodeDepth
+
+        node.depth = self.nodeDepth + 1
         node.parent = parent
-        try:
+
+        if handler is False:
+            return
+
+        if not handler:
             handler = self.getNodeHandler(node.__class__)
+
+        self.nodeDepth += 1
+        try:
             handler(node)
         finally:
             self.nodeDepth -= 1
@@ -964,21 +1009,22 @@ def ignore(self, node):
         pass
 
     # "stmt" type nodes
-    DELETE = PRINT = FOR = ASYNCFOR = WHILE = IF = WITH = WITHITEM = \
-        ASYNCWITH = ASYNCWITHITEM = TRYFINALLY = EXEC = \
-        EXPR = ASSIGN = handleChildren
+    DELETE = PRINT = EXEC = EXPR = handleChildrenFlattened
+    ASSIGN = TRYFINALLY = handleChildren
+    FOR = ASYNCFOR = WHILE = IF = WITH = ASYNCWITH = handleChildren
+    WITHITEM = ASYNCWITHITEM = handleChildrenFlattened
 
     PASS = ignore
 
     # "expr" type nodes
     BOOLOP = BINOP = UNARYOP = IFEXP = SET = \
         COMPARE = CALL = REPR = ATTRIBUTE = SUBSCRIPT = \
-        STARRED = NAMECONSTANT = handleChildren
+        STARRED = NAMECONSTANT = handleChildrenFlattened
 
     NUM = STR = BYTES = ELLIPSIS = ignore
 
     # "slice" type nodes
-    SLICE = EXTSLICE = INDEX = handleChildren
+    SLICE = EXTSLICE = INDEX = handleChildrenFlattened
 
     # expression contexts are node instances too, though being constants
     LOAD = STORE = DEL = AUGLOAD = AUGSTORE = PARAM = ignore
@@ -1003,7 +1049,8 @@ def RAISE(self, node):
             self.report(messages.RaiseNotImplemented, node)
 
     # additional node types
-    COMPREHENSION = KEYWORD = FORMATTEDVALUE = JOINEDSTR = handleChildren
+    COMPREHENSION = handleChildren
+    KEYWORD = FORMATTEDVALUE = JOINEDSTR = handleChildrenFlattened
 
     def DICT(self, node):
         # Complain if there are duplicate keys with different values
@@ -1083,7 +1130,7 @@ def GENERATOREXP(self, node):
         self.handleChildren(node)
         self.popScope()
 
-    LISTCOMP = handleChildren if PY2 else GENERATOREXP
+    LISTCOMP = handleChildrenNested if PY2 else GENERATOREXP
 
     DICTCOMP = SETCOMP = GENERATOREXP
 

pyflakes/test/test_other.py

@@ -2,6 +2,8 @@
 Tests for various Pyflakes behavior.
 """
 
+import sys
+
 from sys import version_info
 
 from pyflakes import messages as m
@@ -1100,6 +1102,50 @@ def test_containment(self):
         x not in y
         ''')
 
+    def test_flattened(self):
+        """
+        Suppress warning when a defined name is used by a binop.
+        """
+        self.flakes('''
+        w = 5
+        x = 10
+        y = 20
+        z = w + x + y
+        ''')
+
+        self.flakes('''
+        a = 10
+        x = {}
+        y = {}
+        z = x + {a: a} + y
+        ''')
+
+    def test_flattened_with_lambda(self):
+        """
+        Suppress warning when a defined name is used in an expression
+        containing flattened and recursed nodes.
+        """
+        self.flakes('''
+        a = 10
+        b = 10
+        l = True and (lambda x: a) or (lambda x: b)
+        ''')
+        self.flakes('''
+        a = 10
+        l = []
+        l = l + (lambda x: a)
+        ''')
+
+    def test_flattened_with_comprehension(self):
+        """
+        Suppress warning when a defined name is used in an expression
+        containing flattened and recursed nodes.
+        """
+        self.flakes('''
+        l = []
+        l = l + [x for x in range(10)]
+        ''')
+
     def test_loopControl(self):
         """
         break and continue statements are supported.
@@ -1184,6 +1230,11 @@ def a():
             b = 1
             return locals()
         ''')
+        self.flakes('''
+        def a():
+            b = 1
+            return '{b}' % locals()
+        ''')
 
     def test_unusedVariableNoLocals(self):
         """
@@ -1390,6 +1441,13 @@ def test_ifexp(self):
         self.flakes("a = foo if True else 'oink'", m.UndefinedName)
         self.flakes("a = 'moo' if True else bar", m.UndefinedName)
 
+    def test_withStatement(self):
+        self.flakes('''
+        with open('foo'):
+            baz = 1
+            assert baz
+        ''')
+
     def test_withStatementNoNames(self):
         """
         No warnings are emitted for using inside or after a nameless C{with}
@@ -1743,7 +1801,9 @@ def test_asyncFor(self):
         async def read_data(db):
             output = []
             async for row in db.cursor():
+                foo = 1
                 output.append(row)
+                assert foo
             return output
         ''')
 
@@ -1810,6 +1870,8 @@ def test_asyncWith(self):
         async def commit(session, data):
             async with session.transaction():
                 await session.update(data)
+                foo = 1
+                assert foo
         ''')
 
     @skipIf(version_info < (3, 5), 'new in Python 3.5')
@@ -1818,7 +1880,9 @@ def test_asyncWithItem(self):
         async def commit(session, data):
             async with session.transaction() as trans:
                 await trans.begin()
+                foo = 1
                 ...
+                assert foo
                 await trans.end()
         ''')
 
@@ -1993,3 +2057,17 @@ def test_raise_notimplemented(self):
         self.flakes('''
         raise NotImplemented
         ''', m.RaiseNotImplemented)
+
+
+class TestMaximumRecursion(TestCase):
+
+    def setUp(self):
+        self._recursionlimit = sys.getrecursionlimit()
+
+    def test_flattened(self):
+        sys.setrecursionlimit(100)
+        s = 'x = ' + ' + '.join(str(n) for n in range(100))
+        self.flakes(s)
+
+    def tearDown(self):
+        sys.setrecursionlimit(self._recursionlimit)