From c379c30af4630b90b06930e3222e5116f7bf83a4 Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Thu, 16 Jul 2026 20:15:04 +0200 Subject: [PATCH] Add heuristic to remove superfluous empty lines when deleting a symbol #1697 --- CHANGELOG.md | 1 + src/serena/code_editor.py | 24 +++++++++++++- src/solidlsp/ls_utils.py | 19 +++++++++-- test/serena/test_symbol_editing.py | 52 +++++++++++++++++++++++++++--- test/solidlsp/test_text_utils.py | 9 +++++- 5 files changed, 97 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e805d7d5..ea53bb24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Status of the `main` branch. Changes prior to the next official version change w break, because the match's exclusive end index was mapped to a line number directly and therefore resolved to the start of the following line. The line the match ends on is now determined correctly, which also keeps `context_lines_after` aligned. + - `safe_delete`: Add heuristic to delete superfluous empty lines after a deletion * JetBrains: - Allow external files from dependencies (specified via references like "") to be diff --git a/src/serena/code_editor.py b/src/serena/code_editor.py index c4894a08..7652de09 100644 --- a/src/serena/code_editor.py +++ b/src/serena/code_editor.py @@ -10,7 +10,7 @@ from serena.jetbrains.jetbrains_plugin_client import JetBrainsPluginClient from serena.symbol import JetBrainsSymbol, LanguageServerSymbol, LanguageServerSymbolRetriever, PositionInFile, Symbol from solidlsp import SolidLanguageServer, ls_types from solidlsp.ls import LSPFileBuffer -from solidlsp.ls_utils import PathUtils, TextUtils +from solidlsp.ls_utils import PathUtils, TextStepper, TextUtils from .project import Project from .util.file_proxy import FileProxy @@ -242,9 +242,31 @@ class CodeEditor(Generic[TSymbol], ABC): symbol = self._find_unique_symbol(name_path, relative_file_path) start_pos = symbol.get_body_start_position_or_raise() end_pos = symbol.get_body_end_position_or_raise() + with self.edited_file_context(relative_file_path) as edited_file: + # do the actual deletion edited_file.delete_text_between_positions(start_pos, end_pos) + # empty line removal heuristic: + # After the deletion, go to the line where the deleted symbol started, + # check whether it and any subsequent lines are empty, and if so, delete them. + # The empty lines preceding the deleted symbol's body should normally suffice as a separator + # between the symbols before and after the deleted symbol. + start_pos.col = 0 + stepper = TextStepper(edited_file.get_contents()) + stepper.step_to(start_pos.line, start_pos.col) + apply_empty_line_deletion = False + while stepper.step_line(): + line = stepper.get_last_line(with_end=False) + if line.strip() == "": + end_pos.line = stepper.line + end_pos.col = stepper.col + apply_empty_line_deletion = True + else: + break + if apply_empty_line_deletion: + edited_file.delete_text_between_positions(start_pos, end_pos) + @abstractmethod def rename_symbol(self, name_path: str, relative_path: str, new_name: str) -> str: pass diff --git a/src/solidlsp/ls_utils.py b/src/solidlsp/ls_utils.py index f00e1f04..ea4b5641 100644 --- a/src/solidlsp/ls_utils.py +++ b/src/solidlsp/ls_utils.py @@ -122,6 +122,21 @@ class TextStepper: self.is_newline = False return True + def step_to(self, line: int, col: int): + """ + Steps through the text until the given line and column are reached, or until the end of the text is reached. + + :param line: the 0-based line number to step to + :param col: the 0-based column number to step to + """ + while self.line < line: + if not self.step_line(): + break + if self.line != line: + raise InvalidTextLocationError + self.idx += col + self.col = col + def process_all(self): """ Processes all characters in the text, updating the line and column numbers accordingly. @@ -129,7 +144,7 @@ class TextStepper: while self.step_line(): pass - def _get_last_line(self, with_end: bool) -> str: + def get_last_line(self, with_end: bool) -> str: """ Returns the last line processed, optionally including the newline character(s) at the end """ @@ -147,7 +162,7 @@ class TextStepper: lines = [] while self.step_line(): if self.is_newline: - lines.append(self._get_last_line(with_end=with_ends)) + lines.append(self.get_last_line(with_end=with_ends)) # add the last line (which was not followed by a newline), even if empty last_line = self._chars[self.line_start_idx :] diff --git a/test/serena/test_symbol_editing.py b/test/serena/test_symbol_editing.py index 97a02aa6..669740fe 100644 --- a/test/serena/test_symbol_editing.py +++ b/test/serena/test_symbol_editing.py @@ -216,7 +216,7 @@ class EditingTest(ABC): with open(file_path, encoding="utf-8") as f: return f.read() - def run_test(self, content_after_ground_truth: SnapshotAssertion) -> None: + def run_test(self, content_after_ground_truth: SnapshotAssertion | None) -> None: with self._setup() as symbol_retriever: content_before = self._read_file(self.rel_path) code_editor = LanguageServerCodeEditor(symbol_retriever) @@ -229,9 +229,10 @@ class EditingTest(ABC): def _apply_edit(self, code_editor: CodeEditor) -> None: pass - def _test_diff(self, code_diff: CodeDiff, snapshot: SnapshotAssertion) -> None: + def _test_diff(self, code_diff: CodeDiff, snapshot: SnapshotAssertion | None) -> None: assert code_diff.has_changes, f"Sanity check failed: No changes detected in {code_diff.relative_path}" - assert code_diff.modified_content == snapshot + if snapshot is not None: + assert code_diff.modified_content == snapshot # Python test file path @@ -483,7 +484,7 @@ class RenameSymbolTest(EditingTest): code_editor.rename_symbol(self.symbol_name, self.rel_path, self.new_name) @overrides - def _test_diff(self, code_diff: CodeDiff, snapshot: SnapshotAssertion) -> None: + def _test_diff(self, code_diff: CodeDiff, snapshot: SnapshotAssertion | None) -> None: # sanity check (e.g., for newly generated snapshots) that the new name is actually in the modified content assert self.new_name in code_diff.modified_content, f"New name '{self.new_name}' not found in modified content." return super()._test_diff(code_diff, snapshot) @@ -500,6 +501,49 @@ def test_rename_symbol(snapshot: SnapshotAssertion): test_case.run_test(content_after_ground_truth=snapshot) +class InsertAndDeleteSymbolTest(EditingTest): + """ + Inserts a symbol after another symbol and then deletes the inserted symbol, which should result + in no change to the file content. + """ + + def __init__(self, language: Language, rel_path: str, rel_symbol: str, deleted_symbol: str, insertion: str): + """ + :param language: specifies the language server to use + :param rel_path: relative path of the file + :param rel_symbol: symbol after which the insertion is made + :param deleted_symbol: the symbol to be deleted after the insertion + :param insertion: text which inserts the symbol `deleted_symbol` + """ + super().__init__(language, rel_path) + self.rel_symbol = rel_symbol + self.deleted_symbol = deleted_symbol + self.rel_path = rel_path + self.insertion = insertion + + def _apply_edit(self, code_editor: CodeEditor) -> None: + code_editor.insert_after_symbol(self.rel_symbol, self.rel_path, self.insertion) + code_editor.delete_symbol(self.deleted_symbol, self.rel_path) + + def _test_diff(self, code_diff: CodeDiff, snapshot: SnapshotAssertion | None) -> None: + assert not code_diff.has_changes + + +def test_insert_and_delete_no_change(): + """ + Tests that inserting and immediately deleting a symbol results in no change to the file content. + """ + insertion = " def inserted_function():\n pass" + test_case = InsertAndDeleteSymbolTest( + Language.PYTHON, + PYTHON_TEST_REL_FILE_PATH, + "VariableContainer/modify_instance_var", + "VariableContainer/inserted_function", + insertion, + ) + test_case.run_test(None) + + # ===== VUE WRITE OPERATIONS TESTS ===== VUE_TEST_FILE = os.path.join("src", "components", "CalculatorButton.vue") diff --git a/test/solidlsp/test_text_utils.py b/test/solidlsp/test_text_utils.py index 0d0fca42..f6122148 100644 --- a/test/solidlsp/test_text_utils.py +++ b/test/solidlsp/test_text_utils.py @@ -1,6 +1,6 @@ import pytest -from solidlsp.ls_utils import InvalidTextLocationError, TextUtils +from solidlsp.ls_utils import InvalidTextLocationError, TextStepper, TextUtils class TestTextUtils: @@ -35,6 +35,13 @@ class TestTextUtils: assert TextUtils.get_index_from_line_col(self.TEXT, 1, 1) == 3 + 1 + 1 assert TextUtils.get_index_from_line_col(self.TEXT, 2, 1) == 3 + 1 + 3 + 2 + 1 + def test_step_to(self): + stepper = TextStepper(self.TEXT) + stepper.step_to(2, 1) + assert stepper.line == 2 + assert stepper.col == 1 + assert stepper.idx == 3 + 1 + 3 + 2 + 1 + def test_insert_text_at_index(self): insertion = "XXX" new_text, l, c = TextUtils.insert_text_at_position(self.TEXT, 0, 1, insertion)