diff --git a/CHANGELOG.md b/CHANGELOG.md index ba328bd9a..474b30b5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # **Upcoming release** +- #927 Validate convert local variable to field eligibility in the constructor. (@Narek-Khachikyan) - #895 patchedast Starred and keyword now consumes their syntactically expected and ** (@lieryan) - #896 patchedast cleanup and refactoring (@lieryan) - #905 Ignore `node_modules` directories by default when scanning project files and source folders. (@marcoeangeli) diff --git a/rope/refactor/localtofield.py b/rope/refactor/localtofield.py index e936d5161..22da77f25 100644 --- a/rope/refactor/localtofield.py +++ b/rope/refactor/localtofield.py @@ -8,17 +8,17 @@ def __init__(self, project, resource, offset): self.resource = resource self.offset = offset - def get_changes(self): - name = worder.get_name_at(self.resource, self.offset) this_pymodule = self.project.get_pymodule(self.resource) - pyname = evaluate.eval_location(this_pymodule, self.offset) - if not self._is_a_method_local(pyname): + self.pyname = evaluate.eval_location(this_pymodule, self.offset) + if not self._is_a_method_local(self.pyname): raise exceptions.RefactoringError( "Convert local variable to field should be performed on " "a local variable of a method." ) - pymodule, lineno = pyname.get_definition_location() + def get_changes(self): + name = worder.get_name_at(self.resource, self.offset) + pymodule, lineno = self.pyname.get_definition_location() function_scope = pymodule.get_scope().get_inner_scope_for_line(lineno) # Not checking redefinition # self._check_redefinition(name, function_scope) @@ -40,12 +40,13 @@ def _get_field_name(self, pyfunction, name): return new_name def _is_a_method_local(self, pyname): + if not isinstance(pyname, pynames.AssignedName): + return False pymodule, lineno = pyname.get_definition_location() holding_scope = pymodule.get_scope().get_inner_scope_for_line(lineno) parent = holding_scope.parent return ( - isinstance(pyname, pynames.AssignedName) - and pyname in holding_scope.get_names().values() + pyname in holding_scope.get_names().values() and holding_scope.get_kind() == "Function" and parent is not None and parent.get_kind() == "Class" diff --git a/ropetest/refactor/__init__.py b/ropetest/refactor/__init__.py index 4d5bee1ad..37333393f 100644 --- a/ropetest/refactor/__init__.py +++ b/ropetest/refactor/__init__.py @@ -877,11 +877,11 @@ def a_func(self): self.assertEqual(expected, self.mod.read()) def test_raising_exception_when_performed_on_a_global_var(self): - self.mod.write("var = 10\n") + self.mod.write(dedent("""\ + var = 10 + """)) with self.assertRaises(RefactoringError): - self._perform_convert_local_variable_to_field( - self.mod, self.mod.read().index("var") + 1 - ) + LocalToField(self.project, self.mod, 0) def test_raising_exception_when_performed_on_field(self): code = dedent("""\ @@ -891,9 +891,7 @@ def a_func(self): """) self.mod.write(code) with self.assertRaises(RefactoringError): - self._perform_convert_local_variable_to_field( - self.mod, self.mod.read().index("var") + 1 - ) + LocalToField(self.project, self.mod, code.index("var") + 1) def test_raising_exception_when_performed_on_a_parameter(self): code = dedent("""\ @@ -903,9 +901,45 @@ def a_func(self, var): """) self.mod.write(code) with self.assertRaises(RefactoringError): - self._perform_convert_local_variable_to_field( - self.mod, self.mod.read().index("var") + 1 - ) + LocalToField(self.project, self.mod, code.index("var") + 1) + + def test_raising_exception_when_performed_on_a_function_local(self): + code = dedent("""\ + def a_func(): + var = 10 + """) + self.mod.write(code) + with self.assertRaises(RefactoringError): + LocalToField(self.project, self.mod, code.index("var")) + + def test_raising_exception_when_performed_on_a_class_variable(self): + code = dedent("""\ + class A: + var = 10 + """) + self.mod.write(code) + with self.assertRaises(RefactoringError): + LocalToField(self.project, self.mod, code.index("var")) + + def test_raising_exception_when_performed_on_an_unresolved_name(self): + code = dedent("""\ + class A: + def a_func(self): + missing + """) + self.mod.write(code) + with self.assertRaises(RefactoringError): + LocalToField(self.project, self.mod, code.index("missing")) + + def test_raising_exception_when_performed_on_a_builtin(self): + code = dedent("""\ + class A: + def a_func(self): + len([]) + """) + self.mod.write(code) + with self.assertRaises(RefactoringError): + LocalToField(self.project, self.mod, code.index("len")) # NOTE: This situation happens a lot and is normally not an error # @testutils.assert_raises(RefactoringError)