From 6f68b4d32c9050514ac8d3106dd2db85ee025d32 Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Sun, 9 Nov 2025 21:48:21 +0100 Subject: [PATCH] UnifiedSymbolInformation: Add overload_idx to support overloads #515 Adapt SolidLanguageServer.request_document_symbols to fill overload_idx Introduce symbol cache versioning within the saved file, avoiding storage of old cache files. --- src/solidlsp/ls.py | 126 ++++++++++++++++++++----------------- src/solidlsp/ls_types.py | 8 +++ src/solidlsp/util/cache.py | 23 +++++++ 3 files changed, 98 insertions(+), 59 deletions(-) create mode 100644 src/solidlsp/util/cache.py diff --git a/src/solidlsp/ls.py b/src/solidlsp/ls.py index 20d737bf..e1859dd9 100644 --- a/src/solidlsp/ls.py +++ b/src/solidlsp/ls.py @@ -4,7 +4,6 @@ import json import logging import os import pathlib -import pickle import shutil import subprocess import threading @@ -31,13 +30,14 @@ from solidlsp.ls_utils import FileUtils, PathUtils, TextUtils from solidlsp.lsp_protocol_handler import lsp_types from solidlsp.lsp_protocol_handler import lsp_types as LSPTypes from solidlsp.lsp_protocol_handler.lsp_constants import LSPConstants -from solidlsp.lsp_protocol_handler.lsp_types import Definition, DefinitionParams, LocationLink, SymbolKind +from solidlsp.lsp_protocol_handler.lsp_types import Definition, DefinitionParams, LocationLink from solidlsp.lsp_protocol_handler.server import ( LSPError, ProcessLaunchInfo, StringDict, ) from solidlsp.settings import SolidLSPSettings +from solidlsp.util.cache import load_cache, save_cache GenericDocumentSymbol = Union[LSPTypes.DocumentSymbol, LSPTypes.SymbolInformation, ls_types.UnifiedSymbolInformation] @@ -85,6 +85,7 @@ class SolidLanguageServer(ABC): """ CACHE_FOLDER_NAME = "cache" + CACHE_VERSION = 2 # To be overridden and extended by subclasses def is_ignored_dirname(self, dirname: str) -> bool: @@ -830,12 +831,26 @@ class SolidLanguageServer(ABC): return [json.loads(json_repr) for json_repr in set(json.dumps(item, sort_keys=True) for item in completions_list)] + def _request_document_symbols(self, relative_file_path: str, include_body: bool = False) -> list[GenericDocumentSymbol]: + """ + Sends a [textDocument/documentSymbol](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_documentSymbol) request to the Language Server + to find symbols in the given file. Wait for the response and return the result. + + :param relative_file_path: + :param include_body: + :return: + """ + self.logger.log(f"Requesting document symbols for {relative_file_path} from the Language Server", logging.DEBUG) + response = self.server.send.document_symbol( + {"textDocument": {"uri": pathlib.Path(os.path.join(self.repository_root_path, relative_file_path)).as_uri()}} + ) + return response + def request_document_symbols( self, relative_file_path: str, include_body: bool = False ) -> tuple[list[ls_types.UnifiedSymbolInformation], list[ls_types.UnifiedSymbolInformation]]: """ - Raise a [textDocument/documentSymbol](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_documentSymbol) request to the Language Server - to find symbols in the given file. Wait for the response and return the result. + Retrieves the full list of symbols in the given file, along with the root symbols that represent the tree structure of the symbols. :param relative_file_path: The relative path of the file that has the symbols :param include_body: whether to include the body of the symbols in the result. @@ -846,6 +861,7 @@ class SolidLanguageServer(ABC): where the parent attribute will be the file symbol which in turn may have a package symbol as parent. If you need a symbol tree that contains file symbols as well, you should use `request_full_symbol_tree` instead. """ + # check if the desired result is cached # TODO: it's kinda dumb to not use the cache if include_body is False after include_body was True once # Should be fixed in the future, it's a small performance optimization cache_key = f"{relative_file_path}-{include_body}" @@ -862,25 +878,30 @@ class SolidLanguageServer(ABC): else: self.logger.log(f"No cache hit for symbols with {include_body=} in {relative_file_path}", logging.DEBUG) - self.logger.log(f"Requesting document symbols for {relative_file_path} from the Language Server", logging.DEBUG) - response = self.server.send.document_symbol( - {"textDocument": {"uri": pathlib.Path(os.path.join(self.repository_root_path, relative_file_path)).as_uri()}} - ) - if response is None: - self.logger.log( - f"Received None response from the Language Server for document symbols in {relative_file_path}. " - f"This means the language server can't understand this file (possibly due to syntax errors). It may also be due to a bug or misconfiguration of the LS. " - f"Returning empty list", - logging.WARNING, - ) - return [], [] - assert isinstance(response, list), f"Unexpected response from Language Server: {response}" + # request the symbols from the language server + response = self._request_document_symbols(relative_file_path, include_body=include_body) + if response is None: self.logger.log( - f"Received {len(response)} document symbols for {relative_file_path} from the Language Server", - logging.DEBUG, + f"Received None response from the Language Server for document symbols in {relative_file_path}. " + f"This means the language server can't understand this file (possibly due to syntax errors). It may also be due to a bug or misconfiguration of the LS. " + f"Returning empty list", + logging.WARNING, ) + return [], [] + assert isinstance(response, list), f"Unexpected response from Language Server: {response}" + self.logger.log( + f"Received {len(response)} document symbols for {relative_file_path} from the Language Server", + logging.DEBUG, + ) - def turn_item_into_symbol_with_children(item: GenericDocumentSymbol): + def turn_item_into_symbol_with_children(item: GenericDocumentSymbol) -> ls_types.UnifiedSymbolInformation: + """ + Ensures that the given item is augmented with location, children, and body (if requested), + such that it satisfies the UnifiedSymbolInformation structure. + + :param item: the item to augment + :return: the augmented item (same object as input, but with more attributes and cast to UnifiedSymbolInformation) + """ item = cast(ls_types.UnifiedSymbolInformation, item) absolute_path = os.path.join(self.repository_root_path, relative_file_path) @@ -912,49 +933,36 @@ class SolidLanguageServer(ABC): for child in children: child["parent"] = item item[LSPConstants.CHILDREN] = children + return item flat_all_symbol_list: list[ls_types.UnifiedSymbolInformation] = [] - root_nodes: list[ls_types.UnifiedSymbolInformation] = [] - for root_item in response: - if "range" not in root_item and "location" not in root_item: - if root_item["kind"] in [SymbolKind.File, SymbolKind.Module]: - ... - # mutation is more convenient than creating a new dict, - # so we cast and rename the var after the mutating call to turn_item_into_symbol_with_children - # which turned and item into a "symbol" - turn_item_into_symbol_with_children(root_item) - root_symbol = cast(ls_types.UnifiedSymbolInformation, root_item) - root_symbol["parent"] = None + def convert_nodes_with_common_parent(nodes: list[GenericDocumentSymbol], parent: ls_types.UnifiedSymbolInformation | None) -> None: + """ + Converts the given nodes into UnifiedSymbolInformation with proper parent-child relationships, + adding overload indices for symbols with the same name under the same parent. + """ + name_counts = defaultdict(lambda: 0) + for node in nodes: + flat_all_symbol_list.append(node) + usymbol_node = turn_item_into_symbol_with_children(node) + usymbol_node["overload_idx"] = name_counts[usymbol_node["name"]] + name_counts[usymbol_node["name"]] += 1 + usymbol_node["parent"] = parent + if usymbol_node["children"]: + convert_nodes_with_common_parent(usymbol_node["children"], usymbol_node) - root_nodes.append(root_symbol) - assert isinstance(root_symbol, dict) - assert LSPConstants.NAME in root_symbol - assert LSPConstants.KIND in root_symbol - - if LSPConstants.CHILDREN in root_symbol: - # TODO: l_tree should be a list of TreeRepr. Define the following function to return TreeRepr as well - - def visit_tree_nodes_and_build_tree_repr(node: GenericDocumentSymbol) -> list[ls_types.UnifiedSymbolInformation]: - node = cast(ls_types.UnifiedSymbolInformation, node) - l: list[ls_types.UnifiedSymbolInformation] = [] - turn_item_into_symbol_with_children(node) - assert LSPConstants.CHILDREN in node - children = node[LSPConstants.CHILDREN] - l.append(node) - for child in children: - l.extend(visit_tree_nodes_and_build_tree_repr(child)) - return l - - flat_all_symbol_list.extend(visit_tree_nodes_and_build_tree_repr(root_symbol)) - else: - flat_all_symbol_list.append(ls_types.UnifiedSymbolInformation(**root_symbol)) + root_nodes: list[ls_types.UnifiedSymbolInformation] = response + convert_nodes_with_common_parent(root_nodes, None) result = flat_all_symbol_list, root_nodes + + # update cache self.logger.log(f"Caching document symbols for {relative_file_path}", logging.DEBUG) with self._cache_lock: self._document_symbols_cache[cache_key] = (file_data.content_hash, result) self._cache_has_changed = True + return result def request_full_symbol_tree( @@ -1561,7 +1569,7 @@ class SolidLanguageServer(ABC): / self._solidlsp_settings.project_data_relative_path / self.CACHE_FOLDER_NAME / self.language_id - / "document_symbols_cache_v23-06-25.pkl" + / "document_symbols_cache.pkl" ) def save_cache(self): @@ -1573,8 +1581,7 @@ class SolidLanguageServer(ABC): self.logger.log(f"Saving updated document symbols cache to {self.cache_path}", logging.INFO) self.cache_path.parent.mkdir(parents=True, exist_ok=True) try: - with open(self.cache_path, "wb") as f: - pickle.dump(self._document_symbols_cache, f) + save_cache(str(self.cache_path), self.CACHE_VERSION, self._document_symbols_cache) self._cache_has_changed = False except Exception as e: self.logger.log( @@ -1590,9 +1597,10 @@ class SolidLanguageServer(ABC): with self._cache_lock: self.logger.log(f"Loading document symbols cache from {self.cache_path}", logging.INFO) try: - with open(self.cache_path, "rb") as f: - self._document_symbols_cache = pickle.load(f) - self.logger.log(f"Loaded {len(self._document_symbols_cache)} document symbols from cache.", logging.INFO) + saved_cache = load_cache(str(self.cache_path), self.CACHE_VERSION) + if saved_cache is not None: + self._document_symbols_cache = saved_cache + self.logger.log(f"Loaded {len(self._document_symbols_cache)} document symbols from cache.", logging.INFO) except Exception as e: # cache often becomes corrupt, so just skip loading it self.logger.log( diff --git a/src/solidlsp/ls_types.py b/src/solidlsp/ls_types.py index 4a8de6d4..8bc6f303 100644 --- a/src/solidlsp/ls_types.py +++ b/src/solidlsp/ls_types.py @@ -243,6 +243,14 @@ class UnifiedSymbolInformation(TypedDict): All symbols except the root packages will have a parent. """ + overload_idx: NotRequired[int] + """ + The overload index of the symbol, if applicable. If the symbol is an overloaded function or method + (same symbol name with the same parent), this index indicates which overload it is. + The index is 0-based. + Added for Serena, not part of the LSP. + """ + class MarkupKind(Enum): """Describes the content type that a client supports in various diff --git a/src/solidlsp/util/cache.py b/src/solidlsp/util/cache.py new file mode 100644 index 00000000..64ee1345 --- /dev/null +++ b/src/solidlsp/util/cache.py @@ -0,0 +1,23 @@ +import logging +from typing import Any, Optional + +from sensai.util.pickle import dump_pickle, load_pickle + +log = logging.getLogger(__name__) + + +def load_cache(path: str, version: Any) -> Optional[Any]: + data = load_pickle(path) + if not isinstance(data, dict) or "__cache_version" not in data: + log.info("Cache is outdated (expected version %s). Ignoring cache at %s", version, path) + return None + saved_version = data["__cache_version"] + if saved_version != version: + log.info("Cache is outdated (expected version %s, got %s). Ignoring cache at %s", version, saved_version, path) + return None + return data["obj"] + + +def save_cache(path: str, version: Any, obj: Any) -> None: + data = {"__cache_version": version, "obj": obj} + dump_pickle(data, path)