Preserve relative imports and aliases when moving globals

rope/refactor/move.py

@@ -22,6 +22,7 @@
 )
 from rope.base.change import ChangeContents, ChangeSet, MoveResource
 from rope.refactor import functionutils, importutils, occurrences, rename, sourceutils
+from rope.refactor.importutils import importinfo
 from rope.refactor.importutils.module_imports import (
     get_first_decorator_or_function_start_line,
 )
@@ -263,9 +264,27 @@ def __init__(self, project, resource, offset):
         )
         self.import_tools = self.tools.import_tools
 
-    def _import_filter(self, stmt):
+    def _import_filter(self, stmt, folder=None):
+        """Select the statements that import the moved name or its source module.
+
+        ``folder`` is the importing module's folder; relative statements are
+        resolved from it to the module they import.
+        """
         module_name = libutils.modname(self.source)
 
+        if (
+            isinstance(stmt.import_info, importutils.FromImport)
+            and stmt.import_info.level
+        ):
+            context = importinfo.ImportContext(self.project, folder)
+            imported = stmt.import_info.get_imported_resource(context)
+            source = self._source_import_resource()
+            if imported == source:
+                return True
+            basename = module_name.rsplit(".", 1)[-1]
+            return imported == source.parent and any(
+                basename == name for name, alias in stmt.import_info.names_and_aliases
+            )
         if isinstance(stmt.import_info, importutils.NormalImport):
             # Affect any statement that imports the source module
             return any(
@@ -358,30 +377,66 @@ def _calculate_changes(self, dest, resources, task_handle):
                 changes.add_change(self._source_module_changes(dest))
             elif file_ == dest:
                 changes.add_change(self._dest_module_changes(dest))
-            elif self.tools.occurs_in_module(resource=file_):
-                pymodule = self.project.get_pymodule(file_)
-                # Changing occurrences
-                placeholder = "__rope_renaming_%s_" % self.old_name
-                source = self.tools.rename_in_module(placeholder, resource=file_)
-                should_import = source is not None
-                # Removing out of date imports
-                pymodule = self.tools.new_pymodule(pymodule, source)
-                source = self.import_tools.organize_imports(
-                    pymodule, sort=False, import_filter=self._import_filter
-                )
-                # Adding new import
-                if should_import:
-                    pymodule = self.tools.new_pymodule(pymodule, source)
-                    source, imported = importutils.add_import(
-                        self.project, pymodule, self._new_modname(dest), self.old_name
-                    )
-                    source = source.replace(placeholder, imported)
-                source = self.tools.new_source(pymodule, source)
-                if source != file_.read():
+            else:
+                original = source = file_.read()
+                if self.tools.occurs_in_module(resource=file_):
+                    source = self._rewrite_global_occurrences(file_, dest)
+                source = self._retarget_aliased_imports(file_, source, dest)
+                if source != original:
                     changes.add_change(ChangeContents(file_, source))
             job_set.finished_job()
         return changes
 
+    def _rewrite_global_occurrences(self, file_, dest):
+        pymodule = self.project.get_pymodule(file_)
+        placeholder = "__rope_renaming_%s_" % self.old_name
+        source = self.tools.rename_in_module(placeholder, resource=file_)
+        should_import = source is not None
+        pymodule = self.tools.new_pymodule(pymodule, source)
+        source = self.import_tools.organize_imports(
+            pymodule,
+            sort=False,
+            import_filter=lambda stmt: self._import_filter(stmt, file_.parent),
+        )
+        if should_import:
+            pymodule = self.tools.new_pymodule(pymodule, source)
+            source, imported = importutils.add_import(
+                self.project, pymodule, self._new_modname(dest), self.old_name
+            )
+            source = source.replace(placeholder, imported)
+        return self.tools.new_source(pymodule, source)
+
+    def _source_import_resource(self):
+        if self.source.name == "__init__.py":
+            return self.source.parent
+        return self.source
+
+    def _retarget_aliased_imports(self, resource, source, dest):
+        pymodule = libutils.get_string_module(self.project, source, resource)
+        imports = self.import_tools.module_imports(pymodule)
+        context = importinfo.ImportContext(self.project, resource.parent)
+        origin = self._source_import_resource()
+        for stmt in tuple(imports.imports):
+            info = stmt.import_info
+            if not isinstance(info, importutils.FromImport):
+                continue
+            aliases = [
+                (name, alias)
+                for name, alias in info.names_and_aliases
+                if name == self.old_name and alias is not None
+            ]
+            if not aliases or info.get_imported_resource(context) != origin:
+                continue
+            stmt.import_info = importutils.FromImport(
+                info.module_name,
+                info.level,
+                [pair for pair in info.names_and_aliases if pair not in aliases],
+            )
+            imports.add_import(
+                importutils.FromImport(self._new_modname(dest), 0, aliases)
+            )
+        return imports.get_changed_source()
+
     def _source_module_changes(self, dest):
         placeholder = "__rope_moving_%s_" % self.old_name
         handle = _ChangeMoveOccurrencesHandle(placeholder)
@@ -441,7 +496,7 @@ def _dest_module_changes(self, dest):
             sort=False,
             selfs=False,
             unused=True,
-            import_filter=self._import_filter,
+            import_filter=lambda stmt: self._import_filter(stmt, dest.parent),
         )
         return ChangeContents(dest, source)
 

ropetest/refactor/movetest.py

@@ -563,6 +563,58 @@ def a_func():
         self.assertEqual(expected, self.destination_module.read())
         self.assertEqual("", self.origin_module_in_pkg.read())
 
+    def test_moving_global_replaces_relative_from_imports_in_consumers(self) -> None:
+        """A relative from-import of the moved name is rewritten like an absolute one"""
+        self.origin_module_in_pkg.write("class AClass:\n    pass\n")
+        self.mod4.write("from .origin_module_in_pkg import AClass\na_var = AClass()\n")
+        self._move(self.origin_module_in_pkg, 6, self.destination_module_in_pkg)
+        self.assertEqual(
+            "import pkg.destination_module_in_pkg\n"
+            "a_var = pkg.destination_module_in_pkg.AClass()\n",
+            self.mod4.read(),
+        )
+
+    def test_moving_global_replaces_relative_package_imports_in_consumers(self) -> None:
+        """A relative import of the source module is rewritten to the destination"""
+        self.origin_module_in_pkg.write("class AClass:\n    pass\n")
+        self.mod4.write(
+            "from . import origin_module_in_pkg\n"
+            "a_var = origin_module_in_pkg.AClass()\n"
+        )
+        self._move(self.origin_module_in_pkg, 6, self.destination_module_in_pkg)
+        self.assertEqual(
+            "import pkg.destination_module_in_pkg\n"
+            "a_var = pkg.destination_module_in_pkg.AClass()\n",
+            self.mod4.read(),
+        )
+
+    def test_moving_global_retargets_package_init_import_and_alias(self) -> None:
+        package = testutils.create_package(self.project, "origin", self.pkg)
+        origin = package.get_child("__init__.py")
+        origin.write("class Thing: pass\nother = 1\n")
+        self.mod4.write(
+            "from . import origin\n"
+            "from .origin import Thing as Alias, other\n"
+            "values = origin.Thing(), Alias(), other\n"
+        )
+        self._move(origin, 6, self.destination_module_in_pkg)
+        result = self.mod4.read()
+        self.assertNotIn("from . import origin", result)
+        self.assertIn("from .origin import other", result)
+        self.assertIn("from pkg.destination_module_in_pkg import Thing as Alias", result)
+        self.assertIn(
+            "values = pkg.destination_module_in_pkg.Thing(), Alias(), other", result
+        )
+
+    def test_moving_global_retargets_a_consumer_with_only_an_alias(self) -> None:
+        self.origin_module_in_pkg.write("value = 1\n")
+        self.mod4.write("from .origin_module_in_pkg import value as old\nresult = old\n")
+        self._move(self.origin_module_in_pkg, 0, self.destination_module_in_pkg)
+        self.assertEqual(
+            "from pkg.destination_module_in_pkg import value as old\nresult = old\n",
+            self.mod4.read(),
+        )
+
     def test_moving_modules_into_package(self) -> None:
         """Move global function where the destination module is in a package"""
         code = dedent("""\