From 25da7a505fbbfe4f1931537a2c3e75bf921d0c05 Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Thu, 16 Oct 2025 09:24:30 +0100 Subject: [PATCH] Add rename_symbol method (#660) Tested through a snapshot test on python. Calling the tool with language servers not supporting renaming will raise an error Co-authored-by: Don Syme --- src/serena/code_editor.py | 74 ++++++++++++- src/serena/tools/symbol_tools.py | 28 +++++ src/solidlsp/ls.py | 46 ++++++++ src/solidlsp/ls_types.py | 61 +++++++++++ .../__snapshots__/test_symbol_editing.ambr | 101 ++++++++++++++++++ test/serena/test_symbol_editing.py | 56 ++++++++-- 6 files changed, 353 insertions(+), 13 deletions(-) diff --git a/src/serena/code_editor.py b/src/serena/code_editor.py index 6fe41da1..ef00970b 100644 --- a/src/serena/code_editor.py +++ b/src/serena/code_editor.py @@ -4,12 +4,13 @@ import os from abc import ABC, abstractmethod from collections.abc import Iterable, Iterator, Reversible from contextlib import contextmanager -from typing import TYPE_CHECKING, Generic, Optional, TypeVar +from typing import TYPE_CHECKING, Generic, Optional, TypeVar, cast from serena.symbol import JetBrainsSymbol, LanguageServerSymbol, LanguageServerSymbolRetriever, PositionInFile, Symbol -from solidlsp import SolidLanguageServer +from solidlsp import SolidLanguageServer, ls_types from solidlsp.ls import LSPFileBuffer -from solidlsp.ls_utils import TextUtils +from solidlsp.ls_types import extract_text_edits +from solidlsp.ls_utils import PathUtils, TextUtils from .project import Project from .tools.jetbrains_plugin_client import JetBrainsPluginClient @@ -207,6 +208,17 @@ class CodeEditor(Generic[TSymbol], ABC): with self._edited_file_context(relative_file_path) as edited_file: edited_file.delete_text_between_positions(start_pos, end_pos) + @abstractmethod + def rename_symbol(self, name_path: str, relative_file_path: str, new_name: str) -> list[str]: + """ + Renames the symbol with the given name throughout the codebase. + + :param name_path: the name path of the symbol to rename + :param relative_file_path: the relative path of the file containing the symbol + :param new_name: the new name for the symbol + :return: list of files that were modified + """ + class LanguageServerCodeEditor(CodeEditor[LanguageServerSymbol]): def __init__(self, symbol_retriever: LanguageServerSymbolRetriever, agent: Optional["SerenaAgent"] = None): @@ -232,6 +244,9 @@ class LanguageServerCodeEditor(CodeEditor[LanguageServerSymbol]): def insert_text_at_position(self, pos: PositionInFile, text: str) -> None: self._lang_server.insert_text_at_position(self._relative_path, pos.line, pos.col, text) + def apply_text_edits(self, text_edits: list[ls_types.TextEdit]) -> None: + return self._lang_server.apply_text_edits_to_file(self._relative_path, text_edits) + @contextmanager def _open_file_context(self, relative_path: str) -> Iterator["CodeEditor.EditedFile"]: with self._lang_server.open_file(relative_path) as file_buffer: @@ -252,6 +267,52 @@ class LanguageServerCodeEditor(CodeEditor[LanguageServerSymbol]): ) return symbol_candidates[0] + def _apply_workspace_edit(self, workspace_edit: ls_types.WorkspaceEdit) -> list[str]: + """ + Apply a WorkspaceEdit by making the changes to files. + + :param workspace_edit: The WorkspaceEdit containing the changes to apply + :return: List of relative file paths that were modified + """ + uri_to_edits = extract_text_edits(workspace_edit) + modified_relative_paths = [] + + # Handle the 'changes' format (URI -> list of TextEdits) + for uri, edits in uri_to_edits.items(): + file_path = PathUtils.uri_to_path(uri) + relative_path = os.path.relpath(file_path, self._lang_server.repository_root_path) + modified_relative_paths.append(relative_path) + with self._edited_file_context(relative_path) as edited_file: + edited_file = cast(self.EditedFile, edited_file) + edited_file.apply_text_edits(edits) + return modified_relative_paths + + def rename_symbol(self, name_path: str, relative_file_path: str, new_name: str) -> list[str]: + """ + Renames the symbol with the given name throughout the codebase. + :param name_path: + :param relative_file_path: + :param new_name: + :return: list of files that were modified + """ + symbol = self._find_unique_symbol(name_path, relative_file_path) + if not symbol.location.has_position_in_file(): + raise ValueError(f"Symbol '{name_path}' does not have a valid position in file for renaming") + + # After has_position_in_file check, line and column are guaranteed to be non-None + assert symbol.location.line is not None + assert symbol.location.column is not None + + rename_result = self._lang_server.request_rename_symbol_edit( + relative_file_path=relative_file_path, line=symbol.location.line, column=symbol.location.column, new_name=new_name + ) + if rename_result is None: + raise ValueError( + f"Language server for {self._lang_server.language_id} returned no rename edits for symbol '{name_path}'. " + f"The symbol might not support renaming." + ) + return self._apply_workspace_edit(rename_result) + class JetBrainsCodeEditor(CodeEditor[JetBrainsSymbol]): def __init__(self, project: Project, agent: Optional["SerenaAgent"] = None) -> None: @@ -292,3 +353,10 @@ class JetBrainsCodeEditor(CodeEditor[JetBrainsSymbol]): "Their locations are: \n " + json.dumps([s["location"] for s in symbols], indent=2) ) return JetBrainsSymbol(symbols[0], self._project) + + def rename_symbol(self, name_path: str, relative_file_path: str, new_name: str) -> list[str]: + """ + Renames the symbol with the given name throughout the codebase. + Not yet implemented for JetBrains code editor. + """ + raise NotImplementedError("Symbol renaming is not yet supported for JetBrains code editor") diff --git a/src/serena/tools/symbol_tools.py b/src/serena/tools/symbol_tools.py index 14099ef7..7526d20e 100644 --- a/src/serena/tools/symbol_tools.py +++ b/src/serena/tools/symbol_tools.py @@ -82,6 +82,7 @@ class FindSymbolTool(Tool, ToolMarkerSymbolicRead): Performs a global (or local) search for symbols with/containing a given name/substring (optionally filtered by type). """ + # noinspection PyDefaultArgument def apply( self, name_path: str, @@ -165,6 +166,7 @@ class FindReferencingSymbolsTool(Tool, ToolMarkerSymbolicRead): Finds symbols that reference the symbol at the given location (optionally filtered by type). """ + # noinspection PyDefaultArgument def apply( self, name_path: str, @@ -293,3 +295,29 @@ class InsertBeforeSymbolTool(Tool, ToolMarkerSymbolicEdit): code_editor = self.create_code_editor() code_editor.insert_before_symbol(name_path, relative_file_path=relative_path, body=body) return SUCCESS_RESULT + + +class RenameSymbolTool(Tool, ToolMarkerSymbolicEdit): + """ + Renames a symbol throughout the codebase using language server refactoring capabilities. + """ + + def apply( + self, + name_path: str, + relative_path: str, + new_name: str, + ) -> str: + """ + Renames the symbol with the given `name_path` to `new_name` throughout the entire codebase. + Note: for languages with method overloading, like Java, name_path may have to include a method's + signature to uniquely identify a method. + + :param name_path: name path of the symbol to rename (definitions in the `find_symbol` tool apply) + :param relative_path: the relative path to the file containing the symbol to rename + :param new_name: the new name for the symbol + :return: result summary indicating success or failure + """ + code_editor = self.create_code_editor() + modified_files = code_editor.rename_symbol(name_path, relative_file_path=relative_path, new_name=new_name) + return f"Successfully renamed '{name_path}' to '{new_name}' in {len(modified_files)} file(s)" diff --git a/src/solidlsp/ls.py b/src/solidlsp/ls.py index c91ff55e..29df1ee4 100644 --- a/src/solidlsp/ls.py +++ b/src/solidlsp/ls.py @@ -1626,6 +1626,52 @@ class SolidLanguageServer(ABC): return ret + def request_rename_symbol_edit( + self, + relative_file_path: str, + line: int, + column: int, + new_name: str, + ) -> ls_types.WorkspaceEdit | None: + """ + Retrieve a WorkspaceEdit for renaming the symbol at the given location to the new name. + Does not apply the edit, just retrieves it. In order to actually rename the symbol, call apply_workspace_edit. + + :param relative_file_path: The relative path to the file containing the symbol + :param line: The 0-indexed line number of the symbol + :param column: The 0-indexed column number of the symbol + :param new_name: The new name for the symbol + :return: A WorkspaceEdit containing the changes needed to rename the symbol, or None if rename is not supported + """ + params = ls_types.RenameParams( + textDocument=ls_types.TextDocumentIdentifier( + uri=pathlib.Path(os.path.join(self.repository_root_path, relative_file_path)).as_uri() + ), + position=ls_types.Position(line=line, character=column), + newName=new_name, + ) + + return self.server.send.rename(params) + + def apply_text_edits_to_file(self, relative_path: str, edits: list[ls_types.TextEdit]) -> None: + """ + Apply a list of text edits to a file. + + :param relative_path: The relative path of the file to edit + :param edits: List of TextEdit dictionaries to apply + """ + with self.open_file(relative_path): + # Sort edits by position (latest first) to avoid position shifts + sorted_edits = sorted(edits, key=lambda e: (e["range"]["start"]["line"], e["range"]["start"]["character"]), reverse=True) + + for edit in sorted_edits: + start_pos = ls_types.Position(line=edit["range"]["start"]["line"], character=edit["range"]["start"]["character"]) + end_pos = ls_types.Position(line=edit["range"]["end"]["line"], character=edit["range"]["end"]["character"]) + + # Delete the old text and insert the new text + self.delete_text_between_positions(relative_path, start_pos, end_pos) + self.insert_text_at_position(relative_path, start_pos["line"], start_pos["character"], edit["newText"]) + def start(self) -> "SolidLanguageServer": """ Starts the language server process and connects to it. Call shutdown when ready. diff --git a/src/solidlsp/ls_types.py b/src/solidlsp/ls_types.py index d8ff8903..4a8de6d4 100644 --- a/src/solidlsp/ls_types.py +++ b/src/solidlsp/ls_types.py @@ -326,6 +326,42 @@ class DiagnosticsSeverity(IntEnum): HINT = 4 +class TextDocumentIdentifier(TypedDict): + """A literal to identify a text document in the client.""" + + uri: DocumentUri + """ The text document's uri. """ + + +class TextEdit(TypedDict): + """A textual edit applicable to a text document.""" + + range: Range + """ The range of the text document to be manipulated. """ + newText: str + """ The string to be inserted. For delete operations use an empty string. """ + + +class WorkspaceEdit(TypedDict): + """A workspace edit represents changes to many resources managed in the workspace.""" + + changes: NotRequired[dict[DocumentUri, list[TextEdit]]] + """ Holds changes to existing resources. """ + documentChanges: NotRequired[list] + """ Document changes array for versioned edits. """ + + +class RenameParams(TypedDict): + """The parameters of a RenameRequest.""" + + textDocument: TextDocumentIdentifier + """ The document to rename. """ + position: Position + """ The position at which this request was sent. """ + newName: str + """ The new name of the symbol. """ + + class Diagnostic(TypedDict): """Diagnostic information for a text document.""" @@ -341,3 +377,28 @@ class Diagnostic(TypedDict): """ The code of the diagnostic. """ source: NotRequired[str] """ The source of the diagnostic, e.g. the name of the tool that produced it. """ + + +def extract_text_edits(workspace_edit: WorkspaceEdit) -> dict[str, list[TextEdit]]: + """ + Extracts the text changes from a WorkspaceEdit object. + + Args: + workspace_edit (WorkspaceEdit): The WorkspaceEdit object to extract text changes from. + + Returns: + dict[str, list[TextEdit]]: A dictionary mapping document URIs to lists of TextEdit objects. + + """ + if "changes" in workspace_edit: + return workspace_edit["changes"] + elif "documentChanges" in workspace_edit: + changes = {} + for change in workspace_edit["documentChanges"]: + if "textDocument" in change and "edits" in change: + uri = change["textDocument"]["uri"] + edits = change["edits"] + changes[uri] = edits + return changes + else: + raise f"Invalid WorkspaceEdit (expected 'changes' or 'documentChanges' key):\n{workspace_edit}" diff --git a/test/serena/__snapshots__/test_symbol_editing.ambr b/test/serena/__snapshots__/test_symbol_editing.ambr index 3278113d..92603c2d 100644 --- a/test/serena/__snapshots__/test_symbol_editing.ambr +++ b/test/serena/__snapshots__/test_symbol_editing.ambr @@ -983,6 +983,107 @@ ''' # --- +# name: test_rename_symbol + ''' + """ + Test module for variable declarations and usage. + + This module tests various types of variable declarations and usages including: + - Module-level variables + - Class-level variables + - Instance variables + - Variable reassignments + """ + + from dataclasses import dataclass, field + + # Module-level variables + module_var = "Initial module value" + + reassignable_module_var = 10 + reassignable_module_var = 20 # Reassigned + + # Module-level variable with type annotation + renamed_typed_module_var: int = 42 + + + # Regular class with class and instance variables + class VariableContainer: + """Class that contains various variables.""" + + # Class-level variables + class_var = "Initial class value" + + reassignable_class_var = True + reassignable_class_var = False # Reassigned #noqa: PIE794 + + # Class-level variable with type annotation + typed_class_var: str = "typed value" + + def __init__(self): + # Instance variables + self.instance_var = "Initial instance value" + self.reassignable_instance_var = 100 + + # Instance variable with type annotation + self.typed_instance_var: list[str] = ["item1", "item2"] + + def modify_instance_var(self): + # Reassign instance variable + self.instance_var = "Modified instance value" + self.reassignable_instance_var = 200 # Reassigned + + def use_module_var(self): + # Use module-level variables + result = module_var + " used in method" + other_result = reassignable_module_var + 5 + return result, other_result + + def use_class_var(self): + # Use class-level variables + result = VariableContainer.class_var + " used in method" + other_result = VariableContainer.reassignable_class_var + return result, other_result + + + # Dataclass with variables + @dataclass + class VariableDataclass: + """Dataclass that contains various fields.""" + + # Field variables with type annotations + id: int + name: str + items: list[str] = field(default_factory=list) + metadata: dict[str, str] = field(default_factory=dict) + optional_value: float | None = None + + # This will be reassigned in various places + 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" + other_result = reassignable_module_var * 2 + return result, other_result + + + # Create instances and use variables + dataclass_instance = VariableDataclass(id=1, name="Test") + dataclass_instance.status = "active" # Reassign dataclass field + + # Use variables at module level + module_result = module_var + " used at module level" + other_module_result = reassignable_module_var + 30 + + # Create a second dataclass instance with different status + second_dataclass = VariableDataclass(id=2, name="Another Test") + second_dataclass.status = "completed" # Another reassignment of status + + ''' +# --- # name: test_replace_body[test_case0] ''' """ diff --git a/test/serena/test_symbol_editing.py b/test/serena/test_symbol_editing.py index 15fb35ed..ee97149a 100644 --- a/test/serena/test_symbol_editing.py +++ b/test/serena/test_symbol_editing.py @@ -1,10 +1,15 @@ +""" +Snapshot tests using the (awesome) syrupy pytest plugin https://github.com/syrupy-project/syrupy. +Recreate the snapshots with `pytest --snapshot-update`. +""" + import logging import os import shutil import sys import tempfile import time -from abc import abstractmethod +from abc import ABC, abstractmethod from collections.abc import Iterator from contextlib import contextmanager from dataclasses import dataclass, field @@ -13,6 +18,8 @@ from pathlib import Path from typing import Literal, NamedTuple import pytest +from overrides import overrides +from syrupy import SnapshotAssertion from serena.code_editor import CodeEditor, LanguageServerCodeEditor from solidlsp.ls_config import Language @@ -169,7 +176,7 @@ class CodeDiff: return "".join(diff) -class EditingTest: +class EditingTest(ABC): def __init__(self, language: Language, rel_path: str): """ :param language: the language @@ -221,7 +228,7 @@ class EditingTest: with open(file_path, encoding="utf-8") as f: return f.read() - def run_test(self, content_after_ground_truth: str) -> None: + def run_test(self, content_after_ground_truth: str | SnapshotAssertion) -> None: with self._setup() as symbol_retriever: content_before = self._read_file(self.rel_path) code_editor = LanguageServerCodeEditor(symbol_retriever) @@ -234,7 +241,8 @@ class EditingTest: def _apply_edit(self, code_editor: CodeEditor) -> None: pass - def _test_diff(self, code_diff: CodeDiff, snapshot: str) -> None: + def _test_diff(self, code_diff: CodeDiff, snapshot: SnapshotAssertion) -> None: + assert code_diff.has_changes, f"Sanity check failed: No changes detected in {code_diff.relative_path}" assert code_diff.modified_content == snapshot @@ -276,7 +284,7 @@ class DeleteSymbolTest(EditingTest): ), ], ) -def test_delete_symbol(test_case, snapshot): +def test_delete_symbol(test_case, snapshot: SnapshotAssertion): test_case.run_test(content_after_ground_truth=snapshot) @@ -369,13 +377,13 @@ class InsertInRelToSymbolTest(EditingTest): ), ], ) -def test_insert_in_rel_to_symbol(test_case: InsertInRelToSymbolTest, mode: Literal["before", "after"], snapshot): +def test_insert_in_rel_to_symbol(test_case: InsertInRelToSymbolTest, mode: Literal["before", "after"], snapshot: SnapshotAssertion): test_case.set_mode(mode) test_case.run_test(content_after_ground_truth=snapshot) @pytest.mark.python -def test_insert_python_class_before(snapshot): +def test_insert_python_class_before(snapshot: SnapshotAssertion): InsertInRelToSymbolTest( Language.PYTHON, PYTHON_TEST_REL_FILE_PATH, @@ -386,7 +394,7 @@ def test_insert_python_class_before(snapshot): @pytest.mark.python -def test_insert_python_class_after(snapshot): +def test_insert_python_class_after(snapshot: SnapshotAssertion): InsertInRelToSymbolTest( Language.PYTHON, PYTHON_TEST_REL_FILE_PATH, @@ -442,7 +450,8 @@ class ReplaceBodyTest(EditingTest): ), ], ) -def test_replace_body(test_case: ReplaceBodyTest, snapshot): +def test_replace_body(test_case: ReplaceBodyTest, snapshot: SnapshotAssertion): + # assert "a" in snapshot test_case.run_test(content_after_ground_truth=snapshot) @@ -463,7 +472,7 @@ class NixAttrReplacementTest(EditingTest): @pytest.mark.nix @pytest.mark.skipif(sys.platform == "win32", reason="nixd language server doesn't run on Windows") -def test_nix_symbol_replacement_no_double_semicolon(snapshot): +def test_nix_symbol_replacement_no_double_semicolon(snapshot: SnapshotAssertion): """ Test that replacing a Nix attribute does not result in double semicolons. @@ -483,3 +492,30 @@ def test_nix_symbol_replacement_no_double_semicolon(snapshot): NIX_ATTR_REPLACEMENT, ) test_case.run_test(content_after_ground_truth=snapshot) + + +class RenameSymbolTest(EditingTest): + def __init__(self, language: Language, rel_path: str, symbol_name: str, new_name: str): + super().__init__(language, rel_path) + self.symbol_name = symbol_name + self.new_name = new_name + + def _apply_edit(self, code_editor: CodeEditor) -> None: + code_editor.rename_symbol(self.symbol_name, self.rel_path, self.new_name) + + @overrides + def _test_diff(self, code_diff: CodeDiff, snapshot: SnapshotAssertion) -> 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) + + +@pytest.mark.python +def test_rename_symbol(snapshot: SnapshotAssertion): + test_case = RenameSymbolTest( + Language.PYTHON, + PYTHON_TEST_REL_FILE_PATH, + "typed_module_var", + "renamed_typed_module_var", + ) + test_case.run_test(content_after_ground_truth=snapshot)