From af5d792e6b26267cfabfc45ed977e7f9f349f8d7 Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Thu, 25 Dec 2025 13:25:13 +0700 Subject: [PATCH] Support for symbol_info in LSP backend --- .serena/memories/glossary.md | 2 + src/serena/agent.py | 6 ++ src/serena/ls_manager.py | 8 +++ src/serena/symbol.py | 50 ++++++++++++- src/serena/tools/symbol_tools.py | 16 +++-- .../language_servers/eclipse_jdtls.py | 67 ++++++++++++++--- src/solidlsp/ls.py | 50 ++++++++++++- src/solidlsp/ls_types.py | 72 +++++++++++++++++++ .../src/main/java/test_repo/Model.java | 3 + test/serena/test_serena_agent.py | 18 ++++- test/serena/test_symbol.py | 14 +++- 11 files changed, 283 insertions(+), 23 deletions(-) create mode 100644 .serena/memories/glossary.md diff --git a/.serena/memories/glossary.md b/.serena/memories/glossary.md new file mode 100644 index 00000000..0002784b --- /dev/null +++ b/.serena/memories/glossary.md @@ -0,0 +1,2 @@ + +us: UnifiedSymbolInformation (return format of many low-level language server symbol requests) diff --git a/src/serena/agent.py b/src/serena/agent.py index ca763b87..a43ec287 100644 --- a/src/serena/agent.py +++ b/src/serena/agent.py @@ -708,3 +708,9 @@ class SerenaAgent: def get_tool_by_name(self, tool_name: str) -> Tool: tool_class = ToolRegistry().get_tool_class_by_name(tool_name) return self.get_tool(tool_class) + + def get_active_lsp_languages(self) -> list[Language]: + ls_manager = self.get_language_server_manager() + if ls_manager is None: + return [] + return ls_manager.get_active_languages() diff --git a/src/serena/ls_manager.py b/src/serena/ls_manager.py index 8f3681bf..6fdf0c24 100644 --- a/src/serena/ls_manager.py +++ b/src/serena/ls_manager.py @@ -182,6 +182,14 @@ class LanguageServerManager: ls = self._language_servers.pop(language) self._stop_language_server(ls, save_cache=save_cache) + def get_active_languages(self) -> list[Language]: + """ + Returns the list of languages for which language servers are currently managed. + + :return: list of languages + """ + return list(self._language_servers.keys()) + @staticmethod def _stop_language_server(ls: SolidLanguageServer, save_cache: bool = False, timeout: float = 2.0) -> None: if ls.is_running(): diff --git a/src/serena/symbol.py b/src/serena/symbol.py index ecc13353..caef4bd5 100644 --- a/src/serena/symbol.py +++ b/src/serena/symbol.py @@ -188,7 +188,9 @@ class NamePathMatcher(ToStringMixin): class LanguageServerSymbol(Symbol, ToStringMixin): - def __init__(self, symbol_root_from_ls: UnifiedSymbolInformation) -> None: + def __init__( + self, symbol_root_from_ls: UnifiedSymbolInformation, docstring: str | None = None, signature_string: str | None = None + ) -> None: self.symbol_root = symbol_root_from_ls def _tostring_includes(self) -> list[str]: @@ -496,6 +498,52 @@ class LanguageServerSymbolRetriever: self._ls_manager: LanguageServerManager = ls_manager self.agent = agent + def request_info(self, relative_file_path: str, line: int, column: int) -> str | None: + """Retrieves information (in a sanitized format) about the symbol at the desired location, + typically containing the docstring and signature. + + Returns None if no information is available. + """ + lang_server = self.get_language_server(relative_file_path) + hover_info = lang_server.request_hover(relative_file_path=relative_file_path, line=line, column=column) + if hover_info is None: + return None + contents = hover_info["contents"] + try: + # Handle various response formats + if isinstance(contents, list): + # Array format: extract all parts and join them + stripped_parts = [] + for part in contents: + if isinstance(part, str) and (stripped_part := part.strip()): + stripped_parts.append(stripped_part) + else: + # should be a dict with "value" key + stripped_parts.append(part["value"].strip()) # type: ignore + return "\n".join(stripped_parts) if stripped_parts else None + if isinstance(contents, dict) and (stripped_contents := contents.get("value", "").strip()): + return stripped_contents + if isinstance(contents, str) and (stripped_contents := contents.strip()): + return stripped_contents + except (KeyError, IndexError, TypeError): + pass + return None + + def request_info_for_us(self, us: UnifiedSymbolInformation) -> str | None: + relative_path = us["location"]["relativePath"] + if relative_path is None: + return None + return self.request_info( + relative_file_path=relative_path, + # we need to use selectionRange for hover, + # as range typically starts at the symbol definition (at def, class, etc.) where no hover info is available + line=us["selectionRange"]["start"]["line"], + column=us["selectionRange"]["start"]["character"], + ) + + def request_info_for_symbol(self, symbol: LanguageServerSymbol) -> str | None: + return self.request_info_for_us(symbol.symbol_root) + def get_root_path(self) -> str: return self._ls_manager.get_root_path() diff --git a/src/serena/tools/symbol_tools.py b/src/serena/tools/symbol_tools.py index 75d0e5c9..814c8d5c 100644 --- a/src/serena/tools/symbol_tools.py +++ b/src/serena/tools/symbol_tools.py @@ -89,6 +89,7 @@ class FindSymbolTool(Tool, ToolMarkerSymbolicRead): depth: int = 0, relative_path: str = "", include_body: bool = False, + include_info: bool = True, include_kinds: list[int] = [], # noqa: B006 exclude_kinds: list[int] = [], # noqa: B006 substring_matching: bool = False, @@ -114,17 +115,14 @@ class FindSymbolTool(Tool, ToolMarkerSymbolicRead): :param name_path_pattern: the name path matching pattern (see above) :param depth: depth up to which descendants shall be retrieved (e.g. use 1 to also retrieve immediate children; for the case where the symbol is a class, this will return its methods). - Default 0. :param relative_path: Optional. Restrict search to this file or directory. If None, searches entire codebase. If a directory is passed, the search will be restricted to the files in that directory. If a file is passed, the search will be restricted to that file. If you have some knowledge about the codebase, you should use this parameter, as it will significantly speed up the search as well as reduce the number of results. - :param include_body: If True, include the symbol's source code. Use judiciously. - :param include_kinds: Optional. List of LSP symbol kind integers to include. (e.g., 5 for Class, 12 for Function). - Valid kinds: 1=file, 2=module, 3=namespace, 4=package, 5=class, 6=method, 7=property, 8=field, 9=constructor, 10=enum, - 11=interface, 12=function, 13=variable, 14=constant, 15=string, 16=number, 17=boolean, 18=array, 19=object, - 20=key, 21=null, 22=enum member, 23=struct, 24=event, 25=operator, 26=type parameter. + :param include_body: whether to include the symbol's source code. Use judiciously. + :param include_info: whether to include additional info about the symbol (ignored if include_body is True). + :param include_kinds: List of LSP symbol kind integers to include. If not provided, all kinds are included. :param exclude_kinds: Optional. List of LSP symbol kind integers to exclude. Takes precedence over `include_kinds`. If not provided, no kinds are excluded. @@ -145,6 +143,12 @@ class FindSymbolTool(Tool, ToolMarkerSymbolicRead): within_relative_path=relative_path, ) symbol_dicts = [_sanitize_symbol_dict(s.to_dict(kind=True, location=True, depth=depth, include_body=include_body)) for s in symbols] + if not include_body and include_info: + # we add an info field to the symbol dicts if requested + for s, s_dict in zip(symbols, symbol_dicts, strict=True): + if symbol_info := symbol_retriever.request_info_for_symbol(s): + s_dict["info"] = symbol_info + s_dict.pop("name", None) # name is included in the info result = self._to_json(symbol_dicts) return self._limit_length(result, max_answer_chars) diff --git a/src/solidlsp/language_servers/eclipse_jdtls.py b/src/solidlsp/language_servers/eclipse_jdtls.py index d058521e..312cf444 100644 --- a/src/solidlsp/language_servers/eclipse_jdtls.py +++ b/src/solidlsp/language_servers/eclipse_jdtls.py @@ -10,10 +10,12 @@ import shutil import threading import uuid from pathlib import PurePath +from time import sleep from typing import cast from overrides import override +from solidlsp import ls_types from solidlsp.ls import LSPFileBuffer, SolidLanguageServer from solidlsp.ls_config import LanguageServerConfig from solidlsp.ls_types import UnifiedSymbolInformation @@ -144,9 +146,9 @@ class EclipseJDTLS(SolidLanguageServer): f"{data_dir}", ] - self.service_ready_event = threading.Event() - self.intellicode_enable_command_available = threading.Event() - self.initialize_searcher_command_available = threading.Event() + self._service_ready_event = threading.Event() + self._project_ready_event = threading.Event() + self._intellicode_enable_command_available = threading.Event() super().__init__( config, repository_root_path, ProcessLaunchInfo(cmd, proc_env, proc_cwd), "java", solidlsp_settings=solidlsp_settings @@ -612,6 +614,7 @@ class EclipseJDTLS(SolidLanguageServer): "maven": {"downloadSources": True, "updateSnapshots": False}, "eclipse": {"downloadSources": True}, "signatureHelp": {"enabled": True, "description": {"enabled": True}}, + "hover": {"javadoc": {"enabled": True}}, "implementationsCodeLens": {"enabled": True}, "format": { "enabled": True, @@ -721,15 +724,15 @@ class EclipseJDTLS(SolidLanguageServer): self.completions_available.set() if registration["method"] == "workspace/executeCommand": if "java.intellicode.enable" in registration["registerOptions"]["commands"]: - self.intellicode_enable_command_available.set() + self._intellicode_enable_command_available.set() return def lang_status_handler(params: dict) -> None: - # TODO: Should we wait for - # server -> client: {'jsonrpc': '2.0', 'method': 'language/status', 'params': {'type': 'ProjectStatus', 'message': 'OK'}} - # Before proceeding? if params["type"] == "ServiceReady" and params["message"] == "ServiceReady": - self.service_ready_event.set() + self._service_ready_event.set() + if params["type"] == "ProjectStatus": + if params["message"] == "OK": + self._project_ready_event.set() def execute_client_command_handler(params: dict) -> list: assert params["command"] == "_java.reloadBundles.command" @@ -764,7 +767,7 @@ class EclipseJDTLS(SolidLanguageServer): self.server.notify.workspace_did_change_configuration({"settings": initialize_params["initializationOptions"]["settings"]}) # type: ignore - self.intellicode_enable_command_available.wait() + self._intellicode_enable_command_available.wait() java_intellisense_members_path = self.runtime_dependency_paths.intellisense_members_path assert os.path.exists(java_intellisense_members_path) @@ -776,8 +779,50 @@ class EclipseJDTLS(SolidLanguageServer): ) assert intellicode_enable_result - # TODO: Add comments about why we wait here, and how this can be optimized - self.service_ready_event.wait() + self._service_ready_event.wait() + self._project_ready_event.wait() + + @override + def _request_hover(self, uri: str, line: int, column: int) -> ls_types.Hover | None: + # Eclipse JDTLS is flaky on request_hover, sometimes ignoring the javadoc of a symbol, and on top of that changes the output format + # depending on whether javadoc is present or not (contents: list[...] if present, contents: {value: info} if not) + # The following is a rather hacky way to keep calling _request_hover until "a larger" response comes back or we run out of retries + # Note: The file is kept open by the caller (request_hover), so retries don't cause repeated open/close cycles + max_retries = 5 + last_result = super()._request_hover(uri, line, column) + for _ in range(max_retries): + sleep(0.05) + new_result = super()._request_hover(uri, line, column) + if new_result: + if not last_result: + last_result = new_result + continue + last_contents = last_result["contents"] + new_contents = new_result["contents"] + if last_contents == new_contents: + continue + if isinstance(new_contents, list) and isinstance(last_contents, dict): + return new_result + if isinstance(last_contents, list) and isinstance(new_contents, dict): + return last_result + # Get comparable values - prefer list (full javadoc) over dict/str + if isinstance(last_contents, list): + last_value: list | str = last_contents + elif isinstance(last_contents, dict): + last_value = last_contents["value"] + else: + last_value = last_contents + if isinstance(new_contents, list): + new_value: list | str = new_contents + elif isinstance(new_contents, dict): + new_value = new_contents["value"] + else: + new_value = new_contents + if len(last_value) > len(new_value): + return last_result + else: + return new_result + return last_result def _request_document_symbols( self, relative_file_path: str, file_data: LSPFileBuffer | None diff --git a/src/solidlsp/ls.py b/src/solidlsp/ls.py index be66e36c..e5e9e785 100644 --- a/src/solidlsp/ls.py +++ b/src/solidlsp/ls.py @@ -1322,11 +1322,57 @@ class SolidLanguageServer(ABC): :param relative_file_path: The relative path of the file that has the hover information :param line: The line number of the symbol :param column: The column number of the symbol + """ + with self.open_file(relative_file_path): + uri = pathlib.Path(os.path.join(self.repository_root_path, relative_file_path)).as_uri() + return self._request_hover(uri, line, column) + + def _request_hover(self, uri: str, line: int, column: int) -> ls_types.Hover | None: + """ + Internal method that performs the actual hover request. + The file must already be open when calling this method. + Subclasses can override this to customize hover behavior (e.g., retries). + + :param uri: The URI of the file + :param line: The line number of the symbol + :param column: The column number of the symbol + """ + response = self.server.send.hover( + { + "textDocument": {"uri": uri}, + "position": { + "line": line, + "character": column, + }, + } + ) + + if response is None: + return None + + assert isinstance(response, dict) + contents = response.get("contents") + if not contents: + return None + if isinstance(contents, dict) and not contents.get("value"): + return None + return ls_types.Hover(**response) # type: ignore + + def request_signature_help(self, relative_file_path: str, line: int, column: int) -> ls_types.SignatureHelp | None: + """ + Raise a [textDocument/signatureHelp](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_signatureHelp) + request to the Language Server to find the signature help at the given line and column in the given file. + Note: contrary to `hover`, this only returns something on the position of a *call* and not on a symbol definition. + This means for Serena's purposes, this method is not particularly useful. The result is also fairly verbose (but well structured). + + :param relative_file_path: The relative path of the file that has the signature help + :param line: The line number of the symbol + :param column: The column number of the symbol :return None """ with self.open_file(relative_file_path): - response = self.server.send.hover( + response = self.server.send.signature_help( { "textDocument": {"uri": pathlib.Path(os.path.join(self.repository_root_path, relative_file_path)).as_uri()}, "position": { @@ -1341,7 +1387,7 @@ class SolidLanguageServer(ABC): assert isinstance(response, dict) - return ls_types.Hover(**response) # type: ignore + return ls_types.SignatureHelp(**response) # type: ignore def retrieve_symbol_body( self, diff --git a/src/solidlsp/ls_types.py b/src/solidlsp/ls_types.py index e8899602..06e6ae9b 100644 --- a/src/solidlsp/ls_types.py +++ b/src/solidlsp/ls_types.py @@ -371,3 +371,75 @@ 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. """ + + +class SignatureHelp(TypedDict): + """ + Signature help represents the signature of something + callable. There can be multiple signature but only one + active and only one active parameter. + + See https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#signatureHelp + """ + + signatures: list[SignatureInformation] + """ One or more signatures. """ + activeSignature: NotRequired[int] + """ The active signature. If omitted or the value lies outside the + range of `signatures` the value defaults to zero or is ignored if + the `SignatureHelp` has no signatures. + + Whenever possible implementers should make an active decision about + the active signature and shouldn't rely on a default value. + + In future version of the protocol this property might become + mandatory to better express this. """ + activeParameter: NotRequired[int] + """ The active parameter of the active signature. If omitted or the value + lies outside the range of `signatures[activeSignature].parameters` + defaults to 0 if the active signature has parameters. If + the active signature has no parameters it is ignored. + In future version of the protocol this property might become + mandatory to better express the active parameter if the + active signature does have any. """ + + +class SignatureInformation(TypedDict): + """Represents the signature of something callable. A signature + can have a label, like a function-name, a doc-comment, and + a set of parameters. + """ + + label: str + """ The label of this signature. Will be shown in + the UI. """ + documentation: NotRequired[MarkupContent | str] + """ The human-readable doc-comment of this signature. Will be shown + in the UI but can be omitted. """ + parameters: NotRequired[list[ParameterInformation]] + """ The parameters of this signature. """ + activeParameter: NotRequired[int] + """ The index of the active parameter. + + If provided, this is used in place of `SignatureHelp.activeParameter`. + + @since 3.16.0 """ + + +class ParameterInformation(TypedDict): + """Represents a parameter of a callable-signature. A parameter can + have a label and a doc-comment. + """ + + label: str | list[int] + """ The label of this parameter information. + + Either a string or an inclusive start and exclusive end offsets within its containing + signature label. (see SignatureInformation.label). The offsets are based on a UTF-16 + string representation as `Position` and `Range` does. + + *Note*: a label of type string should be a substring of its containing signature label. + Its intended use case is to highlight the parameter label part in the `SignatureInformation.label`. """ + documentation: NotRequired[MarkupContent | str] + """ The human-readable doc-comment of this parameter. Will be shown + in the UI but can be omitted. """ diff --git a/test/resources/repos/java/test_repo/src/main/java/test_repo/Model.java b/test/resources/repos/java/test_repo/src/main/java/test_repo/Model.java index 5a6a044e..8834f3ca 100644 --- a/test/resources/repos/java/test_repo/src/main/java/test_repo/Model.java +++ b/test/resources/repos/java/test_repo/src/main/java/test_repo/Model.java @@ -1,5 +1,8 @@ package test_repo; +/** + * A simple model class that holds a name and provides methods to retrieve it. + */ public class Model { private String name; diff --git a/test/serena/test_serena_agent.py b/test/serena/test_serena_agent.py index 761affc2..52055975 100644 --- a/test/serena/test_serena_agent.py +++ b/test/serena/test_serena_agent.py @@ -14,6 +14,7 @@ from serena.config.serena_config import ProjectConfig, RegisteredProject, Serena from serena.project import Project from serena.tools import SUCCESS_RESULT, FindReferencingSymbolsTool, FindSymbolTool, ReplaceContentTool, ReplaceSymbolBodyTool from solidlsp.ls_config import Language +from solidlsp.ls_types import SymbolKind from test.conftest import get_repo_path, language_tests_enabled from test.solidlsp import clojure as clj @@ -120,7 +121,7 @@ class TestSerenaAgent: ], indirect=["serena_agent"], ) - def test_find_symbol(self, serena_agent, symbol_name: str, expected_kind: str, expected_file: str): + def test_find_symbol(self, serena_agent: SerenaAgent, symbol_name: str, expected_kind: str, expected_file: str): agent = serena_agent find_symbol_tool = agent.get_tool(FindSymbolTool) result = find_symbol_tool.apply_ex(name_path_pattern=symbol_name) @@ -130,6 +131,19 @@ class TestSerenaAgent: symbol_name in s["name_path"] and expected_kind.lower() in s["kind"].lower() and expected_file in s["relative_path"] for s in symbols ), f"Expected to find {symbol_name} ({expected_kind}) in {expected_file}" + # testing retrieval of symbol info + for s in symbols: + if s["kind"] == SymbolKind.File.name: + # we ignore file symbols for the info test + continue + symbol_info = s.get("info") + assert symbol_info, f"Expected symbol info to be present for symbol: {s}" + assert ( + symbol_name in s["info"] + ), f"[{serena_agent.get_active_lsp_languages()[0]}] Expected symbol info to contain symbol name {symbol_name}. Info: {s['info']}" + # special additional test for Java, since Eclipse returns hover in a complex format and we want to make sure to get it right + if s["kind"] == SymbolKind.Class.name and serena_agent.get_active_lsp_languages() == [Language.JAVA]: + assert "A simple model class" in symbol_info, f"Java class docstring not found in symbol info: {s}" @pytest.mark.parametrize( "serena_agent,symbol_name,def_file,ref_file", @@ -172,7 +186,7 @@ class TestSerenaAgent: ], indirect=["serena_agent"], ) - def test_find_symbol_references(self, serena_agent, symbol_name: str, def_file: str, ref_file: str) -> None: + def test_find_symbol_references(self, serena_agent: SerenaAgent, symbol_name: str, def_file: str, ref_file: str) -> None: agent = serena_agent # Find the symbol location first diff --git a/test/serena/test_symbol.py b/test/serena/test_symbol.py index d7dc772d..77558ecd 100644 --- a/test/serena/test_symbol.py +++ b/test/serena/test_symbol.py @@ -1,6 +1,8 @@ import pytest -from serena.symbol import NamePathMatcher +from serena.symbol import LanguageServerSymbolRetriever, NamePathMatcher +from solidlsp import SolidLanguageServer +from solidlsp.ls_config import Language class TestSymbolNameMatching: @@ -176,3 +178,13 @@ class TestSymbolNameMatching: result = matcher.matches_components(symbol_name_path_parts, symbol_overload_idx) error_msg = self._create_assertion_error_message(name_path_pattern, symbol_name_path_parts, False, expected, result) assert result == expected, error_msg + + +@pytest.mark.python +class TestLanguageServerSymbolRetriever: + @pytest.mark.parametrize("language_server", [Language.PYTHON], indirect=True) + def test_request_info(self, language_server: SolidLanguageServer): + symbol_retriever = LanguageServerSymbolRetriever(language_server) + create_user_method_symbol = symbol_retriever.find("UserService/create_user", within_relative_path="test_repo/services.py")[0] + create_user_method_symbol_info = symbol_retriever.request_info_for_symbol(create_user_method_symbol) + assert "Create a new user and store it" in create_user_method_symbol_info