From fc0512ee780f1e48d88b816181e086c322efa47b Mon Sep 17 00:00:00 2001 From: Marlon Costa Date: Sat, 14 Nov 2026 20:47:54 -0300 Subject: [PATCH] Preserve relative imports and aliases when moving globals --- rope/refactor/move.py | 99 +++++++++++++++++++++++++++-------- ropetest/refactor/movetest.py | 52 ++++++++++++++++++ 2 files changed, 129 insertions(+), 22 deletions(-) diff --git a/rope/refactor/move.py b/rope/refactor/move.py index f13aca6d1..bea323eb6 100644 --- a/rope/refactor/move.py +++ b/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) diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index 2e467b82b..dfa5a9e6c 100644 --- a/ropetest/refactor/movetest.py +++ b/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("""\