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/resources/repos/python/test_repo/test_repo/variables.py b/test/resources/repos/python/test_repo/test_repo/variables.py index 81d6fbde..62241660 100644 --- a/test/resources/repos/python/test_repo/test_repo/variables.py +++ b/test/resources/repos/python/test_repo/test_repo/variables.py @@ -20,7 +20,6 @@ reassignable_module_var = 20 # Reassigned typed_module_var: int = 42 -# Regular class with class and instance variables class VariableContainer: """Class that contains various variables.""" @@ -59,7 +58,6 @@ class VariableContainer: return result, other_result -# Dataclass with variables @dataclass class VariableDataclass: """Dataclass that contains various fields.""" @@ -75,7 +73,6 @@ class VariableDataclass: status: str = "pending" -# Function that uses the module variables def use_module_variables(): """Function that uses module-level variables.""" result = module_var + " used in function" diff --git a/test/serena/__snapshots__/test_symbol_editing.ambr b/test/serena/__snapshots__/test_symbol_editing.ambr index ec23cf33..7e6c6a8f 100644 --- a/test/serena/__snapshots__/test_symbol_editing.ambr +++ b/test/serena/__snapshots__/test_symbol_editing.ambr @@ -23,11 +23,6 @@ typed_module_var: int = 42 - # Regular class with class and instance variables - - - - # Dataclass with variables @dataclass class VariableDataclass: """Dataclass that contains various fields.""" @@ -43,7 +38,6 @@ status: str = "pending" - # Function that uses the module variables def use_module_variables(): """Function that uses module-level variables.""" result = module_var + " used in function" @@ -69,8 +63,6 @@ ''' import { ConsoleGreeter, Greeter } from "./formatters"; - - export function helperFunction() { const demo = new DemoClass(42); demo.printValue(); @@ -334,7 +326,6 @@ typed_module_var: int = 42 - # Regular class with class and instance variables class VariableContainer: """Class that contains various variables.""" @@ -373,7 +364,6 @@ return result, other_result - # Dataclass with variables @dataclass class VariableDataclass: """Dataclass that contains various fields.""" @@ -389,7 +379,6 @@ status: str = "pending" - # Function that uses the module variables def use_module_variables(): """Function that uses module-level variables.""" result = module_var + " used in function" @@ -438,7 +427,6 @@ typed_module_var: int = 42 - # Regular class with class and instance variables class VariableContainer: """Class that contains various variables.""" @@ -477,7 +465,6 @@ return result, other_result - # Dataclass with variables @dataclass class VariableDataclass: """Dataclass that contains various fields.""" @@ -493,7 +480,6 @@ status: str = "pending" - # Function that uses the module variables def new_inserted_function(): print("This is a new function inserted before another.") @@ -1138,7 +1124,6 @@ typed_module_var: int = 42 - # Regular class with class and instance variables class VariableContainer: """Class that contains various variables.""" @@ -1177,7 +1162,6 @@ return result, other_result - # Dataclass with variables @dataclass class VariableDataclass: """Dataclass that contains various fields.""" @@ -1197,7 +1181,6 @@ pass - # Function that uses the module variables def use_module_variables(): """Function that uses module-level variables.""" result = module_var + " used in function" @@ -1243,7 +1226,6 @@ typed_module_var: int = 42 - # Regular class with class and instance variables class VariableContainer: """Class that contains various variables.""" @@ -1282,7 +1264,6 @@ return result, other_result - # Dataclass with variables class NewInsertedClass: pass @@ -1302,7 +1283,6 @@ status: str = "pending" - # Function that uses the module variables def use_module_variables(): """Function that uses module-level variables.""" result = module_var + " used in function" @@ -1531,7 +1511,6 @@ renamed_typed_module_var: int = 42 - # Regular class with class and instance variables class VariableContainer: """Class that contains various variables.""" @@ -1570,7 +1549,6 @@ return result, other_result - # Dataclass with variables @dataclass class VariableDataclass: """Dataclass that contains various fields.""" @@ -1586,7 +1564,6 @@ status: str = "pending" - # Function that uses the module variables def use_module_variables(): """Function that uses module-level variables.""" result = module_var + " used in function" @@ -1632,7 +1609,6 @@ typed_module_var: int = 42 - # Regular class with class and instance variables class VariableContainer: """Class that contains various variables.""" @@ -1671,7 +1647,6 @@ return result, other_result - # Dataclass with variables @dataclass class VariableDataclass: """Dataclass that contains various fields.""" @@ -1687,7 +1662,6 @@ status: str = "pending" - # Function that uses the module variables def use_module_variables(): """Function that uses module-level variables.""" result = module_var + " used in function" 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/python/test_symbol_retrieval.py b/test/solidlsp/python/test_symbol_retrieval.py index b95155d2..38185f31 100644 --- a/test/solidlsp/python/test_symbol_retrieval.py +++ b/test/solidlsp/python/test_symbol_retrieval.py @@ -11,6 +11,7 @@ import os import pytest from serena.symbol import LanguageServerSymbol +from serena.util.text_utils import find_text_coordinates from solidlsp import SolidLanguageServer from solidlsp.ls_types import SymbolKind from test.solidlsp.conftest import PYTHON_BACKEND_LANGUAGES @@ -40,14 +41,15 @@ class TestLanguageServerSymbols: def test_references_to_variables(self, language_server: SolidLanguageServer) -> None: """Test request_referencing_symbols for a variable.""" file_path = os.path.join("test_repo", "variables.py") - # Line 75 contains the field status that is later modified - ref_symbols = [ref.symbol for ref in language_server.request_referencing_symbols(file_path, 74, 4)] + + # find references to field `status` + with language_server.open_file(file_path, open_in_ls=False) as f: + file_content = f.contents + coords = find_text_coordinates(file_content, r"(status): str") + ref_symbols = [ref.symbol for ref in language_server.request_referencing_symbols(file_path, coords.line, coords.col)] assert len(ref_symbols) > 0 - ref_lines = [ref["location"]["range"]["start"]["line"] for ref in ref_symbols if "location" in ref and "range" in ref["location"]] ref_names = [ref["name"] for ref in ref_symbols] - assert 87 in ref_lines - assert 95 in ref_lines assert "dataclass_instance" in ref_names assert "second_dataclass" in ref_names 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)