From 9735733802fba8d4c77bcc77acf11065bf4f5e2f Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Tue, 24 Mar 2026 11:53:51 +0100 Subject: [PATCH] Added SafeDeleteSymbol in LS world --- .../config/internal_modes/jetbrains.yml | 1 + src/serena/tools/symbol_tools.py | 41 ++++++++ src/serena/tools/tools_base.py | 13 ++- .../repos/typescript/test_repo/index.ts | 4 + .../__snapshots__/test_symbol_editing.ambr | 24 +++++ test/serena/test_serena_agent.py | 98 ++++++++++++++++++- 6 files changed, 177 insertions(+), 4 deletions(-) diff --git a/src/serena/resources/config/internal_modes/jetbrains.yml b/src/serena/resources/config/internal_modes/jetbrains.yml index 090c6b8f..f9e60a4b 100644 --- a/src/serena/resources/config/internal_modes/jetbrains.yml +++ b/src/serena/resources/config/internal_modes/jetbrains.yml @@ -9,6 +9,7 @@ excluded_tools: - find_referencing_symbols - get_symbols_overview - restart_language_server + - safe_delete_symbol included_optional_tools: - jet_brains_find_symbol - jet_brains_find_referencing_symbols diff --git a/src/serena/tools/symbol_tools.py b/src/serena/tools/symbol_tools.py index 2df83bc1..2cf5c178 100644 --- a/src/serena/tools/symbol_tools.py +++ b/src/serena/tools/symbol_tools.py @@ -430,3 +430,44 @@ class RenameSymbolTool(Tool, ToolMarkerSymbolicEdit): code_editor = self.create_code_editor() status_message = code_editor.rename_symbol(name_path, relative_file_path=relative_path, new_name=new_name) return status_message + + +class SafeDeleteSymbol(Tool, ToolMarkerSymbolicEdit): + def apply( + self, + name_path_pattern: str, + relative_path: str, + ) -> str: + """ + Deletes the symbol if it is safe to do so (i.e., if there are no references to it) + or returns a list of references to it. + + :param name_path_pattern: name path of the symbol to delete (definitions in the `find_symbol` tool apply) + :param relative_path: the relative path to the file containing the symbol to delete + """ + ls_symbol_retriever = self.create_language_server_symbol_retriever() + symbol = ls_symbol_retriever.find_unique(name_path_pattern, substring_matching=False, within_relative_path=relative_path) + symbol_rel_path = symbol.relative_path + assert symbol_rel_path is not None, f"Symbol {name_path_pattern} has no relative path, this is likely a bug." + assert symbol_rel_path == relative_path, f"Symbol {name_path_pattern} is not in the expected relative path {relative_path}." + symbol_name_path = symbol.get_name_path() + + symbol_line = symbol.line + symbol_col = symbol.column + assert symbol_line is not None and symbol_col is not None, ( + f"Symbol {name_path_pattern} has no identifier position, this is likely a bug." + ) + lang_server = ls_symbol_retriever.get_language_server(symbol_rel_path) + references_locations = lang_server.request_references(symbol_rel_path, symbol_line, symbol_col) + file_to_lines: dict[str, list[int]] = defaultdict(list) + if references_locations: + for ref_loc in references_locations: + ref_relative_path = ref_loc.get("relativePath") + if ref_relative_path is None: + continue + file_to_lines[ref_relative_path].append(ref_loc["range"]["start"]["line"]) + if file_to_lines: + return f"Cannot delete, the symbol {symbol_name_path} is referenced in: {self._to_json(file_to_lines)}" + code_editor = self.create_ls_code_editor() + code_editor.delete_symbol(symbol_name_path, relative_file_path=symbol_rel_path) + return SUCCESS_RESULT diff --git a/src/serena/tools/tools_base.py b/src/serena/tools/tools_base.py index 18eed68f..200c2ae3 100644 --- a/src/serena/tools/tools_base.py +++ b/src/serena/tools/tools_base.py @@ -21,7 +21,7 @@ from solidlsp.ls_exceptions import SolidLSPException if TYPE_CHECKING: from serena.agent import SerenaAgent - from serena.code_editor import CodeEditor + from serena.code_editor import CodeEditor, LanguageServerCodeEditor from serena.symbol import LanguageServerSymbolRetriever log = logging.getLogger(__name__) @@ -58,16 +58,23 @@ class Component(ABC): return self.agent.get_active_project_or_raise() def create_code_editor(self) -> "CodeEditor": - from ..code_editor import JetBrainsCodeEditor, LanguageServerCodeEditor + from ..code_editor import JetBrainsCodeEditor match self.agent.get_language_backend(): case LanguageBackend.LSP: - return LanguageServerCodeEditor(self.create_language_server_symbol_retriever()) + return self.create_ls_code_editor() case LanguageBackend.JETBRAINS: return JetBrainsCodeEditor(project=self.project) case _: raise ValueError + def create_ls_code_editor(self) -> "LanguageServerCodeEditor": + from ..code_editor import LanguageServerCodeEditor + + if not self.agent.is_using_language_server(): + raise Exception("Cannot create LanguageServerCodeEditor; agent is not in language server mode.") + return LanguageServerCodeEditor(self.create_language_server_symbol_retriever()) + class ToolMarker: """ diff --git a/test/resources/repos/typescript/test_repo/index.ts b/test/resources/repos/typescript/test_repo/index.ts index 7b8bd1da..724a67c6 100644 --- a/test/resources/repos/typescript/test_repo/index.ts +++ b/test/resources/repos/typescript/test_repo/index.ts @@ -14,3 +14,7 @@ export function helperFunction() { } helperFunction(); + +export function unusedStandaloneFunction(): string { + return "This function is not referenced anywhere"; +} diff --git a/test/serena/__snapshots__/test_symbol_editing.ambr b/test/serena/__snapshots__/test_symbol_editing.ambr index 347862ca..f614627e 100644 --- a/test/serena/__snapshots__/test_symbol_editing.ambr +++ b/test/serena/__snapshots__/test_symbol_editing.ambr @@ -76,6 +76,10 @@ helperFunction(); + export function unusedStandaloneFunction(): string { + return "This function is not referenced anywhere"; + } + ''' # --- # name: test_delete_symbol_vue[test_case0] @@ -736,6 +740,10 @@ helperFunction(); + export function unusedStandaloneFunction(): string { + return "This function is not referenced anywhere"; + } + ''' # --- # name: test_insert_in_rel_to_symbol[test_case2-before] @@ -761,6 +769,10 @@ helperFunction(); + export function unusedStandaloneFunction(): string { + return "This function is not referenced anywhere"; + } + ''' # --- # name: test_insert_in_rel_to_symbol[test_case3-after] @@ -786,6 +798,10 @@ helperFunction(); + export function unusedStandaloneFunction(): string { + return "This function is not referenced anywhere"; + } + ''' # --- # name: test_insert_in_rel_to_symbol[test_case3-before] @@ -811,6 +827,10 @@ helperFunction(); + export function unusedStandaloneFunction(): string { + return "This function is not referenced anywhere"; + } + ''' # --- # name: test_insert_in_rel_to_symbol_vue[test_case0-after] @@ -1888,6 +1908,10 @@ helperFunction(); + export function unusedStandaloneFunction(): string { + return "This function is not referenced anywhere"; + } + ''' # --- # name: test_replace_body_vue[test_case0] diff --git a/test/serena/test_serena_agent.py b/test/serena/test_serena_agent.py index 0b40ab28..e9dc6b2e 100644 --- a/test/serena/test_serena_agent.py +++ b/test/serena/test_serena_agent.py @@ -12,7 +12,14 @@ import pytest from serena.agent import SerenaAgent from serena.config.serena_config import ProjectConfig, RegisteredProject, SerenaConfig from serena.project import Project -from serena.tools import SUCCESS_RESULT, FindReferencingSymbolsTool, FindSymbolTool, ReplaceContentTool, ReplaceSymbolBodyTool +from serena.tools import ( + SUCCESS_RESULT, + FindReferencingSymbolsTool, + FindSymbolTool, + ReplaceContentTool, + ReplaceSymbolBodyTool, + SafeDeleteSymbol, +) from solidlsp.ls_config import Language from solidlsp.ls_types import SymbolKind from test.conftest import get_repo_path, is_ci, language_tests_enabled @@ -589,3 +596,92 @@ class TestSerenaAgent: relative_path="ws_manager.js", mode="regex", ) + + @pytest.mark.parametrize( + "serena_agent,name_path,relative_path", + [ + pytest.param( + Language.PYTHON, + "User", + os.path.join("test_repo", "models.py"), + marks=pytest.mark.python, + ), + pytest.param( + Language.JAVA, + "Model", + os.path.join("src", "main", "java", "test_repo", "Model.java"), + marks=pytest.mark.java, + ), + pytest.param( + Language.KOTLIN, + "Model", + os.path.join("src", "main", "kotlin", "test_repo", "Model.kt"), + marks=[pytest.mark.kotlin] + ([pytest.mark.skip(reason="Kotlin LSP JVM crashes on restart in CI")] if is_ci else []), + ), + pytest.param( + Language.TYPESCRIPT, + "helperFunction", + "index.ts", + marks=pytest.mark.typescript, + ), + ], + indirect=["serena_agent"], + ) + def test_safe_delete_symbol_blocked_by_references(self, serena_agent: SerenaAgent, name_path: str, relative_path: str): + """ + Tests that SafeDeleteSymbol refuses to delete a symbol that is referenced elsewhere + and returns a message listing the referencing files. + """ + # wrap in modification context as a safety net: if the tool has a bug and deletes anyway, + # the file will be restored, preventing corruption of test resources + with project_file_modification_context(serena_agent, relative_path): + safe_delete_tool = serena_agent.get_tool(SafeDeleteSymbol) + result = safe_delete_tool.apply(name_path_pattern=name_path, relative_path=relative_path) + assert "Cannot delete" in result, f"Expected deletion to be blocked due to existing references, but got: {result}" + assert "referenced in" in result, f"Expected reference information in result, but got: {result}" + + @pytest.mark.parametrize( + "serena_agent,name_path,relative_path", + [ + pytest.param( + Language.PYTHON, + "Timer", + os.path.join("test_repo", "utils.py"), + marks=pytest.mark.python, + ), + pytest.param( + Language.JAVA, + "ModelUser", + os.path.join("src", "main", "java", "test_repo", "ModelUser.java"), + marks=pytest.mark.java, + ), + pytest.param( + Language.KOTLIN, + "ModelUser", + os.path.join("src", "main", "kotlin", "test_repo", "ModelUser.kt"), + marks=[pytest.mark.kotlin] + ([pytest.mark.skip(reason="Kotlin LSP JVM crashes on restart in CI")] if is_ci else []), + ), + pytest.param( + Language.TYPESCRIPT, + "unusedStandaloneFunction", + "index.ts", + marks=pytest.mark.typescript, + ), + ], + indirect=["serena_agent"], + ) + def test_safe_delete_symbol_succeeds_when_no_references(self, serena_agent: SerenaAgent, name_path: str, relative_path: str): + """ + Tests that SafeDeleteSymbol successfully deletes a symbol that has no references + and that the symbol is actually removed from the file. + """ + with project_file_modification_context(serena_agent, relative_path): + safe_delete_tool = serena_agent.get_tool(SafeDeleteSymbol) + result = safe_delete_tool.apply(name_path_pattern=name_path, relative_path=relative_path) + assert result == SUCCESS_RESULT, f"Expected successful deletion, but got: {result}" + + # verify the symbol was actually removed from the file + file_content = read_project_file(serena_agent.get_active_project(), relative_path) + assert name_path not in file_content, ( + f"Expected symbol {name_path} to be removed from {relative_path}, but it still appears in the file content" + )