diff --git a/CHANGELOG.md b/CHANGELOG.md index 1650aa77..4e24edca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # **Upcoming release** +- #831 Preserve external imported names when renaming their local bindings - #850 Update and pin black version in pre-commit and Github Actions - #851 Bump supported python version to up to Python 3.14 - #852 Implement patchedast handlers for TypeAlias diff --git a/rope/refactor/rename.py b/rope/refactor/rename.py index fcdd1796..e2c1ede4 100644 --- a/rope/refactor/rename.py +++ b/rope/refactor/rename.py @@ -2,6 +2,7 @@ from keyword import iskeyword from rope.base import ( + ast, codeanalyze, evaluate, exceptions, @@ -120,7 +121,12 @@ def unsure_func(value=unsure): job_set = task_handle.create_jobset("Collecting Changes", len(resources)) for file_ in resources: job_set.started_job(file_.path) - new_content = rename_in_module(finder, new_name, resource=file_) + new_content = rename_in_module( + finder, + new_name, + resource=file_, + preserve_imports=self._get_preserved_imports(file_), + ) if new_content is not None: changes.add_change(ChangeContents(file_, new_content)) job_set.finished_job() @@ -130,6 +136,33 @@ def unsure_func(value=unsure): self._rename_module(resource, new_name, changes) return changes + def _get_preserved_imports(self, resource): + """Find imported names whose definitions are outside the project.""" + pymodule = self.project.get_pymodule(resource) + lines = codeanalyze.ASTLinesAdapter(pymodule.source_code) + for node in ast.walk(pymodule.get_ast()): + if not isinstance(node, ast.ImportFrom): + continue + names = [alias for alias in node.names if alias.name == self.old_name] + if not names: + continue + imported_module = pynames.ImportedModule( + pymodule, node.module or "", level=node.level + ).get_object() + imported_resource = ( + imported_module.get_resource() + if isinstance(imported_module, pyobjects.AbstractModule) + else None + ) + if imported_resource is not None and ( + imported_resource.project == self.project + and not self.project.is_ignored(imported_resource) + ): + continue + for alias in names: + start, _ = lines[alias] + yield start, alias.asname is None + def validate_changes( self, new_name: str, @@ -233,6 +266,7 @@ def rename_in_module( region=None, reads=True, writes=True, + preserve_imports=None, ): """Returns the changed source or `None` if there is no changes""" if resource is not None: @@ -240,6 +274,7 @@ def rename_in_module( else: source_code = pymodule.source_code change_collector = codeanalyze.ChangeCollector(source_code) + preserved_imports = None for occurrence in occurrences_finder.find_occurrences(resource, pymodule): if replace_primary and occurrence.is_a_fixed_primary(): continue @@ -252,7 +287,16 @@ def rename_in_module( ): continue if region is None or region[0] <= start < region[1]: - change_collector.add_change(start, end, new_name) + replacement = new_name + if preserve_imports is not None and occurrence.is_in_import_statement(): + if preserved_imports is None: + preserved_imports = dict(preserve_imports) + if start in preserved_imports: + if not preserved_imports[start]: + continue + if source_code[start:end] != new_name: + replacement = source_code[start:end] + " as " + new_name + change_collector.add_change(start, end, replacement) return change_collector.get_changed() diff --git a/ropetest/refactor/renametest.py b/ropetest/refactor/renametest.py index 9303fb15..8247b442 100644 --- a/ropetest/refactor/renametest.py +++ b/ropetest/refactor/renametest.py @@ -582,6 +582,111 @@ def new_func(): mod2.read(), ) + def test_renaming_external_import_preserves_imported_name(self): + self.project.prefs.add("extension_modules", "builtins") + code = "from builtins import zip\nresult = list(zip([1], [2]))\n" + refactored = self._local_rename(code, code.rindex("zip"), "pairs") + self.assertEqual( + "from builtins import zip as pairs\nresult = list(pairs([1], [2]))\n", + refactored, + ) + namespace = {} + exec(refactored, namespace) + self.assertEqual([(1, 2)], namespace["result"]) + + def test_renaming_external_import_with_same_alias(self): + code = "from builtins import zip as zip\nresult = zip([], [])\n" + refactored = self._local_rename(code, code.rindex("zip"), "pairs") + self.assertEqual( + "from builtins import zip as pairs\nresult = pairs([], [])\n", + refactored, + ) + + def test_renaming_external_import_with_different_alias(self): + code = "from builtins import zip as pairs\nresult = pairs([], [])\n" + refactored = self._local_rename(code, code.rindex("pairs"), "new_pairs") + self.assertEqual( + "from builtins import zip as new_pairs\nresult = new_pairs([], [])\n", + refactored, + ) + + def test_renaming_multiline_external_import_after_unicode(self): + code = dedent("""\ + café = 1; from builtins import ( + zip as # Keep the imported name. + zip, + ) + result = zip([], []) + """) + refactored = self._local_rename(code, code.rindex("zip"), "pairs") + self.assertEqual( + dedent("""\ + café = 1; from builtins import ( + zip as # Keep the imported name. + pairs, + ) + result = pairs([], []) + """), + refactored, + ) + + def test_renaming_conditional_external_import_from_another_module(self): + compat = testutils.create_module(self.project, "compat") + compat.write(dedent("""\ + import sys + if sys.version_info[0] >= 3: + from builtins import map, zip + else: + zip = lambda *args: None + """)) + mod = testutils.create_module(self.project, "mod") + mod.write("from compat import zip\nresult = list(zip([1], [2]))\n") + self._rename(mod, mod.read().rindex("zip"), "pairs") + self.assertEqual( + dedent("""\ + import sys + if sys.version_info[0] >= 3: + from builtins import map, zip as pairs + else: + pairs = lambda *args: None + """), + compat.read(), + ) + self.assertEqual( + "from compat import pairs\nresult = list(pairs([1], [2]))\n", + mod.read(), + ) + + def test_renaming_import_from_external_python_module(self): + external_project = testutils.sample_project() + self.addCleanup(testutils.remove_project, external_project) + external = testutils.create_module(external_project, "external") + external.write("value = 42\n") + self.project.prefs.add("python_path", external_project.address) + code = "from external import value\nresult = value\n" + refactored = self._local_rename(code, code.rindex("value"), "new_value") + self.assertEqual( + "from external import value as new_value\nresult = new_value\n", + refactored, + ) + self.assertEqual("value = 42\n", external.read()) + + def test_renaming_external_import_does_not_resolve_unrelated_import(self): + invalid = testutils.create_module(self.project, "invalid") + invalid.write("def broken(:\n") + code = ( + "from invalid import other\n" + "from builtins import zip\n" + "result = zip([], [])\n" + ) + refactored = self._local_rename(code, code.rindex("zip"), "pairs") + self.assertEqual( + "from invalid import other\n" + "from builtins import zip as pairs\n" + "result = pairs([], [])\n", + refactored, + ) + def test_applying_all_changes_together(self): mod1 = testutils.create_module(self.project, "mod1") mod1.write(dedent("""\