From 44c1b69ddee9969583655e9abc5268ace3bd4abd Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Wed, 12 Nov 2025 03:23:18 +0100 Subject: [PATCH] Open every file exactly once during request_full_symbol_tree --- .../language_servers/bash_language_server.py | 29 +++------ .../fortran_language_server.py | 28 ++++----- src/solidlsp/language_servers/nixd_ls.py | 14 ++--- src/solidlsp/ls.py | 62 ++++++++++++------- 4 files changed, 66 insertions(+), 67 deletions(-) diff --git a/src/solidlsp/language_servers/bash_language_server.py b/src/solidlsp/language_servers/bash_language_server.py index 9942e1f4..f7dd1d5e 100644 --- a/src/solidlsp/language_servers/bash_language_server.py +++ b/src/solidlsp/language_servers/bash_language_server.py @@ -10,7 +10,7 @@ import shutil import threading from solidlsp.language_servers.common import RuntimeDependency, RuntimeDependencyCollection -from solidlsp.ls import DocumentSymbols, SolidLanguageServer +from solidlsp.ls import DocumentSymbols, LSPFileBuffer, SolidLanguageServer from solidlsp.ls_config import LanguageServerConfig from solidlsp.ls_logger import LanguageServerLogger from solidlsp.lsp_protocol_handler.lsp_types import InitializeParams @@ -193,29 +193,18 @@ class BashLanguageServer(SolidLanguageServer): else: self.logger.log("Bash server initialization complete", logging.INFO) - def request_document_symbols(self, relative_file_path: str) -> DocumentSymbols: - """ - Request document symbols from bash-language-server via LSP. + def request_document_symbols(self, relative_file_path: str, file_buffer: LSPFileBuffer | None = None) -> DocumentSymbols: + # Uses the standard LSP documentSymbol request which provides reliable function detection + # for all bash function syntaxes including: + # - function name() { ... } (with function keyword) + # - name() { ... } (traditional syntax) + # - Functions with various indentation levels + # - Functions with comments before/after/inside - Uses the standard LSP documentSymbol request which provides reliable function detection - for all bash function syntaxes including: - - function name() { ... } (with function keyword) - - name() { ... } (traditional syntax) - - Functions with various indentation levels - - Functions with comments before/after/inside - - Args: - relative_file_path: Path to the bash file relative to repository root - include_body: Whether to include function bodies in symbol information - - Returns: - The symbols - - """ self.logger.log(f"Requesting document symbols via LSP for {relative_file_path}", logging.DEBUG) # Use the standard LSP approach - bash-language-server handles all function syntaxes correctly - document_symbols = super().request_document_symbols(relative_file_path) + document_symbols = super().request_document_symbols(relative_file_path, file_buffer=file_buffer) # Log detection results for debugging functions = [s for s in document_symbols.iter_symbols() if s.get("kind") == 12] diff --git a/src/solidlsp/language_servers/fortran_language_server.py b/src/solidlsp/language_servers/fortran_language_server.py index 69ead2d0..782df7b3 100644 --- a/src/solidlsp/language_servers/fortran_language_server.py +++ b/src/solidlsp/language_servers/fortran_language_server.py @@ -12,7 +12,7 @@ import threading from overrides import override from solidlsp import ls_types -from solidlsp.ls import DocumentSymbols, SolidLanguageServer +from solidlsp.ls import DocumentSymbols, LSPFileBuffer, SolidLanguageServer from solidlsp.ls_config import LanguageServerConfig from solidlsp.ls_logger import LanguageServerLogger from solidlsp.lsp_protocol_handler.lsp_types import InitializeParams @@ -138,22 +138,20 @@ class FortranLanguageServer(SolidLanguageServer): return symbol @override - def request_document_symbols(self, relative_file_path: str) -> DocumentSymbols: - """ - Override to fix fortls's incorrect selectionRange bug. + def request_document_symbols(self, relative_file_path: str, file_buffer: LSPFileBuffer | None = None) -> DocumentSymbols: + # Override to fix fortls's incorrect selectionRange bug. + # + # fortls returns selectionRange pointing to line start (character 0) instead of the + # identifier name position. This breaks MCP server features that rely on exact positions. + # + # This override: + # 1. Gets symbols from fortls via parent implementation + # 2. Parses each symbol's line to find the correct identifier position + # 3. Fixes selectionRange for all symbols recursively + # 4. Returns corrected symbols - fortls returns selectionRange pointing to line start (character 0) instead of the - identifier name position. This breaks MCP server features that rely on exact positions. - - This override: - 1. Gets symbols from fortls via parent implementation - 2. Parses each symbol's line to find the correct identifier position - 3. Fixes selectionRange for all symbols recursively - 4. Returns corrected symbols - - """ # Get symbols from fortls (with incorrect selectionRange) - document_symbols = super().request_document_symbols(relative_file_path) + document_symbols = super().request_document_symbols(relative_file_path, file_buffer=file_buffer) # Get file content for parsing with self.open_file(relative_file_path) as file_data: diff --git a/src/solidlsp/language_servers/nixd_ls.py b/src/solidlsp/language_servers/nixd_ls.py index 4af41c7f..beea7c6b 100644 --- a/src/solidlsp/language_servers/nixd_ls.py +++ b/src/solidlsp/language_servers/nixd_ls.py @@ -16,7 +16,7 @@ from pathlib import Path from overrides import override from solidlsp import ls_types -from solidlsp.ls import DocumentSymbols, SolidLanguageServer +from solidlsp.ls import DocumentSymbols, LSPFileBuffer, SolidLanguageServer from solidlsp.ls_config import LanguageServerConfig from solidlsp.ls_logger import LanguageServerLogger from solidlsp.lsp_protocol_handler.lsp_types import InitializeParams @@ -70,15 +70,13 @@ class NixLanguageServer(SolidLanguageServer): return symbol @override - def request_document_symbols(self, relative_file_path: str) -> DocumentSymbols: - """ - Override to extend Nix symbol ranges to include trailing semicolons. + def request_document_symbols(self, relative_file_path: str, file_buffer: LSPFileBuffer | None = None) -> DocumentSymbols: + # Override to extend Nix symbol ranges to include trailing semicolons. + # nixd provides expression-level ranges (excluding semicolons) but serena needs + # statement-level ranges (including semicolons) for proper symbol replacement. - nixd provides expression-level ranges (excluding semicolons) but serena needs - statement-level ranges (including semicolons) for proper symbol replacement. - """ # Get symbols from parent implementation - document_symbols = super().request_document_symbols(relative_file_path) + document_symbols = super().request_document_symbols(relative_file_path, file_buffer=file_buffer) # Get file content for range extension file_content = self.language_server.retrieve_full_file_content(relative_file_path) diff --git a/src/solidlsp/ls.py b/src/solidlsp/ls.py index 0df1487a..2341abba 100644 --- a/src/solidlsp/ls.py +++ b/src/solidlsp/ls.py @@ -484,6 +484,20 @@ class SolidLanguageServer(ABC): ) del self.open_file_buffers[uri] + @contextmanager + def _open_file_context(self, relative_file_path: str, file_buffer: LSPFileBuffer | None = None) -> Iterator[LSPFileBuffer]: + """ + Internal context manager to open a file, optionally reusing an existing file buffer. + + :param relative_file_path: the relative path of the file to open. + :param file_buffer: an optional existing file buffer to reuse. + """ + if file_buffer is not None: + yield file_buffer + else: + with self.open_file(relative_file_path) as fb: + yield fb + def insert_text_at_position(self, relative_file_path: str, line: int, column: int, text_to_be_inserted: str) -> ls_types.Position: """ Insert text at the given line and column in the given file and return @@ -941,11 +955,12 @@ class SolidLanguageServer(ABC): with self.open_file(relative_file_path) as opened_file_data: return get_raw_document_symbols(opened_file_data) - def request_document_symbols(self, relative_file_path: str) -> DocumentSymbols: + def request_document_symbols(self, relative_file_path: str, file_buffer: LSPFileBuffer | None = None) -> DocumentSymbols: """ Retrieves the collection of symbols in the given file :param relative_file_path: The relative path of the file that has the symbols + :param file_buffer: an optional file buffer if the file is already opened. :return: the collection of symbols in the file. All contained symbols will have a location, children, and a parent attribute, where the parent attribute is None for root symbols. @@ -953,7 +968,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. """ - with self.open_file(relative_file_path) as file_data: + with self._open_file_context(relative_file_path, file_buffer) as file_data: # check if the desired result is cached cache_key = relative_file_path file_hash_and_result = self._document_symbols_cache.get(cache_key) @@ -1148,29 +1163,28 @@ class SolidLanguageServer(ABC): child["parent"] = package_symbol elif os.path.isfile(contained_dir_or_file_abs_path): - document_symbols = self.request_document_symbols(contained_dir_or_file_rel_path) - file_root_nodes = document_symbols.root_symbols + with self._open_file_context(contained_dir_or_file_rel_path) as file_data: + document_symbols = self.request_document_symbols(contained_dir_or_file_rel_path, file_data) + file_root_nodes = document_symbols.root_symbols - # Create file symbol, link with children - file_rel_path = str(Path(contained_dir_or_file_abs_path).resolve().relative_to(self.repository_root_path)) - with self.open_file(file_rel_path) as file_data: - fileRange = self._get_range_from_file_content(file_data.contents) - file_symbol = ls_types.UnifiedSymbolInformation( # type: ignore - name=os.path.splitext(contained_dir_or_file_name)[0], - kind=ls_types.SymbolKind.File, - range=fileRange, - selectionRange=fileRange, - location=ls_types.Location( - uri=str(pathlib.Path(contained_dir_or_file_abs_path).as_uri()), - range=fileRange, - absolutePath=str(contained_dir_or_file_abs_path), - relativePath=str(Path(contained_dir_or_file_abs_path).resolve().relative_to(self.repository_root_path)), - ), - children=file_root_nodes, - parent=package_symbol, - ) - for child in file_root_nodes: - child["parent"] = file_symbol + # Create file symbol, link with children + file_range = self._get_range_from_file_content(file_data.contents) + file_symbol = ls_types.UnifiedSymbolInformation( # type: ignore + name=os.path.splitext(contained_dir_or_file_name)[0], + kind=ls_types.SymbolKind.File, + range=file_range, + selectionRange=file_range, + location=ls_types.Location( + uri=str(pathlib.Path(contained_dir_or_file_abs_path).as_uri()), + range=file_range, + absolutePath=str(contained_dir_or_file_abs_path), + relativePath=str(Path(contained_dir_or_file_abs_path).resolve().relative_to(self.repository_root_path)), + ), + children=file_root_nodes, + parent=package_symbol, + ) + for child in file_root_nodes: + child["parent"] = file_symbol # Link file symbol with package package_symbol["children"].append(file_symbol)