From bb79bf98d2c82631fa559e2d9b59562a41db228b Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Mon, 8 Dec 2025 14:58:41 +0100 Subject: [PATCH 1/3] Refactor name path-based symbol retrieval methods * Improve parameter and method names * Consolidate documentation * Add unique symbol retrieval method to facilitate reuse Fix FindReferencingSymbolsTool glossing over non-unique symbol specification (must raise an error if the name path is not unique) --- src/serena/code_editor.py | 10 +-- src/serena/symbol.py | 110 +++++++++++++++++-------------- src/serena/tools/symbol_tools.py | 2 +- 3 files changed, 62 insertions(+), 60 deletions(-) diff --git a/src/serena/code_editor.py b/src/serena/code_editor.py index 6e9f65b1..7220ef90 100644 --- a/src/serena/code_editor.py +++ b/src/serena/code_editor.py @@ -267,15 +267,7 @@ class LanguageServerCodeEditor(CodeEditor[LanguageServerSymbol]): return lang_server.language_server.retrieve_full_file_content(relative_path) def _find_unique_symbol(self, name_path: str, relative_file_path: str) -> LanguageServerSymbol: - symbol_candidates = self._symbol_retriever.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}") - if len(symbol_candidates) > 1: - raise ValueError( - f"Found multiple {len(symbol_candidates)} symbols with name {name_path} in file {relative_file_path}. " - "Their locations are: \n " + json.dumps([s.location.to_dict() for s in symbol_candidates], indent=2) - ) - return symbol_candidates[0] + return self._symbol_retriever.find_unique(name_path, within_relative_path=relative_file_path) def _apply_workspace_edit(self, workspace_edit: ls_types.WorkspaceEdit) -> list[str]: """ diff --git a/src/serena/symbol.py b/src/serena/symbol.py index e3753041..b2301826 100644 --- a/src/serena/symbol.py +++ b/src/serena/symbol.py @@ -115,16 +115,31 @@ class Symbol(ToStringMixin, ABC): class NamePathMatcher(ToStringMixin): - def __init__(self, name_path_expr: str, substring_matching: bool) -> None: + """ + Matches name paths of symbols against search patterns. + + A name path is a path in the symbol tree *within a source file*. + For example, the method `my_method` defined in class `MyClass` would have the name path `MyClass/my_method`. + If a symbol is overloaded (e.g., in Java), a 0-based index is appended (e.g. "MyClass/my_method[0]") to + uniquely identify it. + + A matching pattern can be: + * a simple name (e.g. "method"), which will match any symbol with that name + * a relative path like "class/method", which will match any symbol with that name path suffix + * an absolute name path "/class/method" (absolute name path), which requires an exact match of the full name path within the source file. + Append an index `[i]` to match a specific overload only, e.g. "MyClass/my_method[1]". + """ + + def __init__(self, name_path_pattern: str, substring_matching: bool) -> None: """ - :param name_path_expr: the name path expression to match against + :param name_path_pattern: the name path expression to match against :param substring_matching: whether to use substring matching for the last segment """ - assert name_path_expr, "name_path must not be empty" - self._expr = name_path_expr + assert name_path_pattern, "name_path must not be empty" + self._expr = name_path_pattern self._substring_matching = substring_matching - self._is_absolute_pattern = name_path_expr.startswith(NAME_PATH_SEP) - self._pattern_parts = name_path_expr.lstrip(NAME_PATH_SEP).rstrip(NAME_PATH_SEP).split(NAME_PATH_SEP) + self._is_absolute_pattern = name_path_pattern.startswith(NAME_PATH_SEP) + self._pattern_parts = name_path_pattern.lstrip(NAME_PATH_SEP).rstrip(NAME_PATH_SEP).split(NAME_PATH_SEP) # extract overload index "[idx]" if present at end of last part self._overload_idx: int | None = None @@ -323,34 +338,15 @@ class LanguageServerSymbol(Symbol, ToStringMixin): def find( self, - name_path: str, + name_path_pattern: str, substring_matching: bool = False, include_kinds: Sequence[SymbolKind] | None = None, exclude_kinds: Sequence[SymbolKind] | None = None, ) -> list[Self]: """ - Find all symbols within the symbol's subtree that match the given `name_path`. - The matching behavior is determined by the structure of `name_path`, which can - either be a simple name (e.g. "method") or a name path like "class/method" (relative name path) - or "/class/method" (absolute name path). + Find all symbols within the symbol's subtree that match the given name path pattern. - Key aspects of the name path matching behavior: - - Trailing slashes in `name_path` play no role and are ignored. - - The name of the retrieved symbols will match (either exactly or as a substring) - the last segment of `name_path`, while other segments will restrict the search to symbols that - have a desired sequence of ancestors. - - If there is no starting or intermediate slash in `name_path`, there is no - restriction on the ancestor symbols. For example, passing `method` will match - against symbols with name paths like `method`, `class/method`, `class/nested_class/method`, etc. - - If `name_path` contains a `/` but doesn't start with a `/`, the matching is restricted to symbols - with the same ancestors as the last segment of `name_path`. For example, passing `class/method` will match against - `class/method` as well as `nested_class/class/method` but not `method`. - - If `name_path` starts with a `/`, it will be treated as an absolute name path pattern, meaning - that the first segment of it must match the first segment of the symbol's name path. - For example, passing `/class` will match only against top-level symbols like `class` but not against `nested_class/class`. - Passing `/class/method` will match against `class/method` but not `nested_class/class/method` or `method`. - - :param name_path: the name path expression to match against + :param name_path_pattern: the name path pattern to match against (see class :class:`NamePathMatcher` for details) :param substring_matching: whether to use substring matching (as opposed to exact matching) of the last segment of `name_path` against the symbol name. :param include_kinds: an optional sequence of ints representing the LSP symbol kind. @@ -358,7 +354,7 @@ class LanguageServerSymbol(Symbol, ToStringMixin): :param exclude_kinds: If provided, symbols of the given kinds will be excluded from the result. """ result = [] - name_path_matcher = NamePathMatcher(name_path, substring_matching) + name_path_matcher = NamePathMatcher(name_path_pattern, substring_matching) def should_include(s: "LanguageServerSymbol") -> bool: if include_kinds is not None and s.symbol_kind not in include_kinds: @@ -488,18 +484,17 @@ class LanguageServerSymbolRetriever: def get_language_server(self, relative_path: str) -> SolidLanguageServer: return self._ls_manager.get_language_server(relative_path) - def find_by_name( + def find( self, - name_path: str, + name_path_pattern: str, include_kinds: Sequence[SymbolKind] | None = None, exclude_kinds: Sequence[SymbolKind] | None = None, substring_matching: bool = False, within_relative_path: str | None = None, ) -> 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. + Finds all symbols that match the given name path pattern (see class :class:`NamePathMatcher` for details), + optionally limited to a specific file and filtered by kind. """ symbols: list[LanguageServerSymbol] = [] for lang_server in self._ls_manager.iter_language_servers(): @@ -507,11 +502,37 @@ class LanguageServerSymbolRetriever: for root in symbol_roots: symbols.extend( LanguageServerSymbol(root).find( - name_path, include_kinds=include_kinds, exclude_kinds=exclude_kinds, substring_matching=substring_matching + name_path_pattern, include_kinds=include_kinds, exclude_kinds=exclude_kinds, substring_matching=substring_matching ) ) return symbols + def find_unique( + self, + name_path_pattern: str, + include_kinds: Sequence[SymbolKind] | None = None, + exclude_kinds: Sequence[SymbolKind] | None = None, + substring_matching: bool = False, + within_relative_path: str | None = None, + ) -> LanguageServerSymbol: + symbol_candidates = self.find( + name_path_pattern, + include_kinds=include_kinds, + exclude_kinds=exclude_kinds, + substring_matching=substring_matching, + within_relative_path=within_relative_path, + ) + if len(symbol_candidates) == 0: + raise ValueError(f"No symbol matching '{name_path_pattern}' found") + if len(symbol_candidates) > 1: + include_rel_path = within_relative_path is not None + raise ValueError( + f"Found multiple {len(symbol_candidates)} symbols matching '{name_path_pattern}'. " + "They are: \n" + + json.dumps([s.to_dict(kind=True, include_relative_path=include_rel_path) for s in symbol_candidates], indent=2) + ) + return symbol_candidates[0] + def find_by_location(self, location: LanguageServerSymbolLocation) -> LanguageServerSymbol | None: if location.relative_path is None: return None @@ -532,28 +553,17 @@ class LanguageServerSymbolRetriever: exclude_kinds: Sequence[SymbolKind] | None = None, ) -> 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. + Find all symbols that reference the specified symbol, which is assumed to be unique. - :param name_path: the name path of the symbol to find + :param name_path: the name path of the symbol to find. (While this can be a matching pattern, it should + usually be the full path to ensure uniqueness.) :param relative_file_path: the relative path of the file in which the referenced symbol is defined. :param include_body: whether to include the body of all symbols in the result. Not recommended, as the referencing symbols will often be files, and thus the bodies will be very long. :param include_kinds: which kinds of symbols to include in the result. :param exclude_kinds: which kinds of symbols to exclude from the result. """ - symbol_candidates = self.find_by_name(name_path, substring_matching=False, within_relative_path=relative_file_path) - if len(symbol_candidates) == 0: - log.warning(f"No symbol with name {name_path} found in file {relative_file_path}") - return [] - if len(symbol_candidates) > 1: - log.error( - f"Found {len(symbol_candidates)} symbols with name {name_path} in file {relative_file_path}." - f"May be an overwritten variable, in which case you can ignore this error. Proceeding with the first one. " - f"Found symbols for {name_path=} in {relative_file_path=}: \n" - f"{json.dumps([s.location.to_dict() for s in symbol_candidates], indent=2)}" - ) - symbol = symbol_candidates[0] + symbol = self.find_unique(name_path, substring_matching=False, within_relative_path=relative_file_path) return self.find_referencing_symbols_by_location( symbol.location, include_body=include_body, include_kinds=include_kinds, exclude_kinds=exclude_kinds ) diff --git a/src/serena/tools/symbol_tools.py b/src/serena/tools/symbol_tools.py index 9d8cffaa..72a73f48 100644 --- a/src/serena/tools/symbol_tools.py +++ b/src/serena/tools/symbol_tools.py @@ -136,7 +136,7 @@ class FindSymbolTool(Tool, ToolMarkerSymbolicRead): 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 symbol_retriever = self.create_language_server_symbol_retriever() - symbols = symbol_retriever.find_by_name( + symbols = symbol_retriever.find( name_path_pattern, include_kinds=parsed_include_kinds, exclude_kinds=parsed_exclude_kinds, From a38311984ac65c45173b3fb93aedad564471e9bd Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Mon, 8 Dec 2025 15:12:55 +0100 Subject: [PATCH 2/3] Add tests relating to overloaded symbols * Test whether retrieval returns all overloads * Test whether tools operating on well-defined symbols raise an error if the symbol is non-unique --- .../src/main/java/test_repo/Model.java | 8 +++ test/serena/test_serena_agent.py | 66 ++++++++++++++++++- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/test/resources/repos/java/test_repo/src/main/java/test_repo/Model.java b/test/resources/repos/java/test_repo/src/main/java/test_repo/Model.java index a2c6459b..5a6a044e 100644 --- a/test/resources/repos/java/test_repo/src/main/java/test_repo/Model.java +++ b/test/resources/repos/java/test_repo/src/main/java/test_repo/Model.java @@ -10,4 +10,12 @@ public class Model { public String getName() { return name; } + + public String getName(int maxChars) { + if (name.length() <= maxChars) { + return name; + } else { + return name.substring(0, maxChars) + "..."; + } + } } diff --git a/test/serena/test_serena_agent.py b/test/serena/test_serena_agent.py index f027e3f8..2b9060b7 100644 --- a/test/serena/test_serena_agent.py +++ b/test/serena/test_serena_agent.py @@ -9,7 +9,7 @@ import pytest from serena.agent import SerenaAgent from serena.config.serena_config import ProjectConfig, RegisteredProject, SerenaConfig from serena.project import Project -from serena.tools import FindReferencingSymbolsTool, FindSymbolTool +from serena.tools import FindReferencingSymbolsTool, FindSymbolTool, ReplaceSymbolBodyTool from solidlsp.ls_config import Language from test.conftest import get_repo_path, language_tests_enabled from test.solidlsp import clojure as clj @@ -64,6 +64,10 @@ def serena_agent(request: pytest.FixtureRequest, serena_config) -> Iterator[Sere project_name = f"test_repo_{language}" agent = SerenaAgent(project=project_name, serena_config=serena_config) + + # wait for agent to be ready + agent.execute_task(lambda: None) + yield agent # explicitly delete to free resources agent.shutdown(timeout=5) @@ -288,3 +292,63 @@ class TestSerenaAgent: symbols = json.loads(result) assert not symbols, f"Expected to find no symbols for {name_path}. Symbols found: {symbols}" + + @pytest.mark.parametrize( + "serena_agent,name_path,num_expected", + [ + pytest.param( + Language.JAVA, + "Model/getName", + 2, + id="overloaded_java_method", + marks=pytest.mark.java, + ), + ], + indirect=["serena_agent"], + ) + def test_find_symbol_overloaded_function(self, serena_agent: SerenaAgent, name_path: str, num_expected: int): + """ + Tests whether the FindSymbolTool can find all overloads of a function/method + (provided that the overload id remains unspecified in the name path) + """ + agent = serena_agent + + find_symbol_tool = agent.get_tool(FindSymbolTool) + result = find_symbol_tool.apply_ex( + name_path_pattern=name_path, + depth=0, + substring_matching=False, + ) + + symbols = json.loads(result) + assert ( + len(symbols) == num_expected + ), f"Expected to find {num_expected} symbols for overloaded function {name_path}. Symbols found: {symbols}" + + @pytest.mark.parametrize( + "serena_agent,name_path,relative_path", + [ + pytest.param( + Language.JAVA, + "Model/getName", + os.path.join("src", "main", "java", "test_repo", "Model.java"), + id="overloaded_java_method", + marks=pytest.mark.java, + ), + ], + indirect=["serena_agent"], + ) + def test_non_unique_symbol_reference_error(self, serena_agent: SerenaAgent, name_path: str, relative_path: str): + """ + Tests whether the tools operating on a well-defined symbol raises an error when the symbol reference is non-unique. + We exemplarily test a retrieval tool (FindReferencingSymbolsTool) and an editing tool (ReplaceSymbolBodyTool). + """ + match_text = "multiple" + + find_refs_tool = serena_agent.get_tool(FindReferencingSymbolsTool) + with pytest.raises(ValueError, match=match_text): + find_refs_tool.apply(name_path=name_path, relative_path=relative_path) + + replace_symbol_body_tool = serena_agent.get_tool(ReplaceSymbolBodyTool) + with pytest.raises(ValueError, match=match_text): + replace_symbol_body_tool.apply(name_path=name_path, relative_path=relative_path, body="") From 874732b939a61c3338945e2f55752aeb6142af4f Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Mon, 8 Dec 2025 16:09:42 +0100 Subject: [PATCH 3/3] LanguageServerSymbolRetriever.find_unique: For case of multiple matches, assume that a match where the name patch matches the search expression fully is the right one (if there is only one such match) --- src/serena/symbol.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/serena/symbol.py b/src/serena/symbol.py index b2301826..1640cafb 100644 --- a/src/serena/symbol.py +++ b/src/serena/symbol.py @@ -522,16 +522,23 @@ class LanguageServerSymbolRetriever: substring_matching=substring_matching, within_relative_path=within_relative_path, ) - if len(symbol_candidates) == 0: + if len(symbol_candidates) == 1: + return symbol_candidates[0] + elif len(symbol_candidates) == 0: raise ValueError(f"No symbol matching '{name_path_pattern}' found") - if len(symbol_candidates) > 1: + else: + # There are multiple candidates. + # If only one of the candidates has the given pattern as its exact name path, return that one + exact_matches = [s for s in symbol_candidates if s.get_name_path() == name_path_pattern] + if len(exact_matches) == 1: + return exact_matches[0] + # otherwise, raise an error include_rel_path = within_relative_path is not None raise ValueError( f"Found multiple {len(symbol_candidates)} symbols matching '{name_path_pattern}'. " "They are: \n" + json.dumps([s.to_dict(kind=True, include_relative_path=include_rel_path) for s in symbol_candidates], indent=2) ) - return symbol_candidates[0] def find_by_location(self, location: LanguageServerSymbolLocation) -> LanguageServerSymbol | None: if location.relative_path is None: