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 change from always being the
ast depth, which varied based on 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 e3b323c
Show file tree
Hide file tree
Showing 2 changed files with 150 additions and 11 deletions.
83 changes: 72 additions & 11 deletions pyflakes/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -734,9 +734,53 @@ 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, node, omit, _fields_order=_FieldsOrder()):
"""
Yield child nodes of *node* and their children, with a recurse flag.
The value yielded is a tuple of the node, its parent and a boolean flag
to indicate whether the node needs to be recursed by the caller because
all child nodes can not be processed as a flat list.
e.g. flag is True for lambda and comprehension nodes,
as their children must be handled by the appropriate node handler.
"""
nodes = [(node, None)]
while nodes:
new_nodes = []
for node, parent in nodes:
yield_children = True
if parent:
handler = self.getNodeHandler(node.__class__)
if handler in (self.handleChildren,
self.handleChildrenFlattened,
self.handleNode):
handler = False
else:
yield_children = False
yield node, parent, handler

if yield_children:
new_nodes += list(
(child, node)
for child in iter_child_nodes(node,
omit,
_fields_order))

nodes[:] = new_nodes

def handleChildrenFlattened(self, tree, omit=None):
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 +810,7 @@ def getDocstring(self, node):

return (node.s, doctest_lineno)

def handleNode(self, node, parent):
def handleNode(self, node, parent, handler=None):
if node is None:
return
if self.offset and getattr(node, 'lineno', None) is not None:
Expand All @@ -777,11 +821,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 +884,30 @@ def ignore(self, node):
pass

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

# many of these handleChildrenFlattened are wrong, and need tests
# Anywhere a function/class can be defined probably caueses a failure,
# due to the delayed processing.
DELETE = PRINT = handleChildrenFlattened
FOR = ASYNCFOR = handleChildren
WHILE = IF = WITH = ASYNCWITH = handleChildren
WITHITEM = ASYNCWITHITEM = handleChildrenFlattened
RAISE = handleChildrenFlattened
TRYFINALLY = handleChildren
EXEC = EXPR = handleChildrenFlattened
ASSIGN = handleChildren

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 +919,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 +964,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 @@ -1565,6 +1616,13 @@ def test_withStatementUndefinedInExpression(self):
pass
''', m.UndefinedName)

def test_with_statement_order(self):
self.flakes('''
with open('foo'):
baz = 1
assert baz
''')

@skipIf(version_info < (2, 7), "Python >= 2.7 only")
def test_dictComprehension(self):
"""
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 e3b323c

Please sign in to comment.