Simplify language server factory interfaces (one factory for all fits current requirements best)

src/serena/ls_manager.py

@@ -1,6 +1,5 @@
 import logging
 from collections.abc import Iterator
-from dataclasses import dataclass
 
 from serena.constants import SERENA_MANAGED_DIR_IN_HOME, SERENA_MANAGED_DIR_NAME
 from solidlsp import SolidLanguageServer
@@ -11,65 +10,67 @@ from solidlsp.settings import SolidLSPSettings
 log = logging.getLogger(__name__)
 
 
-@dataclass
-class LanguageServerFactoryParams:
-    project_root: str
-    encoding: str
-    ignored_patterns: list[str]
-    ls_timeout: float | None = None
-    ls_specific_settings: dict | None = None
-    log_level: int = logging.INFO
-    trace_lsp_communication: bool = False
-
-
 class LanguageServerFactory:
-    def __init__(self, language: Language):
-        self.language = language
+    def __init__(
+        self,
+        project_root: str,
+        encoding: str,
+        ignored_patterns: list[str],
+        ls_timeout: float | None = None,
+        ls_specific_settings: dict | None = None,
+        log_level: int = logging.INFO,
+        trace_lsp_communication: bool = False,
+    ):
+        self.project_root = project_root
+        self.encoding = encoding
+        self.ignored_patterns = ignored_patterns
+        self.ls_timeout = ls_timeout
+        self.ls_specific_settings = ls_specific_settings
+        self.log_level = log_level
+        self.trace_lsp_communication = trace_lsp_communication
 
-    def create_language_server(self, params: LanguageServerFactoryParams) -> SolidLanguageServer:
+    def create_language_server(self, language: Language) -> SolidLanguageServer:
         ls_config = LanguageServerConfig(
-            code_language=self.language,
-            ignored_paths=params.ignored_patterns,
-            trace_lsp_communication=params.trace_lsp_communication,
-            encoding=params.encoding,
+            code_language=language,
+            ignored_paths=self.ignored_patterns,
+            trace_lsp_communication=self.trace_lsp_communication,
+            encoding=self.encoding,
         )
-        ls_logger = LanguageServerLogger(log_level=params.log_level)
+        ls_logger = LanguageServerLogger(log_level=self.log_level)
 
-        log.info(f"Creating language server instance for {params.project_root}, language={self.language}.")
+        log.info(f"Creating language server instance for {self.project_root}, language={language}.")
         return SolidLanguageServer.create(
             ls_config,
             ls_logger,
-            params.project_root,
-            timeout=params.ls_timeout,
+            self.project_root,
+            timeout=self.ls_timeout,
             solidlsp_settings=SolidLSPSettings(
                 solidlsp_dir=SERENA_MANAGED_DIR_IN_HOME,
                 project_data_relative_path=SERENA_MANAGED_DIR_NAME,
-                ls_specific_settings=params.ls_specific_settings or {},
+                ls_specific_settings=self.ls_specific_settings or {},
             ),
         )
 
 
 class LanguageServerManager:
     """
-    Manages one or more language servers
+    Manages one or more language servers for a project.
     """
 
     def __init__(
         self,
         language_servers: dict[Language, SolidLanguageServer],
-        language_server_factories: dict[Language, LanguageServerFactory] | None = None,
-        language_server_factory_params: LanguageServerFactoryParams | None = None,
+        language_server_factory: LanguageServerFactory | None = None,
     ) -> None:
         """
         :param language_servers: a mapping from language to language server; the servers are assumed to be already started.
             The first server in the iteration order is used as the default server.
             All servers are assumed to serve the same project root.
-        :param language_server_factories: factories for the language servers, to be used for restarting them if needed
-        :param language_server_factory_params: parameters to be used when (re-)creating language servers via factories
+        :param language_server_factory: factory for language server creation; if None, dynamic (re)creation of language servers
+            is not supported
         """
         self._language_servers = language_servers
-        self._language_server_factories = language_server_factories or {}
-        self._language_server_factory_params = language_server_factory_params
+        self._language_server_factory = language_server_factory
         self._default_language_server = next(iter(language_servers.values()))
         self._root_path = self._default_language_server.repository_root_path
 
@@ -94,14 +95,12 @@ class LanguageServerManager:
         return self._ensure_functional_ls(ls)
 
     def _create_and_start_language_server(self, language: Language) -> SolidLanguageServer:
-        factory = self._language_server_factories.get(language)
-        if factory is None:
-            raise ValueError(f"No language server factory found for language {language}; cannot create new instance")
-        if self._language_server_factory_params is None:
-            raise ValueError("No language server factory parameters provided; cannot instantiate new language server instance")
-        self._language_servers[language] = factory.create_language_server(self._language_server_factory_params)
+        if self._language_server_factory is None:
+            raise ValueError(f"No language server factory available to create language server for {language}")
+        language_server = self._language_server_factory.create_language_server(language)
         self._language_servers[language].start()
-        return self._language_servers[language]
+        self._language_servers[language] = language_server
+        return language_server
 
     def restart_language_server(self, language: Language) -> SolidLanguageServer:
         """
@@ -111,6 +110,8 @@ class LanguageServerManager:
         :param language: the language
         :return: the newly created language server
         """
+        if language not in self._language_servers:
+            raise ValueError(f"No language server for language {language.value} present; cannot restart")
         return self._create_and_start_language_server(language)
 
     def add_language_server(self, language: Language) -> SolidLanguageServer:
@@ -122,15 +123,8 @@ class LanguageServerManager:
         :return: the newly created language server
         """
         if language in self._language_servers:
-            raise ValueError(f"Language server for language {language} already present")
-        self._language_server_factories[language] = LanguageServerFactory(language)
-        try:
-            return self._create_and_start_language_server(language)
-        except Exception as e:
-            del self._language_server_factories[language]
-            if language in self._language_servers:
-                del self._language_servers[language]
-            raise Exception(f"Addition of language server for {language.value} failed") from e
+            raise ValueError(f"Language server for language {language.value} already present")
+        return self._create_and_start_language_server(language)
 
     def iter_language_servers(self) -> Iterator[SolidLanguageServer]:
         for ls in self._language_servers.values():

src/serena/project.py

@@ -11,7 +11,7 @@ from sensai.util.string import ToStringMixin
 
 from serena.config.serena_config import DEFAULT_TOOL_TIMEOUT, ProjectConfig, get_serena_managed_in_project_dir
 from serena.constants import SERENA_FILE_ENCODING, SERENA_MANAGED_DIR_NAME
-from serena.ls_manager import LanguageServerFactory, LanguageServerFactoryParams, LanguageServerManager
+from serena.ls_manager import LanguageServerFactory, LanguageServerManager
 from serena.text_utils import MatchedConsecutiveLines, search_files
 from serena.util.file_system import GitignoreParser, match_path
 from serena.util.general import load_yaml, save_yaml
@@ -384,7 +384,7 @@ class Project(ToStringMixin):
             self.language_server_manager = None
 
         log.info(f"Creating language server manager for {self.project_root}")
-        language_server_factory_params = LanguageServerFactoryParams(
+        language_server_factory = LanguageServerFactory(
             project_root=self.project_root,
             encoding=self.project_config.encoding,
             ignored_patterns=self._ignored_patterns,
@@ -394,15 +394,14 @@ class Project(ToStringMixin):
             trace_lsp_communication=trace_lsp_communication,
         )
         language_servers: dict[Language, SolidLanguageServer] = {}
-        language_server_factories: dict[Language, LanguageServerFactory] = {}
         threads = []
         exceptions = {}
         lock = threading.Lock()
 
-        def start_language_server(language: Language, factory: LanguageServerFactory) -> None:
+        def start_language_server(language: Language) -> None:
             try:
                 with LogTime(f"Language server startup (language={language.value})"):
-                    language_server = factory.create_language_server(language_server_factory_params)
+                    language_server = language_server_factory.create_language_server(language)
                     language_server.start()
                     if not language_server.is_running():
                         raise RuntimeError(
@@ -417,9 +416,7 @@ class Project(ToStringMixin):
 
         # start language servers in parallel threads
         for language in self.project_config.languages:
-            factory = LanguageServerFactory(language)
-            language_server_factories[language] = factory
-            thread = threading.Thread(target=start_language_server, args=(language, factory), name="StartLS:" + language.value)
+            thread = threading.Thread(target=start_language_server, args=(language,), name="StartLS:" + language.value)
             thread.start()
             threads.append(thread)
         for thread in threads:
@@ -432,7 +429,7 @@ class Project(ToStringMixin):
             failure_messages = "\n".join([f"{lang.value}: {e}" for lang, e in exceptions.items()])
             raise Exception(f"Failed to start language servers:\n{failure_messages}")
 
-        self.language_server_manager = LanguageServerManager(language_servers, language_server_factories, language_server_factory_params)
+        self.language_server_manager = LanguageServerManager(language_servers, language_server_factory)
         return self.language_server_manager
 
     def add_language(self, language: Language) -> None: