From 0cf0ff3d48bf3d97a7f7d97400cb0d88c58fcebc Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Wed, 21 May 2025 12:35:26 +0200 Subject: [PATCH] Improvements and renamings of symbol name_path matching --- src/serena/agent.py | 105 ++-- src/serena/symbol.py | 1004 +++++++++++++++--------------- test/serena/test_serena_agent.py | 65 +- test/serena/test_symbol.py | 197 +++--- 4 files changed, 718 insertions(+), 653 deletions(-) diff --git a/src/serena/agent.py b/src/serena/agent.py index a8a6fcd..7d0a692 100644 --- a/src/serena/agent.py +++ b/src/serena/agent.py @@ -10,11 +10,11 @@ import sys import traceback from abc import ABC from collections import defaultdict -from collections.abc import Callable, Generator, Iterable +from collections.abc import Callable, Generator, Iterable, Sequence from dataclasses import dataclass, field from logging import Logger from pathlib import Path -from typing import TYPE_CHECKING, Any, Self, TypeVar, Union, cast +from typing import TYPE_CHECKING, Any, Self, TypeVar, Union import yaml from sensai.util import logging @@ -778,7 +778,7 @@ class FindSymbolTool(Tool): def apply( self, - name: str, + name_path: str, depth: int = 0, within_relative_path: str | None = None, include_body: bool = False, @@ -788,63 +788,56 @@ class FindSymbolTool(Tool): max_answer_chars: int = _DEFAULT_MAX_ANSWER_LENGTH, ) -> str: """ - Retrieves information on all symbols/code entities, i.e. classes, methods, attributes, variables, etc. - with the given name. - The returned symbol location information can subsequently be used to edit the returned symbols - or to retrieve further information using other tools. - If you already anticipate that you will need to reference children of the symbol (like methods or fields contained in a class), - you can specify a depth > 0. + Retrieves information on all symbols/code entities (classes, methods, etc.) based on the given `name_path`, + which represents a pattern for the symbol's path within the symbol tree of a single file. + The returned symbol location can be used for edits or further queries. + Specify `depth > 0` to retrieve children (e.g., methods of a class). - The name matching behavior depends on whether a qualified name or a simple name is provided. - It is assumed that the provided name is a qualified name if it contains the `/` character. - If substring matching is allowed, only the last element of the qualified name will be checked against - the symbol name using substring matching. + 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). Note that the name path is not a path in the file system + but rather a path in the symbol tree **within a single file**. Thus, file or directory names should never + be included in the `name_path`. For restricting the search to a single file or directory, + the `within_relative_path` parameter should be used instead. The retrieved symbols' `name_path` attribute + will always be composed of symbol names, never file or directory names. - Examples: - - Providing "foo" will find all symbols named "foo" regardless where they are contained in the symbol tree. - - Providing "bar/foo" will only find symbols named "foo" that are direct children of a symbol called "bar". - - Providing "foo/" will only find symbols named "foo" that are top-level symbols (have no parent). - - Allowing substring matching with "bar" will find symbols with names containing "foo" anywhere in the symbol tree. - - Allowing substring matching with "foo/" will find only top-level symbols with names containing "foo". - - Allowing substring matching with "bar/foo" will find only symbols with names containing "foo" that are direct children of a symbol named "bar". + 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: the name of the symbols to find. A "qualified" name that includes the symbol's parents - separated by `/` (e.g. "class/method/inner_function") can be used to restrict the search. - :param depth: specifies the depth up to which descendants of the symbol are to be retrieved - (e.g. depth 1 will retrieve methods and attributes for the case where the symbol refers to a class). - Provide a non-zero depth if you intend to subsequently query symbols that are contained in the - retrieved symbol. - :param within_relative_path: pass a relative path to only consider symbols within this path. - If a file is passed, only the symbols within this file will be considered. - If a directory is passed, all files within this directory will be considered. - If None, the entire codebase will be considered. - :param include_body: whether to include the body of all symbols in the result. You should only use this - if you actually need the body of the symbol for the task at hand (for example, for a deep analysis - of the functionality or for an editing task). - :param include_kinds: an optional list of ints representing the LSP symbol kind. - If provided, only symbols of the given kinds will be included in the result. - Valid kinds: - 1=file, 2=module, 3=namespace, 4=package, 5=class, 6=method, 7=property, 8=field, 9=constructor, 10=enum, + + :param name_path: The name path pattern to search for, see above for details. + :param depth: Depth to retrieve descendants (e.g., 1 for class methods/attributes). + :param within_relative_path: Optional. Restrict search to this file or directory. If None, searches entire codebase. + :param include_body: If True, include the symbol's source code. Use judiciously. + :param include_kinds: Optional. List of LSP symbol kind integers to include. (e.g., 5 for Class, 12 for Function). + Valid kinds: 1=file, 2=module, 3=namespace, 4=package, 5=class, 6=method, 7=property, 8=field, 9=constructor, 10=enum, 11=interface, 12=function, 13=variable, 14=constant, 15=string, 16=number, 17=boolean, 18=array, 19=object, 20=key, 21=null, 22=enum member, 23=struct, 24=event, 25=operator, 26=type parameter - :param exclude_kinds: If provided, symbols of the given kinds will be excluded from the result. - Takes precedence over include_kinds. - :param substring_matching: whether to use substring matching for the symbol name. - If True, the symbol name will be matched if it contains the given name as a substring. - :param max_answer_chars: if the output is longer than this number of characters, - no content will be returned. Don't adjust unless there is really no other way to get the content - required for the task. Instead, if the output is too long, you should - make a stricter query. - :return: a list of symbols (with symbol locations) that match the given name in JSON format - + :param exclude_kinds: Optional. List of LSP symbol kind integers to exclude. Takes precedence over `include_kinds`. + :param substring_matching: If True, use substring matching for the last segment of `name`. + :param max_answer_chars: Max characters for the JSON result. If exceeded, no content is returned. + :return: JSON string: a list of symbols (with locations) matching the name. """ - include_kinds = cast(list[SymbolKind] | None, include_kinds) - exclude_kinds = cast(list[SymbolKind] | None, exclude_kinds) + 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( - name, + name_path, include_body=include_body, - include_kinds=include_kinds, - exclude_kinds=exclude_kinds, + include_kinds=parsed_include_kinds, + exclude_kinds=parsed_exclude_kinds, substring_matching=substring_matching, within_relative_path=within_relative_path, ) @@ -895,13 +888,13 @@ class FindReferencingSymbolsTool(Tool): make a stricter query. :return: a list of JSON objects with the symbols referencing the requested symbol """ - include_kinds = cast(list[SymbolKind] | None, include_kinds) - exclude_kinds = cast(list[SymbolKind] | None, exclude_kinds) + 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_referencing_symbols( SymbolLocation(relative_path, line, column), include_body=include_body, - include_kinds=include_kinds, - exclude_kinds=exclude_kinds, + include_kinds=parsed_include_kinds, + exclude_kinds=parsed_exclude_kinds, ) symbol_dicts = [s.to_dict(kind=True, location=True, depth=0, include_body=include_body) for s in symbols] result = json.dumps(symbol_dicts) diff --git a/src/serena/symbol.py b/src/serena/symbol.py index 7a6e227..171e209 100644 --- a/src/serena/symbol.py +++ b/src/serena/symbol.py @@ -1,508 +1,496 @@ -import logging -import os -from collections.abc import Iterator, Sequence -from contextlib import contextmanager -from copy import copy -from dataclasses import asdict, dataclass -from typing import TYPE_CHECKING, Any, Self - -from sensai.util.string import ToStringMixin - -from multilspy import SyncLanguageServer -from multilspy.multilspy_types import Position, SymbolKind, UnifiedSymbolInformation - -if TYPE_CHECKING: - from .agent import SerenaAgent - -log = logging.getLogger(__name__) - - -@dataclass -class SymbolLocation: - """ - Represents the (start) location of a symbol identifier - """ - - relative_path: str | None - """ - the relative path of the file containing the symbol; if None, the symbol is defined outside of the project's scope - """ - line: int | None - """ - the line number in which the symbol identifier is defined (if the symbol is a function, class, etc.); - may be None for some types of symbols (e.g. SymbolKind.File) - """ - column: int | None - """ - the column number in which the symbol identifier is defined (if the symbol is a function, class, etc.); - may be None for some types of symbols (e.g. SymbolKind.File) - """ - - def __post_init__(self) -> None: - if self.relative_path is not None: - self.relative_path = self.relative_path.replace("/", os.path.sep) - - def to_dict(self) -> dict[str, Any]: - return asdict(self) - - def has_position_in_file(self) -> bool: - return self.relative_path is not None and self.line is not None and self.column is not None - - -class Symbol(ToStringMixin): - _QAULNAME_SEPARATOR = "/" - - @staticmethod - def match_against_qualname( - name_pattern: str, - qual_name_parts: list[str], - substring_matching: bool, - ) -> bool: - """ - Checks if a given name/pattern matches a symbol's qualified name parts. - - :param name_pattern: The name or pattern to match. Can be a simple name - (e.g., "my_func") or a qualified name pattern - (e.g., "MyClass/my_method", "MyClass/"). - :param qual_name_parts: A list of strings representing the parts of the - a qualified name. - :param substring_matching: If True, allows substring matching for the relevant part(s). - - For simple names, the whole `name_pattern` is checked as a substring - of the symbol's simple name (`qual_name_parts[-1]`). - - For qualified name patterns, only the *last* part of the - `name_to_match` pattern is checked as a substring. - Other parts must match exactly. - :return: True if the name matches, False otherwise. - """ - assert name_pattern, "name_to_match must not be empty" - assert qual_name_parts, "symbol_qual_name_parts must not be empty" - qname_separator = Symbol._QAULNAME_SEPARATOR - is_qualified_pattern = qname_separator in name_pattern - - if not is_qualified_pattern: - # Simple name matching - symbol_simple_name = qual_name_parts[-1] - if substring_matching: - return name_pattern in symbol_simple_name - else: - return name_pattern == symbol_simple_name - # Qualified name pattern matching - name_parts = name_pattern.rstrip(qname_separator).split(qname_separator) - - if len(name_parts) != len(qual_name_parts): - return False - - # Segments before the last one must be exact matches - if name_parts[:-1] != qual_name_parts[:-1]: - return False - - # Match the last segment of the pattern against the last part of the symbol's qualified name. - last_pattern_segment = name_parts[-1] - last_qual_name_segment = qual_name_parts[-1] - if substring_matching: - return last_pattern_segment in last_qual_name_segment - else: - return last_pattern_segment == last_qual_name_segment - - def __init__(self, symbol_root_from_ls: UnifiedSymbolInformation) -> None: - self.symbol_root = symbol_root_from_ls - - def _tostring_includes(self) -> list[str]: - return [] - - def _tostring_additional_entries(self) -> dict[str, Any]: - return dict(name=self.name, kind=self.kind, num_children=len(self.symbol_root["children"])) - - @property - def name(self) -> str: - return self.symbol_root["name"] - - @property - def kind(self) -> str: - return SymbolKind(self.symbol_kind).name - - @property - def symbol_kind(self) -> SymbolKind: - return self.symbol_root["kind"] - - @property - def relative_path(self) -> str | None: - location = self.symbol_root.get("location") - if location: - return location.get("relativePath") - return None - - @property - def location(self) -> SymbolLocation: - """ - :return: the start location of the actual symbol identifier - """ - return SymbolLocation(relative_path=self.relative_path, line=self.line, column=self.column) - - @property - def body_start_position(self) -> Position | None: - location = self.symbol_root.get("location") - if location: - range_info = location.get("range") - if range_info: - start_pos = range_info.get("start") - if start_pos: - return start_pos - return None - - @property - def body_end_position(self) -> Position | None: - location = self.symbol_root.get("location") - if location: - range_info = location.get("range") - if range_info: - end_pos = range_info.get("end") - if end_pos: - return end_pos - return None - - @property - def line(self) -> int | None: - if "selectionRange" in self.symbol_root: - return self.symbol_root["selectionRange"]["start"]["line"] - else: - # line is expected to be undefined for some types of symbols (e.g. SymbolKind.File) - return None - - @property - def column(self) -> int | None: - if "selectionRange" in self.symbol_root: - return self.symbol_root["selectionRange"]["start"]["character"] - else: - # precise location is expected to be undefined for some types of symbols (e.g. SymbolKind.File) - return None - - @property - def body(self) -> str | None: - return self.symbol_root.get("body") - - def get_qualified_name(self) -> str: - """ - Get the qualified name of the symbol (e.g. "class/method/inner_function"). - """ - return self._QAULNAME_SEPARATOR.join(self.get_qualified_name_parts()) - - def get_qualified_name_parts(self) -> list[str]: - """ - Get the parts of the qualified name of the symbol (e.g. ["class", "method", "inner_function"]). - """ - ancestors_within_file = list(self.iter_ancestors(up_to_symbol_kind=SymbolKind.File)) - ancestors_within_file.reverse() - return [a.name for a in ancestors_within_file] + [self.name] - - def iter_children(self) -> Iterator[Self]: - for c in self.symbol_root["children"]: - yield self.__class__(c) - - def iter_ancestors(self, up_to_symbol_kind: SymbolKind | None = None) -> Iterator[Self]: - """ - Iterate over all ancestors of the symbol, starting with the parent and going up to the root or - the given symbol kind. - - :param up_to_symbol_kind: if provided, iteration will stop *before* the first ancestor of the given kind. - A typical use case is to pass `SymbolKind.File` or `SymbolKind.Package`. - """ - parent = self.get_parent() - if parent is not None: - if up_to_symbol_kind is None or parent.symbol_kind != up_to_symbol_kind: - yield parent - yield from parent.iter_ancestors(up_to_symbol_kind=up_to_symbol_kind) - - def get_parent(self) -> Self | None: - parent_root = self.symbol_root.get("parent") - if parent_root is None: - return None - return self.__class__(parent_root) - - def find( - self, - name: 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. - The name matching behavior depends on whether a qualified name or a simple name is provided. - It is assumed that the provided name is a qualified name if it contains the `/` character. - If substring matching is allowed, only the last element of the qualified name will be checked against - the symbol name using substring matching. - - Examples: - - Providing "foo" will find all symbols named "foo" regardless where they are contained in the symbol tree. - - Providing "bar/foo" will only find symbols named "foo" that are direct children of a symbol called "bar". - - Providing "foo/" will only find symbols named "foo" that are top-level symbols (have no parent). - - Allowing substring matching with "bar" will find symbols with names containing "foo" anywhere in the symbol tree. - - Allowing substring matching with "foo/" will find only top-level symbols with names containing "foo". - - Allowing substring matching with "bar/foo" will find only symbols with names containing "foo" that are direct children of a symbol named "bar". - - :param name: the name of the symbols to find. A "qualified" name that includes the symbol's parents - separated by `/` (e.g. "class/method/inner_function") can be used to restrict the search. - :param substring_matching: whether to use substring matching for the symbol name. - If a qualified name is provided, the last element of the qualified name will be checked against - the symbol name using substring matching. - :param include_kinds: an optional sequence of ints representing the LSP symbol kind. - If provided, only symbols of the given kinds will be included in the result. - :param exclude_kinds: If provided, symbols of the given kinds will be excluded from the result. - - """ - result = [] - - def should_include(s: "Symbol") -> 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_against_qualname( - name_pattern=name, - qual_name_parts=s.get_qualified_name_parts(), - substring_matching=substring_matching, - ) - - def traverse(s: "Symbol") -> None: - if should_include(s): - result.append(s) - for c in s.iter_children(): - traverse(c) - - traverse(self) - return result - - def to_dict( - self, kind: bool = False, location: bool = False, depth: int = 0, include_body: bool = False, include_children_body: bool = False - ) -> dict[str, Any]: - """ - Convert the symbol to a dictionary. - - :param kind: whether to include the kind of the symbol - :param location: whether to include the location of the symbol - :param depth: the depth of the symbol - :param include_body: whether to include the body of the top-level symbol. - :param include_children_body: whether to also include the body of the children. - Note that the body of the children is part of the body of the parent symbol, - so there is usually no need to set this to True unless you want process the output - and pass the children without passing the parent body to the LM. - :return: a dictionary representation of the symbol - """ - result: dict[str, Any] = {"name": self.name, "qualname": self.get_qualified_name()} - - if kind: - result["kind"] = self.kind - - if location: - result["location"] = self.location.to_dict() - - if include_body: - if self.body is None: - log.warning("Requested body for symbol, but it is not present. The symbol might have been loaded with include_body=False.") - result["body"] = self.body - - def add_children(s: Self) -> list[dict[str, Any]]: - children = [] - for c in s.iter_children(): - children.append( - c.to_dict( - kind=kind, - location=location, - depth=depth - 1, - include_body=include_children_body, - include_children_body=include_children_body, - ) - ) - return children - - if depth > 0: - result["children"] = add_children(self) - - return result - - -class SymbolManager: - def __init__(self, lang_server: SyncLanguageServer, agent: "SerenaAgent") -> None: - self.lang_server = lang_server - self.agent = agent - - def _to_symbols(self, items: list[UnifiedSymbolInformation]) -> list[Symbol]: - return [Symbol(s) for s in items] - - def find_by_name( - self, - name: str, - include_body: bool = False, - include_kinds: Sequence[SymbolKind] | None = None, - exclude_kinds: Sequence[SymbolKind] | None = None, - substring_matching: bool = False, - within_relative_path: str | None = None, - ) -> list[Symbol]: - """ - 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] = [] - 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(name, include_kinds=include_kinds, exclude_kinds=exclude_kinds, substring_matching=substring_matching) - ) - return symbols - - def get_document_symbols(self, relative_path: str) -> list[Symbol]: - symbol_dicts, roots = self.lang_server.request_document_symbols(relative_path, include_body=False) - symbols = [Symbol(s) for s in symbol_dicts] - return symbols - - def find_by_location(self, location: SymbolLocation) -> Symbol | 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) - if symbol.location == location: - return symbol - return None - - def find_referencing_symbols( - self, - symbol_location: SymbolLocation, - include_body: bool = False, - include_kinds: Sequence[SymbolKind] | None = None, - exclude_kinds: Sequence[SymbolKind] | None = None, - ) -> list[Symbol]: - """ - Find all symbols that reference the symbol at the given location. - - :param symbol_location: the location of the symbol for which to find references - :param include_body: whether to include the body of all symbols in the result. - Note: you can filter out the bodies of the children if you set include_children_body=False - in the to_dict method. - :param include_kinds: an optional sequence of ints representing the LSP symbol kind. - If provided, only symbols of the given kinds will be included in the result. - :param exclude_kinds: If provided, symbols of the given kinds will be excluded from the result. - Takes precedence over include_kinds. - :return: a list of symbols that reference the given symbol - """ - if not symbol_location.has_position_in_file(): - raise ValueError("Symbol location does not contain a valid position in a file") - assert symbol_location.relative_path is not None - assert symbol_location.line is not None - assert symbol_location.column is not None - symbol_dicts = self.lang_server.request_referencing_symbols( - relative_file_path=symbol_location.relative_path, - line=symbol_location.line, - column=symbol_location.column, - include_imports=False, - include_self=False, - include_body=include_body, - ) - - if include_kinds is not None: - symbol_dicts = [s for s in symbol_dicts if s["kind"] in include_kinds] - - if exclude_kinds is not None: - symbol_dicts = [s for s in symbol_dicts if s["kind"] not in exclude_kinds] - - return self._to_symbols(symbol_dicts) - - @contextmanager - def _edited_file(self, relative_path: str) -> Iterator[None]: - with self.lang_server.open_file(relative_path) as file_buffer: - yield - root_path = self.lang_server.language_server.repository_root_path - abs_path = os.path.join(root_path, relative_path) - with open(abs_path, "w", encoding="utf-8") as f: - f.write(file_buffer.contents) - self.agent.mark_file_modified(relative_path) - - @contextmanager - def _edited_symbol_location(self, location: SymbolLocation) -> Iterator[Symbol]: - symbol = self.find_by_location(location) - if symbol is None: - raise ValueError("Symbol not found/has no defined location within a file") - assert location.relative_path is not None - with self._edited_file(location.relative_path): - yield symbol - - def replace_body(self, location: SymbolLocation, body: str) -> None: - """ - Replace the body of the symbol at the given location with the given body - - :param location: the location of the symbol to replace - :param body: the new body - """ - # make sure body always ends with at least one newline - if not body.endswith("\n"): - body += "\n" - with self._edited_symbol_location(location) as symbol: - assert location.relative_path is not None - start_pos = symbol.body_start_position - end_pos = symbol.body_end_position - if start_pos is None or end_pos is None: - raise ValueError(f"Symbol at {location} does not have a defined body range.") - # At this point, start_pos and end_pos are guaranteed to be Position objects - self.lang_server.delete_text_between_positions(location.relative_path, start_pos, end_pos) - self.lang_server.insert_text_at_position(location.relative_path, start_pos["line"], start_pos["character"], body) - - def insert_after(self, location: SymbolLocation, body: str) -> None: - """ - Appends content after the given symbol - - :param location: the location of the symbol after which to add new lines - :param body: the body of the entity to append - """ - # make sure body always ends with at least one newline - if not body.endswith("\n"): - body += "\n" - with self._edited_symbol_location(location) as symbol: - pos = symbol.body_end_position - if pos is None: - raise ValueError(f"Symbol at {location} does not have a defined end position.") - # At this point, pos is guaranteed to be a Position object - assert location.relative_path is not None - self.lang_server.insert_text_at_position(location.relative_path, pos["line"], pos["character"], body) - - def insert_before(self, location: SymbolLocation, body: str) -> None: - """ - Inserts content before the given symbol - - :param location: the location of the symbol before which to add new lines - :param body: the body of the entity to insert - """ - # make sure body always ends with at least one newline - if not body.endswith("\n"): - body += "\n" - with self._edited_symbol_location(location) as symbol: - original_start_pos = symbol.body_start_position - if original_start_pos is None: - raise ValueError(f"Symbol at {location} does not have a defined start position.") - # At this point, original_start_pos is guaranteed to be a Position object, so copying is safe. - pos = copy(original_start_pos) - assert location.relative_path is not None - self.lang_server.insert_text_at_position(location.relative_path, pos["line"], pos["character"], body) - - def insert_at_line(self, relative_path: str, line: int, content: str) -> None: - """ - Inserts content at the given line in the given file. - - :param line: the 0-based index of the line to insert content at - :param content: the content to insert - """ - with self._edited_file(relative_path): - self.lang_server.insert_text_at_position(relative_path, line, 0, content) - - def delete_lines(self, relative_path: str, start_line: int, end_line: int) -> None: - """ - Deletes lines in the given file. - - :param start_line: the 0-based index of the first line to delete (inclusive) - :param end_line: the 0-based index of the last line to delete (inclusive) - """ - with self._edited_file(relative_path): - start_pos = Position(line=start_line, character=0) - end_pos = Position(line=end_line + 1, character=0) - self.lang_server.delete_text_between_positions(relative_path, start_pos, end_pos) +import logging +import os +from collections.abc import Iterator, Sequence +from contextlib import contextmanager +from copy import copy +from dataclasses import asdict, dataclass +from typing import TYPE_CHECKING, Any, Self + +from sensai.util.string import ToStringMixin + +from multilspy import SyncLanguageServer +from multilspy.multilspy_types import Position, SymbolKind, UnifiedSymbolInformation + +if TYPE_CHECKING: + from .agent import SerenaAgent + +log = logging.getLogger(__name__) + + +@dataclass +class SymbolLocation: + """ + Represents the (start) location of a symbol identifier + """ + + relative_path: str | None + """ + the relative path of the file containing the symbol; if None, the symbol is defined outside of the project's scope + """ + line: int | None + """ + the line number in which the symbol identifier is defined (if the symbol is a function, class, etc.); + may be None for some types of symbols (e.g. SymbolKind.File) + """ + column: int | None + """ + the column number in which the symbol identifier is defined (if the symbol is a function, class, etc.); + may be None for some types of symbols (e.g. SymbolKind.File) + """ + + def __post_init__(self) -> None: + if self.relative_path is not None: + self.relative_path = self.relative_path.replace("/", os.path.sep) + + def to_dict(self) -> dict[str, Any]: + return asdict(self) + + def has_position_in_file(self) -> bool: + return self.relative_path is not None and self.line is not None and self.column is not None + + +class Symbol(ToStringMixin): + _NAME_PATH_SEP = "/" + + @staticmethod + def match_name_path( + name_path: str, + symbol_name_path_parts: list[str], + substring_matching: bool, + ) -> bool: + """ + Checks if a given `name_path` matches a symbol's qualified name parts. + See docstring of `Symbol.find` for more details. + """ + 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 + + 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) + + # filtering based on ancestors + if len(pattern_parts) > len(symbol_name_path_parts): + # can't possibly match if pattern has more parts than symbol + return False + if is_absolute_pattern and len(pattern_parts) != len(symbol_name_path_parts): + # for absolute patterns, the number of parts must match exactly + return False + if symbol_name_path_parts[-len(pattern_parts) : -1] != pattern_parts[:-1]: + # ancestors must match + return False + + # matching the last part of the symbol name + name_to_match = pattern_parts[-1] + symbol_name = symbol_name_path_parts[-1] + if substring_matching: + return name_to_match in symbol_name + else: + return name_to_match == symbol_name + + def __init__(self, symbol_root_from_ls: UnifiedSymbolInformation) -> None: + self.symbol_root = symbol_root_from_ls + + def _tostring_includes(self) -> list[str]: + return [] + + def _tostring_additional_entries(self) -> dict[str, Any]: + return dict(name=self.name, kind=self.kind, num_children=len(self.symbol_root["children"])) + + @property + def name(self) -> str: + return self.symbol_root["name"] + + @property + def kind(self) -> str: + return SymbolKind(self.symbol_kind).name + + @property + def symbol_kind(self) -> SymbolKind: + return self.symbol_root["kind"] + + @property + def relative_path(self) -> str | None: + location = self.symbol_root.get("location") + if location: + return location.get("relativePath") + return None + + @property + def location(self) -> SymbolLocation: + """ + :return: the start location of the actual symbol identifier + """ + return SymbolLocation(relative_path=self.relative_path, line=self.line, column=self.column) + + @property + def body_start_position(self) -> Position | None: + location = self.symbol_root.get("location") + if location: + range_info = location.get("range") + if range_info: + start_pos = range_info.get("start") + if start_pos: + return start_pos + return None + + @property + def body_end_position(self) -> Position | None: + location = self.symbol_root.get("location") + if location: + range_info = location.get("range") + if range_info: + end_pos = range_info.get("end") + if end_pos: + return end_pos + return None + + @property + def line(self) -> int | None: + if "selectionRange" in self.symbol_root: + return self.symbol_root["selectionRange"]["start"]["line"] + else: + # line is expected to be undefined for some types of symbols (e.g. SymbolKind.File) + return None + + @property + def column(self) -> int | None: + if "selectionRange" in self.symbol_root: + return self.symbol_root["selectionRange"]["start"]["character"] + else: + # precise location is expected to be undefined for some types of symbols (e.g. SymbolKind.File) + return None + + @property + def body(self) -> str | None: + return self.symbol_root.get("body") + + def get_name_path(self) -> str: + """ + Get the name path of the symbol (e.g. "class/method/inner_function"). + """ + return self._NAME_PATH_SEP.join(self.get_name_path_parts()) + + def get_name_path_parts(self) -> list[str]: + """ + Get the parts of the name path of the symbol (e.g. ["class", "method", "inner_function"]). + """ + ancestors_within_file = list(self.iter_ancestors(up_to_symbol_kind=SymbolKind.File)) + ancestors_within_file.reverse() + return [a.name for a in ancestors_within_file] + [self.name] + + def iter_children(self) -> Iterator[Self]: + for c in self.symbol_root["children"]: + yield self.__class__(c) + + def iter_ancestors(self, up_to_symbol_kind: SymbolKind | None = None) -> Iterator[Self]: + """ + Iterate over all ancestors of the symbol, starting with the parent and going up to the root or + the given symbol kind. + + :param up_to_symbol_kind: if provided, iteration will stop *before* the first ancestor of the given kind. + A typical use case is to pass `SymbolKind.File` or `SymbolKind.Package`. + """ + parent = self.get_parent() + if parent is not None: + if up_to_symbol_kind is None or parent.symbol_kind != up_to_symbol_kind: + yield parent + yield from parent.iter_ancestors(up_to_symbol_kind=up_to_symbol_kind) + + def get_parent(self) -> Self | None: + parent_root = self.symbol_root.get("parent") + if parent_root is None: + return None + return self.__class__(parent_root) + + def find( + self, + name_path: 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). + + 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 to match against + :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. + If provided, only symbols of the given kinds will be included in the result. + :param exclude_kinds: If provided, symbols of the given kinds will be excluded from the result. + """ + result = [] + + def should_include(s: "Symbol") -> 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( + name_path=name_path, + symbol_name_path_parts=s.get_name_path_parts(), + substring_matching=substring_matching, + ) + + def traverse(s: "Symbol") -> None: + if should_include(s): + result.append(s) + for c in s.iter_children(): + traverse(c) + + traverse(self) + return result + + def to_dict( + self, kind: bool = False, location: bool = False, depth: int = 0, include_body: bool = False, include_children_body: bool = False + ) -> dict[str, Any]: + """ + Convert the symbol to a dictionary. + + :param kind: whether to include the kind of the symbol + :param location: whether to include the location of the symbol + :param depth: the depth of the symbol + :param include_body: whether to include the body of the top-level symbol. + :param include_children_body: whether to also include the body of the children. + Note that the body of the children is part of the body of the parent symbol, + so there is usually no need to set this to True unless you want process the output + and pass the children without passing the parent body to the LM. + :return: a dictionary representation of the symbol + """ + result: dict[str, Any] = {"name": self.name, "name_path": self.get_name_path()} + + if kind: + result["kind"] = self.kind + + if location: + result["location"] = self.location.to_dict() + + if include_body: + if self.body is None: + log.warning("Requested body for symbol, but it is not present. The symbol might have been loaded with include_body=False.") + result["body"] = self.body + + def add_children(s: Self) -> list[dict[str, Any]]: + children = [] + for c in s.iter_children(): + children.append( + c.to_dict( + kind=kind, + location=location, + depth=depth - 1, + include_body=include_children_body, + include_children_body=include_children_body, + ) + ) + return children + + if depth > 0: + result["children"] = add_children(self) + + return result + + +class SymbolManager: + def __init__(self, lang_server: SyncLanguageServer, agent: "SerenaAgent") -> None: + self.lang_server = lang_server + self.agent = agent + + def _to_symbols(self, items: list[UnifiedSymbolInformation]) -> list[Symbol]: + return [Symbol(s) for s in items] + + def find_by_name( + self, + name: str, + include_body: bool = False, + include_kinds: Sequence[SymbolKind] | None = None, + exclude_kinds: Sequence[SymbolKind] | None = None, + substring_matching: bool = False, + within_relative_path: str | None = None, + ) -> list[Symbol]: + """ + 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] = [] + 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(name, include_kinds=include_kinds, exclude_kinds=exclude_kinds, substring_matching=substring_matching) + ) + return symbols + + def get_document_symbols(self, relative_path: str) -> list[Symbol]: + symbol_dicts, roots = self.lang_server.request_document_symbols(relative_path, include_body=False) + symbols = [Symbol(s) for s in symbol_dicts] + return symbols + + def find_by_location(self, location: SymbolLocation) -> Symbol | 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) + if symbol.location == location: + return symbol + return None + + def find_referencing_symbols( + self, + symbol_location: SymbolLocation, + include_body: bool = False, + include_kinds: Sequence[SymbolKind] | None = None, + exclude_kinds: Sequence[SymbolKind] | None = None, + ) -> list[Symbol]: + """ + Find all symbols that reference the symbol at the given location. + + :param symbol_location: the location of the symbol for which to find references + :param include_body: whether to include the body of all symbols in the result. + Note: you can filter out the bodies of the children if you set include_children_body=False + in the to_dict method. + :param include_kinds: an optional sequence of ints representing the LSP symbol kind. + If provided, only symbols of the given kinds will be included in the result. + :param exclude_kinds: If provided, symbols of the given kinds will be excluded from the result. + Takes precedence over include_kinds. + :return: a list of symbols that reference the given symbol + """ + if not symbol_location.has_position_in_file(): + raise ValueError("Symbol location does not contain a valid position in a file") + assert symbol_location.relative_path is not None + assert symbol_location.line is not None + assert symbol_location.column is not None + symbol_dicts = self.lang_server.request_referencing_symbols( + relative_file_path=symbol_location.relative_path, + line=symbol_location.line, + column=symbol_location.column, + include_imports=False, + include_self=False, + include_body=include_body, + ) + + if include_kinds is not None: + symbol_dicts = [s for s in symbol_dicts if s["kind"] in include_kinds] + + if exclude_kinds is not None: + symbol_dicts = [s for s in symbol_dicts if s["kind"] not in exclude_kinds] + + return self._to_symbols(symbol_dicts) + + @contextmanager + def _edited_file(self, relative_path: str) -> Iterator[None]: + with self.lang_server.open_file(relative_path) as file_buffer: + yield + root_path = self.lang_server.language_server.repository_root_path + abs_path = os.path.join(root_path, relative_path) + with open(abs_path, "w", encoding="utf-8") as f: + f.write(file_buffer.contents) + self.agent.mark_file_modified(relative_path) + + @contextmanager + def _edited_symbol_location(self, location: SymbolLocation) -> Iterator[Symbol]: + symbol = self.find_by_location(location) + if symbol is None: + raise ValueError("Symbol not found/has no defined location within a file") + assert location.relative_path is not None + with self._edited_file(location.relative_path): + yield symbol + + def replace_body(self, location: SymbolLocation, body: str) -> None: + """ + Replace the body of the symbol at the given location with the given body + + :param location: the location of the symbol to replace + :param body: the new body + """ + # make sure body always ends with at least one newline + if not body.endswith("\n"): + body += "\n" + with self._edited_symbol_location(location) as symbol: + assert location.relative_path is not None + start_pos = symbol.body_start_position + end_pos = symbol.body_end_position + if start_pos is None or end_pos is None: + raise ValueError(f"Symbol at {location} does not have a defined body range.") + # At this point, start_pos and end_pos are guaranteed to be Position objects + self.lang_server.delete_text_between_positions(location.relative_path, start_pos, end_pos) + self.lang_server.insert_text_at_position(location.relative_path, start_pos["line"], start_pos["character"], body) + + def insert_after(self, location: SymbolLocation, body: str) -> None: + """ + Appends content after the given symbol + + :param location: the location of the symbol after which to add new lines + :param body: the body of the entity to append + """ + # make sure body always ends with at least one newline + if not body.endswith("\n"): + body += "\n" + with self._edited_symbol_location(location) as symbol: + pos = symbol.body_end_position + if pos is None: + raise ValueError(f"Symbol at {location} does not have a defined end position.") + # At this point, pos is guaranteed to be a Position object + assert location.relative_path is not None + self.lang_server.insert_text_at_position(location.relative_path, pos["line"], pos["character"], body) + + def insert_before(self, location: SymbolLocation, body: str) -> None: + """ + Inserts content before the given symbol + + :param location: the location of the symbol before which to add new lines + :param body: the body of the entity to insert + """ + # make sure body always ends with at least one newline + if not body.endswith("\n"): + body += "\n" + with self._edited_symbol_location(location) as symbol: + original_start_pos = symbol.body_start_position + if original_start_pos is None: + raise ValueError(f"Symbol at {location} does not have a defined start position.") + # At this point, original_start_pos is guaranteed to be a Position object, so copying is safe. + pos = copy(original_start_pos) + assert location.relative_path is not None + self.lang_server.insert_text_at_position(location.relative_path, pos["line"], pos["character"], body) + + def insert_at_line(self, relative_path: str, line: int, content: str) -> None: + """ + Inserts content at the given line in the given file. + + :param line: the 0-based index of the line to insert content at + :param content: the content to insert + """ + with self._edited_file(relative_path): + self.lang_server.insert_text_at_position(relative_path, line, 0, content) + + def delete_lines(self, relative_path: str, start_line: int, end_line: int) -> None: + """ + Deletes lines in the given file. + + :param start_line: the 0-based index of the first line to delete (inclusive) + :param end_line: the 0-based index of the last line to delete (inclusive) + """ + with self._edited_file(relative_path): + start_pos = Position(line=start_line, character=0) + end_pos = Position(line=end_line + 1, character=0) + self.lang_server.delete_text_between_positions(relative_path, start_pos, end_pos) diff --git a/test/serena/test_serena_agent.py b/test/serena/test_serena_agent.py index 9fb3783..2b0616a 100644 --- a/test/serena/test_serena_agent.py +++ b/test/serena/test_serena_agent.py @@ -89,7 +89,7 @@ class TestSerenaAgent: ), f"Expected to find reference to {symbol_name} in {ref_file} for {agent.project_config.language.name}. refs={refs}" @pytest.mark.parametrize( - "serena_agent,qualified_name,substring_matching,expected_symbol_name,expected_kind,expected_file", + "serena_agent,name_path,substring_matching,expected_symbol_name,expected_kind,expected_file", [ pytest.param( Language.PYTHON, @@ -131,13 +131,33 @@ class TestSerenaAgent: id="substring_qualname_method", marks=pytest.mark.python, ), + pytest.param( + Language.PYTHON, + "/OuterClass", # Absolute path + False, + "OuterClass", + "Class", + os.path.join("test_repo", "nested.py"), + id="absolute_qualname_class", + marks=pytest.mark.python, + ), + pytest.param( + Language.PYTHON, + "/OuterClass/NestedClass/find_m", # Absolute path with substring + True, + "find_me", + "Method", + os.path.join("test_repo", "nested.py"), + id="absolute_substring_qualname_method", + marks=pytest.mark.python, + ), ], indirect=["serena_agent"], ) - def test_find_symbol_qualified_name( + def test_find_symbol_name_path( self, serena_agent: SerenaAgent, - qualified_name: str, + name_path: str, substring_matching: bool, expected_symbol_name: str, expected_kind: str, @@ -146,7 +166,7 @@ class TestSerenaAgent: agent = serena_agent find_symbol_tool = agent.get_tool(FindSymbolTool) result = find_symbol_tool.apply( - name=qualified_name, + name_path=name_path, depth=0, within_relative_path=None, include_body=False, @@ -160,4 +180,39 @@ class TestSerenaAgent: and expected_kind.lower() in s["kind"].lower() and expected_file in s["location"]["relative_path"] for s in symbols - ), f"Expected to find {qualified_name} ({expected_kind}) in {expected_file} for {agent.project_config.language.name}. Symbols: {symbols}" + ), f"Expected to find {name_path} ({expected_kind}) in {expected_file} for {agent.project_config.language.name}. Symbols: {symbols}" + + @pytest.mark.parametrize( + "serena_agent,name_path", + [ + pytest.param( + Language.PYTHON, + "/NestedClass", # Absolute path, NestedClass is not top-level + id="absolute_path_non_top_level_no_match", + marks=pytest.mark.python, + ), + pytest.param( + Language.PYTHON, + "/NoSuchParent/NestedClass", # Absolute path with non-existent parent + id="absolute_path_non_existent_parent_no_match", + marks=pytest.mark.python, + ), + ], + indirect=["serena_agent"], + ) + def test_find_symbol_name_path_no_match( + self, + serena_agent: SerenaAgent, + name_path: str, + ): + agent = serena_agent + find_symbol_tool = agent.get_tool(FindSymbolTool) + result = find_symbol_tool.apply( + name_path=name_path, + depth=0, + substring_matching=True, + ) + symbols = json.loads(result) + assert ( + not symbols + ), f"Expected to find no symbols for {name_path} for {agent.project_config.language.name}. Symbols found: {symbols}" diff --git a/test/serena/test_symbol.py b/test/serena/test_symbol.py index fdd48f7..7c21215 100644 --- a/test/serena/test_symbol.py +++ b/test/serena/test_symbol.py @@ -6,128 +6,157 @@ from src.serena.symbol import Symbol class TestSymbolNameMatching: def _create_assertion_error_message( self, - name_pattern: str, - qual_name_parts: list[str], + name_path_pattern: str, + symbol_name_path_parts: list[str], is_substring_match: bool, expected_result: bool, actual_result: bool, ) -> str: """Helper to create a detailed error message for assertions.""" - qnp_repr = "/".join(qual_name_parts) + qnp_repr = "/".join(symbol_name_path_parts) return ( - f"Pattern '{name_pattern}' (substring: {is_substring_match}) vs " - f"Qualname parts {qual_name_parts} (as '{qnp_repr}'). " + f"Pattern '{name_path_pattern}' (substring: {is_substring_match}) vs " + f"Qualname parts {symbol_name_path_parts} (as '{qnp_repr}'). " f"Expected: {expected_result}, Got: {actual_result}" ) @pytest.mark.parametrize( - "name_pattern, qual_name_parts, is_substring_match, expected", + "name_path_pattern, symbol_name_path_parts, is_substring_match, expected", [ - # Exact matches (is_substring_match=False) - pytest.param("foo", ["foo"], False, True, id="'foo' matches 'foo' exactly"), - pytest.param("foo", ["bar", "foo"], False, True, id="'foo' matches 'bar,foo' exactly"), - pytest.param("foo", ["foobar"], False, False, id="'foo' does not match 'foobar' exactly"), - pytest.param("foo", ["bar", "foobar"], False, False, id="'foo' does not match 'bar,foobar' exactly"), - pytest.param("foo", ["path", "to", "foo"], False, True, id="'foo' matches 'path,to,foo' exactly"), - # Substring matches (is_substring_match=True) - pytest.param("foo", ["foobar"], True, True, id="'foo' matches 'foobar' as substring"), - pytest.param("foo", ["bar", "foobar"], True, True, id="'foo' matches 'bar,foobar' as substring"), - pytest.param("foo", ["barfoo"], True, True, id="'foo' matches 'barfoo' as substring"), - pytest.param("foo", ["baz"], True, False, id="'foo' does not match 'baz' as substring"), - pytest.param("foo", ["bar", "baz"], True, False, id="'foo' does not match 'bar,baz' as substring"), - pytest.param("foo", ["my_foobar_func"], True, True, id="'foo' matches 'my_foobar_func' as substring"), - pytest.param("foo", ["ClassA", "my_foobar_method"], True, True, id="'foo' matches 'ClassA,my_foobar_method' as substring"), - pytest.param("foo", ["my_bar_func"], True, False, id="'foo' does not match 'my_bar_func' as substring"), + # Exact matches, anywhere in the name (is_substring_match=False) + pytest.param("foo", ["foo"], False, True, id="'foo' matches 'foo' exactly (simple)"), + pytest.param("foo/", ["foo"], False, True, id="'foo/' matches 'foo' exactly (simple)"), + pytest.param("foo", ["bar", "foo"], False, True, id="'foo' matches ['bar', 'foo'] exactly (simple, last element)"), + pytest.param("foo", ["foobar"], False, False, id="'foo' does not match 'foobar' exactly (simple)"), + pytest.param( + "foo", ["bar", "foobar"], False, False, id="'foo' does not match ['bar', 'foobar'] exactly (simple, last element)" + ), + pytest.param( + "foo", ["path", "to", "foo"], False, True, id="'foo' matches ['path', 'to', 'foo'] exactly (simple, last element)" + ), + # Exact matches, absolute patterns (is_substring_match=False) + pytest.param("/foo", ["foo"], False, True, id="'/foo' matches ['foo'] exactly (absolute simple)"), + pytest.param("/foo", ["foo", "bar"], False, False, id="'/foo' does not match ['foo', 'bar'] (absolute simple, len mismatch)"), + pytest.param("/foo", ["bar"], False, False, id="'/foo' does not match ['bar'] (absolute simple, name mismatch)"), + pytest.param( + "/foo", ["bar", "foo"], False, False, id="'/foo' does not match ['bar', 'foo'] (absolute simple, position mismatch)" + ), + # Substring matches, anywhere in the name (is_substring_match=True) + pytest.param("foo", ["foobar"], True, True, id="'foo' matches 'foobar' as substring (simple)"), + pytest.param("foo", ["bar", "foobar"], True, True, id="'foo' matches ['bar', 'foobar'] as substring (simple, last element)"), + pytest.param( + "foo", ["barfoo"], True, True, id="'foo' matches 'barfoo' as substring (simple)" + ), # This was potentially ambiguous before + pytest.param("foo", ["baz"], True, False, id="'foo' does not match 'baz' as substring (simple)"), + pytest.param("foo", ["bar", "baz"], True, False, id="'foo' does not match ['bar', 'baz'] as substring (simple, last element)"), + pytest.param("foo", ["my_foobar_func"], True, True, id="'foo' matches 'my_foobar_func' as substring (simple)"), + pytest.param( + "foo", + ["ClassA", "my_foobar_method"], + True, + True, + id="'foo' matches ['ClassA', 'my_foobar_method'] as substring (simple, last element)", + ), + pytest.param("foo", ["my_bar_func"], True, False, id="'foo' does not match 'my_bar_func' as substring (simple)"), + # Substring matches, absolute patterns (is_substring_match=True) + pytest.param("/foo", ["foobar"], True, True, id="'/foo' matches ['foobar'] as substring (absolute simple)"), + pytest.param("/foo/", ["foobar"], True, True, id="'/foo/' matches ['foobar'] as substring (absolute simple, last element)"), + pytest.param("/foo", ["barfoobaz"], True, True, id="'/foo' matches ['barfoobaz'] as substring (absolute simple)"), + pytest.param( + "/foo", ["foo", "bar"], True, False, id="'/foo' does not match ['foo', 'bar'] as substring (absolute simple, len mismatch)" + ), + pytest.param("/foo", ["bar"], True, False, id="'/foo' does not match ['bar'] (absolute simple, no substr)"), + pytest.param( + "/foo", ["bar", "foo"], True, False, id="'/foo' does not match ['bar', 'foo'] (absolute simple, position mismatch)" + ), + pytest.param( + "/foo/", ["bar", "foo"], True, False, id="'/foo/' does not match ['bar', 'foo'] (absolute simple, position mismatch)" + ), ], ) - def test_match_simple_name(self, name_pattern, qual_name_parts, is_substring_match, expected): + 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_against_qualname(name_pattern, qual_name_parts, is_substring_match) - error_msg = self._create_assertion_error_message(name_pattern, qual_name_parts, is_substring_match, expected, result) + result = Symbol.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 @pytest.mark.parametrize( - "name_pattern, qual_name_parts, is_substring_match, expected", + "name_path_pattern, symbol_name_path_parts, is_substring_match, expected", [ - # Exact matches (is_substring_match=False) - pytest.param("bar/foo", ["bar", "foo"], False, True, id="'bar/foo' matches 'bar,foo' exactly"), + # --- Relative patterns (suffix matching) --- + # Exact matches, relative patterns (is_substring_match=False) + pytest.param("bar/foo", ["bar", "foo"], False, True, id="R: 'bar/foo' matches ['bar', 'foo'] exactly"), + pytest.param("bar/foo", ["mod", "bar", "foo"], False, True, id="R: 'bar/foo' matches ['mod', 'bar', 'foo'] exactly (suffix)"), pytest.param( - "bar/foo", ["bar", "foo", "baz"], False, False, id="'bar/foo' does not match 'bar,foo,baz' exactly (len mismatch)" + "bar/foo", ["bar", "foo", "baz"], False, False, id="R: 'bar/foo' does not match ['bar', 'foo', 'baz'] (pattern shorter)" ), - pytest.param("bar/foo", ["bar"], False, False, id="'bar/foo' does not match 'bar' exactly (len mismatch)"), - pytest.param("bar/foo", ["baz", "foo"], False, False, id="'bar/foo' does not match 'baz,foo' exactly (first mismatch)"), - pytest.param("bar/foo", ["bar", "baz"], False, False, id="'bar/foo' does not match 'bar,baz' exactly (last mismatch)"), - # from docstring examples - pytest.param("bar/foo", ["foo"], False, False, id="'bar/foo' does not match 'foo' exactly"), - pytest.param("bar/foo", ["other", "foo"], False, False, id="'bar/foo' does not match 'other,foo' exactly"), - pytest.param("bar/foo", ["bar", "otherfoo"], False, False, id="'bar/foo' does not match 'bar,otherfoo' exactly"), - # Substring matches (is_substring_match=True) - pytest.param("bar/foo", ["bar", "foobar"], True, True, id="'bar/foo' matches 'bar,foobar' as substring"), - pytest.param("bar/foo", ["bar", "bazfoo"], True, True, id="'bar/foo' matches 'bar,bazfoo' as substring"), - pytest.param("bar/fo", ["bar", "foo"], True, True, id="'bar/fo' matches 'bar,foo' as substring"), - pytest.param("bar/foo", ["bar", "baz"], True, False, id="'bar/foo' does not match 'bar,baz' as substring (last no substr)"), + pytest.param("bar/foo", ["bar"], False, False, id="R: 'bar/foo' does not match ['bar'] (pattern longer)"), + pytest.param("bar/foo", ["baz", "foo"], False, False, id="R: 'bar/foo' does not match ['baz', 'foo'] (first part mismatch)"), + pytest.param("bar/foo", ["bar", "baz"], False, False, id="R: 'bar/foo' does not match ['bar', 'baz'] (last part mismatch)"), + pytest.param("bar/foo", ["foo"], False, False, id="R: 'bar/foo' does not match ['foo'] (pattern longer)"), pytest.param( - "bar/foo", ["baz", "foobar"], True, False, id="'bar/foo' does not match 'baz,foobar' as substring (first mismatch)" - ), - # from docstring examples - pytest.param("bar/foo", ["bar", "my_foobar_method"], True, True, id="'bar/foo' matches 'bar,my_foobar_method' as substring"), - pytest.param( - "bar/foo", ["bar", "another_method"], True, False, id="'bar/foo' does not match 'bar,another_method' as substring" + "bar/foo", ["other", "foo"], False, False, id="R: 'bar/foo' does not match ['other', 'foo'] (first part mismatch)" ), pytest.param( - "bar/foo", ["other", "my_foobar_method"], True, False, id="'bar/foo' does not match 'other,my_foobar_method' as substring" + "bar/foo", ["bar", "otherfoo"], False, False, id="R: 'bar/foo' does not match ['bar', 'otherfoo'] (last part mismatch)" ), - pytest.param("bar/f", ["bar", "foo"], True, True, id="'bar/f' matches 'bar,foo' as substring"), - ], - ) - def test_match_name_pattern_ends_without_slash(self, name_pattern, qual_name_parts, is_substring_match, expected): - """Tests matching for qualified names (e.g. 'module/class/func').""" - result = Symbol.match_against_qualname(name_pattern, qual_name_parts, is_substring_match) - error_msg = self._create_assertion_error_message(name_pattern, qual_name_parts, is_substring_match, expected, result) - assert result == expected, error_msg - - @pytest.mark.parametrize( - "name_pattern, qual_name_parts, is_substring_match, expected", - [ - # Exact matches (is_substring_match=False) - pytest.param("foo/", ["foo"], False, True, id="'foo/' matches 'foo' exactly"), - pytest.param("bar/foo/", ["bar", "foo"], False, True, id="'bar/foo/' matches 'bar,foo' exactly"), - pytest.param("foo/", ["foobar"], False, False, id="'foo/' does not match 'foobar' exactly"), - pytest.param("foo/", ["bar", "foo"], False, False, id="'foo/' does not match 'bar,foo' exactly (not toplevel)"), - # Substring matches (is_substring_match=True) - pytest.param("foo/", ["foobar"], True, True, id="'foo/' matches 'foobar' as substring"), + # Substring matches, relative patterns (is_substring_match=True) + pytest.param("bar/foo", ["bar", "foobar"], True, True, id="R: 'bar/foo' matches ['bar', 'foobar'] as substring"), pytest.param( - "bar/foo/", ["bar", "the_foobar_method"], True, True, id="'bar/foo/' matches 'bar,the_foobar_method' as substring" + "bar/foo", ["mod", "bar", "foobar"], True, True, id="R: 'bar/foo' matches ['mod', 'bar', 'foobar'] as substring (suffix)" + ), + pytest.param("bar/foo", ["bar", "bazfoo"], True, True, id="R: 'bar/foo' matches ['bar', 'bazfoo'] as substring"), + pytest.param("bar/fo", ["bar", "foo"], True, True, id="R: 'bar/fo' matches ['bar', 'foo'] as substring"), + pytest.param("bar/foo", ["bar", "baz"], True, False, id="R: 'bar/foo' does not match ['bar', 'baz'] (last no substr)"), + pytest.param( + "bar/foo", ["baz", "foobar"], True, False, id="R: 'bar/foo' does not match ['baz', 'foobar'] (first part mismatch)" ), pytest.param( - "bar/foo/", + "bar/foo", ["bar", "my_foobar_method"], True, True, id="R: 'bar/foo' matches ['bar', 'my_foobar_method'] as substring" + ), + pytest.param( + "bar/foo", + ["mod", "bar", "my_foobar_method"], + True, + True, + id="R: 'bar/foo' matches ['mod', 'bar', 'my_foobar_method'] as substring (suffix)", + ), + pytest.param( + "bar/foo", ["bar", "another_method"], True, False, - id="'bar/foo/' does not match 'bar,another_method' as substring (last no substr)", + id="R: 'bar/foo' does not match ['bar', 'another_method'] (last no substr)", ), pytest.param( - "bar/foo/", - ["baz", "the_foobar_method"], + "bar/foo", + ["other", "my_foobar_method"], True, False, - id="'bar/foo/' does not match 'baz,the_foobar_method' as substring (first mismatch)", + id="R: 'bar/foo' does not match ['other', 'my_foobar_method'] (first part mismatch)", ), - # from docstring examples - pytest.param("foo/", ["my_foobar_func"], True, True, id="'foo/' matches 'my_foobar_func' as substring"), + pytest.param("bar/f", ["bar", "foo"], True, True, id="R: 'bar/f' matches ['bar', 'foo'] as substring"), + # Exact matches, absolute patterns (is_substring_match=False) + pytest.param("/bar/foo", ["bar", "foo"], False, True, id="A: '/bar/foo' matches ['bar', 'foo'] exactly"), pytest.param( - "foo/", - ["ClassA", "my_foobar_method"], - True, - False, - id="'foo/' does not match 'ClassA,my_foobar_method' as substring (not toplevel)", + "/bar/foo", ["bar", "foo", "baz"], False, False, id="A: '/bar/foo' does not match ['bar', 'foo', 'baz'] (pattern shorter)" + ), + pytest.param("/bar/foo", ["bar"], False, False, id="A: '/bar/foo' does not match ['bar'] (pattern longer)"), + pytest.param("/bar/foo", ["baz", "foo"], False, False, id="A: '/bar/foo' does not match ['baz', 'foo'] (first part mismatch)"), + pytest.param("/bar/foo", ["bar", "baz"], False, False, id="A: '/bar/foo' does not match ['bar', 'baz'] (last part mismatch)"), + # Substring matches (is_substring_match=True) + pytest.param("/bar/foo", ["bar", "foobar"], True, True, id="A: '/bar/foo' matches ['bar', 'foobar'] as substring"), + pytest.param("/bar/foo", ["bar", "bazfoo"], True, True, id="A: '/bar/foo' matches ['bar', 'bazfoo'] as substring"), + pytest.param("/bar/fo", ["bar", "foo"], True, True, id="A: '/bar/fo' matches ['bar', 'foo'] as substring"), + pytest.param("/bar/foo", ["bar", "baz"], True, False, id="A: '/bar/foo' does not match ['bar', 'baz'] (last no substr)"), + pytest.param( + "/bar/foo", ["baz", "foobar"], True, False, id="A: '/bar/foo' does not match ['baz', 'foobar'] (first part mismatch)" ), - pytest.param("foo/", ["foo"], True, True, id="'foo/' matches 'foo' as substring (exact is substr)"), ], ) - def test_match_name_pattern_ending_in_slash(self, name_pattern, qual_name_parts, is_substring_match, expected): - """Tests matching for patterns ending with '/' (prefix/namespace style).""" - result = Symbol.match_against_qualname(name_pattern, qual_name_parts, is_substring_match) - error_msg = self._create_assertion_error_message(name_pattern, qual_name_parts, is_substring_match, expected, result) + 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) + error_msg = self._create_assertion_error_message(name_path_pattern, symbol_name_path_parts, is_substring_match, expected, result) assert result == expected, error_msg