From c7d144681aaaddb5370283e55afc7ae2fb16bc87 Mon Sep 17 00:00:00 2001 From: climbus Date: Thu, 16 Sep 2021 20:48:26 +0200 Subject: [PATCH 01/24] extraction with global simple case --- rope/refactor/extract.py | 14 ++++++++++++++ ropetest/refactor/extracttest.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index dfab070dc..04348fc8e 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -596,10 +596,20 @@ def _get_unindented_function_body(self, returns): return 'return ' + _join_lines(self.info.extracted) extracted_body = self.info.extracted unindented_body = sourceutils.fix_indentation(extracted_body, 0) + unindented_body = self._insert_globals(unindented_body) if returns: unindented_body += '\nreturn %s' % self._get_comma_form(returns) return unindented_body + def _insert_globals(self, unindented_body): + globals_ = self.info_collector.globals_ & self._get_internal_variables() + if globals_: + unindented_body = "global {}\n{}".format(", ".join(globals_), unindented_body) + return unindented_body + + def _get_internal_variables(self): + return self.info_collector.read | self.info_collector.written | self.info_collector.maybe_written + class _ExtractVariableParts(object): @@ -635,6 +645,7 @@ def __init__(self, start, end, is_global): self.postwritten = OrderedSet() self.host_function = True self.conditional = False + self.globals_ = OrderedSet() def _read_variable(self, name, lineno): if self.start <= lineno <= self.end: @@ -671,6 +682,9 @@ def _FunctionDef(self, node): for name in visitor.read - visitor.written: self._read_variable(name, node.lineno) + def _Global(self, node): + self.globals_.add(*node.names) + def _Name(self, node): if isinstance(node.ctx, (ast.Store, ast.AugStore)): self._written_variable(node.id, node.lineno) diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index f1670dfcc..0076a54d2 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1349,6 +1349,38 @@ def new_func(): ''') self.assertEqual(expected, refactored) + def test_extraction_method_with_global_variable(self): + code = dedent('''\ + g = None + + def f(): + global g + + g = 2 + + f() + print(g) + ''') + extract_target = 'g = 2' + start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) + refactored = self.do_extract_method(code, start, end, '_g') + expected = dedent('''\ + g = None + + def f(): + global g + + _g() + + def _g(): + global g + g = 2 + + f() + print(g) + ''') + self.assertEqual(expected, refactored) + if __name__ == '__main__': unittest.main() From 6c7654daa107a3b2c3e536042985f08a69017b76 Mon Sep 17 00:00:00 2001 From: climbus Date: Thu, 16 Sep 2021 21:47:27 +0200 Subject: [PATCH 02/24] extract with global definition --- rope/refactor/extract.py | 19 +++++++++++++++++++ ropetest/refactor/extracttest.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index 04348fc8e..fb6156e6c 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -602,11 +602,21 @@ def _get_unindented_function_body(self, returns): return unindented_body def _insert_globals(self, unindented_body): + globals_in_body = self._get_globals_in_body(unindented_body) globals_ = self.info_collector.globals_ & self._get_internal_variables() + globals_ = globals_ - globals_in_body + if globals_: unindented_body = "global {}\n{}".format(", ".join(globals_), unindented_body) return unindented_body + @staticmethod + def _get_globals_in_body(unindented_body): + node = _parse_text(unindented_body) + visitor = _GlobalFinder() + ast.walk(node, visitor) + return visitor.globals_ + def _get_internal_variables(self): return self.info_collector.read | self.info_collector.written | self.info_collector.maybe_written @@ -834,6 +844,15 @@ def has_errors(code): return visitor.error +class _GlobalFinder(object): + def __init__(self): + self.globals_ = OrderedSet() + + def _Global(self, node): + for name in node.names: + self.globals_.add(name) + + def _get_function_kind(scope): return scope.pyobject.get_kind() diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index 0076a54d2..1a38af02e 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1381,6 +1381,35 @@ def _g(): ''') self.assertEqual(expected, refactored) + def test_extraction_method_with_global_variable_and_global_declaration(self): + code = dedent('''\ + g = None + + def f(): + global g + + g = 2 + + f() + print(g) + ''') + start, end = 23, 42 + refactored = self.do_extract_method(code, start, end, '_g') + expected = dedent('''\ + g = None + + def f(): + _g() + + def _g(): + global g + + g = 2 + + f() + print(g) + ''') + self.assertEqual(expected, refactored) if __name__ == '__main__': unittest.main() From ef4cb6797b027c564251e77c6c37e27d70fa6424 Mon Sep 17 00:00:00 2001 From: climbus Date: Fri, 17 Sep 2021 09:14:59 +0200 Subject: [PATCH 03/24] global in oneliner --- rope/refactor/extract.py | 5 +++-- ropetest/refactor/extracttest.py | 33 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index fb6156e6c..90d24ad02 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -591,9 +591,10 @@ def _find_function_returns(self): def _get_unindented_function_body(self, returns): if self.info.one_line: if self.info.returning_named_expr: - return 'return ' + '(' + _join_lines(self.info.extracted) + ')' + body = 'return ' + '(' + _join_lines(self.info.extracted) + ')' else: - return 'return ' + _join_lines(self.info.extracted) + body = 'return ' + _join_lines(self.info.extracted) + return self._insert_globals(body) extracted_body = self.info.extracted unindented_body = sourceutils.fix_indentation(extracted_body, 0) unindented_body = self._insert_globals(unindented_body) diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index 1a38af02e..97f94a076 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1411,5 +1411,38 @@ def _g(): ''') self.assertEqual(expected, refactored) + def test_extraction_one_line_with_global_variable(self): + code = dedent('''\ + g = None + + def f(): + global g + + a = g + + f() + print(g) + ''') + extract_target = '= g' + start, end = code.index(extract_target) + 2, code.index(extract_target) + 3 + refactored = self.do_extract_method(code, start, end, '_g') + print(refactored) + expected = dedent('''\ + g = None + + def f(): + global g + + a = _g() + + def _g(): + global g + return g + + f() + print(g) + ''') + self.assertEqual(expected, refactored) + if __name__ == '__main__': unittest.main() From d31065d54dc32a5eb136d92d8601e338befb6c25 Mon Sep 17 00:00:00 2001 From: climbus Date: Fri, 17 Sep 2021 11:09:43 +0200 Subject: [PATCH 04/24] refactored getting function body --- rope/refactor/extract.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index 90d24ad02..4b1458dc4 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -590,18 +590,23 @@ def _find_function_returns(self): def _get_unindented_function_body(self, returns): if self.info.one_line: - if self.info.returning_named_expr: - body = 'return ' + '(' + _join_lines(self.info.extracted) + ')' - else: - body = 'return ' + _join_lines(self.info.extracted) - return self._insert_globals(body) - extracted_body = self.info.extracted - unindented_body = sourceutils.fix_indentation(extracted_body, 0) + return self._get_one_line_function_body() + return self._get_multiline_function_body(returns) + + def _get_multiline_function_body(self, returns): + unindented_body = sourceutils.fix_indentation(self.info.extracted, 0) unindented_body = self._insert_globals(unindented_body) if returns: unindented_body += '\nreturn %s' % self._get_comma_form(returns) return unindented_body + def _get_one_line_function_body(self): + if self.info.returning_named_expr: + body = 'return ' + '(' + _join_lines(self.info.extracted) + ')' + else: + body = 'return ' + _join_lines(self.info.extracted) + return self._insert_globals(body) + def _insert_globals(self, unindented_body): globals_in_body = self._get_globals_in_body(unindented_body) globals_ = self.info_collector.globals_ & self._get_internal_variables() From ba4746fe486c2ce2962d981bb650dc744c555063 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Sun, 12 Sep 2021 21:09:09 +1000 Subject: [PATCH 05/24] Fix detection of read/written variables when extract method of code in a loop --- rope/refactor/extract.py | 15 +++- ropetest/refactor/extracttest.py | 115 ++++++++++++++++++++++++++----- 2 files changed, 109 insertions(+), 21 deletions(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index 4b1458dc4..7ed5464b6 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -661,6 +661,7 @@ def __init__(self, start, end, is_global): self.postwritten = OrderedSet() self.host_function = True self.conditional = False + self.surrounded_by_loop = 0 self.globals_ = OrderedSet() def _read_variable(self, name, lineno): @@ -674,10 +675,12 @@ def _read_variable(self, name, lineno): def _written_variable(self, name, lineno): if self.start <= lineno <= self.end: - if self.conditional: + if self.conditional or self.surrounded_by_loop: self.maybe_written.add(name) else: self.written.add(name) + if self.surrounded_by_loop and name in self.read: + self.postread.add(name) if self.start > lineno: self.prewritten.add(name) if self.end < lineno: @@ -732,10 +735,17 @@ def _If(self, node): self._handle_conditional_node(node) def _While(self, node): - self._handle_conditional_node(node) + if node.lineno < self.start: + self.surrounded_by_loop += 1 + try: + self._handle_conditional_node(node) + finally: + self.surrounded_by_loop -= 1 def _For(self, node): self.conditional = True + if node.lineno < self.start: + self.surrounded_by_loop += 1 try: # iter has to be checked before the target variables ast.walk(node.iter, self) @@ -747,6 +757,7 @@ def _For(self, node): ast.walk(child, self) finally: self.conditional = False + self.surrounded_by_loop -= 1 def _get_argnames(arguments): diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index 97f94a076..6c0c375f7 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -433,6 +433,74 @@ def test_breaks_and_continues_outside_loops(self): with self.assertRaises(rope.base.exceptions.RefactoringError): self.do_extract_method(code, start, end, 'new_func') + def test_for_loop_variable_scope(self): + code = dedent('''\ + def my_func(): + i = 0 + for dummy in range(10): + i += 1 + print(i) + ''') + start, end = self._convert_line_range_to_offset(code, 4, 5) + refactored = self.do_extract_method(code, start, end, 'new_func') + expected = dedent('''\ + def my_func(): + i = 0 + for dummy in range(10): + i = new_func(i) + + def new_func(i): + i += 1 + print(i) + return i + ''') + self.assertEqual(expected, refactored) + + def test_for_loop_variable_scope_read_then_write(self): + code = dedent('''\ + def my_func(): + i = 0 + for dummy in range(10): + a = i + 1 + i = a + 1 + ''') + start, end = self._convert_line_range_to_offset(code, 4, 5) + refactored = self.do_extract_method(code, start, end, 'new_func') + expected = dedent('''\ + def my_func(): + i = 0 + for dummy in range(10): + i = new_func(i) + + def new_func(i): + a = i + 1 + i = a + 1 + return i + ''') + self.assertEqual(expected, refactored) + + def test_for_loop_variable_scope_write_then_read(self): + code = dedent('''\ + def my_func(): + i = 0 + for dummy in range(10): + i = 'hello' + print(i) + ''') + start, end = self._convert_line_range_to_offset(code, 4, 5) + refactored = self.do_extract_method(code, start, end, 'new_func') + expected = dedent('''\ + def my_func(): + i = 0 + for dummy in range(10): + new_func() + + def new_func(): + i = 'hello' + print(i) + ''') + self.assertEqual(expected, refactored) + def test_variable_writes_followed_by_variable_reads_after_extraction(self): code = 'def a_func():\n a = 1\n a = 2\n b = a\n' start = code.index('a = 1') @@ -1126,25 +1194,29 @@ def test_extract_method_with_list_comprehension(self): self.assertEqual(expected, refactored) def test_extract_method_with_list_comprehension_and_iter(self): - code = "def foo():\n" \ - " x = [e for e in []]\n" \ - " f = 23\n" \ - "\n" \ - " for x, f in x:\n" \ - " def bar():\n" \ - " x[42] = 1\n" + code = dedent("""\ + def foo(): + x = [e for e in []] + f = 23 + + for x, f in x: + def bar(): + x[42] = 1 + """) start, end = self._convert_line_range_to_offset(code, 4, 7) refactored = self.do_extract_method(code, start, end, 'baz') - expected = "def foo():\n" \ - " x = [e for e in []]\n" \ - " f = 23\n" \ - "\n" \ - " baz(x)\n" \ - "\n" \ - "def baz(x):\n" \ - " for x, f in x:\n" \ - " def bar():\n" \ - " x[42] = 1\n" + expected = dedent("""\ + def foo(): + x = [e for e in []] + f = 23 + + baz(x) + + def baz(x): + for x, f in x: + def bar(): + x[42] = 1 + """) self.assertEqual(expected, refactored) def test_extract_method_with_list_comprehension_and_orelse(self): @@ -1209,8 +1281,13 @@ def test_extract_function_with_for_else_statemant_more(self): self.assertEqual(expected, refactored) def test_extract_function_with_for_else_statemant_outside_loops(self): - code = 'def a_func():\n for i in range(10):\n a = i\n' \ - ' else:\n a=None\n' + code = dedent('''\ + def a_func(): + for i in range(10): + a = i + else: + a=None + ''') start = code.index('a = i') end = len(code) - 1 with self.assertRaises(rope.base.exceptions.RefactoringError): From ef6f23a93f513e1871469cabaf88f05c46e32dde Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Sun, 12 Sep 2021 21:16:32 +1000 Subject: [PATCH 06/24] Refactor handling of loop_depth to use contextmanager --- rope/refactor/extract.py | 50 ++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index 7ed5464b6..04bfb4471 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -1,10 +1,11 @@ import re +from contextlib import contextmanager -from rope.base.utils.datastructures import OrderedSet from rope.base import ast, codeanalyze from rope.base.change import ChangeSet, ChangeContents from rope.base.exceptions import RefactoringError from rope.base.utils import pycompat +from rope.base.utils.datastructures import OrderedSet from rope.refactor import (sourceutils, similarfinder, patchedast, suites, usefunction) @@ -663,6 +664,8 @@ def __init__(self, start, end, is_global): self.conditional = False self.surrounded_by_loop = 0 self.globals_ = OrderedSet() + self.surrounded_by_loop = 0 + self.loop_depth = 0 def _read_variable(self, name, lineno): if self.start <= lineno <= self.end: @@ -675,11 +678,11 @@ def _read_variable(self, name, lineno): def _written_variable(self, name, lineno): if self.start <= lineno <= self.end: - if self.conditional or self.surrounded_by_loop: + if self.conditional or self.loop_depth > 0: self.maybe_written.add(name) else: self.written.add(name) - if self.surrounded_by_loop and name in self.read: + if self.loop_depth > 0 and name in self.read: self.postread.add(name) if self.start > lineno: self.prewritten.add(name) @@ -731,33 +734,36 @@ def _handle_conditional_node(self, node): finally: self.conditional = False + @contextmanager + def _handle_loop_node(self, node): + if node.lineno < self.start: + self.loop_depth += 1 + try: + yield + finally: + self.loop_depth -= 1 + def _If(self, node): self._handle_conditional_node(node) def _While(self, node): - if node.lineno < self.start: - self.surrounded_by_loop += 1 - try: + with self._handle_loop_node(node): self._handle_conditional_node(node) - finally: - self.surrounded_by_loop -= 1 def _For(self, node): self.conditional = True - if node.lineno < self.start: - self.surrounded_by_loop += 1 - try: - # iter has to be checked before the target variables - ast.walk(node.iter, self) - ast.walk(node.target, self) - - for child in node.body: - ast.walk(child, self) - for child in node.orelse: - ast.walk(child, self) - finally: - self.conditional = False - self.surrounded_by_loop -= 1 + with self._handle_loop_node(node): + try: + # iter has to be checked before the target variables + ast.walk(node.iter, self) + ast.walk(node.target, self) + + for child in node.body: + ast.walk(child, self) + for child in node.orelse: + ast.walk(child, self) + finally: + self.conditional = False def _get_argnames(arguments): From 681fd0a5a01d428afe32126fa37d5b55e6376212 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Sun, 12 Sep 2021 22:22:12 +1000 Subject: [PATCH 07/24] Rename _FunctionInformationCollector.{_handle_loop_node->_handle_loop_context} --- rope/refactor/extract.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index 04bfb4471..116bb95ef 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -735,7 +735,7 @@ def _handle_conditional_node(self, node): self.conditional = False @contextmanager - def _handle_loop_node(self, node): + def _handle_loop_context(self, node): if node.lineno < self.start: self.loop_depth += 1 try: @@ -747,12 +747,12 @@ def _If(self, node): self._handle_conditional_node(node) def _While(self, node): - with self._handle_loop_node(node): + with self._handle_loop_context(node): self._handle_conditional_node(node) def _For(self, node): self.conditional = True - with self._handle_loop_node(node): + with self._handle_loop_context(node): try: # iter has to be checked before the target variables ast.walk(node.iter, self) From 11f38c55213762c8d70e54229a0832f49684d9c2 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Sun, 12 Sep 2021 22:31:39 +1000 Subject: [PATCH 08/24] Refactor _handle_conditional_context() --- rope/refactor/extract.py | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index 116bb95ef..69b315d0a 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -726,23 +726,6 @@ def _AugAssign(self, node): def _ClassDef(self, node): self._written_variable(node.name, node.lineno) - def _handle_conditional_node(self, node): - self.conditional = True - try: - for child in ast.get_child_nodes(node): - ast.walk(child, self) - finally: - self.conditional = False - - @contextmanager - def _handle_loop_context(self, node): - if node.lineno < self.start: - self.loop_depth += 1 - try: - yield - finally: - self.loop_depth -= 1 - def _If(self, node): self._handle_conditional_node(node) @@ -765,6 +748,28 @@ def _For(self, node): finally: self.conditional = False + def _handle_conditional_node(self, node): + with self._handle_conditional_context(node): + for child in ast.get_child_nodes(node): + ast.walk(child, self) + + @contextmanager + def _handle_conditional_context(self, node): + self.conditional = True + try: + yield + finally: + self.conditional = False + + @contextmanager + def _handle_loop_context(self, node): + if node.lineno < self.start: + self.loop_depth += 1 + try: + yield + finally: + self.loop_depth -= 1 + def _get_argnames(arguments): result = [pycompat.get_ast_arg_arg(node) for node in arguments.args From 91743c489974cdd92f25b91643e8cb246d5e9bb0 Mon Sep 17 00:00:00 2001 From: Lie Ryan Date: Sun, 12 Sep 2021 23:05:06 +1000 Subject: [PATCH 09/24] Fix write-only variable when extracting method in a loop --- rope/refactor/extract.py | 27 ++++++++--------- ropetest/refactor/extracttest.py | 50 +++++++++++++++++++++++++------- 2 files changed, 51 insertions(+), 26 deletions(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index 69b315d0a..8fba0ce44 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -678,7 +678,7 @@ def _read_variable(self, name, lineno): def _written_variable(self, name, lineno): if self.start <= lineno <= self.end: - if self.conditional or self.loop_depth > 0: + if self.conditional: self.maybe_written.add(name) else: self.written.add(name) @@ -734,19 +734,15 @@ def _While(self, node): self._handle_conditional_node(node) def _For(self, node): - self.conditional = True - with self._handle_loop_context(node): - try: - # iter has to be checked before the target variables - ast.walk(node.iter, self) - ast.walk(node.target, self) - - for child in node.body: - ast.walk(child, self) - for child in node.orelse: - ast.walk(child, self) - finally: - self.conditional = False + with self._handle_loop_context(node), self._handle_conditional_context(node): + # iter has to be checked before the target variables + ast.walk(node.iter, self) + ast.walk(node.target, self) + + for child in node.body: + ast.walk(child, self) + for child in node.orelse: + ast.walk(child, self) def _handle_conditional_node(self, node): with self._handle_conditional_context(node): @@ -755,7 +751,8 @@ def _handle_conditional_node(self, node): @contextmanager def _handle_conditional_context(self, node): - self.conditional = True + if self.start <= node.lineno <= self.end: + self.conditional = True try: yield finally: diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index 6c0c375f7..9978c0e96 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -501,6 +501,29 @@ def new_func(): ''') self.assertEqual(expected, refactored) + def test_for_loop_variable_scope_write_only(self): + code = dedent('''\ + def my_func(): + i = 0 + for num in range(10): + i = 'hello' + num + print(i) + ''') + start, end = self._convert_line_range_to_offset(code, 4, 4) + refactored = self.do_extract_method(code, start, end, 'new_func') + expected = dedent('''\ + def my_func(): + i = 0 + for num in range(10): + i = new_func(num) + print(i) + + def new_func(num): + i = 'hello' + num + return i + ''') + self.assertEqual(expected, refactored) + def test_variable_writes_followed_by_variable_reads_after_extraction(self): code = 'def a_func():\n a = 1\n a = 2\n b = a\n' start = code.index('a = 1') @@ -1140,19 +1163,24 @@ def test_passing_conditional_updated_vars_in_extracted(self): self.assertEqual(expected, refactored) def test_returning_conditional_updated_vars_in_extracted(self): - code = 'def f(a):\n' \ - ' if 0:\n' \ - ' a = 1\n' \ - ' print(a)\n' + code = dedent("""\ + def f(a): + if 0: + a = 1 + print(a) + """) start, end = self._convert_line_range_to_offset(code, 2, 3) refactored = self.do_extract_method(code, start, end, 'g') - expected = 'def f(a):\n' \ - ' a = g(a)\n' \ - ' print(a)\n\n' \ - 'def g(a):\n' \ - ' if 0:\n' \ - ' a = 1\n' \ - ' return a\n' + expected = dedent("""\ + def f(a): + a = g(a) + print(a) + + def g(a): + if 0: + a = 1 + return a + """) self.assertEqual(expected, refactored) def test_extract_method_with_variables_possibly_written_to(self): From a450ebbf6b79cec9669ce31f6e06b5355dccb9b9 Mon Sep 17 00:00:00 2001 From: climbus Date: Thu, 9 Sep 2021 08:52:40 +0200 Subject: [PATCH 10/24] extracting static WIP --- rope/refactor/extract.py | 26 ++++++++++++++++---------- ropetest/refactor/extracttest.py | 23 +++++++++++++++++++++++ 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index 8fba0ce44..eb6b7e0b1 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -53,7 +53,7 @@ def _fix_end(self, source, offset): offset -= 1 return offset - def get_changes(self, extracted_name, similar=False, global_=False): + def get_changes(self, extracted_name, similar=False, global_=False, static=False): """Get the changes this refactoring makes :parameters: @@ -66,7 +66,7 @@ def get_changes(self, extracted_name, similar=False, global_=False): info = _ExtractInfo( self.project, self.resource, self.start_offset, self.end_offset, extracted_name, variable=self.kind == 'variable', - similar=similar, make_global=global_) + similar=similar, make_global=global_, static=static) new_contents = _ExtractPerformer(info).extract() changes = ChangeSet('Extract %s <%s>' % (self.kind, extracted_name)) @@ -96,7 +96,7 @@ class _ExtractInfo(object): """Holds information about the extract to be performed""" def __init__(self, project, resource, start, end, new_name, - variable, similar, make_global): + variable, similar, make_global, static): self.project = project self.resource = resource self.pymodule = project.get_pymodule(resource) @@ -107,6 +107,7 @@ def __init__(self, project, resource, start, end, new_name, self.variable = variable self.similar = similar self._init_parts(start, end) + self.static = static self._init_scope() self.make_global = make_global @@ -440,6 +441,8 @@ class _ExtractMethodParts(object): def __init__(self, info): self.info = info self.info_collector = self._create_info_collector() + if self.info.method and _get_function_kind(self.info.scope) == "staticmethod": + self.info.static = True def get_definition(self): if self.info.global_: @@ -493,9 +496,9 @@ def _create_info_collector(self): def _get_function_definition(self): args = self._find_function_arguments() returns = self._find_function_returns() + result = [] - if self.info.method and not self.info.make_global and \ - _get_function_kind(self.info.scope) != 'method': + if self._extracting_static(): result.append('@staticmethod\n') result.append('def %s:\n' % self._get_function_signature(args)) unindented_body = self._get_unindented_function_body(returns) @@ -506,6 +509,9 @@ def _get_function_definition(self): return definition + '\n' + def _extracting_static(self): + return self.info.static + def _get_function_signature(self, args): args = list(args) prefix = '' @@ -521,8 +527,8 @@ def _get_function_signature(self, args): '(%s)' % self._get_comma_form(args) def _extracting_method(self): - return self.info.method and not self.info.make_global and \ - _get_function_kind(self.info.scope) == 'method' + return not self._extracting_static() and (self.info.method and not self.info.make_global and \ + _get_function_kind(self.info.scope) == 'method') def _get_self_name(self): param_names = self.info.scope.pyobject.get_param_names() @@ -532,13 +538,13 @@ def _get_self_name(self): def _get_function_call(self, args): prefix = '' if self.info.method and not self.info.make_global: - if _get_function_kind(self.info.scope) == 'method': + if self._extracting_static(): + prefix = self.info.scope.parent.pyobject.get_name() + '.' + else: self_name = self._get_self_name() if self_name in args: args.remove(self_name) prefix = self_name + '.' - else: - prefix = self.info.scope.parent.pyobject.get_name() + '.' return prefix + '%s(%s)' % (self.info.new_name, self._get_comma_form(args)) diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index 9978c0e96..9f62a511a 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1549,5 +1549,28 @@ def _g(): ''') self.assertEqual(expected, refactored) + def test_extract_to_static_method(self): + code = dedent('''\ + class A: + def first_method(self): + a_var = 1 + b_var = a_var + 1 + ''') + extract_target = 'a_var + 1' + start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) + refactored = self.do_extract_method(code, start, end, 'second_method', static=True) + expected = dedent('''\ + class A: + def first_method(self): + a_var = 1 + b_var = A.second_method(a_var) + + @staticmethod + def second_method(a_var): + return a_var + 1 + ''') + self.assertEqual(expected, refactored) + + if __name__ == '__main__': unittest.main() From c88e77c7bd539993f893ba5e072cf344e39963b3 Mon Sep 17 00:00:00 2001 From: climbus Date: Thu, 9 Sep 2021 12:35:49 +0200 Subject: [PATCH 11/24] some refactorings --- rope/refactor/extract.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index eb6b7e0b1..6b6cdd256 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -441,8 +441,10 @@ class _ExtractMethodParts(object): def __init__(self, info): self.info = info self.info_collector = self._create_info_collector() - if self.info.method and _get_function_kind(self.info.scope) == "staticmethod": - self.info.static = True + self.info.static = True if self._extracting_from_static() else self.info.static + + def _extracting_from_static(self): + return self.info.method and _get_function_kind(self.info.scope) == "staticmethod" def get_definition(self): if self.info.global_: @@ -527,8 +529,10 @@ def _get_function_signature(self, args): '(%s)' % self._get_comma_form(args) def _extracting_method(self): - return not self._extracting_static() and (self.info.method and not self.info.make_global and \ - _get_function_kind(self.info.scope) == 'method') + return not self._extracting_static() and \ + (self.info.method and \ + not self.info.make_global and \ + _get_function_kind(self.info.scope) == 'method') def _get_self_name(self): param_names = self.info.scope.pyobject.get_param_names() @@ -536,6 +540,12 @@ def _get_self_name(self): return param_names[0] def _get_function_call(self, args): + return '{prefix}{name}({args})'.format( + prefix=self._get_function_call_prefix(args), + name=self.info.new_name, + args=self._get_comma_form(args)) + + def _get_function_call_prefix(self, args): prefix = '' if self.info.method and not self.info.make_global: if self._extracting_static(): @@ -545,8 +555,7 @@ def _get_function_call(self, args): if self_name in args: args.remove(self_name) prefix = self_name + '.' - return prefix + '%s(%s)' % (self.info.new_name, - self._get_comma_form(args)) + return prefix def _get_comma_form(self, names): return ', '.join(names) From 2eb9e61b90cf47532c7b58d5543cf21c5d5ccb40 Mon Sep 17 00:00:00 2001 From: climbus Date: Thu, 9 Sep 2021 13:25:37 +0200 Subject: [PATCH 12/24] raise error when extracting to static with self in body --- rope/refactor/extract.py | 5 +++++ ropetest/refactor/extracttest.py | 11 +++++++++++ 2 files changed, 16 insertions(+) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index 6b6cdd256..dd06c015b 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -442,6 +442,11 @@ def __init__(self, info): self.info = info self.info_collector = self._create_info_collector() self.info.static = True if self._extracting_from_static() else self.info.static + if self._extracting_static() and self._self_name_in_body(): + raise RefactoringError("Can't extract static method with reference to {}".format(self._get_self_name())) + + def _self_name_in_body(self): + return self._get_self_name() and self._get_self_name() in self.info.extracted def _extracting_from_static(self): return self.info.method and _get_function_kind(self.info.scope) == "staticmethod" diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index 9f62a511a..bd2e90620 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1571,6 +1571,17 @@ def second_method(a_var): ''') self.assertEqual(expected, refactored) + def test_extract_to_static_method_when_self_in_body_should_raise_error(self): + code = dedent('''\ + class A: + def first_method(self): + a_var = 1 + b_var = self.a_var + 1 + ''') + extract_target = 'self.a_var + 1' + start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) + with self.assertRaisesRegexp(rope.base.exceptions.RefactoringError, "extract static method with reference to self"): + self.do_extract_method(code, start, end, 'second_method', static=True) if __name__ == '__main__': unittest.main() From f1ac028ee4ce197073f0fb350d459a7b244e93d5 Mon Sep 17 00:00:00 2001 From: climbus Date: Thu, 9 Sep 2021 13:31:05 +0200 Subject: [PATCH 13/24] kind option to refactoring --- rope/refactor/extract.py | 5 +++-- ropetest/refactor/extracttest.py | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index dd06c015b..9a9ae9ce7 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -53,7 +53,7 @@ def _fix_end(self, source, offset): offset -= 1 return offset - def get_changes(self, extracted_name, similar=False, global_=False, static=False): + def get_changes(self, extracted_name, similar=False, global_=False, kind=None): """Get the changes this refactoring makes :parameters: @@ -61,12 +61,13 @@ def get_changes(self, extracted_name, similar=False, global_=False, static=False replaced. - `global_`: if `True`, the extracted method/variable will be global. + - `kind`: kind of target refactoring to (staticmethod) """ info = _ExtractInfo( self.project, self.resource, self.start_offset, self.end_offset, extracted_name, variable=self.kind == 'variable', - similar=similar, make_global=global_, static=static) + similar=similar, make_global=global_, static=kind == "staticmethod") new_contents = _ExtractPerformer(info).extract() changes = ChangeSet('Extract %s <%s>' % (self.kind, extracted_name)) diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index bd2e90620..cae31f57f 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1558,7 +1558,7 @@ def first_method(self): ''') extract_target = 'a_var + 1' start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) - refactored = self.do_extract_method(code, start, end, 'second_method', static=True) + refactored = self.do_extract_method(code, start, end, 'second_method', kind="staticmethod") expected = dedent('''\ class A: def first_method(self): @@ -1581,7 +1581,7 @@ def first_method(self): extract_target = 'self.a_var + 1' start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) with self.assertRaisesRegexp(rope.base.exceptions.RefactoringError, "extract static method with reference to self"): - self.do_extract_method(code, start, end, 'second_method', static=True) + self.do_extract_method(code, start, end, 'second_method', kind="staticmethod") if __name__ == '__main__': unittest.main() From 8498cf5a81e9b6f928c8ffed9831fe0889606213 Mon Sep 17 00:00:00 2001 From: climbus Date: Thu, 9 Sep 2021 20:57:26 +0200 Subject: [PATCH 14/24] cannot extract from function to staticmethod --- rope/refactor/extract.py | 8 +++++++- ropetest/refactor/extracttest.py | 12 ++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index 9a9ae9ce7..ba8a6a0ce 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -443,8 +443,14 @@ def __init__(self, info): self.info = info self.info_collector = self._create_info_collector() self.info.static = True if self._extracting_from_static() else self.info.static + + self._check_constraints() + + def _check_constraints(self): if self._extracting_static() and self._self_name_in_body(): - raise RefactoringError("Can't extract static method with reference to {}".format(self._get_self_name())) + raise RefactoringError("Cannot extract static method with reference to {}".format(self._get_self_name())) + if self._extracting_static() and not self.info.method: + raise RefactoringError("Cannot extract to staticmethod outside class") def _self_name_in_body(self): return self._get_self_name() and self._get_self_name() in self.info.extracted diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index cae31f57f..2c636dc38 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1583,5 +1583,17 @@ def first_method(self): with self.assertRaisesRegexp(rope.base.exceptions.RefactoringError, "extract static method with reference to self"): self.do_extract_method(code, start, end, 'second_method', kind="staticmethod") + def test_extract_from_function_to_staticmethod_raises_exception(self): + code = dedent('''\ + def first_method(): + a_var = 1 + b_var = a_var + 1 + ''') + extract_target = 'a_var + 1' + start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) + with self.assertRaisesRegexp(rope.base.exceptions.RefactoringError, "Cannot extract to staticmethod outside class"): + self.do_extract_method(code, start, end, 'second_method', kind="staticmethod") + + if __name__ == '__main__': unittest.main() From cf1b6576106b7fca19da08d50cb4b923b3a0912d Mon Sep 17 00:00:00 2001 From: climbus Date: Sat, 11 Sep 2021 16:28:58 +0200 Subject: [PATCH 15/24] extracting from classmethod --- rope/refactor/extract.py | 16 +++++++++++++--- ropetest/refactor/extracttest.py | 13 +++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index ba8a6a0ce..524a9b636 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -443,7 +443,7 @@ def __init__(self, info): self.info = info self.info_collector = self._create_info_collector() self.info.static = True if self._extracting_from_static() else self.info.static - + self.info.classmethod = True if self._extracting_from_classmethod() else False self._check_constraints() def _check_constraints(self): @@ -458,6 +458,9 @@ def _self_name_in_body(self): def _extracting_from_static(self): return self.info.method and _get_function_kind(self.info.scope) == "staticmethod" + def _extracting_from_classmethod(self): + return self.info.method and _get_function_kind(self.info.scope) == "classmethod" + def get_definition(self): if self.info.global_: return '\n%s\n' % self._get_function_definition() @@ -512,8 +515,12 @@ def _get_function_definition(self): returns = self._find_function_returns() result = [] + if self._extracting_static(): result.append('@staticmethod\n') + if self._extracting_classmethod(): + result.append('@classmethod\n') + result.append('def %s:\n' % self._get_function_signature(args)) unindented_body = self._get_unindented_function_body(returns) indents = sourceutils.get_indent(self.info.project) @@ -523,13 +530,16 @@ def _get_function_definition(self): return definition + '\n' + def _extracting_classmethod(self): + return self.info.classmethod + def _extracting_static(self): return self.info.static def _get_function_signature(self, args): args = list(args) prefix = '' - if self._extracting_method(): + if self._extracting_method() or self._extracting_classmethod(): self_name = self._get_self_name() if self_name is None: raise RefactoringError('Extracting a method from a function ' @@ -560,7 +570,7 @@ def _get_function_call(self, args): def _get_function_call_prefix(self, args): prefix = '' if self.info.method and not self.info.make_global: - if self._extracting_static(): + if self._extracting_static() or self._extracting_classmethod(): prefix = self.info.scope.parent.pyobject.get_name() + '.' else: self_name = self._get_self_name() diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index 2c636dc38..82cd2b787 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1594,6 +1594,19 @@ def first_method(): with self.assertRaisesRegexp(rope.base.exceptions.RefactoringError, "Cannot extract to staticmethod outside class"): self.do_extract_method(code, start, end, 'second_method', kind="staticmethod") + def test_extract_method_in_classmethods(self): + code = 'class AClass(object):\n\n' \ + ' @classmethod\n def func2(cls):\n b = 1\n' + start = code.index(' 1') + 1 + refactored = self.do_extract_method(code, start, start + 1, + 'one', similar=True) + expected = 'class AClass(object):\n\n' \ + ' @classmethod\n def func2(cls):\n' \ + ' b = AClass.one()\n\n' \ + ' @classmethod\n def one(cls):\n' \ + ' return 1\n' + self.assertEqual(expected, refactored) + if __name__ == '__main__': unittest.main() From c850ad8855269310e4cfe0429da4ff777751fa14 Mon Sep 17 00:00:00 2001 From: climbus Date: Sat, 11 Sep 2021 16:34:42 +0200 Subject: [PATCH 16/24] refactored test --- ropetest/refactor/extracttest.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index 82cd2b787..5e125c055 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1595,16 +1595,25 @@ def first_method(): self.do_extract_method(code, start, end, 'second_method', kind="staticmethod") def test_extract_method_in_classmethods(self): - code = 'class AClass(object):\n\n' \ - ' @classmethod\n def func2(cls):\n b = 1\n' + code = dedent('''\ + class AClass(object): + @classmethod + def func2(cls): + b = 1 + ''') start = code.index(' 1') + 1 refactored = self.do_extract_method(code, start, start + 1, 'one', similar=True) - expected = 'class AClass(object):\n\n' \ - ' @classmethod\n def func2(cls):\n' \ - ' b = AClass.one()\n\n' \ - ' @classmethod\n def one(cls):\n' \ - ' return 1\n' + expected = dedent('''\ + class AClass(object): + @classmethod + def func2(cls): + b = AClass.one() + + @classmethod + def one(cls): + return 1 + ''') self.assertEqual(expected, refactored) From b4692ef5d6f20c4a7e8a659b1506fbb51ee4c0dc Mon Sep 17 00:00:00 2001 From: climbus Date: Sat, 11 Sep 2021 20:41:55 +0200 Subject: [PATCH 17/24] working classmethod --- rope/refactor/extract.py | 53 ++++++++++++++++++++------------ ropetest/refactor/extracttest.py | 51 ++++++++++++++++++++++++++++-- 2 files changed, 81 insertions(+), 23 deletions(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index 524a9b636..af5c84da1 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -61,13 +61,15 @@ def get_changes(self, extracted_name, similar=False, global_=False, kind=None): replaced. - `global_`: if `True`, the extracted method/variable will be global. - - `kind`: kind of target refactoring to (staticmethod) + - `kind`: kind of target refactoring to (staticmethod, classmethod) """ info = _ExtractInfo( self.project, self.resource, self.start_offset, self.end_offset, extracted_name, variable=self.kind == 'variable', - similar=similar, make_global=global_, static=kind == "staticmethod") + similar=similar, make_global=global_) + info.staticmethod = kind == "staticmethod" + info.classmethod = kind == "classmethod" new_contents = _ExtractPerformer(info).extract() changes = ChangeSet('Extract %s <%s>' % (self.kind, extracted_name)) @@ -97,7 +99,7 @@ class _ExtractInfo(object): """Holds information about the extract to be performed""" def __init__(self, project, resource, start, end, new_name, - variable, similar, make_global, static): + variable, similar, make_global): self.project = project self.resource = resource self.pymodule = project.get_pymodule(resource) @@ -108,7 +110,8 @@ def __init__(self, project, resource, start, end, new_name, self.variable = variable self.similar = similar self._init_parts(start, end) - self.static = static + self.staticmethod = False + self.classmethod = False self._init_scope() self.make_global = make_global @@ -442,18 +445,22 @@ class _ExtractMethodParts(object): def __init__(self, info): self.info = info self.info_collector = self._create_info_collector() - self.info.static = True if self._extracting_from_static() else self.info.static - self.info.classmethod = True if self._extracting_from_classmethod() else False + self.info.staticmethod = True if self._extracting_from_static() else self.info.staticmethod + self.info.classmethod = True if self._extracting_from_classmethod() else self.info.classmethod self._check_constraints() def _check_constraints(self): - if self._extracting_static() and self._self_name_in_body(): - raise RefactoringError("Cannot extract static method with reference to {}".format(self._get_self_name())) - if self._extracting_static() and not self.info.method: + if self._extracting_staticmethod() and self._self_name_in_body(): + raise RefactoringError("Cannot extract staticmethod with reference to {}".format(self._get_self_name())) + elif self._extracting_staticmethod() and not self.info.method: raise RefactoringError("Cannot extract to staticmethod outside class") + elif self._extracting_classmethod() and self._self_name_in_body(): + raise RefactoringError("Cannot extract classmethod with reference to {}".format(self._get_scope_self_name())) + elif self._extracting_classmethod() and not self.info.method: + raise RefactoringError("Cannot extract to classmethod outside class") def _self_name_in_body(self): - return self._get_self_name() and self._get_self_name() in self.info.extracted + return self._get_scope_self_name() and self._get_scope_self_name() in self.info.extracted def _extracting_from_static(self): return self.info.method and _get_function_kind(self.info.scope) == "staticmethod" @@ -515,12 +522,7 @@ def _get_function_definition(self): returns = self._find_function_returns() result = [] - - if self._extracting_static(): - result.append('@staticmethod\n') - if self._extracting_classmethod(): - result.append('@classmethod\n') - + self._append_decorators(result) result.append('def %s:\n' % self._get_function_signature(args)) unindented_body = self._get_unindented_function_body(returns) indents = sourceutils.get_indent(self.info.project) @@ -530,11 +532,17 @@ def _get_function_definition(self): return definition + '\n' + def _append_decorators(self, result): + if self._extracting_staticmethod(): + result.append('@staticmethod\n') + elif self._extracting_classmethod(): + result.append('@classmethod\n') + def _extracting_classmethod(self): return self.info.classmethod - def _extracting_static(self): - return self.info.static + def _extracting_staticmethod(self): + return self.info.staticmethod def _get_function_signature(self, args): args = list(args) @@ -551,12 +559,17 @@ def _get_function_signature(self, args): '(%s)' % self._get_comma_form(args) def _extracting_method(self): - return not self._extracting_static() and \ + return not self._extracting_staticmethod() and \ (self.info.method and \ not self.info.make_global and \ _get_function_kind(self.info.scope) == 'method') def _get_self_name(self): + if self._extracting_classmethod(): + return "cls" + return self._get_scope_self_name() + + def _get_scope_self_name(self): param_names = self.info.scope.pyobject.get_param_names() if param_names: return param_names[0] @@ -570,7 +583,7 @@ def _get_function_call(self, args): def _get_function_call_prefix(self, args): prefix = '' if self.info.method and not self.info.make_global: - if self._extracting_static() or self._extracting_classmethod(): + if self._extracting_staticmethod() or self._extracting_classmethod(): prefix = self.info.scope.parent.pyobject.get_name() + '.' else: self_name = self._get_self_name() diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index 5e125c055..77406d624 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1571,7 +1571,7 @@ def second_method(a_var): ''') self.assertEqual(expected, refactored) - def test_extract_to_static_method_when_self_in_body_should_raise_error(self): + def test_extract_to_staticmethod_when_self_in_body_should_raise_error(self): code = dedent('''\ class A: def first_method(self): @@ -1580,7 +1580,7 @@ def first_method(self): ''') extract_target = 'self.a_var + 1' start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) - with self.assertRaisesRegexp(rope.base.exceptions.RefactoringError, "extract static method with reference to self"): + with self.assertRaisesRegex(rope.base.exceptions.RefactoringError, "extract staticmethod with reference to self"): self.do_extract_method(code, start, end, 'second_method', kind="staticmethod") def test_extract_from_function_to_staticmethod_raises_exception(self): @@ -1591,7 +1591,7 @@ def first_method(): ''') extract_target = 'a_var + 1' start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) - with self.assertRaisesRegexp(rope.base.exceptions.RefactoringError, "Cannot extract to staticmethod outside class"): + with self.assertRaisesRegex(rope.base.exceptions.RefactoringError, "Cannot extract to staticmethod outside class"): self.do_extract_method(code, start, end, 'second_method', kind="staticmethod") def test_extract_method_in_classmethods(self): @@ -1616,6 +1616,51 @@ def one(cls): ''') self.assertEqual(expected, refactored) + def test_extract_from_function_to_classmethod_raises_exception(self): + code = dedent('''\ + def first_method(): + a_var = 1 + b_var = a_var + 1 + ''') + extract_target = 'a_var + 1' + start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) + with self.assertRaisesRegex(rope.base.exceptions.RefactoringError, "Cannot extract to classmethod outside class"): + self.do_extract_method(code, start, end, 'second_method', kind="classmethod") + + def test_extract_to_classmethod_when_self_in_body_should_raise_error(self): + code = dedent('''\ + class A: + def first_method(self): + a_var = 1 + b_var = self.a_var + 1 + ''') + extract_target = 'self.a_var + 1' + start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) + with self.assertRaisesRegex(rope.base.exceptions.RefactoringError, "extract classmethod with reference to self"): + self.do_extract_method(code, start, end, 'second_method', kind="classmethod") + + def test_extract_to_classmethod(self): + code = dedent('''\ + class A: + def first_method(self): + a_var = 1 + b_var = a_var + 1 + ''') + extract_target = 'a_var + 1' + start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) + refactored = self.do_extract_method(code, start, end, 'second_method', kind="classmethod") + expected = dedent('''\ + class A: + def first_method(self): + a_var = 1 + b_var = A.second_method(a_var) + + @classmethod + def second_method(cls, a_var): + return a_var + 1 + ''') + self.assertEqual(expected, refactored) + if __name__ == '__main__': unittest.main() From 48d9b3442a351ce2755f1d3026002daf08c33b4f Mon Sep 17 00:00:00 2001 From: climbus Date: Sun, 12 Sep 2021 11:01:34 +0200 Subject: [PATCH 18/24] use kind like in PyFunctionDef --- rope/refactor/extract.py | 54 ++++++++++++++++++-------------- ropetest/refactor/extracttest.py | 8 ++--- 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index af5c84da1..5e4ad4ffc 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -66,23 +66,27 @@ def get_changes(self, extracted_name, similar=False, global_=False, kind=None): """ info = _ExtractInfo( self.project, self.resource, self.start_offset, self.end_offset, - extracted_name, variable=self.kind == 'variable', + extracted_name, variable=self._get_kind(kind) == 'variable', similar=similar, make_global=global_) - info.staticmethod = kind == "staticmethod" - info.classmethod = kind == "classmethod" + info.kind = self._get_kind(kind) new_contents = _ExtractPerformer(info).extract() - changes = ChangeSet('Extract %s <%s>' % (self.kind, + changes = ChangeSet('Extract %s <%s>' % (info.kind, extracted_name)) changes.add_change(ChangeContents(self.resource, new_contents)) return changes + @classmethod + def _get_kind(cls, kind): + raise NotImplementedError(f"You have to sublass {cls}") -class ExtractMethod(_ExtractRefactoring): - - def __init__(self, *args, **kwds): - super(ExtractMethod, self).__init__(*args, **kwds) +class ExtractMethod(_ExtractRefactoring): kind = 'method' + allowed_kinds = ("function", "method", "staticmethod", "classmethod") + + @classmethod + def _get_kind(cls, kind): + return kind if kind in cls.allowed_kinds else cls.kind class ExtractVariable(_ExtractRefactoring): @@ -94,6 +98,8 @@ def __init__(self, *args, **kwds): kind = 'variable' + def _get_kind(cls, kind): + return cls.kind class _ExtractInfo(object): """Holds information about the extract to be performed""" @@ -110,8 +116,7 @@ def __init__(self, project, resource, start, end, new_name, self.variable = variable self.similar = similar self._init_parts(start, end) - self.staticmethod = False - self.classmethod = False + self.kind = None self._init_scope() self.make_global = make_global @@ -445,24 +450,27 @@ class _ExtractMethodParts(object): def __init__(self, info): self.info = info self.info_collector = self._create_info_collector() - self.info.staticmethod = True if self._extracting_from_static() else self.info.staticmethod - self.info.classmethod = True if self._extracting_from_classmethod() else self.info.classmethod + self.info.kind = self._get_kind_by_scope() self._check_constraints() + def _get_kind_by_scope(self): + if self._extacting_from_staticmethod(): + return "staticmethod" + elif self._extracting_from_classmethod(): + return "classmethod" + return self.info.kind + def _check_constraints(self): - if self._extracting_staticmethod() and self._self_name_in_body(): - raise RefactoringError("Cannot extract staticmethod with reference to {}".format(self._get_self_name())) - elif self._extracting_staticmethod() and not self.info.method: - raise RefactoringError("Cannot extract to staticmethod outside class") - elif self._extracting_classmethod() and self._self_name_in_body(): - raise RefactoringError("Cannot extract classmethod with reference to {}".format(self._get_scope_self_name())) - elif self._extracting_classmethod() and not self.info.method: - raise RefactoringError("Cannot extract to classmethod outside class") + if self._extracting_staticmethod() or self._extracting_classmethod(): + if self._self_name_in_body(): + raise RefactoringError("Cannot extract staticmethod/classmethod with reference to {}".format(self._get_scope_self_name())) + elif not self.info.method: + raise RefactoringError("Cannot extract to staticmethod/classmethod outside class") def _self_name_in_body(self): return self._get_scope_self_name() and self._get_scope_self_name() in self.info.extracted - def _extracting_from_static(self): + def _extacting_from_staticmethod(self): return self.info.method and _get_function_kind(self.info.scope) == "staticmethod" def _extracting_from_classmethod(self): @@ -539,10 +547,10 @@ def _append_decorators(self, result): result.append('@classmethod\n') def _extracting_classmethod(self): - return self.info.classmethod + return self.info.kind == "classmethod" def _extracting_staticmethod(self): - return self.info.staticmethod + return self.info.kind == "staticmethod" def _get_function_signature(self, args): args = list(args) diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index 77406d624..a15a70198 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1580,7 +1580,7 @@ def first_method(self): ''') extract_target = 'self.a_var + 1' start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) - with self.assertRaisesRegex(rope.base.exceptions.RefactoringError, "extract staticmethod with reference to self"): + with self.assertRaisesRegex(rope.base.exceptions.RefactoringError, "extract staticmethod/classmethod with reference to self"): self.do_extract_method(code, start, end, 'second_method', kind="staticmethod") def test_extract_from_function_to_staticmethod_raises_exception(self): @@ -1591,7 +1591,7 @@ def first_method(): ''') extract_target = 'a_var + 1' start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) - with self.assertRaisesRegex(rope.base.exceptions.RefactoringError, "Cannot extract to staticmethod outside class"): + with self.assertRaisesRegex(rope.base.exceptions.RefactoringError, "Cannot extract to staticmethod/classmethod outside class"): self.do_extract_method(code, start, end, 'second_method', kind="staticmethod") def test_extract_method_in_classmethods(self): @@ -1624,7 +1624,7 @@ def first_method(): ''') extract_target = 'a_var + 1' start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) - with self.assertRaisesRegex(rope.base.exceptions.RefactoringError, "Cannot extract to classmethod outside class"): + with self.assertRaisesRegex(rope.base.exceptions.RefactoringError, "Cannot extract to staticmethod/classmethod outside class"): self.do_extract_method(code, start, end, 'second_method', kind="classmethod") def test_extract_to_classmethod_when_self_in_body_should_raise_error(self): @@ -1636,7 +1636,7 @@ def first_method(self): ''') extract_target = 'self.a_var + 1' start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) - with self.assertRaisesRegex(rope.base.exceptions.RefactoringError, "extract classmethod with reference to self"): + with self.assertRaisesRegex(rope.base.exceptions.RefactoringError, "extract staticmethod/classmethod with reference to self"): self.do_extract_method(code, start, end, 'second_method', kind="classmethod") def test_extract_to_classmethod(self): From 1b5d5e277c5c2ccf4cc598eb2bbd823f12d03337 Mon Sep 17 00:00:00 2001 From: climbus Date: Sun, 12 Sep 2021 11:05:29 +0200 Subject: [PATCH 19/24] removed f-string --- rope/refactor/extract.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index 5e4ad4ffc..6a9756655 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -77,7 +77,7 @@ def get_changes(self, extracted_name, similar=False, global_=False, kind=None): @classmethod def _get_kind(cls, kind): - raise NotImplementedError(f"You have to sublass {cls}") + raise NotImplementedError("You have to sublass {}".format(cls)) class ExtractMethod(_ExtractRefactoring): From 80882e93f033181339bc81026420a3810813dafe Mon Sep 17 00:00:00 2001 From: climbus Date: Sun, 12 Sep 2021 11:17:05 +0200 Subject: [PATCH 20/24] assertraisesregex not supported in 2.7 --- ropetest/refactor/extracttest.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index a15a70198..8a9ebd12b 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1580,7 +1580,7 @@ def first_method(self): ''') extract_target = 'self.a_var + 1' start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) - with self.assertRaisesRegex(rope.base.exceptions.RefactoringError, "extract staticmethod/classmethod with reference to self"): + with self.assertRaisesRegexp(rope.base.exceptions.RefactoringError, "extract staticmethod/classmethod with reference to self"): self.do_extract_method(code, start, end, 'second_method', kind="staticmethod") def test_extract_from_function_to_staticmethod_raises_exception(self): @@ -1591,7 +1591,7 @@ def first_method(): ''') extract_target = 'a_var + 1' start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) - with self.assertRaisesRegex(rope.base.exceptions.RefactoringError, "Cannot extract to staticmethod/classmethod outside class"): + with self.assertRaisesRegexp(rope.base.exceptions.RefactoringError, "Cannot extract to staticmethod/classmethod outside class"): self.do_extract_method(code, start, end, 'second_method', kind="staticmethod") def test_extract_method_in_classmethods(self): @@ -1624,7 +1624,7 @@ def first_method(): ''') extract_target = 'a_var + 1' start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) - with self.assertRaisesRegex(rope.base.exceptions.RefactoringError, "Cannot extract to staticmethod/classmethod outside class"): + with self.assertRaisesRegexp(rope.base.exceptions.RefactoringError, "Cannot extract to staticmethod/classmethod outside class"): self.do_extract_method(code, start, end, 'second_method', kind="classmethod") def test_extract_to_classmethod_when_self_in_body_should_raise_error(self): @@ -1636,7 +1636,7 @@ def first_method(self): ''') extract_target = 'self.a_var + 1' start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) - with self.assertRaisesRegex(rope.base.exceptions.RefactoringError, "extract staticmethod/classmethod with reference to self"): + with self.assertRaisesRegexp(rope.base.exceptions.RefactoringError, "extract staticmethod/classmethod with reference to self"): self.do_extract_method(code, start, end, 'second_method', kind="classmethod") def test_extract_to_classmethod(self): From d0b5eaff6cd42f2d4ee3c444a7475a92884dfde1 Mon Sep 17 00:00:00 2001 From: climbus Date: Mon, 13 Sep 2021 19:38:29 +0200 Subject: [PATCH 21/24] refactor and docs --- docs/overview.rst | 4 +++ rope/refactor/extract.py | 19 +++++++++++++ ropetest/refactor/extracttest.py | 48 ++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+) diff --git a/docs/overview.rst b/docs/overview.rst index 3fb32727f..98f143ad9 100644 --- a/docs/overview.rst +++ b/docs/overview.rst @@ -444,6 +444,10 @@ if you extract ``a * 2`` as a method you'll get: def twice(a): return a * 2 +You can indicate method kind prefixing name: + +- @ - classmethod (@name) +- $ - staticmethod ($name) Inline Method Refactoring ------------------------- diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index 6a9756655..f57585cda 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -36,6 +36,8 @@ # classes. class _ExtractRefactoring(object): + kind_prefixes = {} + def __init__(self, project, resource, start_offset, end_offset, variable=False): self.project = project @@ -57,6 +59,8 @@ def get_changes(self, extracted_name, similar=False, global_=False, kind=None): """Get the changes this refactoring makes :parameters: + - `extracted_name`: target name, when starts with @ - set kind to + classmethod, $ - staticmethod - `similar`: if `True`, similar expressions/statements are also replaced. - `global_`: if `True`, the extracted method/variable will @@ -64,6 +68,8 @@ def get_changes(self, extracted_name, similar=False, global_=False, kind=None): - `kind`: kind of target refactoring to (staticmethod, classmethod) """ + extracted_name, kind = self._get_kind_from_name(extracted_name, kind) + info = _ExtractInfo( self.project, self.resource, self.start_offset, self.end_offset, extracted_name, variable=self._get_kind(kind) == 'variable', @@ -75,6 +81,18 @@ def get_changes(self, extracted_name, similar=False, global_=False, kind=None): changes.add_change(ChangeContents(self.resource, new_contents)) return changes + def _get_kind_from_name(self, extracted_name, kind): + for sign, selected_kind in self.kind_prefixes.items(): + if extracted_name.startswith(sign): + self._validate_kind_prefix(kind, selected_kind) + return extracted_name[1:], selected_kind + return extracted_name, kind + + @staticmethod + def _validate_kind_prefix(kind, selected_kind): + if kind and kind != selected_kind: + raise RefactoringError("Kind and shorcut in name missmatch") + @classmethod def _get_kind(cls, kind): raise NotImplementedError("You have to sublass {}".format(cls)) @@ -83,6 +101,7 @@ def _get_kind(cls, kind): class ExtractMethod(_ExtractRefactoring): kind = 'method' allowed_kinds = ("function", "method", "staticmethod", "classmethod") + kind_prefixes = {"@": "classmethod", "$": "staticmethod"} @classmethod def _get_kind(cls, kind): diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index 8a9ebd12b..c3ac59cd7 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1661,6 +1661,54 @@ def second_method(cls, a_var): ''') self.assertEqual(expected, refactored) + def test_extract_to_classmethod_when_name_starts_with_at_sign(self): + code = dedent('''\ + class A: + def first_method(self): + a_var = 1 + b_var = a_var + 1 + ''') + extract_target = 'a_var + 1' + start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) + refactored = self.do_extract_method(code, start, end, '@second_method') + expected = dedent('''\ + class A: + def first_method(self): + a_var = 1 + b_var = A.second_method(a_var) + + @classmethod + def second_method(cls, a_var): + return a_var + 1 + ''') + self.assertEqual(expected, refactored) + + def test_extract_to_staticmethod_when_name_starts_with_dollar_sign(self): + code = dedent('''\ + class A: + def first_method(self): + a_var = 1 + b_var = a_var + 1 + ''') + extract_target = 'a_var + 1' + start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) + refactored = self.do_extract_method(code, start, end, '$second_method') + expected = dedent('''\ + class A: + def first_method(self): + a_var = 1 + b_var = A.second_method(a_var) + + @staticmethod + def second_method(a_var): + return a_var + 1 + ''') + self.assertEqual(expected, refactored) + + def test_raises_exception_when_sign_in_name_and_kind_missmatch(self): + with self.assertRaisesRegexp(rope.base.exceptions.RefactoringError, "Kind and shorcut in name missmatch"): + self.do_extract_method("code", 0,1, '$second_method', kind="classmethod") + if __name__ == '__main__': unittest.main() From 0944ad9bb518302221f3f42c7d900dec002c1147 Mon Sep 17 00:00:00 2001 From: climbus Date: Wed, 15 Sep 2021 19:40:04 +0200 Subject: [PATCH 22/24] fixed bug in self name --- rope/refactor/extract.py | 2 ++ ropetest/refactor/extracttest.py | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index f57585cda..483efeacd 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -597,6 +597,8 @@ def _get_self_name(self): return self._get_scope_self_name() def _get_scope_self_name(self): + if self.info.scope.pyobject.get_kind() == "staticmethod": + return param_names = self.info.scope.pyobject.get_param_names() if param_names: return param_names[0] diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index c3ac59cd7..184f70a3b 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1709,6 +1709,30 @@ def test_raises_exception_when_sign_in_name_and_kind_missmatch(self): with self.assertRaisesRegexp(rope.base.exceptions.RefactoringError, "Kind and shorcut in name missmatch"): self.do_extract_method("code", 0,1, '$second_method', kind="classmethod") + def test_extracting_from_static_with_function_arg(self): + code = dedent('''\ + class A: + @staticmethod + def first_method(someargs): + b_var = someargs + 1 + ''') + + extract_target = 'someargs + 1' + start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) + refactored = self.do_extract_method(code, start, end, 'second_method') + expected = dedent('''\ + class A: + @staticmethod + def first_method(someargs): + b_var = A.second_method(someargs) + + @staticmethod + def second_method(someargs): + return someargs + 1 + ''') + + self.assertEqual(expected, refactored) + if __name__ == '__main__': unittest.main() From b7d7af2e8530319591e94b58f503a8c28d4e9c17 Mon Sep 17 00:00:00 2001 From: climbus Date: Thu, 16 Sep 2021 15:23:58 +0200 Subject: [PATCH 23/24] dont check self in body condition --- rope/refactor/extract.py | 7 +------ ropetest/refactor/extracttest.py | 32 ++++++++++++++++++++++++++------ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index 483efeacd..103b98cf7 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -481,14 +481,9 @@ def _get_kind_by_scope(self): def _check_constraints(self): if self._extracting_staticmethod() or self._extracting_classmethod(): - if self._self_name_in_body(): - raise RefactoringError("Cannot extract staticmethod/classmethod with reference to {}".format(self._get_scope_self_name())) - elif not self.info.method: + if not self.info.method: raise RefactoringError("Cannot extract to staticmethod/classmethod outside class") - def _self_name_in_body(self): - return self._get_scope_self_name() and self._get_scope_self_name() in self.info.extracted - def _extacting_from_staticmethod(self): return self.info.method and _get_function_kind(self.info.scope) == "staticmethod" diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index 184f70a3b..d55debec8 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1571,7 +1571,7 @@ def second_method(a_var): ''') self.assertEqual(expected, refactored) - def test_extract_to_staticmethod_when_self_in_body_should_raise_error(self): + def test_extract_to_staticmethod_when_self_in_body(self): code = dedent('''\ class A: def first_method(self): @@ -1580,8 +1580,18 @@ def first_method(self): ''') extract_target = 'self.a_var + 1' start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) - with self.assertRaisesRegexp(rope.base.exceptions.RefactoringError, "extract staticmethod/classmethod with reference to self"): - self.do_extract_method(code, start, end, 'second_method', kind="staticmethod") + refactored = self.do_extract_method(code, start, end, 'second_method', kind="staticmethod") + expected = dedent('''\ + class A: + def first_method(self): + a_var = 1 + b_var = A.second_method(self) + + @staticmethod + def second_method(self): + return self.a_var + 1 + ''') + self.assertEqual(expected, refactored) def test_extract_from_function_to_staticmethod_raises_exception(self): code = dedent('''\ @@ -1627,7 +1637,7 @@ def first_method(): with self.assertRaisesRegexp(rope.base.exceptions.RefactoringError, "Cannot extract to staticmethod/classmethod outside class"): self.do_extract_method(code, start, end, 'second_method', kind="classmethod") - def test_extract_to_classmethod_when_self_in_body_should_raise_error(self): + def test_extract_to_classmethod_when_self_in_body(self): code = dedent('''\ class A: def first_method(self): @@ -1636,8 +1646,18 @@ def first_method(self): ''') extract_target = 'self.a_var + 1' start, end = code.index(extract_target), code.index(extract_target) + len(extract_target) - with self.assertRaisesRegexp(rope.base.exceptions.RefactoringError, "extract staticmethod/classmethod with reference to self"): - self.do_extract_method(code, start, end, 'second_method', kind="classmethod") + refactored = self.do_extract_method(code, start, end, 'second_method', kind="classmethod") + expected = dedent('''\ + class A: + def first_method(self): + a_var = 1 + b_var = A.second_method(self) + + @classmethod + def second_method(cls, self): + return self.a_var + 1 + ''') + self.assertEqual(expected, refactored) def test_extract_to_classmethod(self): code = dedent('''\ From e1f6103b844805fc15b17ab074b8ee76f02ab460 Mon Sep 17 00:00:00 2001 From: lieryan Date: Sat, 18 Sep 2021 06:14:24 +1000 Subject: [PATCH 24/24] Change workflow to run on pull_request_target The task-completed-checker-action seems to be complaining of "Error: Resource not accessible by integration" whenever it ran from a forked repository. This *should* allow the task-completed-checker-action to run on a forked branch without causing that error. --- .github/workflows/task-completed-checker-action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/task-completed-checker-action.yml b/.github/workflows/task-completed-checker-action.yml index db6961f49..6bd2fae06 100644 --- a/.github/workflows/task-completed-checker-action.yml +++ b/.github/workflows/task-completed-checker-action.yml @@ -1,6 +1,6 @@ name: 'PR Tasks Completed Check' on: - pull_request: + pull_request_target: types: [opened, edited] jobs: