From 1bdbd957bc7ebf6f8ee5fc29f4a38e88a92cd362 Mon Sep 17 00:00:00 2001 From: Yang Fan Date: Sun, 11 Oct 2026 20:33:58 +0800 Subject: [PATCH] Fix namespace package directory renames --- CHANGELOG.md | 1 + docs/library.rst | 7 + rope/refactor/rename.py | 29 ++- .../refactor/namespace_package_rename_test.py | 244 ++++++++++++++++++ 4 files changed, 268 insertions(+), 13 deletions(-) create mode 100644 ropetest/refactor/namespace_package_rename_test.py diff --git a/CHANGELOG.md b/CHANGELOG.md index b3e7311ec..508a3e0ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # **Upcoming release** +- Fix namespace package directory renames and preserve explicit resource limits. - #895 patchedast Starred and keyword now consumes their syntactically expected and ** (@lieryan) - #896 patchedast cleanup and refactoring (@lieryan) diff --git a/docs/library.rst b/docs/library.rst index 35d497337..442d05fcf 100644 --- a/docs/library.rst +++ b/docs/library.rst @@ -610,6 +610,13 @@ them. If it is ``None`` all python modules in the project will be analyzed. Using this parameter, IDEs can let the user limit the files on which a refactoring should be applied. +When renaming a namespace package directory without an ``__init__.py``, +the default project-wide operation also moves the directory. With an +explicit resource list, the directory moves only when all of its Python +files are selected and the package has at least one Python file. Selecting +only importing modules updates those files while leaving the directory +in place, as with an ordinary package whose initializer is not selected. + Examples ======== diff --git a/rope/refactor/rename.py b/rope/refactor/rename.py index fcdd17964..89a058f7e 100644 --- a/rope/refactor/rename.py +++ b/rope/refactor/rename.py @@ -104,6 +104,7 @@ def unsure_func(value=unsure): resources = [self.resource] if _is_local(self.old_pyname): resources = [self.resource] + all_project_resources = resources is None if resources is None: resources = self.project.get_python_files() self.validate_changes(new_name) @@ -126,7 +127,7 @@ def unsure_func(value=unsure): job_set.finished_job() if self._is_renaming_a_module(): resource = self.old_pyname.get_object().get_resource() - if self._is_allowed_to_move(resources, resource): + if self._is_allowed_to_move(resources, resource, all_project_resources): self._rename_module(resource, new_name, changes) return changes @@ -140,12 +141,21 @@ def validate_changes( f"Invalid refactoring target name. '{new_name}' is a Python keyword." ) - def _is_allowed_to_move(self, resources, resource): + def _is_allowed_to_move(self, resources, resource, all_project_resources=False): if resource.is_folder(): try: return resource.get_child("__init__.py") in resources except exceptions.ResourceNotFoundError: - return False + if all_project_resources: + return True + package_files = [ + file + for file in self.project.get_python_files() + if resource.contains(file) + ] + return bool(package_files) and all( + file in resources for file in package_files + ) else: return resource in resources @@ -257,17 +267,10 @@ def rename_in_module( def _is_local(pyname): + if not isinstance(pyname, pynames.AssignedName): + return False module, lineno = pyname.get_definition_location() if lineno is None: return False scope = module.get_scope().get_inner_scope_for_line(lineno) - if isinstance(pyname, pynames.DefinedName) and scope.get_kind() in ( - "Function", - "Class", - ): - scope = scope.parent - return ( - scope.get_kind() == "Function" - and pyname in scope.get_names().values() - and isinstance(pyname, pynames.AssignedName) - ) + return scope.get_kind() == "Function" and pyname in scope.get_names().values() diff --git a/ropetest/refactor/namespace_package_rename_test.py b/ropetest/refactor/namespace_package_rename_test.py new file mode 100644 index 000000000..b6a819393 --- /dev/null +++ b/ropetest/refactor/namespace_package_rename_test.py @@ -0,0 +1,244 @@ +import subprocess +import sys +import unittest +from pathlib import Path +from textwrap import dedent + +from rope.refactor.rename import Rename +from ropetest import testutils + + +class NamespacePackageRenameTest(unittest.TestCase): + def setUp(self): + self.project = testutils.sample_project() + self.root = Path(self.project.address) + self.project.root.create_file("sentinel.txt").write("untouched\n") + + def tearDown(self): + testutils.remove_project(self.project) + + def _file(self, name, contents): + folder = self.project.root + parts = name.split("/") + for part in parts[:-1]: + if not folder.has_child(part): + folder.create_folder(part) + folder = folder.get_child(part) + resource = folder.create_file(parts[-1]) + resource.write(contents) + return resource + + def _run(self, module, expected): + result = subprocess.run( + [sys.executable, "-B", "-m", module], + cwd=self.root, + capture_output=True, + text=True, + timeout=15, + ) + self.assertEqual(0, result.returncode, result.stderr) + self.assertEqual(expected, result.stdout) + self.assertEqual("", result.stderr) + + def _rename(self, package, new_name, **options): + before = { + path.relative_to(self.root).as_posix(): path.read_bytes() + for path in self.root.rglob("*") + if path.is_file() and ".ropeproject" not in path.parts + } + changes = Rename(self.project, package).get_changes(new_name, **options) + after = { + path.relative_to(self.root).as_posix(): path.read_bytes() + for path in self.root.rglob("*") + if path.is_file() and ".ropeproject" not in path.parts + } + self.assertEqual(before, after) + self.project.do(changes) + self.assertEqual("untouched\n", (self.root / "sentinel.txt").read_text()) + + def test_namespace_directory_rename_preserves_saved_module_value(self): + foreign = dedent("""\ + class Foreign: + field = 'FOREIGN' + old = Foreign() + """) + subject = dedent("""\ + import valuespkg.foreign + class Other: + field = 'OTHER' + class Owner: + import valuespkg as slot + saved = Owner.slot + Owner.slot = Other() + print(saved.foreign.old.field) + """) + self._file("valuespkg/foreign.py", foreign) + self._file("subject.py", subject) + self._run("subject", "FOREIGN\n") + self._rename(self.project.root.get_child("valuespkg"), "renamed_pkg") + self.assertFalse((self.root / "valuespkg").exists()) + self.assertFalse((self.root / "renamed_pkg/__init__.py").exists()) + self.assertEqual(foreign, (self.root / "renamed_pkg/foreign.py").read_text()) + self.assertEqual( + subject.replace("import valuespkg", "import renamed_pkg"), + (self.root / "subject.py").read_text(), + ) + self._run("subject", "FOREIGN\n") + + def test_nested_namespace_directory_rename_preserves_relative_import(self): + self._file("outer/inner/value.py", "value = 42\n") + reader = dedent("""\ + from .value import value + print(value) + """) + self._file("outer/inner/reader.py", reader) + self._file( + "subject.py", + dedent("""\ + from outer.inner.value import value + print(value) + """), + ) + self._run("subject", "42\n") + self._run("outer.inner.reader", "42\n") + self._rename(self.project.root.get_child("outer/inner"), "renamed") + self.assertFalse((self.root / "outer/inner").exists()) + self.assertFalse((self.root / "outer/renamed/__init__.py").exists()) + self.assertEqual(reader, (self.root / "outer/renamed/reader.py").read_text()) + self.assertEqual( + dedent("""\ + from outer.renamed.value import value + print(value) + """), + (self.root / "subject.py").read_text(), + ) + self._run("subject", "42\n") + self._run("outer.renamed.reader", "42\n") + + def test_namespace_directory_rename_preserves_import_alias(self): + self._file("valuespkg/value.py", "value = 42\n") + self._file( + "subject.py", + dedent("""\ + from valuespkg import value as saved + print(saved.value) + """), + ) + self._run("subject", "42\n") + self._rename(self.project.root.get_child("valuespkg"), "renamed_pkg") + self.assertEqual( + dedent("""\ + from renamed_pkg import value as saved + print(saved.value) + """), + (self.root / "subject.py").read_text(), + ) + self.assertFalse((self.root / "valuespkg").exists()) + self._run("subject", "42\n") + + def test_empty_namespace_directory_rename_moves_directory(self): + package = self.project.root.create_folder("valuespkg") + self._file( + "subject.py", + dedent("""\ + import valuespkg + print(hasattr(valuespkg, '__path__')) + """), + ) + self._run("subject", "True\n") + self._rename(package, "renamed_pkg") + self.assertTrue((self.root / "renamed_pkg").is_dir()) + self.assertFalse((self.root / "valuespkg").exists()) + self.assertFalse((self.root / "renamed_pkg/__init__.py").exists()) + self._run("subject", "True\n") + + def test_namespace_directory_rename_with_only_importer_keeps_directory(self): + value = self._file("valuespkg/value.py", "value = 42\n") + subject = self._file("subject.py", "import valuespkg.value\n") + self._rename( + self.project.root.get_child("valuespkg"), + "renamed_pkg", + resources=[subject], + ) + self.assertEqual("import renamed_pkg.value\n", subject.read()) + self.assertTrue((self.root / "valuespkg").is_dir()) + self.assertFalse((self.root / "renamed_pkg").exists()) + self.assertEqual("value = 42\n", value.read()) + + def test_regular_package_directory_rename_preserves_initializer(self): + initializer = "value = 42\n" + self._file("valuespkg/__init__.py", initializer) + self._file( + "subject.py", + dedent("""\ + import valuespkg + print(valuespkg.value) + """), + ) + self._run("subject", "42\n") + self._rename(self.project.root.get_child("valuespkg"), "renamed_pkg") + self.assertFalse((self.root / "valuespkg").exists()) + self.assertEqual( + initializer, (self.root / "renamed_pkg/__init__.py").read_text() + ) + self._run("subject", "42\n") + + def test_namespace_directory_rename_with_all_package_files_moves_directory(self): + value = self._file("valuespkg/value.py", "value = 42\n") + other = self._file("valuespkg/other.py", "other = 7\n") + subject = self._file( + "subject.py", + dedent("""\ + from valuespkg.value import value + print(value) + """), + ) + self._run("subject", "42\n") + self._rename( + self.project.root.get_child("valuespkg"), + "renamed_pkg", + resources=[value, other, subject], + ) + self.assertFalse((self.root / "valuespkg").exists()) + self.assertEqual( + "other = 7\n", (self.root / "renamed_pkg/other.py").read_text() + ) + self._run("subject", "42\n") + + def test_namespace_directory_rename_with_partial_package_files_keeps_directory( + self, + ): + value = self._file("valuespkg/value.py", "value = 42\n") + other = self._file("valuespkg/other.py", "other = 7\n") + subject = self._file("subject.py", "import valuespkg.value\n") + self._rename( + self.project.root.get_child("valuespkg"), + "renamed_pkg", + resources=[value, subject], + ) + self.assertTrue((self.root / "valuespkg").is_dir()) + self.assertFalse((self.root / "renamed_pkg").exists()) + self.assertEqual("import renamed_pkg.value\n", subject.read()) + self.assertEqual("value = 42\n", value.read()) + self.assertEqual("other = 7\n", other.read()) + + def test_empty_namespace_directory_with_explicit_resources_keeps_directory(self): + package = self.project.root.create_folder("valuespkg") + subject = self._file("subject.py", "import valuespkg\n") + self._rename(package, "renamed_pkg", resources=[subject]) + self.assertTrue((self.root / "valuespkg").is_dir()) + self.assertFalse((self.root / "renamed_pkg").exists()) + self.assertEqual("import renamed_pkg\n", subject.read()) + + def test_regular_package_with_only_importer_keeps_initializer_and_directory(self): + initializer = self._file("valuespkg/__init__.py", "value = 42\n") + subject = self._file("subject.py", "import valuespkg\n") + self._rename( + self.project.root.get_child("valuespkg"), + "renamed_pkg", + resources=[subject], + ) + self.assertEqual("import renamed_pkg\n", subject.read()) + self.assertEqual("value = 42\n", initializer.read()) + self.assertTrue((self.root / "valuespkg").is_dir()) + self.assertFalse((self.root / "renamed_pkg").exists())