mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-14 00:16:57 +00:00
Refactoring
* Remove SymbolManager instance from agent; instead, create it ad hoc * Rename SymbolManager -> LanguageServerSymbolRetriever * Rename Symbol -> LanguageServerSymbol * Rename AbstractSymbol -> Symbol * Rename SymbolLocation -> LanguageServerSymbolLocation * Rename ReferenceInSymbol -> ReferenceInLanguageServerSymbol
This commit is contained in:
1 parent
17c0b2a949
commit
65b20c4276
7 files changed
+69
-64
No files matched your search
@@ -29,7 +29,6 @@ from serena.constants import (
|
||||
from serena.dashboard import MemoryLogHandler, SerenaDashboardAPI
|
||||
from serena.project import Project
|
||||
from serena.prompt_factory import SerenaPromptFactory
|
||||
from serena.symbol import SymbolManager
|
||||
from serena.tools import Tool, ToolRegistry
|
||||
from solidlsp import SolidLanguageServer
|
||||
|
||||
@@ -220,7 +219,6 @@ class SerenaAgent:
|
||||
self._active_project: Project | None = None
|
||||
self._active_project_root: str | None = None
|
||||
self.language_server: SolidLanguageServer | None = None
|
||||
self.symbol_manager: SymbolManager | None = None
|
||||
self.memories_manager: MemoriesManager | None = None
|
||||
self.lines_read: LinesRead | None = None
|
||||
|
||||
@@ -372,19 +370,12 @@ class SerenaAgent:
|
||||
self.memories_manager = MemoriesManager(project.project_root)
|
||||
self.lines_read = LinesRead()
|
||||
|
||||
# reset project-specific instances that depend on the language server
|
||||
self.symbol_manager = None
|
||||
|
||||
def init_language_server() -> None:
|
||||
# start the language server
|
||||
with LogTime("Language server initialization", logger=log):
|
||||
self.reset_language_server()
|
||||
assert self.language_server is not None
|
||||
|
||||
# initialize project-specific instances which depend on the language server
|
||||
log.debug(f"Initializing symbol and memories manager for {project.project_name} at {project.project_root}")
|
||||
self.symbol_manager = SymbolManager(self.language_server, self)
|
||||
|
||||
# initialize the language server in the background
|
||||
self.issue_task(init_language_server)
|
||||
|
||||
@@ -523,11 +514,6 @@ class SerenaAgent:
|
||||
raise RuntimeError(
|
||||
f"Failed to start the language server for {self._active_project.project_name} at {self._active_project.project_root}"
|
||||
)
|
||||
if self.symbol_manager is not None:
|
||||
log.debug("Setting the language server in the agent's symbol manager")
|
||||
self.symbol_manager.set_language_server(self.language_server)
|
||||
else:
|
||||
log.debug("No symbol manager available yet, skipping setting the language server")
|
||||
|
||||
def get_tool(self, tool_class: type[TTool]) -> TTool:
|
||||
return self._all_tools[tool_class] # type: ignore
|
||||
|
||||
@@ -6,7 +6,7 @@ from collections.abc import Iterable, Iterator, Reversible
|
||||
from contextlib import contextmanager
|
||||
from typing import TYPE_CHECKING, Generic, Optional, TypeVar
|
||||
|
||||
from serena.symbol import AbstractSymbol, JetBrainsSymbol, PositionInFile, Symbol, SymbolManager
|
||||
from serena.symbol import JetBrainsSymbol, LanguageServerSymbol, LanguageServerSymbolRetriever, PositionInFile, Symbol
|
||||
from solidlsp import SolidLanguageServer
|
||||
from solidlsp.ls import LSPFileBuffer
|
||||
from solidlsp.ls_utils import TextUtils
|
||||
@@ -19,7 +19,7 @@ if TYPE_CHECKING:
|
||||
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
TSymbol = TypeVar("TSymbol", bound=AbstractSymbol)
|
||||
TSymbol = TypeVar("TSymbol", bound=Symbol)
|
||||
|
||||
|
||||
class CodeEditor(Generic[TSymbol], ABC):
|
||||
@@ -211,8 +211,8 @@ class CodeEditor(Generic[TSymbol], ABC):
|
||||
edited_file.delete_text_between_positions(start_pos, end_pos)
|
||||
|
||||
|
||||
class LanguageServerCodeEditor(CodeEditor[Symbol]):
|
||||
def __init__(self, symbol_manager: SymbolManager, agent: Optional["SerenaAgent"] = None):
|
||||
class LanguageServerCodeEditor(CodeEditor[LanguageServerSymbol]):
|
||||
def __init__(self, symbol_manager: LanguageServerSymbolRetriever, agent: Optional["SerenaAgent"] = None):
|
||||
super().__init__(project_root=symbol_manager.get_language_server().repository_root_path, agent=agent)
|
||||
self._symbol_manager = symbol_manager
|
||||
|
||||
@@ -244,7 +244,7 @@ class LanguageServerCodeEditor(CodeEditor[Symbol]):
|
||||
"""Get the content of a file using the language server."""
|
||||
return self._lang_server.language_server.retrieve_full_file_content(relative_path)
|
||||
|
||||
def _find_unique_symbol(self, name_path: str, relative_file_path: str) -> Symbol:
|
||||
def _find_unique_symbol(self, name_path: str, relative_file_path: str) -> LanguageServerSymbol:
|
||||
symbol_candidates = self._symbol_manager.find_by_name(name_path, within_relative_path=relative_file_path)
|
||||
if len(symbol_candidates) == 0:
|
||||
raise ValueError(f"No symbol with name {name_path} found in file {relative_file_path}")
|
||||
|
||||
+46
-30
@@ -167,7 +167,7 @@ class CodeDiff:
|
||||
|
||||
|
||||
@dataclass
|
||||
class SymbolLocation:
|
||||
class LanguageServerSymbolLocation:
|
||||
"""
|
||||
Represents the (start) location of a symbol identifier, which, within Serena, uniquely identifies the symbol.
|
||||
"""
|
||||
@@ -203,6 +203,10 @@ class SymbolLocation:
|
||||
|
||||
@dataclass
|
||||
class PositionInFile:
|
||||
"""
|
||||
Represents a character position within a file
|
||||
"""
|
||||
|
||||
line: int
|
||||
"""
|
||||
the 0-based line number in the file
|
||||
@@ -219,7 +223,7 @@ class PositionInFile:
|
||||
return Position(line=self.line, character=self.col)
|
||||
|
||||
|
||||
class AbstractSymbol(ABC):
|
||||
class Symbol(ABC):
|
||||
@abstractmethod
|
||||
def get_body_start_position(self) -> PositionInFile | None:
|
||||
pass
|
||||
@@ -254,7 +258,7 @@ class AbstractSymbol(ABC):
|
||||
"""
|
||||
|
||||
|
||||
class Symbol(AbstractSymbol, ToStringMixin):
|
||||
class LanguageServerSymbol(Symbol, ToStringMixin):
|
||||
_NAME_PATH_SEP = "/"
|
||||
|
||||
@staticmethod
|
||||
@@ -269,7 +273,7 @@ class Symbol(AbstractSymbol, ToStringMixin):
|
||||
"""
|
||||
assert name_path, "name_path must not be empty"
|
||||
assert symbol_name_path_parts, "symbol_name_path_parts must not be empty"
|
||||
name_path_sep = Symbol._NAME_PATH_SEP
|
||||
name_path_sep = LanguageServerSymbol._NAME_PATH_SEP
|
||||
|
||||
is_absolute_pattern = name_path.startswith(name_path_sep)
|
||||
pattern_parts = name_path.lstrip(name_path_sep).rstrip(name_path_sep).split(name_path_sep)
|
||||
@@ -325,11 +329,11 @@ class Symbol(AbstractSymbol, ToStringMixin):
|
||||
return None
|
||||
|
||||
@property
|
||||
def location(self) -> SymbolLocation:
|
||||
def location(self) -> LanguageServerSymbolLocation:
|
||||
"""
|
||||
:return: the start location of the actual symbol identifier
|
||||
"""
|
||||
return SymbolLocation(relative_path=self.relative_path, line=self.line, column=self.column)
|
||||
return LanguageServerSymbolLocation(relative_path=self.relative_path, line=self.line, column=self.column)
|
||||
|
||||
@property
|
||||
def body_start_position(self) -> Position | None:
|
||||
@@ -471,18 +475,18 @@ class Symbol(AbstractSymbol, ToStringMixin):
|
||||
"""
|
||||
result = []
|
||||
|
||||
def should_include(s: "Symbol") -> bool:
|
||||
def should_include(s: "LanguageServerSymbol") -> bool:
|
||||
if include_kinds is not None and s.symbol_kind not in include_kinds:
|
||||
return False
|
||||
if exclude_kinds is not None and s.symbol_kind in exclude_kinds:
|
||||
return False
|
||||
return Symbol.match_name_path(
|
||||
return LanguageServerSymbol.match_name_path(
|
||||
name_path=name_path,
|
||||
symbol_name_path_parts=s.get_name_path_parts(),
|
||||
substring_matching=substring_matching,
|
||||
)
|
||||
|
||||
def traverse(s: "Symbol") -> None:
|
||||
def traverse(s: "LanguageServerSymbol") -> None:
|
||||
if should_include(s):
|
||||
result.append(s)
|
||||
for c in s.iter_children():
|
||||
@@ -553,24 +557,36 @@ class Symbol(AbstractSymbol, ToStringMixin):
|
||||
|
||||
|
||||
@dataclass
|
||||
class ReferenceInSymbol(ToStringMixin):
|
||||
"""Same as the class of the same name in the language server, but using Serena's Symbol class.
|
||||
Be careful to not confuse it with counterpart!
|
||||
class ReferenceInLanguageServerSymbol(ToStringMixin):
|
||||
"""
|
||||
Represents the location of a reference to another symbol within a symbol/file.
|
||||
|
||||
The contained symbol is the symbol within which the reference is located,
|
||||
not the symbol that is referenced.
|
||||
"""
|
||||
|
||||
symbol: Symbol
|
||||
symbol: LanguageServerSymbol
|
||||
"""
|
||||
the symbol within which the reference is located
|
||||
"""
|
||||
line: int
|
||||
"""
|
||||
the line number in which the reference is located (0-based)
|
||||
"""
|
||||
character: int
|
||||
"""
|
||||
the column number in which the reference is located (0-based)
|
||||
"""
|
||||
|
||||
@classmethod
|
||||
def from_lsp_reference(cls, reference: LSPReferenceInSymbol) -> Self:
|
||||
return cls(symbol=LanguageServerSymbol(reference.symbol), line=reference.line, character=reference.character)
|
||||
|
||||
def get_relative_path(self) -> str | None:
|
||||
return self.symbol.location.relative_path
|
||||
|
||||
@classmethod
|
||||
def from_lsp_reference(cls, reference: LSPReferenceInSymbol) -> Self:
|
||||
return cls(symbol=Symbol(reference.symbol), line=reference.line, character=reference.character)
|
||||
|
||||
|
||||
class SymbolManager:
|
||||
class LanguageServerSymbolRetriever:
|
||||
def __init__(self, lang_server: SolidLanguageServer, agent: Union["SerenaAgent", None] = None) -> None:
|
||||
"""
|
||||
:param lang_server: the language server to use for symbol retrieval as well as editing operations.
|
||||
@@ -598,33 +614,33 @@ class SymbolManager:
|
||||
exclude_kinds: Sequence[SymbolKind] | None = None,
|
||||
substring_matching: bool = False,
|
||||
within_relative_path: str | None = None,
|
||||
) -> list[Symbol]:
|
||||
) -> list[LanguageServerSymbol]:
|
||||
"""
|
||||
Find all symbols that match the given name. See docstring of `Symbol.find` for more details.
|
||||
The only parameter not mentioned there is `within_relative_path`, which can be used to restrict the search
|
||||
to symbols within a specific file or directory.
|
||||
"""
|
||||
symbols: list[Symbol] = []
|
||||
symbols: list[LanguageServerSymbol] = []
|
||||
symbol_roots = self._lang_server.request_full_symbol_tree(within_relative_path=within_relative_path, include_body=include_body)
|
||||
for root in symbol_roots:
|
||||
symbols.extend(
|
||||
Symbol(root).find(
|
||||
LanguageServerSymbol(root).find(
|
||||
name_path, include_kinds=include_kinds, exclude_kinds=exclude_kinds, substring_matching=substring_matching
|
||||
)
|
||||
)
|
||||
return symbols
|
||||
|
||||
def get_document_symbols(self, relative_path: str) -> list[Symbol]:
|
||||
def get_document_symbols(self, relative_path: str) -> list[LanguageServerSymbol]:
|
||||
symbol_dicts, roots = self._lang_server.request_document_symbols(relative_path, include_body=False)
|
||||
symbols = [Symbol(s) for s in symbol_dicts]
|
||||
symbols = [LanguageServerSymbol(s) for s in symbol_dicts]
|
||||
return symbols
|
||||
|
||||
def find_by_location(self, location: SymbolLocation) -> Symbol | None:
|
||||
def find_by_location(self, location: LanguageServerSymbolLocation) -> LanguageServerSymbol | None:
|
||||
if location.relative_path is None:
|
||||
return None
|
||||
symbol_dicts, roots = self._lang_server.request_document_symbols(location.relative_path, include_body=False)
|
||||
for symbol_dict in symbol_dicts:
|
||||
symbol = Symbol(symbol_dict)
|
||||
symbol = LanguageServerSymbol(symbol_dict)
|
||||
if symbol.location == location:
|
||||
return symbol
|
||||
return None
|
||||
@@ -636,7 +652,7 @@ class SymbolManager:
|
||||
include_body: bool = False,
|
||||
include_kinds: Sequence[SymbolKind] | None = None,
|
||||
exclude_kinds: Sequence[SymbolKind] | None = None,
|
||||
) -> list[ReferenceInSymbol]:
|
||||
) -> list[ReferenceInLanguageServerSymbol]:
|
||||
"""
|
||||
Find all symbols that reference the symbol with the given name.
|
||||
If multiple symbols fit the name (e.g. for variables that are overwritten), will use the first one.
|
||||
@@ -666,11 +682,11 @@ class SymbolManager:
|
||||
|
||||
def find_referencing_symbols_by_location(
|
||||
self,
|
||||
symbol_location: SymbolLocation,
|
||||
symbol_location: LanguageServerSymbolLocation,
|
||||
include_body: bool = False,
|
||||
include_kinds: Sequence[SymbolKind] | None = None,
|
||||
exclude_kinds: Sequence[SymbolKind] | None = None,
|
||||
) -> list[ReferenceInSymbol]:
|
||||
) -> list[ReferenceInLanguageServerSymbol]:
|
||||
"""
|
||||
Find all symbols that reference the symbol at the given location.
|
||||
|
||||
@@ -707,7 +723,7 @@ class SymbolManager:
|
||||
if exclude_kinds is not None:
|
||||
references = [s for s in references if s.symbol["kind"] not in exclude_kinds]
|
||||
|
||||
return [ReferenceInSymbol.from_lsp_reference(r) for r in references]
|
||||
return [ReferenceInLanguageServerSymbol.from_lsp_reference(r) for r in references]
|
||||
|
||||
@dataclass
|
||||
class SymbolOverviewElement:
|
||||
@@ -724,7 +740,7 @@ class SymbolManager:
|
||||
return result
|
||||
|
||||
|
||||
class JetBrainsSymbol(AbstractSymbol):
|
||||
class JetBrainsSymbol(Symbol):
|
||||
def __init__(self, symbol_dict: dict, project: Project) -> None:
|
||||
"""
|
||||
:param symbol_dict: dictionary as returned by the JetBrains plugin client.
|
||||
|
||||
@@ -61,7 +61,8 @@ class GetSymbolsOverviewTool(Tool):
|
||||
(e.g. a subdirectory).
|
||||
:return: a JSON object mapping relative paths of all contained files to info about top-level symbols in the file (name_path, kind).
|
||||
"""
|
||||
result = self.symbol_manager.get_symbol_overview(relative_path)
|
||||
symbol_retriever = self.create_language_server_symbol_retriever()
|
||||
result = symbol_retriever.get_symbol_overview(relative_path)
|
||||
result_json_str = json.dumps({k: [dataclasses.asdict(i) for i in l] for k, l in result.items()})
|
||||
return self._limit_length(result_json_str, max_answer_chars)
|
||||
|
||||
@@ -132,7 +133,8 @@ class FindSymbolTool(Tool):
|
||||
"""
|
||||
parsed_include_kinds: Sequence[SymbolKind] | None = [SymbolKind(k) for k in include_kinds] if include_kinds else None
|
||||
parsed_exclude_kinds: Sequence[SymbolKind] | None = [SymbolKind(k) for k in exclude_kinds] if exclude_kinds else None
|
||||
symbols = self.symbol_manager.find_by_name(
|
||||
symbol_retriever = self.create_language_server_symbol_retriever()
|
||||
symbols = symbol_retriever.find_by_name(
|
||||
name_path,
|
||||
include_body=include_body,
|
||||
include_kinds=parsed_include_kinds,
|
||||
@@ -175,7 +177,8 @@ class FindReferencingSymbolsTool(Tool):
|
||||
include_body = False # It is probably never a good idea to include the body of the referencing symbols
|
||||
parsed_include_kinds: Sequence[SymbolKind] | None = [SymbolKind(k) for k in include_kinds] if include_kinds else None
|
||||
parsed_exclude_kinds: Sequence[SymbolKind] | None = [SymbolKind(k) for k in exclude_kinds] if exclude_kinds else None
|
||||
references_in_symbols = self.symbol_manager.find_referencing_symbols(
|
||||
symbol_retriever = self.create_language_server_symbol_retriever()
|
||||
references_in_symbols = symbol_retriever.find_referencing_symbols(
|
||||
name_path,
|
||||
relative_file_path=relative_path,
|
||||
include_body=include_body,
|
||||
|
||||
@@ -13,7 +13,7 @@ from sensai.util.string import dict_string
|
||||
|
||||
from serena.project import Project
|
||||
from serena.prompt_factory import PromptFactory
|
||||
from serena.symbol import SymbolManager
|
||||
from serena.symbol import LanguageServerSymbolRetriever
|
||||
from serena.util.class_decorators import singleton
|
||||
from serena.util.inspection import iter_subclasses
|
||||
|
||||
@@ -45,10 +45,10 @@ class Component(ABC):
|
||||
assert self.agent.memories_manager is not None
|
||||
return self.agent.memories_manager
|
||||
|
||||
@property
|
||||
def symbol_manager(self) -> SymbolManager:
|
||||
assert self.agent.symbol_manager is not None
|
||||
return self.agent.symbol_manager
|
||||
def create_language_server_symbol_retriever(self) -> LanguageServerSymbolRetriever:
|
||||
language_server = self.agent.language_server
|
||||
assert language_server is not None
|
||||
return LanguageServerSymbolRetriever(language_server, agent=self.agent)
|
||||
|
||||
@property
|
||||
def project(self) -> Project:
|
||||
@@ -60,7 +60,7 @@ class Component(ABC):
|
||||
if self.agent.serena_config.jetbrains:
|
||||
return JetBrainsCodeEditor(project=self.project, agent=self.agent)
|
||||
else:
|
||||
return LanguageServerCodeEditor(self.symbol_manager, agent=self.agent)
|
||||
return LanguageServerCodeEditor(self.create_language_server_symbol_retriever(), agent=self.agent)
|
||||
|
||||
@property
|
||||
def lines_read(self) -> "LinesRead":
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import pytest
|
||||
|
||||
from src.serena.symbol import Symbol
|
||||
from src.serena.symbol import LanguageServerSymbol
|
||||
|
||||
|
||||
class TestSymbolNameMatching:
|
||||
@@ -77,7 +77,7 @@ class TestSymbolNameMatching:
|
||||
)
|
||||
def test_match_simple_name(self, name_path_pattern, symbol_name_path_parts, is_substring_match, expected):
|
||||
"""Tests matching for simple names (no '/' in pattern)."""
|
||||
result = Symbol.match_name_path(name_path_pattern, symbol_name_path_parts, is_substring_match)
|
||||
result = LanguageServerSymbol.match_name_path(name_path_pattern, symbol_name_path_parts, is_substring_match)
|
||||
error_msg = self._create_assertion_error_message(name_path_pattern, symbol_name_path_parts, is_substring_match, expected, result)
|
||||
assert result == expected, error_msg
|
||||
|
||||
@@ -157,6 +157,6 @@ class TestSymbolNameMatching:
|
||||
)
|
||||
def test_match_name_path_pattern_path_len_2(self, name_path_pattern, symbol_name_path_parts, is_substring_match, expected):
|
||||
"""Tests matching for qualified names (e.g. 'module/class/func')."""
|
||||
result = Symbol.match_name_path(name_path_pattern, symbol_name_path_parts, is_substring_match)
|
||||
result = LanguageServerSymbol.match_name_path(name_path_pattern, symbol_name_path_parts, is_substring_match)
|
||||
error_msg = self._create_assertion_error_message(name_path_pattern, symbol_name_path_parts, is_substring_match, expected, result)
|
||||
assert result == expected, error_msg
|
||||
@@ -14,7 +14,7 @@ import pytest
|
||||
from serena.code_editor import CodeEditor, LanguageServerCodeEditor
|
||||
from serena.symbol import CodeDiff
|
||||
from solidlsp.ls_config import Language
|
||||
from src.serena.symbol import SymbolManager
|
||||
from src.serena.symbol import LanguageServerSymbolRetriever
|
||||
from test.conftest import create_ls, get_repo_path
|
||||
|
||||
pytestmark = pytest.mark.snapshot
|
||||
@@ -34,7 +34,7 @@ class EditingTest:
|
||||
self.repo_path: Path | None = None
|
||||
|
||||
@contextmanager
|
||||
def _setup(self) -> Iterator[SymbolManager]:
|
||||
def _setup(self) -> Iterator[LanguageServerSymbolRetriever]:
|
||||
"""Context manager for setup/teardown with a temporary directory, providing the symbol manager."""
|
||||
temp_dir = Path(tempfile.mkdtemp())
|
||||
self.repo_path = temp_dir / self.original_repo_path.name
|
||||
@@ -51,7 +51,7 @@ class EditingTest:
|
||||
log.info(f"Starting language server for {self.language} {self.rel_path}")
|
||||
language_server.start()
|
||||
log.info(f"Language server started for {self.language} {self.rel_path}")
|
||||
yield SymbolManager(lang_server=language_server)
|
||||
yield LanguageServerSymbolRetriever(lang_server=language_server)
|
||||
finally:
|
||||
if language_server is not None and language_server.is_running():
|
||||
log.info(f"Stopping language server for {self.language} {self.rel_path}")
|
||||
|
||||
Reference in new issue
Block a user