Skip to content

Commit

Permalink
Fix an analyzer crash with multiple list comprehensions in py3
Browse files Browse the repository at this point in the history
The crash was caused by selecting the wrong child symbol table for the
list comprehension. Previously we were always selecting the first one in
the list of subtables. This is wrong if there is more than 1 list
comprehension, since each one gets its own subtable. This is works fine
if the list comprehension only uses symbols defined within itself or
symbols that are shared between it and the first list comprehension in
the enclosing scope, since that will resolve to the same value. The
specific error case we are fixing is when there is a list comprehension
that is not the first one that references a symbol from the enclosing
function scope that the first list comprehension does not reference.

Both the symbol tables and the ListComp node are annotated with a line
number attribute, by matching these up we can select the correct
subtable to go with our list comprehension. This is still naive, but is
better than what we had before.

fixes aws#412
  • Loading branch information
ShotgunCrocodile committed Jul 31, 2017
1 parent 69f18d7 commit bdee4eb
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 4 deletions.
9 changes: 5 additions & 4 deletions chalice/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,8 @@ def visit_GeneratorExp(self, node):

def _handle_comprehension(self, node, comprehension_type):
# type: (Union[ast.ListComp, ast.GeneratorExp], str) -> None
child_scope = self._get_matching_sub_namespace(comprehension_type)
child_scope = self._get_matching_sub_namespace(
comprehension_type, node.lineno)
if child_scope is None:
# If there's no child scope (listcomps in py2) then we can
# just analyze the node.elt node in the current scope instead
Expand All @@ -577,11 +578,11 @@ def _handle_comprehension(self, node, comprehension_type):
ParsedCode(node.elt, child_table), self._binder)
child_infer.bind_types()

def _get_matching_sub_namespace(self, name):
# type: (str) -> symtable.SymbolTable
def _get_matching_sub_namespace(self, name, lineno):
# type: (str, int) -> symtable.SymbolTable
namespaces = [
t for t in self._symbol_table.get_sub_namespaces()
if t.get_name() == name]
if t.get_name() == name and t.get_lineno() == lineno]
if not namespaces:
return
# We're making a simplification and using the genexpr subnamespace.
Expand Down
11 changes: 11 additions & 0 deletions tests/unit/test_analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,17 @@ def test_can_handle_list_expr_with_api_calls():
""") == {'dynamodb': set(['list_tables'])}


def test_can_handle_multiple_listcomps():
assert aws_calls("""\
bar_key = 'bar'
baz_key = 'baz'
items = [{'foo': 'sun', 'bar': 'moon', 'baz': 'stars'}]
foos = [t['foo'] for i in items]
bars = [t[bar_key] for t in items]
bazs = [t[baz_key] for t in items]
""") == {}


# def test_can_handle_dict_comp():
# assert aws_calls("""\
# import boto3
Expand Down

0 comments on commit bdee4eb

Please sign in to comment.