diff --git a/CHANGELOG.md b/CHANGELOG.md index b3e7311ec..64c188c05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # **Upcoming release** +- #811 Preserve still-used imports when moving globals with filtered import cleanup - #895 patchedast Starred and keyword now consumes their syntactically expected and ** (@lieryan) - #896 patchedast cleanup and refactoring (@lieryan) diff --git a/rope/refactor/importutils/actions.py b/rope/refactor/importutils/actions.py index 47e6e8c7f..37d345360 100644 --- a/rope/refactor/importutils/actions.py +++ b/rope/refactor/importutils/actions.py @@ -175,6 +175,11 @@ def visitFromImport(self, import_stmt, import_info): ): if import_info.is_star_import(): return True + if import_stmt.readonly: + return not self.import_info.is_star_import() and all( + pair in import_info.names_and_aliases + for pair in self.import_info.names_and_aliases + ) if self.import_info.is_star_import(): import_stmt.import_info = self.import_info return True diff --git a/ropetest/refactor/importutilstest.py b/ropetest/refactor/importutilstest.py index a2ebd49d1..3e6121e35 100644 --- a/ropetest/refactor/importutilstest.py +++ b/ropetest/refactor/importutilstest.py @@ -1948,6 +1948,37 @@ def import_filter(import_stmt): return import_filter + def test_filtered_duplicate_removal_preserves_distinct_from_imports(self): + for imported in ("mod3", "mod3 as other", "*"): + with self.subTest(imported=imported): + source = dedent(f"""\ + from pkg2 import mod2 + from pkg2 import {imported} + """) + self.mod.write(source) + pymod = self.project.get_pymodule(self.mod) + module_imports = self.import_tools.module_imports( + pymod, self._line_filter(2) + ) + module_imports.remove_duplicates() + self.assertEqual(source, module_imports.get_changed_source()) + + def test_filtered_duplicate_removal_still_removes_covered_imports(self): + for imported in ("mod2", "mod2, mod3", "*"): + with self.subTest(imported=imported): + expected = dedent(f"""\ + from pkg2 import {imported} + """) + self.mod.write(expected + dedent("""\ + from pkg2 import mod2 + """)) + pymod = self.project.get_pymodule(self.mod) + module_imports = self.import_tools.module_imports( + pymod, self._line_filter(2) + ) + module_imports.remove_duplicates() + self.assertEqual(expected, module_imports.get_changed_source()) + def test_filtered_expand_stars(self): self.pkg1.get_child("__init__.py").write("var1 = 1\n") self.pkg2.get_child("__init__.py").write("var2 = 1\n") diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index 90f235d32..d292dce9f 100644 --- a/ropetest/refactor/movetest.py +++ b/ropetest/refactor/movetest.py @@ -73,6 +73,45 @@ def test_move_constant(self) -> None: self.assertEqual("", self.origin_module.read()) self.assertEqual("foo = 123\n", self.destination_module.read()) + def test_move_constant_preserves_used_module_alias(self) -> None: + self.project.prefs["prefer_module_from_imports"] = True + self.mod4.write(dedent("""\ + def keep(): + return 1 + + value = 2 + """)) + self.mod5.write(dedent("""\ + def existing(): + return 3 + """)) + self.mod3.write(dedent("""\ + from pkg import mod5 + from pkg import mod4 as utils_module + + def run(): + return mod5.existing(), utils_module.keep(), utils_module.value + """)) + self._move(self.mod4, self.mod4.read().index("value"), self.mod5) + self.assertEqual( + dedent("""\ + from pkg import mod5 + from pkg import mod4 as utils_module + + def run(): + return mod5.existing(), utils_module.keep(), mod5.value + """), + self.mod3.read(), + ) + self.assertEqual(dedent("""\ + def keep(): + return 1 + + """), self.mod4.read()) + self.assertIn(dedent("""\ + value = 2 + """), self.mod5.read()) + def test_move_constant_2(self) -> None: self.origin_module.write("bar = 321\nfoo = 123\n") self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.destination_module)