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

[edreamleo]

See PR #579.

Background

• rope.base.ast contains five functions: parse, walk, get_child_nodes, call_for_nodes, and get_children.
• Calls to the parse and walk functions appear in many places throughout Rope's code.
• Calls to the get_child_nodes, call_for_nodes, and get_children appear less frequently.
  None of these three functions appear in Python's ast module.
• Only rope.base.ast.parse calls Python's ast.parse function.
• No calls to Python's ast.walk function appear in Rope's code.

Proposal

It should be straightforward and safe to do the following:

• Delete rope/base/ast.py, moving its functions to rope/base/astutils.py.
  As a happy side effect, this will eliminate from ast import * from Rope's codebase.
• Import rope.base.astutils instead of rope.base.ast everywhere.
• Import Python's ast module as needed, including in astutils.py.
• Access all functions in rope.base.astutils using the astutils qualifier, except within astutils itself.

Rationale

• ast will always mean Python's ast module.
• It will always be clear that parse and walk refer to the functions in astutils.

Update: The new code revealed that _search_in_f_string In rope/refactor/occurrences.py calls the "real" ast.parse and ast.walk functions.

[lieryan]

I think we'll still need from ast import * bit. Except for rope.base.ast, other rope code should never be using stdlib ast module directly (there is actually one instance of this in occurences.py, and I consider that to be a bug).

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.

That said, the walk() name collision is unfortunate. I am more keen on renaming this to something else to disambiguate, rather than moving over to ast.

rope should never use 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 stdlib 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 normalizes 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 targetting
6. (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.

There used to be typed_ast, which fulfills most of these criteria (although 2 and 3 is mostly by accident), but typed_ast is no longer active because stdlib essentially merged typed_ast's type comment parsing into the standard library ast.

[edreamleo]

@lieryan I'm not convinced. I never like complicating code on the off chance that it may change later :-)

The proposed PR would not be very large. Any later revision would have similar size.

I agree with your desiderata concerning a wrapper for Python's ast module.

Leo has such a module for the Qt library. It's ugly, but it's mine :-) In other words, I can enhance it or fix bugs in it immediately.

I am willing to write a wrapper for Python's ast module.

[edreamleo]

@lieryan Most importantly: once ast means exactly one thing, it would be easy to change ast to, say, ast2, a wrapper for ast.

[edreamleo]

@lieryan I am going to start working on the PR. Sometimes it's best to let the code do the talking.

[edreamleo]

I am going to close this discussion in favor of discussions in PR #632.