Would a wrapper for Python's ast module be useful?

[edreamleo]

In this comment to #570 @lieryan said (slightly edited):
QQQ
Everything that rope does should go through rope.base.ast, which is a wrapper around stdlib ast module. It doesn't make sense to selectively import from stdlib ast to rope.base.ast, because Rope's rope.base.ast is supposed to be a superset of Python's ast.
...
Rope should never use [Python's] stdlib's ast directly because it is a possibility that we may want to switch to a different ast library in the future.

One of the long-standing issue with rope is that because it uses stdlib ast, rope's ability to parse a syntax has always depended on the version of Python that it is running. If there's a Python ast library that:

1. API must be based on the stdlib's ast, so we don't need to completely rewrite everything
2. It must have its own parser, that does not depend on stdlib's ast.parse()
3. It can parse different python syntax variants (e.g. by specifying with someast.parse(..., version="3.8")
4. It should normalize the API differences in the ast between different python versions (the stdlib ast has often broken backwards compatibility or changed how things are represented between versions)
5. It supports running on all the Python versions that rope is targeting (nice to have) can parse other Python-like languages, e.g. cython, pypy

Then I'd really want to switch rope to that ast library, as it'll allow divorcing rope syntax support from the version of Python it is running on.
QQQ

On second reading, I'm far from convinced:

• By analogy with the "scream test," Rope has done well so far ignoring these problems :-)
  Why (how??) would a Rope user develop code using a lesser version of Python?
• Compatibility problems will be less in Rope 1.x because Python2-related problems will disappear.
• leoAst (and Rope) show that it's easy enough to handle differences in stdlib's ast from Python 3.7 onward.
  Note: leoAst sets itself a particularly challenging task, that of associating tokens with ast nodes.

There are generic problems with using any external package:

• The wrapper creates an extra dependency.
• There is a potential burden of working around bugs in the wrapper.
• There may be complications in accessing the wrapped ast nodes.

Summary

• The goal of cross-version compatibility sounds appealing but has little importance.
• Rope's existing code already handles variations in formats of ast nodes in a straightforward manner.
• Compatibility problems will decrease in Python3-only code.
• This (future!) goal is needlessly complicating Rope's code.

Your comments, please.

[lieryan]

Actually it happens all the time, it's one of the most common new user problems. Users may be hitting something that looks like a bug or a crash, or rope code fails to parse some valid python code, and after some back and forth, we would found out that they're not using the same Python to run rope that they use to run their project. And when they fixed that, the problems usually just disappear.

This happens often because rope usually runs on whatever Python version is embedded into their IDE/editor, which are usually not set up by the user, but rather by whatever is included in the package manager or the IDE/editor. The editor may be running rope in Python 3.8, but the user want to refactor code with Python 3.10 syntax, currently this will break rope.

I very often have to keep telling users that they need to use the same Python for rope and their project, or that they should switch to an editor integration that runs rope in a separate sub process, like pylsp. But that isn't always possible or easy, depending on how the user's editors plugins systems are implemented. In some environments, like Vim, the editor's Python version isn't even configurable because the Python interpreter version is compiled into the editor.

And I'm sure, given the frequency that that kind of reports keeps happening, that there are many more people who had to just find an alternate solutions or workaround instead, or they just avoided using rope or new syntaxes because rope would just broke and they don't bother reporting the bugs.

This is actually one of the loudest and most consistent screams.

[edreamleo]

@lieryan Thanks for your helpful comments. Good to know.

Let's find a way to solve the python-version-mismatch problem without writing our own parser. I would think a clear error message would suffice.

[lieryan]

Collecting all tickets, PRs, and discussions related to this discussions, because it's starting to become a bit too unwieldy to follow the discussions happening in multiple places:

1. @edreamleo brought up the issue with ast

   • Eliminate the clash between rope.base.ast and python's ast module #570

2. @lieryan and @edreamleo agreed that there are some ambiguities with the names, but disagreed on how to fix the issues. We had a couple back and forth in the various PRs discussing how we want to approach the fix:

   • PR for #570: disambiguate the name 'ast' #579
   • PR 3 for #570: the name 'ast' #580

3. We settled to go forward with some of the easy picking first, to have a clearer foundation for further thinking and discussions:

   • remove some legacy functions in rope/base/ast.py (Remove functions in rope.base.ast that has functionally identical implementation in stdlib's ast #581)
   • rename astutils.py, because their current contents are not really actually AST utilities (rope.base.ast and rope.base.astutils #582)
   • keep from ast import * in rope.base.ast, we would just disable the warnings from flake8 caused by these imports
   • the situation with rope.base.ast.parse() isn't really ideal

The issue also spawned a number of other unrelated cleanups, which then gets picked up in their own PRs:

• PR: Clean imports and fix pylint complaints #583
• PR for #576: Improve warning in parse #577

Let me know if I missed anything.

@edreamleo can we keep further discursive discussions on the rope.base.ast topic here? Let's keep the Pull Request space to be more narrow focused towards reviewing the particular aspect of the change being made by that PR rather than having higher level discussions there.

[lieryan]

@edreamleo wrote on #580:

I am enjoying our conversation--it is respectful even when heated. We are beginning to know each other as engineers and as persons.

We are making progress, but imo our debate deserves to continue for as long as it takes. Yes, I could live with the code in master, but I would rather not :-)

Agreed with both points! It's hot, but it has been a very good discussion.

All of us need to be happy with how the end result turned out, even when they're not what we initially desired.

I think we both agreed as well that rope codebase do have accumulated a lot of technical debts over the years. A lot of the common best practices within Python has changed as well due to availability and maturity of toolings that we didn't have back then.

Modernising the codebase is necessary, but as with everything, it has to be done carefully to evaluate whether they suit the project at that point in time. Not all best practices are appropriate for all projects, especially ones as old as rope which had long been following practices from a different time and age.

[edreamleo]

@lieryan I agree completely. Thanks for working with me. And thanks for summarizing our discussions.