Remove special case code from walk (in rope.base.ast)

[edreamleo]

PR #538 contains experimental code, that illustrates the idea.

The walk function in rope/base/ast.py contains the following special case:

if isinstance(node, ast.ImportFrom) and node.module is None:
    # In python < 2.7 ``node.module == ''`` for relative imports
    # but for python 2.7 it is None. Generalizing it to ''.
    node.module = ""

Naively, one would think this code should appear in the visitor for ast.ImportFrom. However, no such "direct" visitor exists. However, all unit tests do pass when the special case code is moved to the start of _ScopeVisitor._ImportFrom.

Discussion

Having all unit tests pass is not the end of the story :-) A cff (Leo's global search) reveals that there are tests for ast.ImportFrom in three places besides walk:

• _GlobalImportFinder.find_import_statements
• ModuleImports._first_import_line
• The get_names_from_file function in rope.contrib.autoimport.

The special case code seems dubious. Distinguishing between None and "" is hardly pythonic. I speculate that this special case (hack) is a premature optimization of some kind. See below.

I have spent several happy hours investigating the effect of setting node.module = "". Clearly, the special case is required. Disabling it entirely causes PyCoreInProjectsTest.test_new_style_relative_imports to fail. But it is, err, "challenging" to discover why the test fails. I'll continue my investigations and report my results here. Aside: For testing I have renamed test_new_style_relative_imports to test_new_style_relative_imports1. This allows me to run this test without running test_new_style_relative_imports2.

I do know that the special case causes rope to instantiate objects of the two different (!!) PyModule classes. In some cases, rope instantiates the placeholder PyModule class in pyobjects.py. In other cases, rope instantiates the "real" PyModule class in pyobjectsdef.py.

Possible fixes

I will not suggest any detailed fix before understanding the code more thoroughly. However, I speculate that the present code attempts to save a bit of space somewhere. Whatever the reason, the present code is difficult to understand.

It might be possible to unify the two PyModule classes by adding an extra ivar. This approach might also help simplify the import structure.

[lieryan]

Resolved in #538