From 7ebf247970f743eb5090a4928167226998e2c70f Mon Sep 17 00:00:00 2001 From: Ray Myers Date: Thu, 14 Dec 2023 23:06:53 -0600 Subject: [PATCH 1/3] Add error handling for extract of incomplete block --- CHANGELOG.md | 1 + rope/refactor/extract.py | 29 ++++++++++++++++ rope/refactor/sourceutils.py | 14 ++++++++ ropetest/refactor/extracttest.py | 58 ++++++++++++++++++++++++++++++++ 4 files changed, 102 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd0c47c1a..28df1506a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - #719 Allows the in-memory db to be shared across threads (@tkrabel) - #720 create one sqlite3.Connection per thread using a thread local (@tkrabel) - #715 change AutoImport's `get_modules` to be case sensitive (@bagel897) +- #734 raise exception when extracting the start of a block without the end # Release 1.10.0 diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index c16c7b0c1..506affaa3 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -444,8 +444,10 @@ def __call__(self, info): def base_conditions(self, info): if info.region[1] > info.scope_region[1]: raise RefactoringError("Bad region selected for extract method") + end_line = info.region_lines[1] end_scope = info.global_scope.get_inner_scope_for_line(end_line) + if end_scope != info.scope and end_scope.get_end() != end_line: raise RefactoringError("Bad region selected for extract method") try: @@ -498,6 +500,33 @@ def multi_line_conditions(self, info): "Extracted piece should contain complete statements." ) + if self._is_region_incomplete_block(info): + raise RefactoringError( + "Extracted piece cannot contain the start of a block without the end" + ) + + def _is_region_incomplete_block(self, info): + """ + Is end more indented than start, and does that level continue outside the region? + If so, this is an incomplete block that cannot be extracted. + """ + + def get_effective_indent(lines, line): + if found_line := sourceutils.find_nonblank_line(lines, line): + return sourceutils.get_indents(info.pymodule.lines, found_line) + return None + + start_line = info.region_lines[0] + end_line = info.region_lines[1] + start_indent = get_effective_indent(info.pymodule.lines, start_line) + end_indent = get_effective_indent(info.pymodule.lines, end_line) + end_next_indent = get_effective_indent(info.pymodule.lines, end_line + 1) + return ( + end_next_indent is not None + and start_indent < end_indent + and end_next_indent >= end_indent + ) + def _is_region_on_a_word(self, info): if ( info.region[0] > 0 diff --git a/rope/refactor/sourceutils.py b/rope/refactor/sourceutils.py index d55aad4c9..5102640eb 100644 --- a/rope/refactor/sourceutils.py +++ b/rope/refactor/sourceutils.py @@ -1,4 +1,5 @@ from rope.base import codeanalyze +from typing import Optional def get_indents(lines, lineno): @@ -91,3 +92,16 @@ def get_body_region(defined): def get_indent(project): return project.prefs.get("indent_size", 4) + + +def find_nonblank_line( + lines, start_line: int, skip_comments: bool = True +) -> Optional[int]: + """Return index of first non-blank line starting with start_line, None if not found""" + next_line = start_line + while next_line < lines.length(): + line_code = lines.get_line(next_line).strip() + if line_code and (not skip_comments or not line_code.startswith("#")): + return next_line + next_line = next_line + 1 + return None diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index ed4054c06..e8c2fc206 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1149,6 +1149,64 @@ def xxx_test_raising_exception_on_function_parens(self): end = code.rindex(")") + 1 with self.assertRaises(rope.base.exceptions.RefactoringError): self.do_extract_method(code, start, end, "new_func") + + def test_raising_exception_on_incomplete_block(self): + code = dedent("""\ + if True: + a = 1 + b = 2 + """) + start = code.index("if") + end = code.index("1") + 1 + with self.assertRaises(rope.base.exceptions.RefactoringError): + self.do_extract_method(code, start, end, "new_func") + + def test_raising_exception_on_incomplete_block_2(self): + code = dedent("""\ + if True: + a = 1 + # + b = 2 + """) + start = code.index("if") + end = code.index("1") + 1 + with self.assertRaises(rope.base.exceptions.RefactoringError): + self.do_extract_method(code, start, end, "new_func") + + def test_raising_exception_on_incomplete_block_3(self): + code = dedent("""\ + if True: + a = 1 + + b = 2 + """) + start = code.index("if") + end = code.index("1") + 1 + with self.assertRaises(rope.base.exceptions.RefactoringError): + self.do_extract_method(code, start, end, "new_func") + + def test_raising_exception_on_incomplete_block_4(self): + code = dedent("""\ + # + if True: + a = 1 + b = 2 + """) + start = code.index("#") + end = code.index("1") + 1 + with self.assertRaises(rope.base.exceptions.RefactoringError): + self.do_extract_method(code, start, end, "new_func") + + def test_raising_exception_on_incomplete_block_5(self): + code = dedent("""\ + if True: + if 0: + a = 1 + """) + start = code.index("if") + end = code.index("0:") + 2 + with self.assertRaises(rope.base.exceptions.RefactoringError): + self.do_extract_method(code, start, end, "new_func") def test_extract_method_and_extra_blank_lines(self): code = dedent("""\ From f4fc0cdb1eb39f5a137477a4bba7287faaa6c4c2 Mon Sep 17 00:00:00 2001 From: Ray Myers Date: Tue, 19 Dec 2023 19:51:59 -0600 Subject: [PATCH 2/3] Add more tests around extract indentation --- ropetest/refactor/extracttest.py | 33 ++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index e8c2fc206..b3818ae0f 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1208,6 +1208,39 @@ def test_raising_exception_on_incomplete_block_5(self): with self.assertRaises(rope.base.exceptions.RefactoringError): self.do_extract_method(code, start, end, "new_func") + def test_no_incomplete_error_for_weird_indentation(self): + code = dedent("""\ + def foo(): + if foo: + s = \""" + blah blah + blah + \""" + print( + a, b, c + ) + """) + start = code.index("s =") + 3 + after_first_triple_quote = code.index('"""') + 3 + end = code.index('"""', after_first_triple_quote) + 3 + self.do_extract_method(code, start, end, "new_func") + + def test_no_incomplete_error_for_weird_indentation2(self): + code = dedent("""\ + def foo(): + print( + a, [ + 3, + 4 + ], + c + ) + """) + start = code.index("[") + end = code.index(']') + 1 + print(code[start:end]) + self.do_extract_method(code, start, end, "new_func") + def test_extract_method_and_extra_blank_lines(self): code = dedent("""\ From 9cb3031e114dbb1d8ce9219988930bae6879eb98 Mon Sep 17 00:00:00 2001 From: Ray Myers Date: Sun, 7 Jan 2024 14:05:47 -0600 Subject: [PATCH 3/3] Use AST visitor for extract incomplete block check --- rope/refactor/extract.py | 59 ++++++++++++++++++-------------- rope/refactor/sourceutils.py | 14 -------- ropetest/refactor/extracttest.py | 33 ------------------ 3 files changed, 34 insertions(+), 72 deletions(-) diff --git a/rope/refactor/extract.py b/rope/refactor/extract.py index 506affaa3..1b1659fad 100644 --- a/rope/refactor/extract.py +++ b/rope/refactor/extract.py @@ -499,34 +499,15 @@ def multi_line_conditions(self, info): raise RefactoringError( "Extracted piece should contain complete statements." ) - - if self._is_region_incomplete_block(info): + unbalanced_region_finder = _UnbalancedRegionFinder( + info.region_lines[0], info.region_lines[1] + ) + unbalanced_region_finder.visit(info.pymodule.ast_node) + if unbalanced_region_finder.error: raise RefactoringError( - "Extracted piece cannot contain the start of a block without the end" + "Extracted piece cannot contain the start of a block without the end." ) - def _is_region_incomplete_block(self, info): - """ - Is end more indented than start, and does that level continue outside the region? - If so, this is an incomplete block that cannot be extracted. - """ - - def get_effective_indent(lines, line): - if found_line := sourceutils.find_nonblank_line(lines, line): - return sourceutils.get_indents(info.pymodule.lines, found_line) - return None - - start_line = info.region_lines[0] - end_line = info.region_lines[1] - start_indent = get_effective_indent(info.pymodule.lines, start_line) - end_indent = get_effective_indent(info.pymodule.lines, end_line) - end_next_indent = get_effective_indent(info.pymodule.lines, end_line + 1) - return ( - end_next_indent is not None - and start_indent < end_indent - and end_next_indent >= end_indent - ) - def _is_region_on_a_word(self, info): if ( info.region[0] > 0 @@ -1122,6 +1103,34 @@ def _ClassDef(self, node): pass +class _UnbalancedRegionFinder(_BaseErrorFinder): + """ + Flag an error if we are including the start of a block without the end. + We detect this by ensuring there is no AST node that starts inside the + selected range but ends outside of it. + """ + + def __init__(self, line_start: int, line_end: int): + self.error = False + self.line_start = line_start + self.line_end = line_end + + def generic_visit(self, node: ast.AST): + if not hasattr(node, "end_lineno"): + super().generic_visit(node) # Visit children + return + ends_before_range_starts = node.end_lineno < self.line_start + starts_after_range_ends = node.lineno > self.line_end + if ends_before_range_starts or starts_after_range_ends: + return # Don't visit children + starts_on_or_after_range_start = node.lineno >= self.line_start + ends_after_range_ends = node.end_lineno > self.line_end + if starts_on_or_after_range_start and ends_after_range_ends: + self.error = True + return # Don't visit children + super().generic_visit(node) # Visit children + + class _GlobalFinder(ast.RopeNodeVisitor): def __init__(self): self.globals_ = OrderedSet() diff --git a/rope/refactor/sourceutils.py b/rope/refactor/sourceutils.py index 5102640eb..d55aad4c9 100644 --- a/rope/refactor/sourceutils.py +++ b/rope/refactor/sourceutils.py @@ -1,5 +1,4 @@ from rope.base import codeanalyze -from typing import Optional def get_indents(lines, lineno): @@ -92,16 +91,3 @@ def get_body_region(defined): def get_indent(project): return project.prefs.get("indent_size", 4) - - -def find_nonblank_line( - lines, start_line: int, skip_comments: bool = True -) -> Optional[int]: - """Return index of first non-blank line starting with start_line, None if not found""" - next_line = start_line - while next_line < lines.length(): - line_code = lines.get_line(next_line).strip() - if line_code and (not skip_comments or not line_code.startswith("#")): - return next_line - next_line = next_line + 1 - return None diff --git a/ropetest/refactor/extracttest.py b/ropetest/refactor/extracttest.py index b3818ae0f..e8c2fc206 100644 --- a/ropetest/refactor/extracttest.py +++ b/ropetest/refactor/extracttest.py @@ -1208,39 +1208,6 @@ def test_raising_exception_on_incomplete_block_5(self): with self.assertRaises(rope.base.exceptions.RefactoringError): self.do_extract_method(code, start, end, "new_func") - def test_no_incomplete_error_for_weird_indentation(self): - code = dedent("""\ - def foo(): - if foo: - s = \""" - blah blah - blah - \""" - print( - a, b, c - ) - """) - start = code.index("s =") + 3 - after_first_triple_quote = code.index('"""') + 3 - end = code.index('"""', after_first_triple_quote) + 3 - self.do_extract_method(code, start, end, "new_func") - - def test_no_incomplete_error_for_weird_indentation2(self): - code = dedent("""\ - def foo(): - print( - a, [ - 3, - 4 - ], - c - ) - """) - start = code.index("[") - end = code.index(']') + 1 - print(code[start:end]) - self.do_extract_method(code, start, end, "new_func") - def test_extract_method_and_extra_blank_lines(self): code = dedent("""\