Improve AST safety parsing error message

[felix-hilden]

Closes #2178, alternative to #2218: This PR continues on the work of Hasan Ramezani in the alternative PR to improve the error messages when parsing formatted AST. His original commit is till at the beginning of the branch, which I then modified (Thank you for the initial push!)

A number of non-obvious details and questions:

• As suggested by Richard in the linked issue, I removed the error messages (and a test) for Python 2 parsing
• Previously versions 3.6 and 3.7 were parsed twice, first by the builtin ast and then by typed_ast. On Discord we determined this was an accident so I removed it.
• The parsing was refactored a bit for clearer flow. It's now two functions: one for parsing a single version, and the wrapping function for constructing a list of versions to parse and then doing it. Probably easier to view the source than the diff on that one.
• Mypy linting fails, even though none of the type signatures were changed. Should we ignore it or is there some trickery that could be done to make Mypy recognise the types?

Everything's in separate commits for an easier review. I'll leave this as a draft at least until the Mypy issue has been resolved.

[felix-hilden]

Oops. Didn't realise tests wouldn't run in a draft!

[JelleZijlstra]

Not sure what's going on with the mypy error. Can look in more detail in a few hours.

[ichard26]

I said this on PyDis but I'll say it here for the record and also just in case someone misses it:

@[felix] I just looked into it and the mypy error seems to be about how ast.parse only started to accept feature_version in 3.8 and higher

ast.parse(source, filename='', mode='exec', *, type_comments=False, feature_version=None)
[...]
Changed in version 3.8: Added type_comments, mode='func_type' and feature_version.

And mypy isn't smart enough to know that ast will only be used on versions where feature_version is supported :slight_smile:

The easiest solution would be just adding an assert that makes sure we're running on 3.8 in that branch. Other than that, I can't think of a solution that wouldn't require redoing and restructuring the patch which is kinda defeating the point :/

edit: and yes mypy is configured to treat the runtime version as 3.6:

black/mypy.ini

Line 5 in a2b5ba2

python_version=3.6

[felix-hilden]

Thanks for digging around! I see now why the error happens only now. So a couple of things we could do:

• # type: ignore, since we know that the calling code has made sure we are running on 3.8+ (tested and works)
• Assert or redundantly test in the if sys.version_info once more (tested and works)
• Revert the refactoring, if you're not too keen on having it (I know it's only a slight improvement)

I'll leave it up to you!

[felix-hilden]

I corrected the parsing logic now! I think this could be ready for another round of review (hopefully the last as well) 👌

[ichard26]

I don't see any issue with this and it's a nice readability improvement while implementing a nice enhancement. Thank you!