Skip to content

Flatten handlers #70

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 58 additions & 11 deletions pyflakes/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the [:] do here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 @@ -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:
Expand All @@ -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__)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've moved this outside of the try. I suspect it was in there for a reason.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will need to do better than suspect. :P


self.nodeDepth += 1
try:
handler(node)
finally:
self.nodeDepth -= 1
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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

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 @@ -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.
Expand Down Expand Up @@ -1184,6 +1230,11 @@ def a():
b = 1
return locals()
''')
self.flakes('''
def a():
b = 1
return '{b}' % locals()
''')

def test_unusedVariableNoLocals(self):
"""
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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
''')

Expand Down Expand Up @@ -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')
Expand All @@ -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()
''')

Expand Down Expand Up @@ -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)