Skip to content

Commit

Permalink
Flatten handlers
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jayvdb committed May 7, 2016
1 parent 29914fc commit ea032f4
Show file tree
Hide file tree
Showing 2 changed files with 136 additions and 11 deletions.
69 changes: 58 additions & 11 deletions pyflakes/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -734,9 +734,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]:
Expand Down Expand Up @@ -766,7 +799,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:
Expand All @@ -777,11 +815,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
Expand Down Expand Up @@ -833,21 +878,22 @@ def ignore(self, node):
pass

# "stmt" type nodes
DELETE = PRINT = FOR = ASYNCFOR = WHILE = IF = WITH = WITHITEM = \
ASYNCWITH = ASYNCWITHITEM = RAISE = TRYFINALLY = EXEC = \
EXPR = ASSIGN = handleChildren
DELETE = PRINT = EXEC = EXPR = RAISE = handleChildrenFlattened
ASSIGN = TRYFINALLY = handleChildren
FOR = ASYNCFOR = WHILE = IF = WITH = ASYNCWITH = handleChildren
WITHITEM = ASYNCWITHITEM = handleChildrenFlattened

PASS = ignore

# "expr" type nodes
BOOLOP = BINOP = UNARYOP = IFEXP = DICT = 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
Expand All @@ -859,7 +905,8 @@ def ignore(self, node):
MATMULT = ignore

# additional node types
COMPREHENSION = KEYWORD = FORMATTEDVALUE = handleChildren
COMPREHENSION = handleChildren
KEYWORD = FORMATTEDVALUE = handleChildrenFlattened

def ASSERT(self, node):
if isinstance(node.test, ast.Tuple) and node.test.elts != []:
Expand Down Expand Up @@ -903,7 +950,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

Expand Down
78 changes: 78 additions & 0 deletions pyflakes/test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
Tests for various Pyflakes behavior.
"""

import sys

from sys import version_info

from pyflakes import messages as m
Expand Down Expand Up @@ -1084,6 +1086,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.
Expand Down Expand Up @@ -1168,6 +1214,11 @@ def a():
b = 1
return locals()
''')
self.flakes('''
def a():
b = 1
return '{b}' % locals()
''')

def test_unusedVariableNoLocals(self):
"""
Expand Down Expand Up @@ -1374,6 +1425,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}
Expand Down Expand Up @@ -1715,7 +1773,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
''')

Expand All @@ -1725,6 +1785,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')
Expand All @@ -1733,7 +1795,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()
''')

Expand All @@ -1743,3 +1807,17 @@ def test_matmul(self):
def foo(a, b):
return a @ b
''')


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)

0 comments on commit ea032f4

Please sign in to comment.