Merge pull request #104 from oraios/config-dataclasses

Better config options, integrations tests for SerenaAgent, minor improvements in LS

.github/workflows/pytest.yml

@@ -1,6 +1,6 @@
 name: Tests on Ubuntu
 
-on: [push, pull_request]
+on: [pull_request]
 
 jobs:
   cpu:

scripts/demo_run_tools.py

@@ -13,7 +13,7 @@ from serena.agent import *
 project_root = Path(__file__).parent.parent
 
 if __name__ == "__main__":
-    agent = SerenaAgent(project_file_path=str(project_root / ".serena" / "project.yml"))
+    agent = SerenaAgent(project_config=str(project_root / ".serena" / "project.yml"))
     overview_tool = agent.get_tool(GetSymbolsOverviewTool)
     find_symbol_tool = agent.get_tool(FindSymbolTool)
 

src/multilspy/language_server.py

@@ -14,19 +14,16 @@ import os
 import pathlib
 import pickle
 import re
-from site import abs_paths
 import threading
 from collections import defaultdict
 from contextlib import asynccontextmanager, contextmanager
 from copy import copy
-from typing import Dict, List, Optional, Tuple, Union
-from fnmatch import fnmatch
 from pathlib import Path, PurePath
 from typing import AsyncIterator, Dict, Iterator, List, Optional, Tuple, Union, cast
 
 import pathspec
 
-from serena.text_utils import LineType, MatchedConsecutiveLines, TextLine, search_files, search_text
+from serena.text_utils import LineType, MatchedConsecutiveLines, TextLine, search_files
 from . import multilspy_types
 from .lsp_protocol_handler import lsp_types as LSPTypes
 from .lsp_protocol_handler.lsp_constants import LSPConstants
@@ -38,9 +35,6 @@ from .lsp_protocol_handler.server import (
 )
 from .multilspy_config import Language, MultilspyConfig
 from .multilspy_exceptions import MultilspyException
-from .multilspy_utils import PathUtils, FileUtils, TextUtils
-from pathlib import PurePath
-from typing import AsyncIterator, Iterator, List, Dict, Optional, Union, Tuple
 from .multilspy_logger import MultilspyLogger
 from .multilspy_utils import FileUtils, PathUtils, TextUtils
 from .type_helpers import ensure_all_methods_implemented
@@ -581,6 +575,16 @@ class LanguageServer:
 
         return ret
 
+    # Some LS cause problems with this, so the call is isolated from the rest to allow overriding in subclasses
+    async def _send_references_request(self, relative_file_path: str, line: int, column: int):
+        return await self.server.send.references(
+            {
+                "textDocument": {"uri": PathUtils.path_to_uri(os.path.join(self.repository_root_path, relative_file_path))},
+                "position": {"line": line, "character": column},
+                "context": {"includeDeclaration": False},
+            }
+        )
+
     async def request_references(
         self, relative_file_path: str, line: int, column: int
     ) -> List[multilspy_types.Location]:
@@ -606,13 +610,7 @@ class LanguageServer:
 
         with self.open_file(relative_file_path):
             try:
-                response = await self.server.send.references(
-                    {
-                        "textDocument": {"uri": PathUtils.path_to_uri(os.path.join(self.repository_root_path, relative_file_path))},
-                        "position": {"line": line, "character": column},
-                        "context": {"includeDeclaration": False},
-                    }
-                )
+                response = await self._send_references_request(relative_file_path, line=line, column=column)
             except Exception as e:
                 # Catch LSP internal error (-32603) and raise a more informative exception
                 if isinstance(e, Error) and getattr(e, 'code', None) == -32603:

src/multilspy/language_servers/gopls/gopls.py

@@ -156,18 +156,16 @@ class Gopls(LanguageServer):
             await self.server.stop()
 
     @override
-    async def request_references(
-        self, relative_file_path: str, line: int, column: int
-    ) -> List[multilspy_types.Location]:
+    async def _send_references_request(self, relative_file_path: str, line: int, column: int) -> List[multilspy_types.Location]:
         NUM_COLS_TO_TRY = 10
-        """selectionRange in Gopls always return the wrong column, 
-        the one at the beginnning of the declaration instead of the symbol. We do a dirty hack here.
+        """selectionRange in Gopls always contains the wrong column, 
+        the one at the beginning of the declaration instead of the symbol. We do a dirty hack here (loop over some columns).
         See https://github.com/golang/go/issues/73521
         """
         actual_column = column
         for actual_column in range(column, column+NUM_COLS_TO_TRY):
             try:
-                return await super().request_references(relative_file_path, line, actual_column)
+                return await super()._send_references_request(relative_file_path, line, actual_column)
             except (Error, RuntimeError, MultilspyException) as e:
                 self.logger.log(f"Cannot find symbol at {line=}, column={actual_column}, trying to bump column by 1", logging.INFO)
-        raise RuntimeError(f"Failed to find references for symbol at {line=}, {column=}")
+        raise MultilspyException(f"Failed to find references for symbol in\n{relative_file_path=} at {line=}, {column=}")

src/multilspy/language_servers/typescript_language_server/typescript_language_server.py

@@ -4,22 +4,23 @@ Provides TypeScript specific instantiation of the LanguageServer class. Contains
 
 import asyncio
 import json
-import shutil
 import logging
 import os
-import subprocess
 import pathlib
+import shutil
+import subprocess
+from collections.abc import AsyncIterator
 from contextlib import asynccontextmanager
-from typing import AsyncIterator, Tuple
+from time import sleep
 
 from overrides import override
 
-from multilspy.multilspy_logger import MultilspyLogger
 from multilspy.language_server import LanguageServer
-from multilspy.lsp_protocol_handler.server import ProcessLaunchInfo
 from multilspy.lsp_protocol_handler.lsp_types import InitializeParams
+from multilspy.lsp_protocol_handler.server import ProcessLaunchInfo
 from multilspy.multilspy_config import MultilspyConfig
-from multilspy.multilspy_utils import PlatformUtils, PlatformId
+from multilspy.multilspy_logger import MultilspyLogger
+from multilspy.multilspy_utils import PlatformId, PlatformUtils
 
 # Platform-specific imports
 if os.name != 'nt':  # Unix-like systems
@@ -82,7 +83,7 @@ class TypeScriptLanguageServer(LanguageServer):
         ]
         assert platform_id in valid_platforms, f"Platform {platform_id} is not supported for multilspy javascript/typescript at the moment"
 
-        with open(os.path.join(os.path.dirname(__file__), "runtime_dependencies.json"), "r") as f:
+        with open(os.path.join(os.path.dirname(__file__), "runtime_dependencies.json")) as f:
             d = json.load(f)
             del d["_description"]
 
@@ -132,7 +133,7 @@ class TypeScriptLanguageServer(LanguageServer):
         """
         Returns the initialize params for the TypeScript Language Server.
         """
-        with open(os.path.join(os.path.dirname(__file__), "initialize_params.json"), "r") as f:
+        with open(os.path.join(os.path.dirname(__file__), "initialize_params.json")) as f:
             d = json.load(f)
 
         del d["_description"]
@@ -222,3 +223,14 @@ class TypeScriptLanguageServer(LanguageServer):
 
             await self.server.shutdown()
             await self.server.stop()
+
+    @override
+    # For some reason, the LS may need longer to process this, so we just retry
+    async def _send_references_request(self, relative_file_path: str, line: int, column: int):
+        # TODO: The LS doesn't return references contained in other files if it doesn't sleep. This is
+        #   despite the LS having processed requests already. I don't know what causes this, but sleeping
+        #   one second helps. It may be that sleeping only once is enough but that's hard to reliably test.
+        #   It may be that even this 1sec is not enough in larger TS projects, at some point we should find what
+        #   causes this and solve it.
+        sleep(1)
+        return await super()._send_references_request(relative_file_path, line, column)

src/serena/agent.py

@@ -11,6 +11,7 @@ import traceback
 from abc import ABC
 from collections import defaultdict
 from collections.abc import Callable, Generator, Iterable
+from dataclasses import dataclass, field
 from logging import Logger
 from pathlib import Path
 from typing import TYPE_CHECKING, Any, Self, TypeVar, Union, cast
@@ -28,7 +29,6 @@ from serena import serena_root_path, serena_version
 from serena.llm.prompt_factory import PromptFactory
 from serena.symbol import SymbolLocation, SymbolManager
 from serena.text_utils import search_files
-from serena.util.class_decorators import singleton
 from serena.util.file_system import scan_directory
 from serena.util.inspection import iter_subclasses
 from serena.util.shell import execute_shell_command
@@ -67,22 +67,40 @@ class SerenaConfigError(Exception):
     pass
 
 
+@dataclass
 class ProjectConfig(ToStringMixin):
+    project_name: str
+    language: Language
+    project_root: str
+    ignored_paths: list[str] = field(default_factory=list)
+    excluded_tools: set[str] = field(default_factory=set)
+    read_only: bool = False
+    ignore_all_files_in_gitignore: bool = True
+
     SERENA_MANAGED_DIR = ".serena"
     SERENA_DEFAULT_PROJECT_FILE = "project.yml"
 
-    def __init__(self, config_dict: dict[str, Any], project_name: str, project_root: Path | None = None):
-        self.project_name: str = project_name
+    @classmethod
+    def rel_path_to_project_yml(cls):
+        return os.path.join(cls.SERENA_MANAGED_DIR, cls.SERENA_DEFAULT_PROJECT_FILE)
+
+    @classmethod
+    def from_config_dict(cls, config_dict: dict[str, Any], project_name: str, project_root: Path | None = None) -> Self:
+        """
+        Create a ProjectConfig instance from a configuration dictionary
+        """
         try:
-            self.language: Language = Language(config_dict["language"].lower())
+            language = Language(config_dict["language"].lower())
         except ValueError as e:
             raise ValueError(f"Invalid language: {config_dict['language']}.\nValid languages are: {[l.value for l in Language]}") from e
+
         if project_root is None:
             project_root = Path(config_dict["project_root"])
-        self.project_root: str = str(project_root.resolve())
-        self.ignored_paths: list[str] = config_dict.get("ignored_paths", [])
-        self.excluded_tools: set[str] = set(config_dict.get("excluded_tools", []))
-        self.read_only: bool = config_dict.get("read_only", False)
+        project_root_str = str(project_root.resolve())
+
+        ignored_paths = config_dict.get("ignored_paths", [])
+        excluded_tools = set(config_dict.get("excluded_tools", []))
+        read_only = config_dict.get("read_only", False)
 
         if "ignore_all_files_in_gitignore" not in config_dict:
             raise SerenaConfigError(
@@ -90,7 +108,7 @@ class ProjectConfig(ToStringMixin):
                 "Please update your `.yml` configuration file for this project. "
                 "It is recommended to set this to `True`."
             )
-        self.ignore_all_files_in_gitignore = config_dict["ignore_all_files_in_gitignore"]
+        ignore_all_files_in_gitignore = config_dict["ignore_all_files_in_gitignore"]
 
         # Raise errors for deprecated keys
         if "ignored_dirs" in config_dict:
@@ -99,9 +117,24 @@ class ProjectConfig(ToStringMixin):
                 "Note that you can also set `ignore_all_files_in_gitignore` to `True`, which will be enough for most cases."
             )
 
+        return cls(
+            project_name=project_name,
+            language=language,
+            project_root=project_root_str,
+            ignored_paths=ignored_paths,
+            excluded_tools=excluded_tools,
+            read_only=read_only,
+            ignore_all_files_in_gitignore=ignore_all_files_in_gitignore,
+        )
+
     @classmethod
-    def from_yml(cls, yml_path: Path) -> Self:
+    def _from_yml(cls, yml_path: Path) -> Self:
+        """
+        Create a ProjectConfig instance from a YAML file
+        """
         log.info(f"Loading project configuration from {yml_path}")
+        if not yml_path.exists():
+            raise FileNotFoundError(f"Project file not found: {yml_path}")
         try:
             with open(yml_path, encoding="utf-8") as f:
                 config_dict = yaml.safe_load(f)
@@ -111,26 +144,50 @@ class ProjectConfig(ToStringMixin):
             else:
                 project_root = None
                 project_name = yml_path.stem
-            return cls(config_dict, project_name=project_name, project_root=project_root)
+            return cls.from_config_dict(config_dict, project_name=project_name, project_root=project_root)
         except Exception as e:
             raise ValueError(f"Error loading project configuration from {yml_path}: {e}") from e
 
+    @classmethod
+    def from_path(cls, path: Path | str) -> Self:
+        """
+        Create a ProjectConfig instance from a path. Can either be a path to a yaml file or a directory
+        containing `.serena/project.yml`.
+        """
+        path = Path(path)
+        if not path.exists():
+            raise FileNotFoundError(f"Path not found: {path}")
+        if path.is_dir():
+            path = path / cls.SERENA_MANAGED_DIR / cls.SERENA_DEFAULT_PROJECT_FILE
+        return cls._from_yml(path)
+
     def get_serena_managed_dir(self) -> str:
         return os.path.join(self.project_root, self.SERENA_MANAGED_DIR)
 
 
-@singleton
+@dataclass
 class SerenaConfig:
     """
     Handles user-defined Serena configuration based on the configuration file
     """
 
+    projects: dict[str, ProjectConfig] = field(default_factory=dict)
+    project_names: list[str] = field(default_factory=list)
+    gui_log_window_enabled: bool = False
+    gui_log_window_level: int = logging.INFO
+    enable_project_activation: bool = True
+
     CONFIG_FILE = "serena_config.yml"
 
-    def __init__(self) -> None:
-        config_file = os.path.join(serena_root_path(), self.CONFIG_FILE)
+    @classmethod
+    def from_config_file(cls) -> "SerenaConfig":
+        """
+        Static constructor to create SerenaConfig from the configuration file
+        """
+        config_file = os.path.join(serena_root_path(), cls.CONFIG_FILE)
         if not os.path.exists(config_file):
             raise FileNotFoundError(f"Serena configuration file not found: {config_file}")
+
         with open(config_file, encoding="utf-8") as f:
             try:
                 log.info(f"Loading Serena configuration from {config_file}")
@@ -138,10 +195,13 @@ class SerenaConfig:
             except Exception as e:
                 raise ValueError(f"Error loading Serena configuration from {config_file}: {e}") from e
 
+        # Create instance
+        instance = cls()
+
         # read projects
-        self.projects: dict[str, ProjectConfig] = {}
         if "projects" not in config_yaml:
             raise SerenaConfigError("`projects` key not found in Serena configuration. Please update your `serena_config.yml` file.")
+        projects = {}
         for project_config_path in config_yaml["projects"]:
             project_config_path = Path(project_config_path)
             if not project_config_path.is_absolute():
@@ -151,15 +211,18 @@ class SerenaConfig:
             if not project_config_path.is_file():
                 raise FileNotFoundError(f"Project file not found: {project_config_path}")
             log.info(f"Loading project configuration from {project_config_path}")
-            project_config = ProjectConfig.from_yml(project_config_path)
-            self.projects[project_config.project_name] = project_config
-        self.project_names = list(self.projects.keys())
+            project_config = ProjectConfig.from_path(project_config_path)
+            projects[project_config.project_name] = project_config
 
-        self.gui_log_window_enabled = config_yaml.get("gui_log_window", False)
-        self.gui_log_window_level = config_yaml.get("gui_log_level", logging.INFO)
-        self.enable_project_activation = config_yaml.get("enable_project_activation", True)
+        instance.projects = projects
+        instance.project_names = list(projects.keys())
+        instance.gui_log_window_enabled = config_yaml.get("gui_log_window", False)
+        instance.gui_log_window_level = config_yaml.get("gui_log_level", logging.INFO)
+        instance.enable_project_activation = config_yaml.get("enable_project_activation", True)
 
-    def get_project_configuration(self, project_name: str) -> ProjectConfig:
+        return instance
+
+    def get_project_configuration(self, project_name: str) -> "ProjectConfig":
         if project_name not in self.projects:
             raise ValueError(f"Project '{project_name}' not found in Serena configuration; valid project names: {self.project_names}")
         return self.projects[project_name]
@@ -182,15 +245,22 @@ class LinesRead:
 
 
 class SerenaAgent:
-    def __init__(self, project_file_path: str | None = None, project_activation_callback: Callable[[], None] | None = None):
+    def __init__(
+        self,
+        project_config: ProjectConfig | str | Path | None = None,
+        project_activation_callback: Callable[[], None] | None = None,
+        serena_config: SerenaConfig | None = None,
+    ) -> None:
         """
-        :param project_file_path: the configuration file (.yml) of the project to load immediately;
+        :param project_config: the project to load immediately; may be a path to the project configuration file
+            or a configuration instance;
             if None, do not load any project (must use project selection tool to activate a project).
             If a project is provided, the corresponding language server will be started.
         :param project_activation_callback: a callback function to be called when a project is activated.
+        :param serena_config: the Serena configuration or None to read the configuration from the default location.
         """
         # obtain serena configuration
-        self.serena_config = SerenaConfig()
+        self.serena_config = serena_config or SerenaConfig.from_config_file()
 
         # open GUI log window if enabled
         self._gui_log_handler: Union["GuiLogViewerHandler", None] = None  # noqa
@@ -240,20 +310,20 @@ class SerenaAgent:
             self._gui_log_handler.log_viewer.set_tool_names(tool_names)
 
         # activate a project configuration (if provided or if there is only a single project available)
-        project_config: ProjectConfig | None = None
-        if project_file_path is not None:
-            if not os.path.exists(project_file_path):
-                raise FileNotFoundError(f"Project file not found: {project_file_path}")
-            log.info(f"Loading project configuration from {project_file_path}")
-            project_config = ProjectConfig.from_yml(Path(project_file_path))
+        project_config_to_load: ProjectConfig | None = None
+        if project_config is not None:
+            if isinstance(project_config, (str, Path)):
+                project_config_to_load = ProjectConfig.from_path(project_config)
+            else:
+                project_config_to_load = project_config
         else:
             match len(self.serena_config.projects):
                 case 0:
                     raise RuntimeError(f"No projects found in {SerenaConfig.CONFIG_FILE} and no project file specified.")
                 case 1:
-                    project_config = self.serena_config.get_project_configuration(self.serena_config.project_names[0])
-        if project_config is not None:
-            self.activate_project(project_config)
+                    project_config_to_load = self.serena_config.get_project_configuration(self.serena_config.project_names[0])
+        if project_config_to_load is not None:
+            self.activate_project(project_config_to_load)
         else:
             if not self.serena_config.enable_project_activation:
                 raise ValueError("Tool-based project activation is disabled in the configuration but no project file was provided.")

test/resources/repos/java/test_repo/src/main/java/test_repo/ModelUser.java

@@ -0,0 +1,8 @@
+package test_repo;
+
+public class ModelUser {
+    public static void main(String[] args) {
+        Model model = new Model("Cascade");
+        System.out.println(model.getName());
+    }
+}

test/resources/repos/typescript/test_repo/tsconfig.json

@@ -0,0 +1,11 @@
+{
+  "compilerOptions": {
+    "target": "ES2017",
+    "module": "commonjs",
+    "strict": true,
+    "esModuleInterop": true,
+    "forceConsistentCasingInFileNames": true,
+    "skipLibCheck": true
+  },
+  "include": ["**/*.ts"]
+}

test/resources/repos/typescript/test_repo/use_helper.ts

@@ -0,0 +1,8 @@
+import {helperFunction} from "./index";
+
+
+export function useHelper() {
+    helperFunction();
+}
+
+useHelper();

test/serena/test_serena_agent.py

@@ -0,0 +1,75 @@
+import json
+import os
+import time
+
+import pytest
+
+from multilspy.multilspy_config import Language
+from serena.agent import FindReferencingSymbolsTool, FindSymbolTool, ProjectConfig, SerenaAgent, SerenaConfig
+from test.conftest import LanguageParamRequest, get_repo_path
+
+
+@pytest.fixture
+def serena_config():
+    return SerenaConfig()
+
+
+@pytest.fixture
+def serena_agent(request: LanguageParamRequest, serena_config) -> SerenaAgent:
+    language = Language(request.param)
+    repo = get_repo_path(language)
+    project_config = ProjectConfig(project_name=os.path.basename(str(repo)), language=language, project_root=str(repo))
+    return SerenaAgent(project_config=project_config, serena_config=serena_config)
+
+
+class TestSerenaAgent:
+    @pytest.mark.parametrize(
+        "serena_agent,symbol_name,expected_kind,expected_file",
+        [
+            (Language.PYTHON, "User", "Class", "models.py"),
+            (Language.GO, "Helper", "Function", "main.go"),
+            (Language.JAVA, "Model", "Class", "Model.java"),
+            (Language.RUST, "add", "Function", "lib.rs"),
+            (Language.TYPESCRIPT, "DemoClass", "Class", "index.ts"),
+        ],
+        indirect=["serena_agent"],
+    )
+    def test_find_symbol(self, serena_agent, symbol_name, expected_kind, expected_file):
+        agent = serena_agent
+        find_symbol_tool = agent.get_tool(FindSymbolTool)
+        result = find_symbol_tool.apply(symbol_name)
+        symbols = json.loads(result)
+        assert any(
+            symbol_name in s["name"] and expected_kind.lower() in s["kind"].lower() and expected_file in s["location"]["relative_path"]
+            for s in symbols
+        ), f"Expected to find {symbol_name} ({expected_kind}) in {expected_file} for {agent.project_config.language.name}"
+
+    @pytest.mark.parametrize(
+        "serena_agent,symbol_name,def_file,ref_file",
+        [
+            (Language.PYTHON, "User", "test_repo/models.py", "test_repo/services.py"),
+            (Language.GO, "Helper", "main.go", "main.go"),
+            (Language.JAVA, "Model", "src/main/java/test_repo/Model.java", "src/main/java/test_repo/Main.java"),
+            (Language.RUST, "add", "src/lib.rs", "src/main.rs"),
+            (Language.TYPESCRIPT, "helperFunction", "index.ts", "use_helper.ts"),
+        ],
+        indirect=["serena_agent"],
+    )
+    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
+        find_symbol_tool = agent.get_tool(FindSymbolTool)
+        result = find_symbol_tool.apply(symbol_name, within_relative_path=def_file)
+        time.sleep(1)
+        symbols = json.loads(result)
+        # Find the definition location
+        def_symbol = symbols[0]
+        loc = def_symbol["location"]
+        # sel_start = def_symbol["location"]["selectionRange"]["start"]
+        # Now find references
+        find_refs_tool = agent.get_tool(FindReferencingSymbolsTool)
+        result = find_refs_tool.apply(def_file, loc["line"], loc["column"])
+        refs = json.loads(result)
+        assert any(
+            ref["location"]["relative_path"] == ref_file for ref in refs
+        ), f"Expected to find reference to {symbol_name} in {ref_file} for {agent.project_config.language.name}. refs={refs}"