From 0d7fdd2036ba60bef5e6d458f77d6bdce8ad41ba Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 11:10:41 +1100 Subject: [PATCH 01/19] Refine type: ignore commment --- rope/refactor/move.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rope/refactor/move.py b/rope/refactor/move.py index 4240288d..95978d4c 100644 --- a/rope/refactor/move.py +++ b/rope/refactor/move.py @@ -335,7 +335,7 @@ def get_changes( # "Resource" has no attribute "has_child" if dest is None or not dest.exists(): raise exceptions.RefactoringError("Move destination does not exist.") - if dest.is_folder() and dest.has_child("__init__.py"): # type:ignore + if dest.is_folder() and dest.has_child("__init__.py"): # type: ignore[attr-defined] dest = dest.get_child("__init__.py") # type:ignore # The previous guards protect against this mypy complaint: # Item "None" of "Union[str, Resource, None]" has no attribute "is_folder" From bb0c437c7248c45110918de036fc5369f1713c32 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 16:30:16 +1100 Subject: [PATCH 02/19] Add dedent to movetest.py --- ropetest/refactor/movetest.py | 134 +++++++++++++++++++++++++++------- 1 file changed, 107 insertions(+), 27 deletions(-) diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index 508069aa..c31b3d60 100644 --- a/ropetest/refactor/movetest.py +++ b/ropetest/refactor/movetest.py @@ -145,8 +145,14 @@ class AClass(foo.FooClass): ) def test_changing_other_modules_replacing_normal_imports(self): - self.mod1.write("class AClass(object):\n pass\n") - self.mod3.write("import mod1\na_var = mod1.AClass()\n") + self.mod1.write(dedent("""\ + class AClass(object): + pass + """)) + self.mod3.write(dedent("""\ + import mod1 + a_var = mod1.AClass() + """)) self._move(self.mod1, self.mod1.read().index("AClass") + 1, self.mod2) self.assertEqual( dedent("""\ @@ -327,7 +333,10 @@ class TestClass: ) def test_raising_exception_for_moving_glob_elements_to_the_same_module(self): - self.mod1.write("def a_func():\n pass\n") + self.mod1.write(dedent("""\ + def a_func(): + pass + """)) with self.assertRaises(exceptions.RefactoringError): self._move(self.mod1, self.mod1.read().index("a_func"), self.mod1) @@ -410,70 +419,117 @@ def a_func(): self.assertEqual("", self.mod4.read()) def test_moving_modules(self): - code = "import mod1\nprint(mod1)" + code = dedent("""\ + import mod1 + print(mod1)""" + ) self.mod2.write(code) self._move(self.mod2, code.index("mod1") + 1, self.pkg) - expected = "import pkg.mod1\nprint(pkg.mod1)" + expected = dedent("""\ + import pkg.mod1 + print(pkg.mod1)""" + ) self.assertEqual(expected, self.mod2.read()) self.assertTrue( not self.mod1.exists() and self.project.find_module("pkg.mod1") is not None ) def test_moving_modules_and_removing_out_of_date_imports(self): - code = "import pkg.mod4\nprint(pkg.mod4)" + code = dedent("""\ + import pkg.mod4 + print(pkg.mod4)""") self.mod2.write(code) self._move(self.mod2, code.index("mod4") + 1, self.project.root) - expected = "import mod4\nprint(mod4)" + expected = dedent("""\ + import mod4 + print(mod4)""") self.assertEqual(expected, self.mod2.read()) self.assertTrue(self.project.find_module("mod4") is not None) def test_moving_modules_and_removing_out_of_date_froms(self): - code = "from pkg import mod4\nprint(mod4)" + code = dedent("""\ + from pkg import mod4 + print(mod4)""") self.mod2.write(code) self._move(self.mod2, code.index("mod4") + 1, self.project.root) - self.assertEqual("import mod4\nprint(mod4)", self.mod2.read()) + self.assertEqual( + dedent("""\ + import mod4 + print(mod4)""" + ), + self.mod2.read(), + ) def test_moving_modules_and_removing_out_of_date_froms2(self): self.mod4.write("a_var = 10") - code = "from pkg.mod4 import a_var\nprint(a_var)\n" + code = dedent("""\ + from pkg.mod4 import a_var + print(a_var) + """) self.mod2.write(code) self._move(self.mod2, code.index("mod4") + 1, self.project.root) - expected = "from mod4 import a_var\nprint(a_var)\n" + expected = dedent("""\ + from mod4 import a_var + print(a_var) + """) self.assertEqual(expected, self.mod2.read()) def test_moving_modules_and_relative_import(self): - self.mod4.write("import mod5\nprint(mod5)\n") - code = "import pkg.mod4\nprint(pkg.mod4)" + self.mod4.write(dedent("""\ + import mod5 + print(mod5) + """)) + code = dedent("""\ + import pkg.mod4 + print(pkg.mod4)""") self.mod2.write(code) self._move(self.mod2, code.index("mod4") + 1, self.project.root) moved = self.project.find_module("mod4") - expected = "import pkg.mod5\nprint(pkg.mod5)\n" + expected = dedent("""\ + import pkg.mod5 + print(pkg.mod5) + """) self.assertEqual(expected, moved.read()) def test_moving_module_kwarg_same_name_as_old(self): - self.mod1.write("def foo(mod1=0):\n pass") - code = "import mod1\nmod1.foo(mod1=1)" + self.mod1.write(dedent("""\ + def foo(mod1=0): + pass""")) + code = dedent("""\ + import mod1 + mod1.foo(mod1=1)""") self.mod2.write(code) self._move(self.mod1, None, self.pkg) moved = self.project.find_module("mod2") - expected = "import pkg.mod1\npkg.mod1.foo(mod1=1)" + expected = dedent("""\ + import pkg.mod1 + pkg.mod1.foo(mod1=1)""") self.assertEqual(expected, moved.read()) def test_moving_packages(self): pkg2 = testutils.create_package(self.project, "pkg2") - code = "import pkg.mod4\nprint(pkg.mod4)" + code = dedent("""\ + import pkg.mod4 + print(pkg.mod4)""") self.mod1.write(code) self._move(self.mod1, code.index("pkg") + 1, pkg2) self.assertFalse(self.pkg.exists()) self.assertTrue(self.project.find_module("pkg2.pkg.mod4") is not None) self.assertTrue(self.project.find_module("pkg2.pkg.mod4") is not None) self.assertTrue(self.project.find_module("pkg2.pkg.mod5") is not None) - expected = "import pkg2.pkg.mod4\nprint(pkg2.pkg.mod4)" + expected = dedent("""\ + import pkg2.pkg.mod4 + print(pkg2.pkg.mod4)""") self.assertEqual(expected, self.mod1.read()) def test_moving_modules_with_self_imports(self): - self.mod1.write("import mod1\nprint(mod1)\n") - self.mod2.write("import mod1\n") + self.mod1.write(dedent("""\ + import mod1 + print(mod1) + """)) + self.mod2.write(dedent("""\ + import mod1 + """)) self._move(self.mod2, self.mod2.read().index("mod1") + 1, self.pkg) moved = self.project.find_module("pkg.mod1") self.assertEqual( @@ -920,7 +976,10 @@ def new_method(self): ) def test_moving_methods_getting_old_method_for_constant_methods(self): - self.mod2.write("class B(object):\n pass\n") + self.mod2.write(dedent("""\ + class B(object): + pass + """)) code = dedent("""\ import mod2 @@ -943,7 +1002,10 @@ def a_method(self): self.assertEqual(expected, self.mod1.read()) def test_moving_methods_getting_getting_changes_for_goal_class(self): - self.mod2.write("class B(object):\n var = 1\n") + self.mod2.write(dedent("""\ + class B(object): + var = 1 + """)) code = dedent("""\ import mod2 @@ -1019,7 +1081,10 @@ def a_method(self): mover.get_changes("attr", "new_method") def test_moving_methods_and_moving_used_imports(self): - self.mod2.write("class B(object):\n var = 1\n") + self.mod2.write(dedent("""\ + class B(object): + var = 1 + """)) code = dedent("""\ import sys import mod2 @@ -1068,7 +1133,10 @@ def new_method(self): self.assertEqual(expected, self.mod2.read()) def test_moving_methods_and_source_class_with_parameters(self): - self.mod2.write("class B(object):\n pass\n") + self.mod2.write(dedent("""\ + class B(object): + pass + """)) code = dedent("""\ import mod2 @@ -1142,7 +1210,13 @@ def f(): print(sys.version, os.path) """) self.mod1.write(code) - self.mod2.write('"""doc\n\nMore docs ...\n\n"""\n') + self.mod2.write(dedent('''\ + """doc + + More docs ... + + """ + ''')) mover = move.create_move( self.project, self.mod1, self.mod1.read().index("f()") + 1 ) @@ -1207,7 +1281,13 @@ def foo(): pass """)) self._move(self.mod1, self.mod1.read().index("foo") + 1, self.mod2) - self.assertEqual("def hello(func):\n return func\n", self.mod1.read()) + self.assertEqual( + dedent("""\ + def hello(func): + return func + """), + self.mod1.read(), + ) self.assertEqual( dedent("""\ from mod1 import hello From ee6b724f44498b0d2dd38ece3acca09db61eb05b Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 18:50:05 +1100 Subject: [PATCH 03/19] Add more test descriptions on movetest.py --- ropetest/refactor/movetest.py | 48 ++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index c31b3d60..69866fe2 100644 --- a/ropetest/refactor/movetest.py +++ b/ropetest/refactor/movetest.py @@ -86,6 +86,7 @@ def test_move_constant_multiple_statements(self): self.assertEqual("foo = 123\n", self.mod2.read()) def test_simple_moving(self): + """Move a global class definition""" self.mod1.write(dedent("""\ class AClass(object): pass @@ -101,6 +102,7 @@ class AClass(object): ) def test_moving_with_comment_prefix(self): + """Comments above the moved class are moved to the destination module""" self.mod1.write(dedent("""\ a = 1 # 1 @@ -145,6 +147,7 @@ class AClass(foo.FooClass): ) def test_changing_other_modules_replacing_normal_imports(self): + """When moving a class from mod1 to mod2, references to the class in mod3 is updated to point to mod2""" self.mod1.write(dedent("""\ class AClass(object): pass @@ -270,6 +273,10 @@ class AClass(object): ) def test_changing_source_module(self): + """ + Add import statements to the source module as the moved class now lives + in mod2. + """ self.mod1.write(dedent("""\ class AClass(object): pass @@ -285,6 +292,10 @@ class AClass(object): ) def test_changing_destination_module(self): + """ + Remove import statements in the destination module as the moved class + can now be referenced from mod2 without import. + """ self.mod1.write(dedent("""\ class AClass(object): pass @@ -309,7 +320,10 @@ def test_folder_destination(self): class AClass(object): pass """)) - with self.assertRaises(exceptions.RefactoringError): + with self.assertRaisesRegex( + exceptions.RefactoringError, + r"Move destination for non-modules should not be folders\.", + ) as e: self._move(self.mod1, self.mod1.read().index("AClass") + 1, folder) def test_raising_exception_for_moving_non_global_elements(self): @@ -318,7 +332,10 @@ def a_func(): class AClass(object): pass """)) - with self.assertRaises(exceptions.RefactoringError): + with self.assertRaisesRegex( + exceptions.RefactoringError, + r"Move only works on global classes/functions/variables, modules and methods\.", + ): self._move(self.mod1, self.mod1.read().index("AClass") + 1, self.mod2) def test_raising_an_exception_for_moving_non_global_variable(self): @@ -327,7 +344,10 @@ class TestClass: CONSTANT = 5 """) self.mod1.write(code) - with self.assertRaises(exceptions.RefactoringError): + with self.assertRaisesRegex( + exceptions.RefactoringError, + "Move refactoring should be performed on a global class, function or variable\.", + ): mover = move.create_move( self.project, self.mod1, code.index("CONSTANT") + 1 ) @@ -337,10 +357,17 @@ def test_raising_exception_for_moving_glob_elements_to_the_same_module(self): def a_func(): pass """)) - with self.assertRaises(exceptions.RefactoringError): + with self.assertRaisesRegex( + exceptions.RefactoringError, + "Moving global elements to the same module\.", + ): self._move(self.mod1, self.mod1.read().index("a_func"), self.mod1) def test_moving_used_imports_to_destination_module(self): + """ + Add import statements for imported references used by the moved + function to the destination module. + """ self.mod3.write("a_var = 10") code = dedent("""\ import mod3 @@ -361,6 +388,10 @@ def a_func(): self.assertEqual(expected, self.mod2.read()) def test_moving_used_names_to_destination_module2(self): + """ + Add import statements for references to globals in the source module + used by the moved function to the destination module. + """ code = dedent("""\ a_var = 10 def a_func(): @@ -401,6 +432,7 @@ def a_func(): self.assertEqual(expected, self.mod2.read()) def test_moving_and_used_relative_imports(self): + """Move global function where the source module is in a package""" code = dedent("""\ import mod5 def a_func(): @@ -418,7 +450,8 @@ def a_func(): self.assertEqual(expected, self.mod1.read()) self.assertEqual("", self.mod4.read()) - def test_moving_modules(self): + def test_moving_modules_into_package(self): + """Move global function where the destination module is in a package""" code = dedent("""\ import mod1 print(mod1)""" @@ -1257,7 +1290,10 @@ def f(): def test_raising_an_exception_when_moving_non_package_folders(self): dir = self.project.root.create_folder("dir") - with self.assertRaises(exceptions.RefactoringError): + with self.assertRaisesRegex( + exceptions.RefactoringError, + r"Cannot move non-package folder\.", + ): move.create_move(self.project, dir) def test_moving_to_a_module_with_encoding_cookie(self): From a2c1cd0bc39d03bd5812b1789b2e36c723d1e02f Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 19:01:56 +1100 Subject: [PATCH 04/19] Disambiguated cases where mod1 is always used as origin_module --- ropetest/refactor/movetest.py | 335 +++++++++++++++++----------------- 1 file changed, 168 insertions(+), 167 deletions(-) diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index 69866fe2..55a52f98 100644 --- a/ropetest/refactor/movetest.py +++ b/ropetest/refactor/movetest.py @@ -10,6 +10,7 @@ class MoveRefactoringTest(unittest.TestCase): def setUp(self): super().setUp() self.project = testutils.sample_project() + self.origin_module = testutils.create_module(self.project, "origin_module") self.mod1 = testutils.create_module(self.project, "mod1") self.mod2 = testutils.create_module(self.project, "mod2") self.mod3 = testutils.create_module(self.project, "mod3") @@ -28,37 +29,37 @@ def _move(self, resource, offset, dest_resource): self.project.do(changes) def test_move_constant(self): - self.mod1.write("foo = 123\n") - self._move(self.mod1, self.mod1.read().index("foo") + 1, self.mod2) - self.assertEqual("", self.mod1.read()) + self.origin_module.write("foo = 123\n") + self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.mod2) + self.assertEqual("", self.origin_module.read()) self.assertEqual("foo = 123\n", self.mod2.read()) def test_move_constant_2(self): - self.mod1.write("bar = 321\nfoo = 123\n") - self._move(self.mod1, self.mod1.read().index("foo") + 1, self.mod2) - self.assertEqual("bar = 321\n", self.mod1.read()) + self.origin_module.write("bar = 321\nfoo = 123\n") + self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.mod2) + self.assertEqual("bar = 321\n", self.origin_module.read()) self.assertEqual("foo = 123\n", self.mod2.read()) def test_move_target_is_module_name(self): - self.mod1.write("foo = 123\n") - self._move(self.mod1, self.mod1.read().index("foo") + 1, "mod2") - self.assertEqual("", self.mod1.read()) + self.origin_module.write("foo = 123\n") + self._move(self.origin_module, self.origin_module.read().index("foo") + 1, "mod2") + self.assertEqual("", self.origin_module.read()) self.assertEqual("foo = 123\n", self.mod2.read()) def test_move_target_is_package_name(self): - self.mod1.write("foo = 123\n") - self._move(self.mod1, self.mod1.read().index("foo") + 1, "pkg.mod4") - self.assertEqual("", self.mod1.read()) + self.origin_module.write("foo = 123\n") + self._move(self.origin_module, self.origin_module.read().index("foo") + 1, "pkg.mod4") + self.assertEqual("", self.origin_module.read()) self.assertEqual("foo = 123\n", self.mod4.read()) def test_move_constant_multiline(self): - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ foo = ( 123 ) """)) - self._move(self.mod1, self.mod1.read().index("foo") + 1, self.mod2) - self.assertEqual("", self.mod1.read()) + self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.mod2) + self.assertEqual("", self.origin_module.read()) self.assertEqual( dedent("""\ foo = ( @@ -69,30 +70,30 @@ def test_move_constant_multiline(self): ) def test_move_constant_multiple_statements(self): - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ foo = 123 foo += 3 foo = 4 """)) - self._move(self.mod1, self.mod1.read().index("foo") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.mod2) self.assertEqual( dedent("""\ import mod2 mod2.foo += 3 mod2.foo = 4 """), - self.mod1.read(), + self.origin_module.read(), ) self.assertEqual("foo = 123\n", self.mod2.read()) def test_simple_moving(self): """Move a global class definition""" - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ class AClass(object): pass """)) - self._move(self.mod1, self.mod1.read().index("AClass") + 1, self.mod2) - self.assertEqual("", self.mod1.read()) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) + self.assertEqual("", self.origin_module.read()) self.assertEqual( dedent("""\ class AClass(object): @@ -103,15 +104,15 @@ class AClass(object): def test_moving_with_comment_prefix(self): """Comments above the moved class are moved to the destination module""" - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ a = 1 # 1 # 2 class AClass(object): pass """)) - self._move(self.mod1, self.mod1.read().index("AClass") + 1, self.mod2) - self.assertEqual("a = 1\n", self.mod1.read()) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) + self.assertEqual("a = 1\n", self.origin_module.read()) self.assertEqual( dedent("""\ # 1 @@ -123,7 +124,7 @@ class AClass(object): ) def test_moving_with_comment_prefix_imports(self): - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ import foo a = 1 # 1 @@ -131,8 +132,8 @@ def test_moving_with_comment_prefix_imports(self): class AClass(foo.FooClass): pass """)) - self._move(self.mod1, self.mod1.read().index("AClass") + 1, self.mod2) - self.assertEqual("a = 1\n", self.mod1.read()) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) + self.assertEqual("a = 1\n", self.origin_module.read()) self.assertEqual( dedent("""\ import foo @@ -147,16 +148,16 @@ class AClass(foo.FooClass): ) def test_changing_other_modules_replacing_normal_imports(self): - """When moving a class from mod1 to mod2, references to the class in mod3 is updated to point to mod2""" - self.mod1.write(dedent("""\ + """When moving a class from origin_module to mod2, references to the class in mod3 is updated to point to mod2""" + self.origin_module.write(dedent("""\ class AClass(object): pass """)) self.mod3.write(dedent("""\ - import mod1 - a_var = mod1.AClass() + import origin_module + a_var = origin_module.AClass() """)) - self._move(self.mod1, self.mod1.read().index("AClass") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) self.assertEqual( dedent("""\ import mod2 @@ -166,104 +167,104 @@ class AClass(object): ) def test_changing_other_modules_adding_normal_imports(self): - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ class AClass(object): pass def a_function(): pass """)) self.mod3.write(dedent("""\ - import mod1 - a_var = mod1.AClass() - mod1.a_function()""")) - self._move(self.mod1, self.mod1.read().index("AClass") + 1, self.mod2) + import origin_module + a_var = origin_module.AClass() + origin_module.a_function()""")) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) self.assertEqual( dedent("""\ - import mod1 + import origin_module import mod2 a_var = mod2.AClass() - mod1.a_function()"""), + origin_module.a_function()"""), self.mod3.read(), ) def test_adding_imports_prefer_from_module(self): self.project.prefs["prefer_module_from_imports"] = True - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ class AClass(object): pass def a_function(): pass """)) self.mod3.write(dedent("""\ - import mod1 - a_var = mod1.AClass() - mod1.a_function()""")) + import origin_module + a_var = origin_module.AClass() + origin_module.a_function()""")) # Move to mod4 which is in a different package - self._move(self.mod1, self.mod1.read().index("AClass") + 1, self.mod4) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod4) self.assertEqual( dedent("""\ - import mod1 + import origin_module from pkg import mod4 a_var = mod4.AClass() - mod1.a_function()"""), + origin_module.a_function()"""), self.mod3.read(), ) def test_adding_imports_noprefer_from_module(self): self.project.prefs["prefer_module_from_imports"] = False - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ class AClass(object): pass def a_function(): pass """)) self.mod3.write(dedent("""\ - import mod1 - a_var = mod1.AClass() - mod1.a_function()""")) + import origin_module + a_var = origin_module.AClass() + origin_module.a_function()""")) # Move to mod4 which is in a different package - self._move(self.mod1, self.mod1.read().index("AClass") + 1, self.mod4) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod4) self.assertEqual( dedent("""\ - import mod1 + import origin_module import pkg.mod4 a_var = pkg.mod4.AClass() - mod1.a_function()"""), + origin_module.a_function()"""), self.mod3.read(), ) def test_adding_imports_prefer_from_module_top_level_module(self): self.project.prefs["prefer_module_from_imports"] = True - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ class AClass(object): pass def a_function(): pass """)) self.mod3.write(dedent("""\ - import mod1 - a_var = mod1.AClass() - mod1.a_function()""")) - self._move(self.mod1, self.mod1.read().index("AClass") + 1, self.mod2) + import origin_module + a_var = origin_module.AClass() + origin_module.a_function()""")) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) self.assertEqual( dedent("""\ - import mod1 + import origin_module import mod2 a_var = mod2.AClass() - mod1.a_function()"""), + origin_module.a_function()"""), self.mod3.read(), ) def test_changing_other_modules_removing_from_imports(self): - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ class AClass(object): pass """)) self.mod3.write(dedent("""\ - from mod1 import AClass + from origin_module import AClass a_var = AClass() """)) - self._move(self.mod1, self.mod1.read().index("AClass") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) self.assertEqual( dedent("""\ import mod2 @@ -277,18 +278,18 @@ def test_changing_source_module(self): Add import statements to the source module as the moved class now lives in mod2. """ - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ class AClass(object): pass a_var = AClass() """)) - self._move(self.mod1, self.mod1.read().index("AClass") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) self.assertEqual( dedent("""\ import mod2 a_var = mod2.AClass() """), - self.mod1.read(), + self.origin_module.read(), ) def test_changing_destination_module(self): @@ -296,15 +297,15 @@ def test_changing_destination_module(self): Remove import statements in the destination module as the moved class can now be referenced from mod2 without import. """ - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ class AClass(object): pass """)) self.mod2.write(dedent("""\ - from mod1 import AClass + from origin_module import AClass a_var = AClass() """)) - self._move(self.mod1, self.mod1.read().index("AClass") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) self.assertEqual( dedent("""\ class AClass(object): @@ -316,7 +317,7 @@ class AClass(object): def test_folder_destination(self): folder = self.project.root.create_folder("folder") - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ class AClass(object): pass """)) @@ -324,10 +325,10 @@ class AClass(object): exceptions.RefactoringError, r"Move destination for non-modules should not be folders\.", ) as e: - self._move(self.mod1, self.mod1.read().index("AClass") + 1, folder) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, folder) def test_raising_exception_for_moving_non_global_elements(self): - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ def a_func(): class AClass(object): pass @@ -336,24 +337,24 @@ class AClass(object): exceptions.RefactoringError, r"Move only works on global classes/functions/variables, modules and methods\.", ): - self._move(self.mod1, self.mod1.read().index("AClass") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) def test_raising_an_exception_for_moving_non_global_variable(self): code = dedent("""\ class TestClass: CONSTANT = 5 """) - self.mod1.write(code) + self.origin_module.write(code) with self.assertRaisesRegex( exceptions.RefactoringError, "Move refactoring should be performed on a global class, function or variable\.", ): mover = move.create_move( - self.project, self.mod1, code.index("CONSTANT") + 1 + self.project, self.origin_module, code.index("CONSTANT") + 1 ) def test_raising_exception_for_moving_glob_elements_to_the_same_module(self): - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ def a_func(): pass """)) @@ -361,7 +362,7 @@ def a_func(): exceptions.RefactoringError, "Moving global elements to the same module\.", ): - self._move(self.mod1, self.mod1.read().index("a_func"), self.mod1) + self._move(self.origin_module, self.origin_module.read().index("a_func"), self.origin_module) def test_moving_used_imports_to_destination_module(self): """ @@ -375,8 +376,8 @@ def test_moving_used_imports_to_destination_module(self): def a_func(): print(mod3, a_var) """) - self.mod1.write(code) - self._move(self.mod1, code.index("a_func") + 1, self.mod2) + self.origin_module.write(code) + self._move(self.origin_module, code.index("a_func") + 1, self.mod2) expected = dedent("""\ import mod3 from mod3 import a_var @@ -397,16 +398,16 @@ def test_moving_used_names_to_destination_module2(self): def a_func(): print(a_var) """) - self.mod1.write(code) - self._move(self.mod1, code.index("a_func") + 1, self.mod2) + self.origin_module.write(code) + self._move(self.origin_module, code.index("a_func") + 1, self.mod2) self.assertEqual( dedent("""\ a_var = 10 """), - self.mod1.read(), + self.origin_module.read(), ) expected = dedent("""\ - from mod1 import a_var + from origin_module import a_var def a_func(): @@ -420,10 +421,10 @@ def test_moving_used_underlined_names_to_destination_module(self): def a_func(): print(_var) """) - self.mod1.write(code) - self._move(self.mod1, code.index("a_func") + 1, self.mod2) + self.origin_module.write(code) + self._move(self.origin_module, code.index("a_func") + 1, self.mod2) expected = dedent("""\ - from mod1 import _var + from origin_module import _var def a_func(): @@ -525,18 +526,18 @@ def test_moving_modules_and_relative_import(self): self.assertEqual(expected, moved.read()) def test_moving_module_kwarg_same_name_as_old(self): - self.mod1.write(dedent("""\ - def foo(mod1=0): + self.origin_module.write(dedent("""\ + def foo(origin_module=0): pass""")) code = dedent("""\ - import mod1 - mod1.foo(mod1=1)""") + import origin_module + origin_module.foo(origin_module=1)""") self.mod2.write(code) - self._move(self.mod1, None, self.pkg) + self._move(self.origin_module, None, self.pkg) moved = self.project.find_module("mod2") expected = dedent("""\ - import pkg.mod1 - pkg.mod1.foo(mod1=1)""") + import pkg.origin_module + pkg.origin_module.foo(origin_module=1)""") self.assertEqual(expected, moved.read()) def test_moving_packages(self): @@ -544,8 +545,8 @@ def test_moving_packages(self): code = dedent("""\ import pkg.mod4 print(pkg.mod4)""") - self.mod1.write(code) - self._move(self.mod1, code.index("pkg") + 1, pkg2) + self.origin_module.write(code) + self._move(self.origin_module, code.index("pkg") + 1, pkg2) self.assertFalse(self.pkg.exists()) self.assertTrue(self.project.find_module("pkg2.pkg.mod4") is not None) self.assertTrue(self.project.find_module("pkg2.pkg.mod4") is not None) @@ -553,7 +554,7 @@ def test_moving_packages(self): expected = dedent("""\ import pkg2.pkg.mod4 print(pkg2.pkg.mod4)""") - self.assertEqual(expected, self.mod1.read()) + self.assertEqual(expected, self.origin_module.read()) def test_moving_modules_with_self_imports(self): self.mod1.write(dedent("""\ @@ -578,15 +579,15 @@ def test_moving_modules_with_from_imports(self): code = dedent("""\ from pkg import mod4 print(mod4)""") - self.mod1.write(code) - self._move(self.mod1, code.index("pkg") + 1, pkg2) + self.origin_module.write(code) + self._move(self.origin_module, code.index("pkg") + 1, pkg2) self.assertFalse(self.pkg.exists()) self.assertTrue(self.project.find_module("pkg2.pkg.mod4") is not None) self.assertTrue(self.project.find_module("pkg2.pkg.mod5") is not None) expected = dedent("""\ from pkg2.pkg import mod4 print(mod4)""") - self.assertEqual(expected, self.mod1.read()) + self.assertEqual(expected, self.origin_module.read()) def test_moving_modules_with_from_import(self): pkg2 = testutils.create_package(self.project, "pkg2") @@ -694,8 +695,8 @@ def test_moving_package_with_from_and_normal_imports(self): import pkg.mod4 print(pkg.mod4) print(mod4)""") - self.mod1.write(code) - self._move(self.mod1, code.index("pkg") + 1, pkg2) + self.origin_module.write(code) + self._move(self.origin_module, code.index("pkg") + 1, pkg2) self.assertFalse(self.pkg.exists()) self.assertTrue(self.project.find_module("pkg2.pkg.mod4") is not None) self.assertTrue(self.project.find_module("pkg2.pkg.mod5") is not None) @@ -704,7 +705,7 @@ def test_moving_package_with_from_and_normal_imports(self): import pkg2.pkg.mod4 print(pkg2.pkg.mod4) print(mod4)""") - self.assertEqual(expected, self.mod1.read()) + self.assertEqual(expected, self.origin_module.read()) def test_moving_package_with_from_and_normal_imports2(self): pkg2 = testutils.create_package(self.project, "pkg2") @@ -713,8 +714,8 @@ def test_moving_package_with_from_and_normal_imports2(self): from pkg import mod4 print(pkg.mod4) print(mod4)""") - self.mod1.write(code) - self._move(self.mod1, code.index("pkg") + 1, pkg2) + self.origin_module.write(code) + self._move(self.origin_module, code.index("pkg") + 1, pkg2) self.assertFalse(self.pkg.exists()) self.assertTrue(self.project.find_module("pkg2.pkg.mod4") is not None) self.assertTrue(self.project.find_module("pkg2.pkg.mod5") is not None) @@ -723,7 +724,7 @@ def test_moving_package_with_from_and_normal_imports2(self): from pkg2.pkg import mod4 print(pkg2.pkg.mod4) print(mod4)""") - self.assertEqual(expected, self.mod1.read()) + self.assertEqual(expected, self.origin_module.read()) def test_moving_package_and_retaining_blank_lines(self): pkg2 = testutils.create_package(self.project, "pkg2", self.pkg) @@ -772,19 +773,19 @@ def a_func(): self.assertEqual(expected, self.mod1.read()) def test_moving_resources_using_move_module_refactoring(self): - self.mod1.write("a_var = 1") + self.origin_module.write("a_var = 1") self.mod2.write(dedent("""\ - import mod1 - my_var = mod1.a_var + import origin_module + my_var = origin_module.a_var """)) - mover = move.create_move(self.project, self.mod1) + mover = move.create_move(self.project, self.origin_module) mover.get_changes(self.pkg).do() expected = dedent("""\ - import pkg.mod1 - my_var = pkg.mod1.a_var + import pkg.origin_module + my_var = pkg.origin_module.a_var """) self.assertEqual(expected, self.mod2.read()) - self.assertTrue(self.pkg.get_child("mod1.py") is not None) + self.assertTrue(self.pkg.get_child("origin_module.py") is not None) def test_moving_resources_using_move_module_for_packages(self): self.mod1.write(dedent("""\ @@ -816,16 +817,16 @@ def test_moving_resources_using_move_module_for_init_dot_py(self): self.assertTrue(pkg2.get_child("pkg") is not None) def test_moving_module_and_star_imports(self): - self.mod1.write("a_var = 1") + self.origin_module.write("a_var = 1") self.mod2.write(dedent("""\ - from mod1 import * + from origin_module import * a = a_var """)) - mover = move.create_move(self.project, self.mod1) + mover = move.create_move(self.project, self.origin_module) mover.get_changes(self.pkg).do() self.assertEqual( dedent("""\ - from pkg.mod1 import * + from pkg.origin_module import * a = a_var """), self.mod2.read(), @@ -872,8 +873,8 @@ class A(object): def a_method(self): pass """) - self.mod1.write(code) - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + self.origin_module.write(code) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) self.assertTrue(isinstance(mover, move.MoveMethod)) def test_moving_methods_getting_new_method_for_empty_methods(self): @@ -882,8 +883,8 @@ class A(object): def a_method(self): pass """) - self.mod1.write(code) - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + self.origin_module.write(code) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) self.assertEqual( dedent("""\ def new_method(self): @@ -898,8 +899,8 @@ class A(object): def a_method(self): return 1 """) - self.mod1.write(code) - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + self.origin_module.write(code) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) self.assertEqual( dedent("""\ def new_method(self): @@ -914,8 +915,8 @@ class A(object): def a_method(self, p): return p """) - self.mod1.write(code) - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + self.origin_module.write(code) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) self.assertEqual( dedent("""\ def new_method(self, p): @@ -931,8 +932,8 @@ class A(object): def a_method(host): return host.attr """) - self.mod1.write(code) - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + self.origin_module.write(code) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) self.assertEqual( dedent("""\ def new_method(self, host): @@ -948,8 +949,8 @@ class A(object): def a_method(self): return self.attr """) - self.mod1.write(code) - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + self.origin_module.write(code) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) self.assertEqual( dedent("""\ def new_method(self, host): @@ -965,8 +966,8 @@ class A(object): def a_method(self, p=None): return p """) - self.mod1.write(code) - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + self.origin_module.write(code) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) self.assertEqual( dedent("""\ def new_method(self, p=None): @@ -982,8 +983,8 @@ class A(object): def a_method(self, p1, *args, **kwds): return self.attr """) - self.mod1.write(code) - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + self.origin_module.write(code) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) expected = dedent("""\ def new_method(self, host, p1, *args, **kwds): return host.attr @@ -997,8 +998,8 @@ def a_method(self): a = 2 return a """) - self.mod1.write(code) - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + self.origin_module.write(code) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) self.assertEqual( dedent("""\ def new_method(self): @@ -1021,8 +1022,8 @@ class A(object): def a_method(self): return 1 """) - self.mod1.write(code) - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + self.origin_module.write(code) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) mover.get_changes("attr", "new_method").do() expected = dedent("""\ import mod2 @@ -1032,7 +1033,7 @@ class A(object): def a_method(self): return self.attr.new_method() """) - self.assertEqual(expected, self.mod1.read()) + self.assertEqual(expected, self.origin_module.read()) def test_moving_methods_getting_getting_changes_for_goal_class(self): self.mod2.write(dedent("""\ @@ -1047,8 +1048,8 @@ class A(object): def a_method(self): return 1 """) - self.mod1.write(code) - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + self.origin_module.write(code) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) mover.get_changes("attr", "new_method").do() expected = dedent("""\ class B(object): @@ -1070,8 +1071,8 @@ class A(object): def a_method(self): return 1 """) - self.mod1.write(code) - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + self.origin_module.write(code) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) mover.get_changes("attr", "new_method").do() self.assertEqual( dedent("""\ @@ -1087,7 +1088,7 @@ class A(object): def a_method(self): return self.attr.new_method() """), - self.mod1.read(), + self.origin_module.read(), ) def test_moving_methods_and_nonexistent_attributes(self): @@ -1096,9 +1097,9 @@ class A(object): def a_method(self): return 1 """) - self.mod1.write(code) + self.origin_module.write(code) with self.assertRaises(exceptions.RefactoringError): - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) mover.get_changes("x", "new_method") def test_unknown_attribute_type(self): @@ -1108,9 +1109,9 @@ class A(object): def a_method(self): return 1 """) - self.mod1.write(code) + self.origin_module.write(code) with self.assertRaises(exceptions.RefactoringError): - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) mover.get_changes("attr", "new_method") def test_moving_methods_and_moving_used_imports(self): @@ -1127,8 +1128,8 @@ class A(object): def a_method(self): return sys.version """) - self.mod1.write(code) - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + self.origin_module.write(code) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) mover.get_changes("attr", "new_method").do() code = dedent("""\ import sys @@ -1154,8 +1155,8 @@ class A(object): def a_method(self): return 1 """) - self.mod1.write(code) - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + self.origin_module.write(code) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) mover.get_changes("attr", "new_method").do() expected = dedent("""\ class B(object): @@ -1178,8 +1179,8 @@ class A(object): def a_method(self, p): return p """) - self.mod1.write(code) - mover = move.create_move(self.project, self.mod1, code.index("a_method")) + self.origin_module.write(code) + mover = move.create_move(self.project, self.origin_module, code.index("a_method")) mover.get_changes("attr", "new_method").do() expected1 = dedent("""\ import mod2 @@ -1189,7 +1190,7 @@ class A(object): def a_method(self, p): return self.attr.new_method(p) """) - self.assertEqual(expected1, self.mod1.read()) + self.assertEqual(expected1, self.origin_module.read()) expected2 = dedent("""\ class B(object): @@ -1199,7 +1200,7 @@ def new_method(self, p): self.assertEqual(expected2, self.mod2.read()) def test_moving_globals_to_a_module_with_only_docstrings(self): - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ import sys @@ -1214,7 +1215,7 @@ def f(): """ ''')) mover = move.create_move( - self.project, self.mod1, self.mod1.read().index("f()") + 1 + self.project, self.origin_module, self.origin_module.read().index("f()") + 1 ) self.project.do(mover.get_changes(self.mod2)) self.assertEqual( @@ -1242,7 +1243,7 @@ def test_moving_globals_to_a_module_with_only_docstrings2(self): def f(): print(sys.version, os.path) """) - self.mod1.write(code) + self.origin_module.write(code) self.mod2.write(dedent('''\ """doc @@ -1251,7 +1252,7 @@ def f(): """ ''')) mover = move.create_move( - self.project, self.mod1, self.mod1.read().index("f()") + 1 + self.project, self.origin_module, self.origin_module.read().index("f()") + 1 ) self.project.do(mover.get_changes(self.mod2)) expected = dedent('''\ @@ -1277,8 +1278,8 @@ def f(): """ r = f() ''') - self.mod1.write(code) - mover = move.create_move(self.project, self.mod1, code.index("f()") + 1) + self.origin_module.write(code) + mover = move.create_move(self.project, self.origin_module, code.index("f()") + 1) self.project.do(mover.get_changes(self.mod2)) expected = dedent('''\ import mod2 @@ -1286,7 +1287,7 @@ def f(): """ r = mod2.f() ''') - self.assertEqual(expected, self.mod1.read()) + self.assertEqual(expected, self.origin_module.read()) def test_raising_an_exception_when_moving_non_package_folders(self): dir = self.project.root.create_folder("dir") @@ -1309,24 +1310,24 @@ def f(): pass self.assertEqual(expected, self.mod1.read()) def test_moving_decorated_function(self): - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ def hello(func): return func @hello def foo(): pass """)) - self._move(self.mod1, self.mod1.read().index("foo") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.mod2) self.assertEqual( dedent("""\ def hello(func): return func """), - self.mod1.read(), + self.origin_module.read(), ) self.assertEqual( dedent("""\ - from mod1 import hello + from origin_module import hello @hello @@ -1337,14 +1338,14 @@ def foo(): ) def test_moving_decorated_class(self): - self.mod1.write(dedent("""\ + self.origin_module.write(dedent("""\ from dataclasses import dataclass @dataclass class AClass: pass """)) - self._move(self.mod1, self.mod1.read().index("AClass") + 1, self.mod2) - self.assertEqual("", self.mod1.read()) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) + self.assertEqual("", self.origin_module.read()) self.assertEqual( dedent("""\ from dataclasses import dataclass From fee9c2ebd9ae92da54c6e33409625d57860c9acc Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 19:22:46 +1100 Subject: [PATCH 05/19] Disambiguated cases where mod2 is always used as destination_module --- ropetest/refactor/movetest.py | 125 ++++++++++++++++++---------------- 1 file changed, 65 insertions(+), 60 deletions(-) diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index 55a52f98..5d6972c5 100644 --- a/ropetest/refactor/movetest.py +++ b/ropetest/refactor/movetest.py @@ -11,6 +11,7 @@ def setUp(self): super().setUp() self.project = testutils.sample_project() self.origin_module = testutils.create_module(self.project, "origin_module") + self.destination_module = testutils.create_module(self.project, "destination_module") self.mod1 = testutils.create_module(self.project, "mod1") self.mod2 = testutils.create_module(self.project, "mod2") self.mod3 = testutils.create_module(self.project, "mod3") @@ -30,21 +31,21 @@ def _move(self, resource, offset, dest_resource): def test_move_constant(self): self.origin_module.write("foo = 123\n") - self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.destination_module) self.assertEqual("", self.origin_module.read()) - self.assertEqual("foo = 123\n", self.mod2.read()) + self.assertEqual("foo = 123\n", self.destination_module.read()) def test_move_constant_2(self): self.origin_module.write("bar = 321\nfoo = 123\n") - self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.destination_module) self.assertEqual("bar = 321\n", self.origin_module.read()) - self.assertEqual("foo = 123\n", self.mod2.read()) + self.assertEqual("foo = 123\n", self.destination_module.read()) def test_move_target_is_module_name(self): self.origin_module.write("foo = 123\n") - self._move(self.origin_module, self.origin_module.read().index("foo") + 1, "mod2") + self._move(self.origin_module, self.origin_module.read().index("foo") + 1, "destination_module") self.assertEqual("", self.origin_module.read()) - self.assertEqual("foo = 123\n", self.mod2.read()) + self.assertEqual("foo = 123\n", self.destination_module.read()) def test_move_target_is_package_name(self): self.origin_module.write("foo = 123\n") @@ -58,7 +59,7 @@ def test_move_constant_multiline(self): 123 ) """)) - self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.destination_module) self.assertEqual("", self.origin_module.read()) self.assertEqual( dedent("""\ @@ -66,7 +67,7 @@ def test_move_constant_multiline(self): 123 ) """), - self.mod2.read(), + self.destination_module.read(), ) def test_move_constant_multiple_statements(self): @@ -75,16 +76,16 @@ def test_move_constant_multiple_statements(self): foo += 3 foo = 4 """)) - self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.destination_module) self.assertEqual( dedent("""\ - import mod2 - mod2.foo += 3 - mod2.foo = 4 + import destination_module + destination_module.foo += 3 + destination_module.foo = 4 """), self.origin_module.read(), ) - self.assertEqual("foo = 123\n", self.mod2.read()) + self.assertEqual("foo = 123\n", self.destination_module.read()) def test_simple_moving(self): """Move a global class definition""" @@ -92,14 +93,14 @@ def test_simple_moving(self): class AClass(object): pass """)) - self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.destination_module) self.assertEqual("", self.origin_module.read()) self.assertEqual( dedent("""\ class AClass(object): pass """), - self.mod2.read(), + self.destination_module.read(), ) def test_moving_with_comment_prefix(self): @@ -111,7 +112,7 @@ def test_moving_with_comment_prefix(self): class AClass(object): pass """)) - self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.destination_module) self.assertEqual("a = 1\n", self.origin_module.read()) self.assertEqual( dedent("""\ @@ -120,7 +121,7 @@ class AClass(object): class AClass(object): pass """), - self.mod2.read(), + self.destination_module.read(), ) def test_moving_with_comment_prefix_imports(self): @@ -132,7 +133,7 @@ def test_moving_with_comment_prefix_imports(self): class AClass(foo.FooClass): pass """)) - self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.destination_module) self.assertEqual("a = 1\n", self.origin_module.read()) self.assertEqual( dedent("""\ @@ -144,11 +145,15 @@ class AClass(foo.FooClass): class AClass(foo.FooClass): pass """), - self.mod2.read(), + self.destination_module.read(), ) def test_changing_other_modules_replacing_normal_imports(self): - """When moving a class from origin_module to mod2, references to the class in mod3 is updated to point to mod2""" + """ + When moving a class from origin_module to destination_module, + references to the class in mod3 is updated to point to + destination_module + """ self.origin_module.write(dedent("""\ class AClass(object): pass @@ -157,11 +162,11 @@ class AClass(object): import origin_module a_var = origin_module.AClass() """)) - self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.destination_module) self.assertEqual( dedent("""\ - import mod2 - a_var = mod2.AClass() + import destination_module + a_var = destination_module.AClass() """), self.mod3.read(), ) @@ -177,12 +182,12 @@ def a_function(): import origin_module a_var = origin_module.AClass() origin_module.a_function()""")) - self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.destination_module) self.assertEqual( dedent("""\ import origin_module - import mod2 - a_var = mod2.AClass() + import destination_module + a_var = destination_module.AClass() origin_module.a_function()"""), self.mod3.read(), ) @@ -245,12 +250,12 @@ def a_function(): import origin_module a_var = origin_module.AClass() origin_module.a_function()""")) - self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.destination_module) self.assertEqual( dedent("""\ import origin_module - import mod2 - a_var = mod2.AClass() + import destination_module + a_var = destination_module.AClass() origin_module.a_function()"""), self.mod3.read(), ) @@ -264,11 +269,11 @@ class AClass(object): from origin_module import AClass a_var = AClass() """)) - self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.destination_module) self.assertEqual( dedent("""\ - import mod2 - a_var = mod2.AClass() + import destination_module + a_var = destination_module.AClass() """), self.mod3.read(), ) @@ -276,18 +281,18 @@ class AClass(object): def test_changing_source_module(self): """ Add import statements to the source module as the moved class now lives - in mod2. + in destination_module. """ self.origin_module.write(dedent("""\ class AClass(object): pass a_var = AClass() """)) - self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.destination_module) self.assertEqual( dedent("""\ - import mod2 - a_var = mod2.AClass() + import destination_module + a_var = destination_module.AClass() """), self.origin_module.read(), ) @@ -295,24 +300,24 @@ class AClass(object): def test_changing_destination_module(self): """ Remove import statements in the destination module as the moved class - can now be referenced from mod2 without import. + can now be referenced from destination_module without import. """ self.origin_module.write(dedent("""\ class AClass(object): pass """)) - self.mod2.write(dedent("""\ + self.destination_module.write(dedent("""\ from origin_module import AClass a_var = AClass() """)) - self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.destination_module) self.assertEqual( dedent("""\ class AClass(object): pass a_var = AClass() """), - self.mod2.read(), + self.destination_module.read(), ) def test_folder_destination(self): @@ -337,7 +342,7 @@ class AClass(object): exceptions.RefactoringError, r"Move only works on global classes/functions/variables, modules and methods\.", ): - self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.destination_module) def test_raising_an_exception_for_moving_non_global_variable(self): code = dedent("""\ @@ -377,7 +382,7 @@ def a_func(): print(mod3, a_var) """) self.origin_module.write(code) - self._move(self.origin_module, code.index("a_func") + 1, self.mod2) + self._move(self.origin_module, code.index("a_func") + 1, self.destination_module) expected = dedent("""\ import mod3 from mod3 import a_var @@ -386,7 +391,7 @@ def a_func(): def a_func(): print(mod3, a_var) """) - self.assertEqual(expected, self.mod2.read()) + self.assertEqual(expected, self.destination_module.read()) def test_moving_used_names_to_destination_module2(self): """ @@ -399,7 +404,7 @@ def a_func(): print(a_var) """) self.origin_module.write(code) - self._move(self.origin_module, code.index("a_func") + 1, self.mod2) + self._move(self.origin_module, code.index("a_func") + 1, self.destination_module) self.assertEqual( dedent("""\ a_var = 10 @@ -413,7 +418,7 @@ def a_func(): def a_func(): print(a_var) """) - self.assertEqual(expected, self.mod2.read()) + self.assertEqual(expected, self.destination_module.read()) def test_moving_used_underlined_names_to_destination_module(self): code = dedent("""\ @@ -422,7 +427,7 @@ def a_func(): print(_var) """) self.origin_module.write(code) - self._move(self.origin_module, code.index("a_func") + 1, self.mod2) + self._move(self.origin_module, code.index("a_func") + 1, self.destination_module) expected = dedent("""\ from origin_module import _var @@ -430,7 +435,7 @@ def a_func(): def a_func(): print(_var) """) - self.assertEqual(expected, self.mod2.read()) + self.assertEqual(expected, self.destination_module.read()) def test_moving_and_used_relative_imports(self): """Move global function where the source module is in a package""" @@ -1207,7 +1212,7 @@ def test_moving_globals_to_a_module_with_only_docstrings(self): def f(): print(sys.version) """)) - self.mod2.write(dedent('''\ + self.destination_module.write(dedent('''\ """doc More docs ... @@ -1217,7 +1222,7 @@ def f(): mover = move.create_move( self.project, self.origin_module, self.origin_module.read().index("f()") + 1 ) - self.project.do(mover.get_changes(self.mod2)) + self.project.do(mover.get_changes(self.destination_module)) self.assertEqual( dedent('''\ """doc @@ -1231,7 +1236,7 @@ def f(): def f(): print(sys.version) '''), - self.mod2.read(), + self.destination_module.read(), ) def test_moving_globals_to_a_module_with_only_docstrings2(self): @@ -1244,7 +1249,7 @@ def f(): print(sys.version, os.path) """) self.origin_module.write(code) - self.mod2.write(dedent('''\ + self.destination_module.write(dedent('''\ """doc More docs ... @@ -1254,7 +1259,7 @@ def f(): mover = move.create_move( self.project, self.origin_module, self.origin_module.read().index("f()") + 1 ) - self.project.do(mover.get_changes(self.mod2)) + self.project.do(mover.get_changes(self.destination_module)) expected = dedent('''\ """doc @@ -1268,7 +1273,7 @@ def f(): def f(): print(sys.version, os.path) ''') - self.assertEqual(expected, self.mod2.read()) + self.assertEqual(expected, self.destination_module.read()) def test_moving_a_global_when_it_is_used_after_a_multiline_str(self): code = dedent('''\ @@ -1280,12 +1285,12 @@ def f(): ''') self.origin_module.write(code) mover = move.create_move(self.project, self.origin_module, code.index("f()") + 1) - self.project.do(mover.get_changes(self.mod2)) + self.project.do(mover.get_changes(self.destination_module)) expected = dedent('''\ - import mod2 + import destination_module s = """\\ """ - r = mod2.f() + r = destination_module.f() ''') self.assertEqual(expected, self.origin_module.read()) @@ -1317,7 +1322,7 @@ def hello(func): def foo(): pass """)) - self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.destination_module) self.assertEqual( dedent("""\ def hello(func): @@ -1334,7 +1339,7 @@ def hello(func): def foo(): pass """), - self.mod2.read(), + self.destination_module.read(), ) def test_moving_decorated_class(self): @@ -1344,7 +1349,7 @@ def test_moving_decorated_class(self): class AClass: pass """)) - self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod2) + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.destination_module) self.assertEqual("", self.origin_module.read()) self.assertEqual( dedent("""\ @@ -1355,5 +1360,5 @@ class AClass: class AClass: pass """), - self.mod2.read(), + self.destination_module.read(), ) From 77f97619307065b66011edd997f7f4b6047548a6 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 19:28:23 +1100 Subject: [PATCH 06/19] Disambiguated cases where mod1 is always used as destination_module --- ropetest/refactor/movetest.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index 5d6972c5..c75b95f5 100644 --- a/ropetest/refactor/movetest.py +++ b/ropetest/refactor/movetest.py @@ -445,7 +445,7 @@ def a_func(): print(mod5) """) self.mod4.write(code) - self._move(self.mod4, code.index("a_func") + 1, self.mod1) + self._move(self.mod4, code.index("a_func") + 1, self.destination_module) expected = dedent("""\ import pkg.mod5 @@ -453,7 +453,7 @@ def a_func(): def a_func(): print(pkg.mod5) """) - self.assertEqual(expected, self.mod1.read()) + self.assertEqual(expected, self.destination_module.read()) self.assertEqual("", self.mod4.read()) def test_moving_modules_into_package(self): @@ -763,19 +763,19 @@ def test_moving_package_and_retaining_blank_lines(self): def test_moving_functions_to_imported_module(self): code = dedent("""\ - import mod1 + import destination_module def a_func(): - var = mod1.a_var + var = destination_module.a_var """) - self.mod1.write("a_var = 1\n") + self.destination_module.write("a_var = 1\n") self.mod2.write(code) - self._move(self.mod2, code.index("a_func") + 1, self.mod1) + self._move(self.mod2, code.index("a_func") + 1, self.destination_module) expected = dedent("""\ def a_func(): var = a_var a_var = 1 """) - self.assertEqual(expected, self.mod1.read()) + self.assertEqual(expected, self.destination_module.read()) def test_moving_resources_using_move_module_refactoring(self): self.origin_module.write("a_var = 1") @@ -1304,15 +1304,15 @@ def test_raising_an_exception_when_moving_non_package_folders(self): def test_moving_to_a_module_with_encoding_cookie(self): code1 = "# -*- coding: utf-8 -*-" - self.mod1.write(code1) + self.destination_module.write(code1) code2 = dedent("""\ def f(): pass """) self.mod2.write(code2) mover = move.create_move(self.project, self.mod2, code2.index("f()") + 1) - self.project.do(mover.get_changes(self.mod1)) + self.project.do(mover.get_changes(self.destination_module)) expected = f"{code1}\n{code2}" - self.assertEqual(expected, self.mod1.read()) + self.assertEqual(expected, self.destination_module.read()) def test_moving_decorated_function(self): self.origin_module.write(dedent("""\ From dfb14d5eb7fb25bc5eaba419838966ffb8bd1b5a Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 19:35:54 +1100 Subject: [PATCH 07/19] Disambiguated cases where mod2 is always used as origin_module --- ropetest/refactor/movetest.py | 36 +++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index c75b95f5..720e1d04 100644 --- a/ropetest/refactor/movetest.py +++ b/ropetest/refactor/movetest.py @@ -462,13 +462,13 @@ def test_moving_modules_into_package(self): import mod1 print(mod1)""" ) - self.mod2.write(code) - self._move(self.mod2, code.index("mod1") + 1, self.pkg) + self.origin_module.write(code) + self._move(self.origin_module, code.index("mod1") + 1, self.pkg) expected = dedent("""\ import pkg.mod1 print(pkg.mod1)""" ) - self.assertEqual(expected, self.mod2.read()) + self.assertEqual(expected, self.origin_module.read()) self.assertTrue( not self.mod1.exists() and self.project.find_module("pkg.mod1") is not None ) @@ -477,26 +477,26 @@ def test_moving_modules_and_removing_out_of_date_imports(self): code = dedent("""\ import pkg.mod4 print(pkg.mod4)""") - self.mod2.write(code) - self._move(self.mod2, code.index("mod4") + 1, self.project.root) + self.origin_module.write(code) + self._move(self.origin_module, code.index("mod4") + 1, self.project.root) expected = dedent("""\ import mod4 print(mod4)""") - self.assertEqual(expected, self.mod2.read()) + self.assertEqual(expected, self.origin_module.read()) self.assertTrue(self.project.find_module("mod4") is not None) def test_moving_modules_and_removing_out_of_date_froms(self): code = dedent("""\ from pkg import mod4 print(mod4)""") - self.mod2.write(code) - self._move(self.mod2, code.index("mod4") + 1, self.project.root) + self.origin_module.write(code) + self._move(self.origin_module, code.index("mod4") + 1, self.project.root) self.assertEqual( dedent("""\ import mod4 print(mod4)""" ), - self.mod2.read(), + self.origin_module.read(), ) def test_moving_modules_and_removing_out_of_date_froms2(self): @@ -505,13 +505,13 @@ def test_moving_modules_and_removing_out_of_date_froms2(self): from pkg.mod4 import a_var print(a_var) """) - self.mod2.write(code) - self._move(self.mod2, code.index("mod4") + 1, self.project.root) + self.origin_module.write(code) + self._move(self.origin_module, code.index("mod4") + 1, self.project.root) expected = dedent("""\ from mod4 import a_var print(a_var) """) - self.assertEqual(expected, self.mod2.read()) + self.assertEqual(expected, self.origin_module.read()) def test_moving_modules_and_relative_import(self): self.mod4.write(dedent("""\ @@ -521,8 +521,8 @@ def test_moving_modules_and_relative_import(self): code = dedent("""\ import pkg.mod4 print(pkg.mod4)""") - self.mod2.write(code) - self._move(self.mod2, code.index("mod4") + 1, self.project.root) + self.origin_module.write(code) + self._move(self.origin_module, code.index("mod4") + 1, self.project.root) moved = self.project.find_module("mod4") expected = dedent("""\ import pkg.mod5 @@ -566,10 +566,10 @@ def test_moving_modules_with_self_imports(self): import mod1 print(mod1) """)) - self.mod2.write(dedent("""\ + self.origin_module.write(dedent("""\ import mod1 """)) - self._move(self.mod2, self.mod2.read().index("mod1") + 1, self.pkg) + self._move(self.origin_module, self.origin_module.read().index("mod1") + 1, self.pkg) moved = self.project.find_module("pkg.mod1") self.assertEqual( dedent("""\ @@ -768,8 +768,8 @@ def a_func(): var = destination_module.a_var """) self.destination_module.write("a_var = 1\n") - self.mod2.write(code) - self._move(self.mod2, code.index("a_func") + 1, self.destination_module) + self.origin_module.write(code) + self._move(self.origin_module, code.index("a_func") + 1, self.destination_module) expected = dedent("""\ def a_func(): var = a_var From a007e3a37427150527a7d303f2f724f8204609d6 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 19:44:48 +1100 Subject: [PATCH 08/19] Disambiguated cases where mod4 is always used as destination_module_in_pkg --- ropetest/refactor/movetest.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index 720e1d04..ff4f090b 100644 --- a/ropetest/refactor/movetest.py +++ b/ropetest/refactor/movetest.py @@ -10,14 +10,15 @@ class MoveRefactoringTest(unittest.TestCase): def setUp(self): super().setUp() self.project = testutils.sample_project() - self.origin_module = testutils.create_module(self.project, "origin_module") - self.destination_module = testutils.create_module(self.project, "destination_module") self.mod1 = testutils.create_module(self.project, "mod1") self.mod2 = testutils.create_module(self.project, "mod2") self.mod3 = testutils.create_module(self.project, "mod3") self.pkg = testutils.create_package(self.project, "pkg") self.mod4 = testutils.create_module(self.project, "mod4", self.pkg) self.mod5 = testutils.create_module(self.project, "mod5", self.pkg) + self.origin_module = testutils.create_module(self.project, "origin_module") + self.destination_module = testutils.create_module(self.project, "destination_module") + self.destination_module_in_pkg = testutils.create_module(self.project, "destination_module_in_pkg", self.pkg) def tearDown(self): testutils.remove_project(self.project) @@ -49,9 +50,9 @@ def test_move_target_is_module_name(self): def test_move_target_is_package_name(self): self.origin_module.write("foo = 123\n") - self._move(self.origin_module, self.origin_module.read().index("foo") + 1, "pkg.mod4") + self._move(self.origin_module, self.origin_module.read().index("foo") + 1, "pkg.destination_module_in_pkg") self.assertEqual("", self.origin_module.read()) - self.assertEqual("foo = 123\n", self.mod4.read()) + self.assertEqual("foo = 123\n", self.destination_module_in_pkg.read()) def test_move_constant_multiline(self): self.origin_module.write(dedent("""\ @@ -204,13 +205,13 @@ def a_function(): import origin_module a_var = origin_module.AClass() origin_module.a_function()""")) - # Move to mod4 which is in a different package - self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod4) + # Move to destination_module_in_pkg which is in a different package + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.destination_module_in_pkg) self.assertEqual( dedent("""\ import origin_module - from pkg import mod4 - a_var = mod4.AClass() + from pkg import destination_module_in_pkg + a_var = destination_module_in_pkg.AClass() origin_module.a_function()"""), self.mod3.read(), ) @@ -227,13 +228,13 @@ def a_function(): import origin_module a_var = origin_module.AClass() origin_module.a_function()""")) - # Move to mod4 which is in a different package - self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.mod4) + # Move to destination_module_in_pkg which is in a different package + self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.destination_module_in_pkg) self.assertEqual( dedent("""\ import origin_module - import pkg.mod4 - a_var = pkg.mod4.AClass() + import pkg.destination_module_in_pkg + a_var = pkg.destination_module_in_pkg.AClass() origin_module.a_function()"""), self.mod3.read(), ) From 75910f0763b0fc23c4959e651ead94def3dfa7bb Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 19:52:13 +1100 Subject: [PATCH 09/19] Disambiguated cases where mod4 is always used as origin_module_in_pkg --- ropetest/refactor/movetest.py | 129 +++++++++++++++++----------------- 1 file changed, 65 insertions(+), 64 deletions(-) diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index ff4f090b..06a46884 100644 --- a/ropetest/refactor/movetest.py +++ b/ropetest/refactor/movetest.py @@ -18,6 +18,7 @@ def setUp(self): self.mod5 = testutils.create_module(self.project, "mod5", self.pkg) self.origin_module = testutils.create_module(self.project, "origin_module") self.destination_module = testutils.create_module(self.project, "destination_module") + self.origin_module_in_pkg = testutils.create_module(self.project, "origin_module_in_pkg", self.pkg) self.destination_module_in_pkg = testutils.create_module(self.project, "destination_module_in_pkg", self.pkg) def tearDown(self): @@ -445,8 +446,8 @@ def test_moving_and_used_relative_imports(self): def a_func(): print(mod5) """) - self.mod4.write(code) - self._move(self.mod4, code.index("a_func") + 1, self.destination_module) + self.origin_module_in_pkg.write(code) + self._move(self.origin_module_in_pkg, code.index("a_func") + 1, self.destination_module) expected = dedent("""\ import pkg.mod5 @@ -455,7 +456,7 @@ def a_func(): print(pkg.mod5) """) self.assertEqual(expected, self.destination_module.read()) - self.assertEqual("", self.mod4.read()) + self.assertEqual("", self.origin_module_in_pkg.read()) def test_moving_modules_into_package(self): """Move global function where the destination module is in a package""" @@ -600,14 +601,14 @@ def test_moving_modules_with_from_import(self): pkg3 = testutils.create_package(self.project, "pkg3", pkg2) pkg4 = testutils.create_package(self.project, "pkg4", pkg3) code = dedent("""\ - from pkg import mod4 - print(mod4)""") + from pkg import origin_module_in_pkg + print(origin_module_in_pkg)""") self.mod1.write(code) - self._move(self.mod4, None, pkg4) - self.assertTrue(self.project.find_module("pkg2.pkg3.pkg4.mod4") is not None) + self._move(self.origin_module_in_pkg, None, pkg4) + self.assertTrue(self.project.find_module("pkg2.pkg3.pkg4.origin_module_in_pkg") is not None) expected = dedent("""\ - from pkg2.pkg3.pkg4 import mod4 - print(mod4)""") + from pkg2.pkg3.pkg4 import origin_module_in_pkg + print(origin_module_in_pkg)""") self.assertEqual(expected, self.mod1.read()) def test_moving_modules_with_multi_from_imports(self): @@ -615,15 +616,15 @@ def test_moving_modules_with_multi_from_imports(self): pkg3 = testutils.create_package(self.project, "pkg3", pkg2) pkg4 = testutils.create_package(self.project, "pkg4", pkg3) code = dedent("""\ - from pkg import mod4, mod5 - print(mod4)""") + from pkg import origin_module_in_pkg, mod5 + print(origin_module_in_pkg)""") self.mod1.write(code) - self._move(self.mod4, None, pkg4) - self.assertTrue(self.project.find_module("pkg2.pkg3.pkg4.mod4") is not None) + self._move(self.origin_module_in_pkg, None, pkg4) + self.assertTrue(self.project.find_module("pkg2.pkg3.pkg4.origin_module_in_pkg") is not None) expected = dedent("""\ from pkg import mod5 - from pkg2.pkg3.pkg4 import mod4 - print(mod4)""") + from pkg2.pkg3.pkg4 import origin_module_in_pkg + print(origin_module_in_pkg)""") self.assertEqual(expected, self.mod1.read()) def test_moving_modules_with_from_and_normal_imports(self): @@ -631,18 +632,18 @@ def test_moving_modules_with_from_and_normal_imports(self): pkg3 = testutils.create_package(self.project, "pkg3", pkg2) pkg4 = testutils.create_package(self.project, "pkg4", pkg3) code = dedent("""\ - from pkg import mod4 - import pkg.mod4 - print(mod4) - print(pkg.mod4)""") + from pkg import origin_module_in_pkg + import pkg.origin_module_in_pkg + print(origin_module_in_pkg) + print(pkg.origin_module_in_pkg)""") self.mod1.write(code) - self._move(self.mod4, None, pkg4) - self.assertTrue(self.project.find_module("pkg2.pkg3.pkg4.mod4") is not None) + self._move(self.origin_module_in_pkg, None, pkg4) + self.assertTrue(self.project.find_module("pkg2.pkg3.pkg4.origin_module_in_pkg") is not None) expected = dedent("""\ - import pkg2.pkg3.pkg4.mod4 - from pkg2.pkg3.pkg4 import mod4 - print(mod4) - print(pkg2.pkg3.pkg4.mod4)""") + import pkg2.pkg3.pkg4.origin_module_in_pkg + from pkg2.pkg3.pkg4 import origin_module_in_pkg + print(origin_module_in_pkg) + print(pkg2.pkg3.pkg4.origin_module_in_pkg)""") self.assertEqual(expected, self.mod1.read()) def test_moving_modules_with_normal_and_from_imports(self): @@ -650,18 +651,18 @@ def test_moving_modules_with_normal_and_from_imports(self): pkg3 = testutils.create_package(self.project, "pkg3", pkg2) pkg4 = testutils.create_package(self.project, "pkg4", pkg3) code = dedent("""\ - import pkg.mod4 - from pkg import mod4 - print(mod4) - print(pkg.mod4)""") + import pkg.origin_module_in_pkg + from pkg import origin_module_in_pkg + print(origin_module_in_pkg) + print(pkg.origin_module_in_pkg)""") self.mod1.write(code) - self._move(self.mod4, None, pkg4) - self.assertTrue(self.project.find_module("pkg2.pkg3.pkg4.mod4") is not None) + self._move(self.origin_module_in_pkg, None, pkg4) + self.assertTrue(self.project.find_module("pkg2.pkg3.pkg4.origin_module_in_pkg") is not None) expected = dedent("""\ - import pkg2.pkg3.pkg4.mod4 - from pkg2.pkg3.pkg4 import mod4 - print(mod4) - print(pkg2.pkg3.pkg4.mod4)""") + import pkg2.pkg3.pkg4.origin_module_in_pkg + from pkg2.pkg3.pkg4 import origin_module_in_pkg + print(origin_module_in_pkg) + print(pkg2.pkg3.pkg4.origin_module_in_pkg)""") self.assertEqual(expected, self.mod1.read()) def test_moving_modules_from_import_variable(self): @@ -669,13 +670,13 @@ def test_moving_modules_from_import_variable(self): pkg3 = testutils.create_package(self.project, "pkg3", pkg2) pkg4 = testutils.create_package(self.project, "pkg4", pkg3) code = dedent("""\ - from pkg.mod4 import foo + from pkg.origin_module_in_pkg import foo print(foo)""") self.mod1.write(code) - self._move(self.mod4, None, pkg4) - self.assertTrue(self.project.find_module("pkg2.pkg3.pkg4.mod4") is not None) + self._move(self.origin_module_in_pkg, None, pkg4) + self.assertTrue(self.project.find_module("pkg2.pkg3.pkg4.origin_module_in_pkg") is not None) expected = dedent("""\ - from pkg2.pkg3.pkg4.mod4 import foo + from pkg2.pkg3.pkg4.origin_module_in_pkg import foo print(foo)""") self.assertEqual(expected, self.mod1.read()) @@ -684,14 +685,14 @@ def test_moving_modules_normal_import(self): pkg3 = testutils.create_package(self.project, "pkg3", pkg2) pkg4 = testutils.create_package(self.project, "pkg4", pkg3) code = dedent("""\ - import pkg.mod4 - print(pkg.mod4)""") + import pkg.origin_module_in_pkg + print(pkg.origin_module_in_pkg)""") self.mod1.write(code) - self._move(self.mod4, None, pkg4) - self.assertTrue(self.project.find_module("pkg2.pkg3.pkg4.mod4") is not None) + self._move(self.origin_module_in_pkg, None, pkg4) + self.assertTrue(self.project.find_module("pkg2.pkg3.pkg4.origin_module_in_pkg") is not None) expected = dedent("""\ - import pkg2.pkg3.pkg4.mod4 - print(pkg2.pkg3.pkg4.mod4)""") + import pkg2.pkg3.pkg4.origin_module_in_pkg + print(pkg2.pkg3.pkg4.origin_module_in_pkg)""") self.assertEqual(expected, self.mod1.read()) def test_moving_package_with_from_and_normal_imports(self): @@ -737,29 +738,29 @@ def test_moving_package_and_retaining_blank_lines(self): code = dedent('''\ """Docstring followed by blank lines.""" - import pkg.mod4 + import pkg.origin_module_in_pkg - from pkg import mod4 + from pkg import origin_module_in_pkg from x import y from y import z from a import b from b import c - print(pkg.mod4) - print(mod4)''') + print(pkg.origin_module_in_pkg) + print(origin_module_in_pkg)''') self.mod1.write(code) - self._move(self.mod4, None, pkg2) + self._move(self.origin_module_in_pkg, None, pkg2) expected = dedent('''\ """Docstring followed by blank lines.""" - import pkg.pkg2.mod4 + import pkg.pkg2.origin_module_in_pkg from x import y from y import z from a import b from b import c - from pkg.pkg2 import mod4 - print(pkg.pkg2.mod4) - print(mod4)''') + from pkg.pkg2 import origin_module_in_pkg + print(pkg.pkg2.origin_module_in_pkg) + print(origin_module_in_pkg)''') self.assertEqual(expected, self.mod1.read()) def test_moving_functions_to_imported_module(self): @@ -839,38 +840,38 @@ def test_moving_module_and_star_imports(self): ) def test_moving_module_and_not_removing_blanks_after_imports(self): - self.mod4.write("a_var = 1") + self.origin_module_in_pkg.write("a_var = 1") self.mod2.write(dedent("""\ - from pkg import mod4 + from pkg import origin_module_in_pkg import os - print(mod4.a_var) + print(origin_module_in_pkg.a_var) """)) - mover = move.create_move(self.project, self.mod4) + mover = move.create_move(self.project, self.origin_module_in_pkg) mover.get_changes(self.project.root).do() self.assertEqual( dedent("""\ import os - import mod4 + import origin_module_in_pkg - print(mod4.a_var) + print(origin_module_in_pkg.a_var) """), self.mod2.read(), ) def test_moving_module_refactoring_and_nonexistent_destinations(self): - self.mod4.write("a_var = 1") + self.origin_module_in_pkg.write("a_var = 1") self.mod2.write(dedent("""\ - from pkg import mod4 + from pkg import origin_module_in_pkg import os - print(mod4.a_var) + print(origin_module_in_pkg.a_var) """)) with self.assertRaises(exceptions.RefactoringError): - mover = move.create_move(self.project, self.mod4) + mover = move.create_move(self.project, self.origin_module_in_pkg) mover.get_changes(None).do() def test_moving_methods_choosing_the_correct_class(self): From 4ff784e6d3ad20be376f24dc7961bc74d0017aaf Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 20:29:59 +1100 Subject: [PATCH 10/19] Disambiguated cases where pkg is always used as destination_pkg_root --- ropetest/refactor/movetest.py | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index 06a46884..ad25debf 100644 --- a/ropetest/refactor/movetest.py +++ b/ropetest/refactor/movetest.py @@ -20,6 +20,7 @@ def setUp(self): self.destination_module = testutils.create_module(self.project, "destination_module") self.origin_module_in_pkg = testutils.create_module(self.project, "origin_module_in_pkg", self.pkg) self.destination_module_in_pkg = testutils.create_module(self.project, "destination_module_in_pkg", self.pkg) + self.destination_pkg_root = testutils.create_package(self.project, "destination_pkg_root") def tearDown(self): testutils.remove_project(self.project) @@ -465,14 +466,14 @@ def test_moving_modules_into_package(self): print(mod1)""" ) self.origin_module.write(code) - self._move(self.origin_module, code.index("mod1") + 1, self.pkg) + self._move(self.origin_module, code.index("mod1") + 1, self.destination_pkg_root) expected = dedent("""\ - import pkg.mod1 - print(pkg.mod1)""" + import destination_pkg_root.mod1 + print(destination_pkg_root.mod1)""" ) self.assertEqual(expected, self.origin_module.read()) self.assertTrue( - not self.mod1.exists() and self.project.find_module("pkg.mod1") is not None + not self.mod1.exists() and self.project.find_module("destination_pkg_root.mod1") is not None ) def test_moving_modules_and_removing_out_of_date_imports(self): @@ -540,11 +541,11 @@ def foo(origin_module=0): import origin_module origin_module.foo(origin_module=1)""") self.mod2.write(code) - self._move(self.origin_module, None, self.pkg) + self._move(self.origin_module, None, self.destination_pkg_root) moved = self.project.find_module("mod2") expected = dedent("""\ - import pkg.origin_module - pkg.origin_module.foo(origin_module=1)""") + import destination_pkg_root.origin_module + destination_pkg_root.origin_module.foo(origin_module=1)""") self.assertEqual(expected, moved.read()) def test_moving_packages(self): @@ -571,12 +572,12 @@ def test_moving_modules_with_self_imports(self): self.origin_module.write(dedent("""\ import mod1 """)) - self._move(self.origin_module, self.origin_module.read().index("mod1") + 1, self.pkg) - moved = self.project.find_module("pkg.mod1") + self._move(self.origin_module, self.origin_module.read().index("mod1") + 1, self.destination_pkg_root) + moved = self.project.find_module("destination_pkg_root.mod1") self.assertEqual( dedent("""\ - import pkg.mod1 - print(pkg.mod1) + import destination_pkg_root.mod1 + print(destination_pkg_root.mod1) """), moved.read(), ) @@ -786,13 +787,13 @@ def test_moving_resources_using_move_module_refactoring(self): my_var = origin_module.a_var """)) mover = move.create_move(self.project, self.origin_module) - mover.get_changes(self.pkg).do() + mover.get_changes(self.destination_pkg_root).do() expected = dedent("""\ - import pkg.origin_module - my_var = pkg.origin_module.a_var + import destination_pkg_root.origin_module + my_var = destination_pkg_root.origin_module.a_var """) self.assertEqual(expected, self.mod2.read()) - self.assertTrue(self.pkg.get_child("origin_module.py") is not None) + self.assertTrue(self.destination_pkg_root.get_child("origin_module.py") is not None) def test_moving_resources_using_move_module_for_packages(self): self.mod1.write(dedent("""\ @@ -830,10 +831,10 @@ def test_moving_module_and_star_imports(self): a = a_var """)) mover = move.create_move(self.project, self.origin_module) - mover.get_changes(self.pkg).do() + mover.get_changes(self.destination_pkg_root).do() self.assertEqual( dedent("""\ - from pkg.origin_module import * + from destination_pkg_root.origin_module import * a = a_var """), self.mod2.read(), From 26a929ee2ca509b5f456b6e116506924c171772e Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 20:30:54 +1100 Subject: [PATCH 11/19] Disambiguated cases where pkg2 is always used as destination_pkg_root --- ropetest/refactor/movetest.py | 66 ++++++++++++++++------------------- 1 file changed, 30 insertions(+), 36 deletions(-) diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index ad25debf..ada09993 100644 --- a/ropetest/refactor/movetest.py +++ b/ropetest/refactor/movetest.py @@ -549,19 +549,18 @@ def foo(origin_module=0): self.assertEqual(expected, moved.read()) def test_moving_packages(self): - pkg2 = testutils.create_package(self.project, "pkg2") code = dedent("""\ import pkg.mod4 print(pkg.mod4)""") self.origin_module.write(code) - self._move(self.origin_module, code.index("pkg") + 1, pkg2) + self._move(self.origin_module, code.index("pkg") + 1, self.destination_pkg_root) self.assertFalse(self.pkg.exists()) - self.assertTrue(self.project.find_module("pkg2.pkg.mod4") is not None) - self.assertTrue(self.project.find_module("pkg2.pkg.mod4") is not None) - self.assertTrue(self.project.find_module("pkg2.pkg.mod5") is not None) + self.assertTrue(self.project.find_module("destination_pkg_root.pkg.mod4") is not None) + self.assertTrue(self.project.find_module("destination_pkg_root.pkg.mod4") is not None) + self.assertTrue(self.project.find_module("destination_pkg_root.pkg.mod5") is not None) expected = dedent("""\ - import pkg2.pkg.mod4 - print(pkg2.pkg.mod4)""") + import destination_pkg_root.pkg.mod4 + print(destination_pkg_root.pkg.mod4)""") self.assertEqual(expected, self.origin_module.read()) def test_moving_modules_with_self_imports(self): @@ -583,17 +582,16 @@ def test_moving_modules_with_self_imports(self): ) def test_moving_modules_with_from_imports(self): - pkg2 = testutils.create_package(self.project, "pkg2") code = dedent("""\ from pkg import mod4 print(mod4)""") self.origin_module.write(code) - self._move(self.origin_module, code.index("pkg") + 1, pkg2) + self._move(self.origin_module, code.index("pkg") + 1, self.destination_pkg_root) self.assertFalse(self.pkg.exists()) - self.assertTrue(self.project.find_module("pkg2.pkg.mod4") is not None) - self.assertTrue(self.project.find_module("pkg2.pkg.mod5") is not None) + self.assertTrue(self.project.find_module("destination_pkg_root.pkg.mod4") is not None) + self.assertTrue(self.project.find_module("destination_pkg_root.pkg.mod5") is not None) expected = dedent("""\ - from pkg2.pkg import mod4 + from destination_pkg_root.pkg import mod4 print(mod4)""") self.assertEqual(expected, self.origin_module.read()) @@ -697,40 +695,38 @@ def test_moving_modules_normal_import(self): self.assertEqual(expected, self.mod1.read()) def test_moving_package_with_from_and_normal_imports(self): - pkg2 = testutils.create_package(self.project, "pkg2") code = dedent("""\ from pkg import mod4 import pkg.mod4 print(pkg.mod4) print(mod4)""") self.origin_module.write(code) - self._move(self.origin_module, code.index("pkg") + 1, pkg2) + self._move(self.origin_module, code.index("pkg") + 1, self.destination_pkg_root) self.assertFalse(self.pkg.exists()) - self.assertTrue(self.project.find_module("pkg2.pkg.mod4") is not None) - self.assertTrue(self.project.find_module("pkg2.pkg.mod5") is not None) + self.assertTrue(self.project.find_module("destination_pkg_root.pkg.mod4") is not None) + self.assertTrue(self.project.find_module("destination_pkg_root.pkg.mod5") is not None) expected = dedent("""\ - from pkg2.pkg import mod4 - import pkg2.pkg.mod4 - print(pkg2.pkg.mod4) + from destination_pkg_root.pkg import mod4 + import destination_pkg_root.pkg.mod4 + print(destination_pkg_root.pkg.mod4) print(mod4)""") self.assertEqual(expected, self.origin_module.read()) def test_moving_package_with_from_and_normal_imports2(self): - pkg2 = testutils.create_package(self.project, "pkg2") code = dedent("""\ import pkg.mod4 from pkg import mod4 print(pkg.mod4) print(mod4)""") self.origin_module.write(code) - self._move(self.origin_module, code.index("pkg") + 1, pkg2) + self._move(self.origin_module, code.index("pkg") + 1, self.destination_pkg_root) self.assertFalse(self.pkg.exists()) - self.assertTrue(self.project.find_module("pkg2.pkg.mod4") is not None) - self.assertTrue(self.project.find_module("pkg2.pkg.mod5") is not None) + self.assertTrue(self.project.find_module("destination_pkg_root.pkg.mod4") is not None) + self.assertTrue(self.project.find_module("destination_pkg_root.pkg.mod5") is not None) expected = dedent("""\ - import pkg2.pkg.mod4 - from pkg2.pkg import mod4 - print(pkg2.pkg.mod4) + import destination_pkg_root.pkg.mod4 + from destination_pkg_root.pkg import mod4 + print(destination_pkg_root.pkg.mod4) print(mod4)""") self.assertEqual(expected, self.origin_module.read()) @@ -799,30 +795,28 @@ def test_moving_resources_using_move_module_for_packages(self): self.mod1.write(dedent("""\ import pkg my_pkg = pkg""")) - pkg2 = testutils.create_package(self.project, "pkg2") mover = move.create_move(self.project, self.pkg) - mover.get_changes(pkg2).do() + mover.get_changes(self.destination_pkg_root).do() expected = dedent("""\ - import pkg2.pkg - my_pkg = pkg2.pkg""") + import destination_pkg_root.pkg + my_pkg = destination_pkg_root.pkg""") self.assertEqual(expected, self.mod1.read()) - self.assertTrue(pkg2.get_child("pkg") is not None) + self.assertTrue(self.destination_pkg_root.get_child("pkg") is not None) def test_moving_resources_using_move_module_for_init_dot_py(self): self.mod1.write(dedent("""\ import pkg my_pkg = pkg""")) - pkg2 = testutils.create_package(self.project, "pkg2") init = self.pkg.get_child("__init__.py") mover = move.create_move(self.project, init) - mover.get_changes(pkg2).do() + mover.get_changes(self.destination_pkg_root).do() self.assertEqual( dedent("""\ - import pkg2.pkg - my_pkg = pkg2.pkg"""), + import destination_pkg_root.pkg + my_pkg = destination_pkg_root.pkg"""), self.mod1.read(), ) - self.assertTrue(pkg2.get_child("pkg") is not None) + self.assertTrue(self.destination_pkg_root.get_child("pkg") is not None) def test_moving_module_and_star_imports(self): self.origin_module.write("a_var = 1") From b7dfa9673ed9cc46fff85577733a639560708734 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 20:49:06 +1100 Subject: [PATCH 12/19] Add explicit exception messages in assertRaisesRegex --- ropetest/refactor/movetest.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index ada09993..6de48408 100644 --- a/ropetest/refactor/movetest.py +++ b/ropetest/refactor/movetest.py @@ -865,7 +865,10 @@ def test_moving_module_refactoring_and_nonexistent_destinations(self): print(origin_module_in_pkg.a_var) """)) - with self.assertRaises(exceptions.RefactoringError): + with self.assertRaisesRegex( + exceptions.RefactoringError, + r"Move destination for modules should be packages.", + ): mover = move.create_move(self.project, self.origin_module_in_pkg) mover.get_changes(None).do() @@ -1100,7 +1103,10 @@ def a_method(self): return 1 """) self.origin_module.write(code) - with self.assertRaises(exceptions.RefactoringError): + with self.assertRaisesRegex( + exceptions.RefactoringError, + r"Destination attribute not found", + ): mover = move.create_move(self.project, self.origin_module, code.index("a_method")) mover.get_changes("x", "new_method") @@ -1112,7 +1118,10 @@ def a_method(self): return 1 """) self.origin_module.write(code) - with self.assertRaises(exceptions.RefactoringError): + with self.assertRaisesRegex( + exceptions.RefactoringError, + r"Unknown class type for attribute ", + ): mover = move.create_move(self.project, self.origin_module, code.index("a_method")) mover.get_changes("attr", "new_method") From c45231d002ae17480ea5dbb462e3802d033458ca Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 21:20:31 +1100 Subject: [PATCH 13/19] Refactor movetest.py to use _move()/_move_to_attr() more consistently --- ropetest/refactor/movetest.py | 67 ++++++++++++++--------------------- 1 file changed, 26 insertions(+), 41 deletions(-) diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index 6de48408..918b4ea3 100644 --- a/ropetest/refactor/movetest.py +++ b/ropetest/refactor/movetest.py @@ -27,9 +27,13 @@ def tearDown(self): super().tearDown() def _move(self, resource, offset, dest_resource): - changes = move.create_move(self.project, resource, offset).get_changes( - dest_resource - ) + mover = move.create_move(self.project, resource, offset) + changes = mover.get_changes(dest_resource) + self.project.do(changes) + + def _move_to_attr(self, resource, offset, dest_attr, *, new_name): + mover = move.create_move(self.project, resource, offset) + changes = mover.get_changes(dest_attr, new_name=new_name) self.project.do(changes) def test_move_constant(self): @@ -782,8 +786,9 @@ def test_moving_resources_using_move_module_refactoring(self): import origin_module my_var = origin_module.a_var """)) - mover = move.create_move(self.project, self.origin_module) - mover.get_changes(self.destination_pkg_root).do() + resource = self.origin_module + dest_resource = self.destination_pkg_root + self._move(resource, None, dest_resource) expected = dedent("""\ import destination_pkg_root.origin_module my_var = destination_pkg_root.origin_module.a_var @@ -795,8 +800,7 @@ def test_moving_resources_using_move_module_for_packages(self): self.mod1.write(dedent("""\ import pkg my_pkg = pkg""")) - mover = move.create_move(self.project, self.pkg) - mover.get_changes(self.destination_pkg_root).do() + self._move(self.pkg, None, self.destination_pkg_root) expected = dedent("""\ import destination_pkg_root.pkg my_pkg = destination_pkg_root.pkg""") @@ -808,8 +812,7 @@ def test_moving_resources_using_move_module_for_init_dot_py(self): import pkg my_pkg = pkg""")) init = self.pkg.get_child("__init__.py") - mover = move.create_move(self.project, init) - mover.get_changes(self.destination_pkg_root).do() + self._move(init, None, self.destination_pkg_root) self.assertEqual( dedent("""\ import destination_pkg_root.pkg @@ -824,8 +827,7 @@ def test_moving_module_and_star_imports(self): from origin_module import * a = a_var """)) - mover = move.create_move(self.project, self.origin_module) - mover.get_changes(self.destination_pkg_root).do() + self._move(self.origin_module, None, self.destination_pkg_root) self.assertEqual( dedent("""\ from destination_pkg_root.origin_module import * @@ -843,8 +845,7 @@ def test_moving_module_and_not_removing_blanks_after_imports(self): print(origin_module_in_pkg.a_var) """)) - mover = move.create_move(self.project, self.origin_module_in_pkg) - mover.get_changes(self.project.root).do() + self._move(self.origin_module_in_pkg, None, self.project.root) self.assertEqual( dedent("""\ import os @@ -869,8 +870,7 @@ def test_moving_module_refactoring_and_nonexistent_destinations(self): exceptions.RefactoringError, r"Move destination for modules should be packages.", ): - mover = move.create_move(self.project, self.origin_module_in_pkg) - mover.get_changes(None).do() + self._move(self.origin_module_in_pkg, None, None) def test_moving_methods_choosing_the_correct_class(self): code = dedent("""\ @@ -1028,8 +1028,7 @@ def a_method(self): return 1 """) self.origin_module.write(code) - mover = move.create_move(self.project, self.origin_module, code.index("a_method")) - mover.get_changes("attr", "new_method").do() + self._move_to_attr(self.origin_module, code.index("a_method"), "attr", new_name="new_method") expected = dedent("""\ import mod2 @@ -1054,8 +1053,7 @@ def a_method(self): return 1 """) self.origin_module.write(code) - mover = move.create_move(self.project, self.origin_module, code.index("a_method")) - mover.get_changes("attr", "new_method").do() + self._move_to_attr(self.origin_module, code.index("a_method"), "attr", new_name="new_method") expected = dedent("""\ class B(object): var = 1 @@ -1077,8 +1075,7 @@ def a_method(self): return 1 """) self.origin_module.write(code) - mover = move.create_move(self.project, self.origin_module, code.index("a_method")) - mover.get_changes("attr", "new_method").do() + self._move_to_attr(self.origin_module, code.index("a_method"), "attr", new_name="new_method") self.assertEqual( dedent("""\ class B(object): @@ -1107,8 +1104,7 @@ def a_method(self): exceptions.RefactoringError, r"Destination attribute not found", ): - mover = move.create_move(self.project, self.origin_module, code.index("a_method")) - mover.get_changes("x", "new_method") + self._move_to_attr(self.origin_module, code.index("a_method"), "x", new_name="new_method") def test_unknown_attribute_type(self): code = dedent("""\ @@ -1122,8 +1118,7 @@ def a_method(self): exceptions.RefactoringError, r"Unknown class type for attribute ", ): - mover = move.create_move(self.project, self.origin_module, code.index("a_method")) - mover.get_changes("attr", "new_method") + self._move_to_attr(self.origin_module, code.index("a_method"), "attr", new_name="new_method") def test_moving_methods_and_moving_used_imports(self): self.mod2.write(dedent("""\ @@ -1140,8 +1135,7 @@ def a_method(self): return sys.version """) self.origin_module.write(code) - mover = move.create_move(self.project, self.origin_module, code.index("a_method")) - mover.get_changes("attr", "new_method").do() + self._move_to_attr(self.origin_module, code.index("a_method"), "attr", new_name="new_method") code = dedent("""\ import sys class B(object): @@ -1167,8 +1161,7 @@ def a_method(self): return 1 """) self.origin_module.write(code) - mover = move.create_move(self.project, self.origin_module, code.index("a_method")) - mover.get_changes("attr", "new_method").do() + self._move_to_attr(self.origin_module, code.index("a_method"), "attr", new_name="new_method") expected = dedent("""\ class B(object): @@ -1191,8 +1184,7 @@ def a_method(self, p): return p """) self.origin_module.write(code) - mover = move.create_move(self.project, self.origin_module, code.index("a_method")) - mover.get_changes("attr", "new_method").do() + self._move_to_attr(self.origin_module, code.index("a_method"), "attr", new_name="new_method") expected1 = dedent("""\ import mod2 @@ -1225,10 +1217,7 @@ def f(): """ ''')) - mover = move.create_move( - self.project, self.origin_module, self.origin_module.read().index("f()") + 1 - ) - self.project.do(mover.get_changes(self.destination_module)) + self._move(self.origin_module, self.origin_module.read().index("f()") + 1, self.destination_module) self.assertEqual( dedent('''\ """doc @@ -1262,10 +1251,7 @@ def f(): """ ''')) - mover = move.create_move( - self.project, self.origin_module, self.origin_module.read().index("f()") + 1 - ) - self.project.do(mover.get_changes(self.destination_module)) + self._move(self.origin_module, self.origin_module.read().index("f()") + 1, self.destination_module) expected = dedent('''\ """doc @@ -1290,8 +1276,7 @@ def f(): r = f() ''') self.origin_module.write(code) - mover = move.create_move(self.project, self.origin_module, code.index("f()") + 1) - self.project.do(mover.get_changes(self.destination_module)) + self._move(self.origin_module, code.index("f()") + 1, self.destination_module) expected = dedent('''\ import destination_module s = """\\ From f5799acee4b7461a3acd5d0387a1b9e0506b2158 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 21:50:50 +1100 Subject: [PATCH 14/19] Add type checking to movetest.py --- rope/contrib/generate.py | 6 +- ropetest/refactor/movetest.py | 190 ++++++++++++++++++++-------------- 2 files changed, 115 insertions(+), 81 deletions(-) diff --git a/rope/contrib/generate.py b/rope/contrib/generate.py index a3391565..06a8335f 100644 --- a/rope/contrib/generate.py +++ b/rope/contrib/generate.py @@ -18,7 +18,7 @@ from typing import Literal, Optional from rope.base.project import Project - from rope.base.resources import Resource + from rope.base.resources import Resource, File, Folder GenerateKind = Literal[ "variable", @@ -51,7 +51,7 @@ def create_generate( return generate(project, resource, offset, goal_resource=goal_resource) -def create_module(project, name, sourcefolder=None): +def create_module(project, name, sourcefolder=None) -> File: """Creates a module and returns a `rope.base.resources.File`""" if sourcefolder is None: sourcefolder = project.root @@ -62,7 +62,7 @@ def create_module(project, name, sourcefolder=None): return parent.create_file(packages[-1] + ".py") -def create_package(project, name, sourcefolder=None): +def create_package(project, name, sourcefolder=None) -> Folder: """Creates a package and returns a `rope.base.resources.Folder`""" if sourcefolder is None: sourcefolder = project.root diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index 918b4ea3..011ad499 100644 --- a/ropetest/refactor/movetest.py +++ b/ropetest/refactor/movetest.py @@ -1,13 +1,32 @@ +from __future__ import annotations + import unittest from textwrap import dedent +from typing import TYPE_CHECKING, Union from rope.base import exceptions from rope.refactor import move from ropetest import testutils +if TYPE_CHECKING: + from rope.base import resources, project + class MoveRefactoringTest(unittest.TestCase): - def setUp(self): + project: project.Project + mod1: resources.File + mod2: resources.File + mod3: resources.File + pkg: resources.Folder + mod4: resources.File + mod5: resources.File + origin_module: resources.File + destination_module: resources.File + origin_module_in_pkg: resources.File + destination_module_in_pkg: resources.File + destination_pkg_root: resources.Folder + + def setUp(self) -> None: super().setUp() self.project = testutils.sample_project() self.mod1 = testutils.create_module(self.project, "mod1") @@ -26,41 +45,53 @@ def tearDown(self): testutils.remove_project(self.project) super().tearDown() - def _move(self, resource, offset, dest_resource): + def _move( + self, + resource: Union[resources.File, resources.Folder], + offset: Union[int, None], + dest_resource: Union[str, resources.File, resources.Folder], + ): mover = move.create_move(self.project, resource, offset) changes = mover.get_changes(dest_resource) self.project.do(changes) - def _move_to_attr(self, resource, offset, dest_attr, *, new_name): + def _move_to_attr( + self, + resource: Union[resources.File, resources.Folder], + offset: Union[int, None], + dest_attr: Union[str, resources.File, resources.Folder], + *, + new_name: str, + ): mover = move.create_move(self.project, resource, offset) changes = mover.get_changes(dest_attr, new_name=new_name) self.project.do(changes) - def test_move_constant(self): + def test_move_constant(self) -> None: self.origin_module.write("foo = 123\n") self._move(self.origin_module, self.origin_module.read().index("foo") + 1, self.destination_module) self.assertEqual("", self.origin_module.read()) self.assertEqual("foo = 123\n", self.destination_module.read()) - def test_move_constant_2(self): + 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) self.assertEqual("bar = 321\n", self.origin_module.read()) self.assertEqual("foo = 123\n", self.destination_module.read()) - def test_move_target_is_module_name(self): + def test_move_target_is_module_name(self) -> None: self.origin_module.write("foo = 123\n") self._move(self.origin_module, self.origin_module.read().index("foo") + 1, "destination_module") self.assertEqual("", self.origin_module.read()) self.assertEqual("foo = 123\n", self.destination_module.read()) - def test_move_target_is_package_name(self): + def test_move_target_is_package_name(self) -> None: self.origin_module.write("foo = 123\n") self._move(self.origin_module, self.origin_module.read().index("foo") + 1, "pkg.destination_module_in_pkg") self.assertEqual("", self.origin_module.read()) self.assertEqual("foo = 123\n", self.destination_module_in_pkg.read()) - def test_move_constant_multiline(self): + def test_move_constant_multiline(self) -> None: self.origin_module.write(dedent("""\ foo = ( 123 @@ -77,7 +108,7 @@ def test_move_constant_multiline(self): self.destination_module.read(), ) - def test_move_constant_multiple_statements(self): + def test_move_constant_multiple_statements(self) -> None: self.origin_module.write(dedent("""\ foo = 123 foo += 3 @@ -94,7 +125,7 @@ def test_move_constant_multiple_statements(self): ) self.assertEqual("foo = 123\n", self.destination_module.read()) - def test_simple_moving(self): + def test_simple_moving(self) -> None: """Move a global class definition""" self.origin_module.write(dedent("""\ class AClass(object): @@ -110,7 +141,7 @@ class AClass(object): self.destination_module.read(), ) - def test_moving_with_comment_prefix(self): + def test_moving_with_comment_prefix(self) -> None: """Comments above the moved class are moved to the destination module""" self.origin_module.write(dedent("""\ a = 1 @@ -131,7 +162,7 @@ class AClass(object): self.destination_module.read(), ) - def test_moving_with_comment_prefix_imports(self): + def test_moving_with_comment_prefix_imports(self) -> None: self.origin_module.write(dedent("""\ import foo a = 1 @@ -155,7 +186,7 @@ class AClass(foo.FooClass): self.destination_module.read(), ) - def test_changing_other_modules_replacing_normal_imports(self): + def test_changing_other_modules_replacing_normal_imports(self) -> None: """ When moving a class from origin_module to destination_module, references to the class in mod3 is updated to point to @@ -178,7 +209,7 @@ class AClass(object): self.mod3.read(), ) - def test_changing_other_modules_adding_normal_imports(self): + def test_changing_other_modules_adding_normal_imports(self) -> None: self.origin_module.write(dedent("""\ class AClass(object): pass @@ -199,7 +230,7 @@ def a_function(): self.mod3.read(), ) - def test_adding_imports_prefer_from_module(self): + def test_adding_imports_prefer_from_module(self) -> None: self.project.prefs["prefer_module_from_imports"] = True self.origin_module.write(dedent("""\ class AClass(object): @@ -222,7 +253,7 @@ def a_function(): self.mod3.read(), ) - def test_adding_imports_noprefer_from_module(self): + def test_adding_imports_noprefer_from_module(self) -> None: self.project.prefs["prefer_module_from_imports"] = False self.origin_module.write(dedent("""\ class AClass(object): @@ -245,7 +276,7 @@ def a_function(): self.mod3.read(), ) - def test_adding_imports_prefer_from_module_top_level_module(self): + def test_adding_imports_prefer_from_module_top_level_module(self) -> None: self.project.prefs["prefer_module_from_imports"] = True self.origin_module.write(dedent("""\ class AClass(object): @@ -267,7 +298,7 @@ def a_function(): self.mod3.read(), ) - def test_changing_other_modules_removing_from_imports(self): + def test_changing_other_modules_removing_from_imports(self) -> None: self.origin_module.write(dedent("""\ class AClass(object): pass @@ -285,7 +316,7 @@ class AClass(object): self.mod3.read(), ) - def test_changing_source_module(self): + def test_changing_source_module(self) -> None: """ Add import statements to the source module as the moved class now lives in destination_module. @@ -304,7 +335,7 @@ class AClass(object): self.origin_module.read(), ) - def test_changing_destination_module(self): + def test_changing_destination_module(self) -> None: """ Remove import statements in the destination module as the moved class can now be referenced from destination_module without import. @@ -327,7 +358,7 @@ class AClass(object): self.destination_module.read(), ) - def test_folder_destination(self): + def test_folder_destination(self) -> None: folder = self.project.root.create_folder("folder") self.origin_module.write(dedent("""\ class AClass(object): @@ -339,7 +370,7 @@ class AClass(object): ) as e: self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, folder) - def test_raising_exception_for_moving_non_global_elements(self): + def test_raising_exception_for_moving_non_global_elements(self) -> None: self.origin_module.write(dedent("""\ def a_func(): class AClass(object): @@ -351,7 +382,7 @@ class AClass(object): ): self._move(self.origin_module, self.origin_module.read().index("AClass") + 1, self.destination_module) - def test_raising_an_exception_for_moving_non_global_variable(self): + def test_raising_an_exception_for_moving_non_global_variable(self) -> None: code = dedent("""\ class TestClass: CONSTANT = 5 @@ -365,7 +396,7 @@ class TestClass: self.project, self.origin_module, code.index("CONSTANT") + 1 ) - def test_raising_exception_for_moving_glob_elements_to_the_same_module(self): + def test_raising_exception_for_moving_glob_elements_to_the_same_module(self) -> None: self.origin_module.write(dedent("""\ def a_func(): pass @@ -376,7 +407,7 @@ def a_func(): ): self._move(self.origin_module, self.origin_module.read().index("a_func"), self.origin_module) - def test_moving_used_imports_to_destination_module(self): + def test_moving_used_imports_to_destination_module(self) -> None: """ Add import statements for imported references used by the moved function to the destination module. @@ -400,7 +431,7 @@ def a_func(): """) self.assertEqual(expected, self.destination_module.read()) - def test_moving_used_names_to_destination_module2(self): + def test_moving_used_names_to_destination_module2(self) -> None: """ Add import statements for references to globals in the source module used by the moved function to the destination module. @@ -427,7 +458,7 @@ def a_func(): """) self.assertEqual(expected, self.destination_module.read()) - def test_moving_used_underlined_names_to_destination_module(self): + def test_moving_used_underlined_names_to_destination_module(self) -> None: code = dedent("""\ _var = 10 def a_func(): @@ -444,7 +475,7 @@ def a_func(): """) self.assertEqual(expected, self.destination_module.read()) - def test_moving_and_used_relative_imports(self): + def test_moving_and_used_relative_imports(self) -> None: """Move global function where the source module is in a package""" code = dedent("""\ import mod5 @@ -463,7 +494,7 @@ def a_func(): self.assertEqual(expected, self.destination_module.read()) self.assertEqual("", self.origin_module_in_pkg.read()) - def test_moving_modules_into_package(self): + def test_moving_modules_into_package(self) -> None: """Move global function where the destination module is in a package""" code = dedent("""\ import mod1 @@ -480,7 +511,7 @@ def test_moving_modules_into_package(self): not self.mod1.exists() and self.project.find_module("destination_pkg_root.mod1") is not None ) - def test_moving_modules_and_removing_out_of_date_imports(self): + def test_moving_modules_and_removing_out_of_date_imports(self) -> None: code = dedent("""\ import pkg.mod4 print(pkg.mod4)""") @@ -492,7 +523,7 @@ def test_moving_modules_and_removing_out_of_date_imports(self): self.assertEqual(expected, self.origin_module.read()) self.assertTrue(self.project.find_module("mod4") is not None) - def test_moving_modules_and_removing_out_of_date_froms(self): + def test_moving_modules_and_removing_out_of_date_froms(self) -> None: code = dedent("""\ from pkg import mod4 print(mod4)""") @@ -506,7 +537,7 @@ def test_moving_modules_and_removing_out_of_date_froms(self): self.origin_module.read(), ) - def test_moving_modules_and_removing_out_of_date_froms2(self): + def test_moving_modules_and_removing_out_of_date_froms2(self) -> None: self.mod4.write("a_var = 10") code = dedent("""\ from pkg.mod4 import a_var @@ -520,7 +551,7 @@ def test_moving_modules_and_removing_out_of_date_froms2(self): """) self.assertEqual(expected, self.origin_module.read()) - def test_moving_modules_and_relative_import(self): + def test_moving_modules_and_relative_import(self) -> None: self.mod4.write(dedent("""\ import mod5 print(mod5) @@ -531,13 +562,14 @@ def test_moving_modules_and_relative_import(self): self.origin_module.write(code) self._move(self.origin_module, code.index("mod4") + 1, self.project.root) moved = self.project.find_module("mod4") + assert moved expected = dedent("""\ import pkg.mod5 print(pkg.mod5) """) self.assertEqual(expected, moved.read()) - def test_moving_module_kwarg_same_name_as_old(self): + def test_moving_module_kwarg_same_name_as_old(self) -> None: self.origin_module.write(dedent("""\ def foo(origin_module=0): pass""")) @@ -547,12 +579,13 @@ def foo(origin_module=0): self.mod2.write(code) self._move(self.origin_module, None, self.destination_pkg_root) moved = self.project.find_module("mod2") + assert moved expected = dedent("""\ import destination_pkg_root.origin_module destination_pkg_root.origin_module.foo(origin_module=1)""") self.assertEqual(expected, moved.read()) - def test_moving_packages(self): + def test_moving_packages(self) -> None: code = dedent("""\ import pkg.mod4 print(pkg.mod4)""") @@ -567,7 +600,7 @@ def test_moving_packages(self): print(destination_pkg_root.pkg.mod4)""") self.assertEqual(expected, self.origin_module.read()) - def test_moving_modules_with_self_imports(self): + def test_moving_modules_with_self_imports(self) -> None: self.mod1.write(dedent("""\ import mod1 print(mod1) @@ -577,6 +610,7 @@ def test_moving_modules_with_self_imports(self): """)) self._move(self.origin_module, self.origin_module.read().index("mod1") + 1, self.destination_pkg_root) moved = self.project.find_module("destination_pkg_root.mod1") + assert moved self.assertEqual( dedent("""\ import destination_pkg_root.mod1 @@ -585,7 +619,7 @@ def test_moving_modules_with_self_imports(self): moved.read(), ) - def test_moving_modules_with_from_imports(self): + def test_moving_modules_with_from_imports(self) -> None: code = dedent("""\ from pkg import mod4 print(mod4)""") @@ -599,7 +633,7 @@ def test_moving_modules_with_from_imports(self): print(mod4)""") self.assertEqual(expected, self.origin_module.read()) - def test_moving_modules_with_from_import(self): + def test_moving_modules_with_from_import(self) -> None: pkg2 = testutils.create_package(self.project, "pkg2") pkg3 = testutils.create_package(self.project, "pkg3", pkg2) pkg4 = testutils.create_package(self.project, "pkg4", pkg3) @@ -614,7 +648,7 @@ def test_moving_modules_with_from_import(self): print(origin_module_in_pkg)""") self.assertEqual(expected, self.mod1.read()) - def test_moving_modules_with_multi_from_imports(self): + def test_moving_modules_with_multi_from_imports(self) -> None: pkg2 = testutils.create_package(self.project, "pkg2") pkg3 = testutils.create_package(self.project, "pkg3", pkg2) pkg4 = testutils.create_package(self.project, "pkg4", pkg3) @@ -630,7 +664,7 @@ def test_moving_modules_with_multi_from_imports(self): print(origin_module_in_pkg)""") self.assertEqual(expected, self.mod1.read()) - def test_moving_modules_with_from_and_normal_imports(self): + def test_moving_modules_with_from_and_normal_imports(self) -> None: pkg2 = testutils.create_package(self.project, "pkg2") pkg3 = testutils.create_package(self.project, "pkg3", pkg2) pkg4 = testutils.create_package(self.project, "pkg4", pkg3) @@ -649,7 +683,7 @@ def test_moving_modules_with_from_and_normal_imports(self): print(pkg2.pkg3.pkg4.origin_module_in_pkg)""") self.assertEqual(expected, self.mod1.read()) - def test_moving_modules_with_normal_and_from_imports(self): + def test_moving_modules_with_normal_and_from_imports(self) -> None: pkg2 = testutils.create_package(self.project, "pkg2") pkg3 = testutils.create_package(self.project, "pkg3", pkg2) pkg4 = testutils.create_package(self.project, "pkg4", pkg3) @@ -668,7 +702,7 @@ def test_moving_modules_with_normal_and_from_imports(self): print(pkg2.pkg3.pkg4.origin_module_in_pkg)""") self.assertEqual(expected, self.mod1.read()) - def test_moving_modules_from_import_variable(self): + def test_moving_modules_from_import_variable(self) -> None: pkg2 = testutils.create_package(self.project, "pkg2") pkg3 = testutils.create_package(self.project, "pkg3", pkg2) pkg4 = testutils.create_package(self.project, "pkg4", pkg3) @@ -683,7 +717,7 @@ def test_moving_modules_from_import_variable(self): print(foo)""") self.assertEqual(expected, self.mod1.read()) - def test_moving_modules_normal_import(self): + def test_moving_modules_normal_import(self) -> None: pkg2 = testutils.create_package(self.project, "pkg2") pkg3 = testutils.create_package(self.project, "pkg3", pkg2) pkg4 = testutils.create_package(self.project, "pkg4", pkg3) @@ -698,7 +732,7 @@ def test_moving_modules_normal_import(self): print(pkg2.pkg3.pkg4.origin_module_in_pkg)""") self.assertEqual(expected, self.mod1.read()) - def test_moving_package_with_from_and_normal_imports(self): + def test_moving_package_with_from_and_normal_imports(self) -> None: code = dedent("""\ from pkg import mod4 import pkg.mod4 @@ -716,7 +750,7 @@ def test_moving_package_with_from_and_normal_imports(self): print(mod4)""") self.assertEqual(expected, self.origin_module.read()) - def test_moving_package_with_from_and_normal_imports2(self): + def test_moving_package_with_from_and_normal_imports2(self) -> None: code = dedent("""\ import pkg.mod4 from pkg import mod4 @@ -734,7 +768,7 @@ def test_moving_package_with_from_and_normal_imports2(self): print(mod4)""") self.assertEqual(expected, self.origin_module.read()) - def test_moving_package_and_retaining_blank_lines(self): + def test_moving_package_and_retaining_blank_lines(self) -> None: pkg2 = testutils.create_package(self.project, "pkg2", self.pkg) code = dedent('''\ """Docstring followed by blank lines.""" @@ -764,7 +798,7 @@ def test_moving_package_and_retaining_blank_lines(self): print(origin_module_in_pkg)''') self.assertEqual(expected, self.mod1.read()) - def test_moving_functions_to_imported_module(self): + def test_moving_functions_to_imported_module(self) -> None: code = dedent("""\ import destination_module def a_func(): @@ -780,7 +814,7 @@ def a_func(): """) self.assertEqual(expected, self.destination_module.read()) - def test_moving_resources_using_move_module_refactoring(self): + def test_moving_resources_using_move_module_refactoring(self) -> None: self.origin_module.write("a_var = 1") self.mod2.write(dedent("""\ import origin_module @@ -796,7 +830,7 @@ def test_moving_resources_using_move_module_refactoring(self): self.assertEqual(expected, self.mod2.read()) self.assertTrue(self.destination_pkg_root.get_child("origin_module.py") is not None) - def test_moving_resources_using_move_module_for_packages(self): + def test_moving_resources_using_move_module_for_packages(self) -> None: self.mod1.write(dedent("""\ import pkg my_pkg = pkg""")) @@ -807,7 +841,7 @@ def test_moving_resources_using_move_module_for_packages(self): self.assertEqual(expected, self.mod1.read()) self.assertTrue(self.destination_pkg_root.get_child("pkg") is not None) - def test_moving_resources_using_move_module_for_init_dot_py(self): + def test_moving_resources_using_move_module_for_init_dot_py(self) -> None: self.mod1.write(dedent("""\ import pkg my_pkg = pkg""")) @@ -821,7 +855,7 @@ def test_moving_resources_using_move_module_for_init_dot_py(self): ) self.assertTrue(self.destination_pkg_root.get_child("pkg") is not None) - def test_moving_module_and_star_imports(self): + def test_moving_module_and_star_imports(self) -> None: self.origin_module.write("a_var = 1") self.mod2.write(dedent("""\ from origin_module import * @@ -836,7 +870,7 @@ def test_moving_module_and_star_imports(self): self.mod2.read(), ) - def test_moving_module_and_not_removing_blanks_after_imports(self): + def test_moving_module_and_not_removing_blanks_after_imports(self) -> None: self.origin_module_in_pkg.write("a_var = 1") self.mod2.write(dedent("""\ from pkg import origin_module_in_pkg @@ -857,7 +891,7 @@ def test_moving_module_and_not_removing_blanks_after_imports(self): self.mod2.read(), ) - def test_moving_module_refactoring_and_nonexistent_destinations(self): + def test_moving_module_refactoring_and_nonexistent_destinations(self) -> None: self.origin_module_in_pkg.write("a_var = 1") self.mod2.write(dedent("""\ from pkg import origin_module_in_pkg @@ -870,9 +904,9 @@ def test_moving_module_refactoring_and_nonexistent_destinations(self): exceptions.RefactoringError, r"Move destination for modules should be packages.", ): - self._move(self.origin_module_in_pkg, None, None) + self._move(self.origin_module_in_pkg, None, None) # type: ignore[arg-type] - def test_moving_methods_choosing_the_correct_class(self): + def test_moving_methods_choosing_the_correct_class(self) -> None: code = dedent("""\ class A(object): def a_method(self): @@ -882,7 +916,7 @@ def a_method(self): mover = move.create_move(self.project, self.origin_module, code.index("a_method")) self.assertTrue(isinstance(mover, move.MoveMethod)) - def test_moving_methods_getting_new_method_for_empty_methods(self): + def test_moving_methods_getting_new_method_for_empty_methods(self) -> None: code = dedent("""\ class A(object): def a_method(self): @@ -898,7 +932,7 @@ def new_method(self): mover.get_new_method("new_method"), ) - def test_moving_methods_getting_new_method_for_constant_methods(self): + def test_moving_methods_getting_new_method_for_constant_methods(self) -> None: code = dedent("""\ class A(object): def a_method(self): @@ -914,7 +948,7 @@ def new_method(self): mover.get_new_method("new_method"), ) - def test_moving_methods_getting_new_method_passing_simple_parameters(self): + def test_moving_methods_getting_new_method_passing_simple_parameters(self) -> None: code = dedent("""\ class A(object): def a_method(self, p): @@ -930,7 +964,7 @@ def new_method(self, p): mover.get_new_method("new_method"), ) - def test_moving_methods_getting_new_method_using_main_object(self): + def test_moving_methods_getting_new_method_using_main_object(self) -> None: code = dedent("""\ class A(object): attr = 1 @@ -947,7 +981,7 @@ def new_method(self, host): mover.get_new_method("new_method"), ) - def test_moving_methods_getting_new_method_renaming_main_object(self): + def test_moving_methods_getting_new_method_renaming_main_object(self) -> None: code = dedent("""\ class A(object): attr = 1 @@ -964,7 +998,7 @@ def new_method(self, host): mover.get_new_method("new_method"), ) - def test_moving_methods_gettin_new_method_with_keyword_arguments(self): + def test_moving_methods_gettin_new_method_with_keyword_arguments(self) -> None: code = dedent("""\ class A(object): attr = 1 @@ -981,7 +1015,7 @@ def new_method(self, p=None): mover.get_new_method("new_method"), ) - def test_moving_methods_gettin_new_method_with_many_kinds_arguments(self): + def test_moving_methods_gettin_new_method_with_many_kinds_arguments(self) -> None: code = dedent("""\ class A(object): attr = 1 @@ -996,7 +1030,7 @@ def new_method(self, host, p1, *args, **kwds): """) self.assertEqual(expected, mover.get_new_method("new_method")) - def test_moving_methods_getting_new_method_for_multi_line_methods(self): + def test_moving_methods_getting_new_method_for_multi_line_methods(self) -> None: code = dedent("""\ class A(object): def a_method(self): @@ -1014,7 +1048,7 @@ def new_method(self): mover.get_new_method("new_method"), ) - def test_moving_methods_getting_old_method_for_constant_methods(self): + def test_moving_methods_getting_old_method_for_constant_methods(self) -> None: self.mod2.write(dedent("""\ class B(object): pass @@ -1039,7 +1073,7 @@ def a_method(self): """) self.assertEqual(expected, self.origin_module.read()) - def test_moving_methods_getting_getting_changes_for_goal_class(self): + def test_moving_methods_getting_getting_changes_for_goal_class(self) -> None: self.mod2.write(dedent("""\ class B(object): var = 1 @@ -1064,7 +1098,7 @@ def new_method(self): """) self.assertEqual(expected, self.mod2.read()) - def test_moving_methods_getting_getting_changes_for_goal_class2(self): + def test_moving_methods_getting_getting_changes_for_goal_class2(self) -> None: code = dedent("""\ class B(object): var = 1 @@ -1093,7 +1127,7 @@ def a_method(self): self.origin_module.read(), ) - def test_moving_methods_and_nonexistent_attributes(self): + def test_moving_methods_and_nonexistent_attributes(self) -> None: code = dedent("""\ class A(object): def a_method(self): @@ -1106,7 +1140,7 @@ def a_method(self): ): self._move_to_attr(self.origin_module, code.index("a_method"), "x", new_name="new_method") - def test_unknown_attribute_type(self): + def test_unknown_attribute_type(self) -> None: code = dedent("""\ class A(object): attr = 1 @@ -1120,7 +1154,7 @@ def a_method(self): ): self._move_to_attr(self.origin_module, code.index("a_method"), "attr", new_name="new_method") - def test_moving_methods_and_moving_used_imports(self): + def test_moving_methods_and_moving_used_imports(self) -> None: self.mod2.write(dedent("""\ class B(object): var = 1 @@ -1147,7 +1181,7 @@ def new_method(self): """) self.assertEqual(code, self.mod2.read()) - def test_moving_methods_getting_getting_changes_for_goal_class3(self): + def test_moving_methods_getting_getting_changes_for_goal_class3(self) -> None: self.mod2.write(dedent("""\ class B(object): pass @@ -1170,7 +1204,7 @@ def new_method(self): """) self.assertEqual(expected, self.mod2.read()) - def test_moving_methods_and_source_class_with_parameters(self): + def test_moving_methods_and_source_class_with_parameters(self) -> None: self.mod2.write(dedent("""\ class B(object): pass @@ -1202,7 +1236,7 @@ def new_method(self, p): """) self.assertEqual(expected2, self.mod2.read()) - def test_moving_globals_to_a_module_with_only_docstrings(self): + def test_moving_globals_to_a_module_with_only_docstrings(self) -> None: self.origin_module.write(dedent("""\ import sys @@ -1234,7 +1268,7 @@ def f(): self.destination_module.read(), ) - def test_moving_globals_to_a_module_with_only_docstrings2(self): + def test_moving_globals_to_a_module_with_only_docstrings2(self) -> None: code = dedent("""\ import os import sys @@ -1267,7 +1301,7 @@ def f(): ''') self.assertEqual(expected, self.destination_module.read()) - def test_moving_a_global_when_it_is_used_after_a_multiline_str(self): + def test_moving_a_global_when_it_is_used_after_a_multiline_str(self) -> None: code = dedent('''\ def f(): pass @@ -1285,7 +1319,7 @@ def f(): ''') self.assertEqual(expected, self.origin_module.read()) - def test_raising_an_exception_when_moving_non_package_folders(self): + def test_raising_an_exception_when_moving_non_package_folders(self) -> None: dir = self.project.root.create_folder("dir") with self.assertRaisesRegex( exceptions.RefactoringError, @@ -1293,7 +1327,7 @@ def test_raising_an_exception_when_moving_non_package_folders(self): ): move.create_move(self.project, dir) - def test_moving_to_a_module_with_encoding_cookie(self): + def test_moving_to_a_module_with_encoding_cookie(self) -> None: code1 = "# -*- coding: utf-8 -*-" self.destination_module.write(code1) code2 = dedent("""\ @@ -1305,7 +1339,7 @@ def f(): pass expected = f"{code1}\n{code2}" self.assertEqual(expected, self.destination_module.read()) - def test_moving_decorated_function(self): + def test_moving_decorated_function(self) -> None: self.origin_module.write(dedent("""\ def hello(func): return func @@ -1333,7 +1367,7 @@ def foo(): self.destination_module.read(), ) - def test_moving_decorated_class(self): + def test_moving_decorated_class(self) -> None: self.origin_module.write(dedent("""\ from dataclasses import dataclass @dataclass From 4ca7a98b80977c3fc6f737c24bbc5f393ba542fb Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 21:59:22 +1100 Subject: [PATCH 15/19] Refactor test_moving_to_a_module_with_encoding_cookie --- ropetest/refactor/movetest.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index 011ad499..03148cb1 100644 --- a/ropetest/refactor/movetest.py +++ b/ropetest/refactor/movetest.py @@ -1328,15 +1328,16 @@ def test_raising_an_exception_when_moving_non_package_folders(self) -> None: move.create_move(self.project, dir) def test_moving_to_a_module_with_encoding_cookie(self) -> None: - code1 = "# -*- coding: utf-8 -*-" - self.destination_module.write(code1) - code2 = dedent("""\ + code = dedent("""\ + def f(): pass + """) + self.origin_module.write(code) + self.destination_module.write("# -*- coding: utf-8 -*-") + self._move(self.origin_module, code.index("f()") + 1, self.destination_module) + expected = dedent("""\ + # -*- coding: utf-8 -*- def f(): pass """) - self.mod2.write(code2) - mover = move.create_move(self.project, self.mod2, code2.index("f()") + 1) - self.project.do(mover.get_changes(self.destination_module)) - expected = f"{code1}\n{code2}" self.assertEqual(expected, self.destination_module.read()) def test_moving_decorated_function(self) -> None: From 6bb67ca45bf5cea6d1ada15379e35e6b1ed8782a Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 22:23:23 +1100 Subject: [PATCH 16/19] Add type annotation to get_changes() in move.py --- rope/refactor/move.py | 11 +++++++---- ropetest/refactor/movetest.py | 3 ++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/rope/refactor/move.py b/rope/refactor/move.py index 95978d4c..35db68a7 100644 --- a/rope/refactor/move.py +++ b/rope/refactor/move.py @@ -82,9 +82,9 @@ def __init__(self, project, resource, offset): def get_changes( self, - dest_attr, - new_name=None, - resources=None, + dest_attr: str, + new_name: Optional[str] = None, + resources: Optional[List[resources.File]] = None, task_handle=taskhandle.DEFAULT_TASK_HANDLE, # FIXME: this is unused ): """Return the changes needed for this refactoring @@ -519,7 +519,10 @@ def __init__(self, project, resource): self.import_tools = self.tools.import_tools def get_changes( - self, dest, resources=None, task_handle=taskhandle.DEFAULT_TASK_HANDLE + self, + dest: resources.Resource, + resources: Optional[List[resources.File]] = None, + task_handle=taskhandle.DEFAULT_TASK_HANDLE, ): if resources is None: resources = self.project.get_python_files() diff --git a/ropetest/refactor/movetest.py b/ropetest/refactor/movetest.py index 03148cb1..804fa688 100644 --- a/ropetest/refactor/movetest.py +++ b/ropetest/refactor/movetest.py @@ -59,11 +59,12 @@ def _move_to_attr( self, resource: Union[resources.File, resources.Folder], offset: Union[int, None], - dest_attr: Union[str, resources.File, resources.Folder], + dest_attr: str, *, new_name: str, ): mover = move.create_move(self.project, resource, offset) + assert isinstance(mover, move.MoveMethod) changes = mover.get_changes(dest_attr, new_name=new_name) self.project.do(changes) From 964dc8e5fb41333289c3e5d8bbc8f2c138ce43e7 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 22:32:58 +1100 Subject: [PATCH 17/19] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0901b86..3aef5641 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # **Upcoming release** +- #785 Refactoring movetest.py (@lieryan) + +# Release 1.13.0 + - #781, #783 Isolate tests that uses external_fixturepkg into a venv (@lieryan) - #751 Check for ast.Attributes when finding occurrences in fstrings (@sandratsy) - #777, #698 add validation to refuse Rename refactoring to a python keyword (@lieryan) From 9ae39bec5f8131f9170d85ab4f104e54a5ef76f3 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 22:35:07 +1100 Subject: [PATCH 18/19] Update black target version --- pyproject.toml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 52fd8070..23a01f13 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -75,10 +75,11 @@ packages = [ [tool.black] target-version = [ - 'py36', - 'py37', 'py38', 'py39', + 'py310', + 'py311', + 'py312', ] include = 'rope/.*\.pyi?$' force-exclude = 'ropetest|rope/base/prefs.py' From 3be1fd41d777659e46944f4bab906cddd6985f70 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Thu, 4 Apr 2024 23:13:34 +1100 Subject: [PATCH 19/19] Add TYPE_CHECKING to coverage exclusion --- pyproject.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 23a01f13..8c17acd2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -84,6 +84,11 @@ target-version = [ include = 'rope/.*\.pyi?$' force-exclude = 'ropetest|rope/base/prefs.py' +[tool.coverage.report] +exclude_also = [ + "if TYPE_CHECKING:", +] + [tool.isort] profile = "black"