Skip to content

Commit

Permalink
closes bpo-27494: Fix 2to3 handling of trailing comma after a generat…
Browse files Browse the repository at this point in the history
…or expression (python#3771)
  • Loading branch information
jstasiak authored and benjaminp committed Oct 5, 2017
1 parent a8ed117 commit af810b3
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 10 deletions.
25 changes: 22 additions & 3 deletions Lib/lib2to3/Grammar.txt
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ atom: ('(' [yield_expr|testlist_gexp] ')' |
'{' [dictsetmaker] '}' |
'`' testlist1 '`' |
NAME | NUMBER | STRING+ | '.' '.' '.')
listmaker: (test|star_expr) ( comp_for | (',' (test|star_expr))* [','] )
testlist_gexp: (test|star_expr) ( comp_for | (',' (test|star_expr))* [','] )
listmaker: (test|star_expr) ( old_comp_for | (',' (test|star_expr))* [','] )
testlist_gexp: (test|star_expr) ( old_comp_for | (',' (test|star_expr))* [','] )
lambdef: 'lambda' [varargslist] ':' test
trailer: '(' [arglist] ')' | '[' subscriptlist ']' | '.' NAME
subscriptlist: subscript (',' subscript)* [',']
Expand Down Expand Up @@ -161,9 +161,28 @@ argument: ( test [comp_for] |
star_expr )

comp_iter: comp_for | comp_if
comp_for: [ASYNC] 'for' exprlist 'in' testlist_safe [comp_iter]
comp_for: [ASYNC] 'for' exprlist 'in' or_test [comp_iter]
comp_if: 'if' old_test [comp_iter]

# As noted above, testlist_safe extends the syntax allowed in list
# comprehensions and generators. We can't use it indiscriminately in all
# derivations using a comp_for-like pattern because the testlist_safe derivation
# contains comma which clashes with trailing comma in arglist.
#
# This was an issue because the parser would not follow the correct derivation
# when parsing syntactically valid Python code. Since testlist_safe was created
# specifically to handle list comprehensions and generator expressions enclosed
# with parentheses, it's safe to only use it in those. That avoids the issue; we
# can parse code like set(x for x in [],).
#
# The syntax supported by this set of rules is not a valid Python 3 syntax,
# hence the prefix "old".
#
# See https://bugs.python.org/issue27494
old_comp_iter: old_comp_for | old_comp_if
old_comp_for: [ASYNC] 'for' exprlist 'in' testlist_safe [old_comp_iter]
old_comp_if: 'if' old_test [old_comp_iter]

testlist1: test (',' test)*

# not used in grammar, but may appear in "node" passed from Parser to Compiler
Expand Down
6 changes: 3 additions & 3 deletions Lib/lib2to3/fixer_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ def ListComp(xp, fp, it, test=None):
test.prefix = " "
if_leaf = Leaf(token.NAME, "if")
if_leaf.prefix = " "
inner_args.append(Node(syms.comp_if, [if_leaf, test]))
inner = Node(syms.listmaker, [xp, Node(syms.comp_for, inner_args)])
inner_args.append(Node(syms.old_comp_if, [if_leaf, test]))
inner = Node(syms.listmaker, [xp, Node(syms.old_comp_for, inner_args)])
return Node(syms.atom,
[Leaf(token.LBRACE, "["),
inner,
Expand Down Expand Up @@ -208,7 +208,7 @@ def attr_chain(obj, attr):
next = getattr(next, attr)

p0 = """for_stmt< 'for' any 'in' node=any ':' any* >
| comp_for< 'for' any 'in' node=any any* >
| old_comp_for< 'for' any 'in' node=any any* >
"""
p1 = """
power<
Expand Down
2 changes: 1 addition & 1 deletion Lib/lib2to3/fixes/fix_dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def transform(self, node, results):
p1 = patcomp.compile_pattern(P1)

P2 = """for_stmt< 'for' any 'in' node=any ':' any* >
| comp_for< 'for' any 'in' node=any any* >
| old_comp_for< 'for' any 'in' node=any any* >
"""
p2 = patcomp.compile_pattern(P2)

Expand Down
4 changes: 2 additions & 2 deletions Lib/lib2to3/fixes/fix_paren.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class FixParen(fixer_base.BaseFix):
PATTERN = """
atom< ('[' | '(')
(listmaker< any
comp_for<
old_comp_for<
'for' NAME 'in'
target=testlist_safe< any (',' any)+ [',']
>
Expand All @@ -24,7 +24,7 @@ class FixParen(fixer_base.BaseFix):
>
|
testlist_gexp< any
comp_for<
old_comp_for<
'for' NAME 'in'
target=testlist_safe< any (',' any)+ [',']
>
Expand Down
2 changes: 1 addition & 1 deletion Lib/lib2to3/fixes/fix_xrange.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def transform_range(self, node, results):
p1 = patcomp.compile_pattern(P1)

P2 = """for_stmt< 'for' any 'in' node=any ':' any* >
| comp_for< 'for' any 'in' node=any any* >
| old_comp_for< 'for' any 'in' node=any any* >
| comparison< any 'in' node=any any*>
"""
p2 = patcomp.compile_pattern(P2)
Expand Down
7 changes: 7 additions & 0 deletions Lib/lib2to3/tests/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,13 @@ def test_multiline_str_literals(self):
self.validate(s)


class TestGeneratorExpressions(GrammarTest):

def test_trailing_comma_after_generator_expression_argument_works(self):
# BPO issue 27494
self.validate("set(x for x in [],)")


def diff(fn, result):
try:
with open('@', 'w') as f:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Make 2to3 accept a trailing comma in generator expressions. For example, ``set(x
for x in [],)`` is now allowed.

0 comments on commit af810b3

Please sign in to comment.