bpo-27494: Fix a 2to3 parser bug (trailing comma)

[jstasiak]

While this is a valid Python 2 and Python 3 syntax lib2to3 would choke
on it:

    set(x for x in [],)

This patch changes the grammar definition used by lib2to3 so that the
actual Python syntax is supported now and backwards compatibility is
preserved.

https://bugs.python.org/issue27494

[codecov]

Codecov Report

Merging #60 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #60      +/-   ##
==========================================
+ Coverage   82.37%   82.37%   +<.01%     
==========================================
  Files        1427     1427              
  Lines      350948   350951       +3     
==========================================
+ Hits       289091   289099       +8     
+ Misses      61857    61852       -5

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 649a7ca...d7f230d. Read the comment docs.

[stuarteberg]

I'm not a core dev, but I took some time to review this anyway. (I recently familiarized myself with lib2to3 in order to make a different PR -- reviews welcome.)

Background:

This PR changes the lib2to3 grammar's handling of for syntax, to distinguish between the following two cases:

1. list comprehensions and generator expressions, e.g.

   • [x for x in foo]
   • (x for x in foo)

2. set and dictionary comprehensions, e.g.

   • {x for x in foo}
   • {x : y for x,y in zip(foo,bar)}

Previously, the grammar used the same node type for those two cases. Unfortunately, that node specification is overly complicated due to backwards compatibility considerations. As a consequence, the grammar failed to handle some valid programs.

Review:

LGTM. The problematic case is indeed fixed, and now covered by the test suite. The three "fixers" that are affected by this Grammar change have been updated. (I see no others that need editing.)

[stuarteberg]

I agree with the need for a verbose comment here, since it's not immediately obvious why old_comp_iter, etc. are necessary. IMHO, you should remove overly cautious words like "seems" and "I believe". Also, I think it might be worth adding a reference to this PR and/or bpo-27494 in the comment itself.

[jstasiak]

Thanks for the feedback, it makes sense; let me improve this a little.

[jstasiak]

Yes, just to be precise:

This PR changes the lib2to3 grammar's handling of for syntax, to distinguish between the following two cases:

1. list comprehensions and generator expressions, e.g. (...)
2. set and dictionary comprehensions, e.g (...)

The second group would be set/dictionary comprehensions and call arguments (like print(x for x in [],))

[jstasiak]

cc @benjaminp @serhiy-storchaka @vadmium @gpshead @1st1 (I looked up who changed lib/lib2to3 recently, apologies for nagging)

I'm happy to make any changes necessary to get this merged, I just need to know what's required.

[jstasiak]

@ambv is this something you could review? I'm asking you because I see you're an author of another 2to3 patch (#1724)

[jstasiak]

Closed by accident, apologies.

[jstasiak]

OK, I'm lost regarding Travis build failures. It seems that the configuration that Travis uses to build the docs (https://travis-ci.org/python/cpython/jobs/279869243/config) is different than in builds of some more recent PR-s (https://travis-ci.org/python/cpython/jobs/279831444/config for example).

I suspect for whatever reason it's not picking up updates to .travis.yml (made since I created my branch even though I pushed them to the branch now) and that makes it fail.

One thing I suspect may work around this is closing this PR and opening a new one...

[jstasiak]

I gave up on trying to make this pull request work with Travis and created another one: #3771. The new one has been actually built successfully now!