Support most Python 2 syntax back to 2.3

[thatch]

trafficstars

Summary

This now parses a good amount of real-world setup.py code, and I've validated back to 2.3 (the earliest version I have handy). It passes most of python-grammar-changes with only a few remaining failures:

                                                  23 24 25 26 27 30 31 32 33 34 35 36 37 38
examples/py27-0024-as-keyword.py                  o. o. o  .. .. .. .. .  .. .  .. .. .. .. 
examples/py30-0004-unpacking-args1.py             o. o. o  o. o. .. .. .  .. .  .. .. .. .. 
examples/py30-0013-kwargs-after-star.py           .o .o .  oo oo oo oo o  oo o  oo oo oo oo 
examples/py30-0018-except-as.py                   .o .o .  oo oo oo oo o  oo o  oo oo oo oo 
examples/py30-0023-relative-imports1.py           .o .o o  oo oo oo oo o  oo o  oo oo oo oo 
examples/py30-0023-relative-imports2.py           .o .o o  oo oo oo oo o  oo o  oo oo oo oo 

Both relative-imports and except-as are important. unpacking-args is a feature I haven't seen used since about 2004. The other two are for additional strictness to reject bad code, but are not necessary for my immediate needs.

Integer validation is reasonably strict at parse time, and handles both the octal changes and L suffix change.

                                                  23 24 25 26 27 30 31 32 33 34 35 36 37 38
examples/py30-0030-int-literals1.py               oo oo o  oo oo .. .. .  .. .  .. .. .. .. 
examples/py30-0030-int-literals2.py               oo oo o  oo oo .. .. .  .. .  .. .. .. .. 
examples/py30-0030-int-literals3.py               oo oo o  oo oo .. .. .  .. .  .. .. .. .. 
examples/py30-0030-int-literals4.py               .. .. .  oo oo oo oo o  oo o  oo oo oo oo 
examples/py30-0030-int-literals5.py               .. .. .  .. .. .. .. .  .. .  .. oo oo oo 
Legend:
 green header means will test with libcst

 first result is python, second [optional] result is libcst
 o   parses
 .   does not parse

New classes have Py2-prefixed names, to make it a conscious decision for people so they don't accidentally using them when constructing trees by hand.

Test Plan

$ ../libcst-venv/bin/pyre
 ƛ No type errors found
$ tox -e py36
...
py36 run-test: commands[0] | python -m unittest
...
Ran 1687 tests in 85.662s

OK (skipped=3, expected failures=2)

This is relevant to #184

[codecov-io]

Codecov Report

Merging #275 (3452f97) into master (31bae01) will decrease coverage by 0.48%. The diff coverage is 74.08%.

@@            Coverage Diff             @@
##           master     #275      +/-   ##
==========================================
- Coverage   94.30%   93.81%   -0.49%     
==========================================
  Files         232      233       +1     
  Lines       22442    23053     +611     
==========================================
+ Hits        21163    21627     +464     
- Misses       1279     1426     +147     

Impacted Files	Coverage Δ
libcst/__init__.py	100.00% <ø> (ø)
libcst/_nodes/tests/test_atom.py	100.00% <ø> (ø)
libcst/_parser/base_parser.py	86.90% <ø> (ø)
libcst/_parser/grammar.py	94.68% <ø> (+4.25%)	:arrow_up:
libcst/matchers/_return_types.py	100.00% <ø> (ø)
libcst/_parser/parso/python/tokenize.py	83.80% <50.00%> (+0.64%)	:arrow_up:
libcst/_nodes/statement.py	90.16% <52.50%> (-5.74%)	:arrow_down:
libcst/_parser/conversions/statement.py	92.38% <61.33%> (-5.52%)	:arrow_down:
libcst/_typed_visitor.py	95.73% <79.51%> (-0.98%)	:arrow_down:
libcst/_nodes/expression.py	96.54% <88.57%> (-0.21%)	:arrow_down:
... and 14 more

---

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 31bae01...3452f97. Read the comment docs.

[thatch]

For completeness, here are future import not covered in the python-grammar-changes automated tests, which are still to do:

• unicode_literals always enabled (needs to store a bit for evaluated_value to be correct on unprefixed strings, and probably some allowed escapes changes)
• absolute_imports always enabled (which may require changes in the imports codemods)
• with_statement always enabled (only affects python 2.5, this was always on in 2.6)

[carljm]

Did another more careful run-through, this looks pretty good to me. In an ideal world I'd prefer to bump the test coverage up a bit; a number of the new Py2 nodes don't really get validation or codegen exercised at all by the test suite, and overall this brings test coverage down. Seems like your change-examples repo goes a long way to giving confidence in the correctness here, but I wish we could get more of that confidence built in to LibCST's own test suite so that future changes are validated too. But I'm also not that invested in Py2, either, so not going to block on that :)

Thanks for all the hard work here!

@jimmylai do you want me to hold off on merging so you can review this too, or just go ahead?

[jimmylai]

@jimmylai do you want me to hold off on merging so you can review this too, or just go ahead?

I can review this today. Maybe just wait a bit for me.

[thatch]

I will work on increasing coverage, if you want to wait a day...

[thatch]

I'll try to rebase this over the holidays.

[thatch]

@jimmylai I think this is ready for another review. I fixed some docs and tests, and pyre is passing locally (but I don't entirely believe that). Is there an easy way to kick off a circleci run?

[jimmylai]

@jimmylai I think this is ready for another review. I fixed some docs and tests, and pyre is passing locally (but I don't entirely believe that). Is there an easy way to kick off a circleci run?

Circle CI shows all tests were passed. It automatically runs whenever you push an update to a PR. Yeah, I'll do another round of review.

[jimmylai]

Some functions are not covered by tests. Can you add some tests?

• convert_py2_raise_stmt
• convert_exec_stmt
• Py2Exec. _codegen_impl
• Py2ExecTarget. _validate

https://codecov.io/gh/Instagram/LibCST/pull/275/diff?src=pr&el=tree#diff-bGliY3N0L19faW5pdF9fLnB5

Did you try running this run on our large codebase through Fixit rules and unit tests? That can help us identify some issue from major changes like this PR. Otherwise, LTGM.

[facebook-github-bot]

Hi @thatch!

Thank you for your pull request.

We require contributors to sign our Contributor License Agreement, and yours needs attention.

You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!