-
Notifications
You must be signed in to change notification settings - Fork 193
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
Support Python 3 syntax back to 3.0 #261
Conversation
This is the first half of #184 |
@jimmylai I could use another pair of eyes on the failing pyre test; I do not see what's different about |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but I'll let @jimmylai look it over as well - I'm not very familiar with the parser details here
@@ -33,8 +33,8 @@ A Concrete Syntax Tree (CST) parser and serializer library for Python | |||
|
|||
.. intro-start | |||
|
|||
LibCST parses Python 3.5, 3.6, 3.7 or 3.8 source code as a CST tree that keeps all | |||
formatting details (comments, whitespaces, parentheses, etc). It's useful for | |||
LibCST parses Python 3.0, 3.1, 3.3, 3.5, 3.6, 3.7 or 3.8 source code as a CST tree that keeps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I would reword this to say
LibCST parses Python 3.0, 3.1, 3.3, 3.5, 3.6, 3.7 or 3.8 source code as a CST tree that keeps | |
LibCST parses source code compatible with all Python versions between 3.0 and 3.8 (inclusive) as a CST tree that keeps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python 3.2 and 3.4 are not mentioned here, assuming they don't have new syntax change?
If that's the case, we can also add those versions as supported version in KNOWN_PYTHON_VERSION_STRINGS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jimmylai there were no grammar changes in 3.2 or 3.4, so I didn't include them (they would be duplicates of the 3.1 and 3.3 grammar if allowed). My intent with KNOWN_PYTHON_VERSION_STRINGS is that you could do
for v in reversed(KNOWN_PYTHON_VERSION_STRINGS):
try:
mod = parse(...python_version=v)
return mod
except ParseError:
pass
to see if it parses on any. I'm happy to add some helpers here (e.g. if you have a /usr/bin/pythonx.y
and want to pick an ideal grammar version).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use pyre-strict
in every file including test files. So each parameter need to have type annotated.
Many of the added test functions miss no type annotations.
Here are pyre errors on my dev machine. Those need to be fixed.
libcst/_nodes/tests/test_atom.py:1040:28 Missing parameter annotation [2]: Parameter `code` has no type specified.
libcst/_nodes/tests/test_atom.py:1040:34 Missing parameter annotation [2]: Parameter `parser` has no type specified.
libcst/_nodes/tests/test_atom.py:1040:42 Missing parameter annotation [2]: Parameter `expect_success` has no type specified.
libcst/_nodes/tests/test_dict.py:190:28 Missing parameter annotation [2]: Parameter `code` has no type specified.
libcst/_nodes/tests/test_dict.py:190:34 Missing parameter annotation [2]: Parameter `parser` has no type specified.
libcst/_nodes/tests/test_dict.py:190:42 Missing parameter annotation [2]: Parameter `expect_success` has no type specified.
libcst/_nodes/tests/test_funcdef.py:2007:28 Missing parameter annotation [2]: Parameter `code` has no type specified.
libcst/_nodes/tests/test_funcdef.py:2007:34 Missing parameter annotation [2]: Parameter `parser` has no type specified.
libcst/_nodes/tests/test_funcdef.py:2007:42 Missing parameter annotation [2]: Parameter `expect_success` has no type specified.
libcst/_nodes/tests/test_list.py:129:28 Missing parameter annotation [2]: Parameter `code` has no type specified.
libcst/_nodes/tests/test_list.py:129:34 Missing parameter annotation [2]: Parameter `parser` has no type specified.
libcst/_nodes/tests/test_list.py:129:42 Missing parameter annotation [2]: Parameter `expect_success` has no type specified.
libcst/_nodes/tests/test_matrix_multiply.py:72:28 Missing parameter annotation [2]: Parameter `code` has no type specified.
libcst/_nodes/tests/test_matrix_multiply.py:72:34 Missing parameter annotation [2]: Parameter `parser` has no type specified.
libcst/_nodes/tests/test_matrix_multiply.py:72:42 Missing parameter annotation [2]: Parameter `expect_success` has no type specified.
libcst/_nodes/tests/test_set.py:136:28 Missing parameter annotation [2]: Parameter `code` has no type specified.
libcst/_nodes/tests/test_set.py:136:34 Missing parameter annotation [2]: Parameter `parser` has no type specified.
libcst/_nodes/tests/test_set.py:136:42 Missing parameter annotation [2]: Parameter `expect_success` has no type specified.
libcst/_nodes/tests/test_tuple.py:282:28 Missing parameter annotation [2]: Parameter `code` has no type specified.
libcst/_nodes/tests/test_tuple.py:282:34 Missing parameter annotation [2]: Parameter `parser` has no type specified.
libcst/_nodes/tests/test_tuple.py:282:42 Missing parameter annotation [2]: Parameter `expect_success` has no type specified.
libcst/_nodes/tests/test_with.py:233:28 Missing parameter annotation [2]: Parameter `code` has no type specified.
libcst/_nodes/tests/test_with.py:233:34 Missing parameter annotation [2]: Parameter `parser` has no type specified.
libcst/_nodes/tests/test_with.py:233:42 Missing parameter annotation [2]: Parameter `expect_success` has no type specified.
libcst/_nodes/tests/test_yield.py:243:28 Missing parameter annotation [2]: Parameter `code` has no type specified.
libcst/_nodes/tests/test_yield.py:243:34 Missing parameter annotation [2]: Parameter `parser` has no type specified.
libcst/_nodes/tests/test_yield.py:243:42 Missing parameter annotation [2]: Parameter `expect_success` has no type specified.
Regarding the weird type errors about One way to fix this is to use the LibCST source of truth in the local repo. |
These changes should almost work, although CI will claim a failure and I don't think I can do anything about that. I only have two issues remaining when I run pyre on my machine:
|
I also see the type error in |
Regarding the tokenize type error. The installed tokenize source code is not typed. When Pyre reads the source code, it generates the type errors. We used a stub file to help pyre understand the types https://github.com/Instagram/LibCST/blob/master/stubs/tokenize.pyi |
Regarding the tokenize type error. python/typeshed#3839 is merged, you shouldn't need the stub anymore if you |
Looks like Pyre packs the typeshed. We can probably get your fix for free in the next release. https://github.com/facebook/pyre-check/tree/master/stubs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My CI change is merged and you can rebase to see if the Pyre errors pass.
The helper function for automatically pick a good grammar version can be useful for some use case and can leave as a follow up.
Otherwise, LGTM.
Several of the python 2 features are gated on these in addition to version (like `with_statement`), and a refactoring tool like Bowler commonly needs this information anyway.
Summary
This adds support for machine-readable supported versions, and the minor changes necessary to support 3.0, 3.1, and 3.3. This includes future detection for future Python 2 parsing, but isn't required here given the PEP 401 findings.
Machine-readable versions are at
libcst.KNOWN_PYTHON_VERSION_STRINGS
.Test Plan
New included tests, and ran against my
python-grammar-changes
project. Each entry should either have ".." or "oo" to say that the LibCST success matches that cpython version.