Fix namespace package directory renames

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)
 

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
 ========

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()

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())