Merge pull request #967 from oraios/name-path-matching-logic

Adjust name path matching logic to consider overload indices at all positions in the name path

src/serena/symbol.py

@@ -115,6 +115,18 @@ class Symbol(ToStringMixin, ABC):
         """
 
 
+class NamePathComponent:
+    def __init__(self, name: str, overload_idx: int | None = None) -> None:
+        self.name = name
+        self.overload_idx = overload_idx
+
+    def __repr__(self) -> str:
+        if self.overload_idx is not None:
+            return f"{self.name}[{self.overload_idx}]"
+        else:
+            return self.name
+
+
 class NamePathMatcher(ToStringMixin):
     """
     Matches name paths of symbols against search patterns.
@@ -131,6 +143,29 @@ class NamePathMatcher(ToStringMixin):
     Append an index `[i]` to match a specific overload only, e.g. "MyClass/my_method[1]".
     """
 
+    class PatternComponent(NamePathComponent):
+        @classmethod
+        def from_string(cls, component_str: str) -> Self:
+            overload_idx = None
+            if component_str.endswith("]") and "[" in component_str:
+                bracket_idx = component_str.rfind("[")
+                index_part = component_str[bracket_idx + 1 : -1]
+                if index_part.isdigit():
+                    component_str = component_str[:bracket_idx]
+                    overload_idx = int(index_part)
+            return cls(name=component_str, overload_idx=overload_idx)
+
+        def matches(self, name_path_component: NamePathComponent, substring_matching: bool) -> bool:
+            if substring_matching:
+                if self.name not in name_path_component.name:
+                    return False
+            else:
+                if self.name != name_path_component.name:
+                    return False
+            if self.overload_idx is not None and self.overload_idx != name_path_component.overload_idx:
+                return False
+            return True
+
     def __init__(self, name_path_pattern: str, substring_matching: bool) -> None:
         """
         :param name_path_pattern: the name path expression to match against
@@ -140,51 +175,32 @@ class NamePathMatcher(ToStringMixin):
         self._expr = name_path_pattern
         self._substring_matching = substring_matching
         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
-        last_part = self._pattern_parts[-1]
-        if last_part.endswith("]") and "[" in last_part:
-            bracket_idx = last_part.rfind("[")
-            index_part = last_part[bracket_idx + 1 : -1]
-            if index_part.isdigit():
-                self._pattern_parts[-1] = last_part[:bracket_idx]
-                self._overload_idx = int(index_part)
+        self._components = [
+            self.PatternComponent.from_string(x) for x in name_path_pattern.lstrip(NAME_PATH_SEP).rstrip(NAME_PATH_SEP).split(NAME_PATH_SEP)
+        ]
 
     def _tostring_includes(self) -> list[str]:
         return ["_expr"]
 
     def matches_ls_symbol(self, symbol: "LanguageServerSymbol") -> bool:
-        return self.matches_components(symbol.get_name_path_parts(), symbol.overload_idx)
+        return self.matches_reversed_components(symbol.iter_name_path_components_reversed())
 
-    def matches_components(self, symbol_name_path_parts: list[str], overload_idx: int | None) -> bool:
-        # filtering based on ancestors
-        if len(self._pattern_parts) > len(symbol_name_path_parts):
-            # can't possibly match if pattern has more parts than symbol
-            return False
-        if self._is_absolute_pattern and len(self._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(self._pattern_parts) : -1] != self._pattern_parts[:-1]:
-            # ancestors must match
-            return False
-
-        # matching the last part of the symbol name
-        name_to_match = self._pattern_parts[-1]
-        symbol_name = symbol_name_path_parts[-1]
-        if self._substring_matching:
-            if name_to_match not in symbol_name:
+    def matches_reversed_components(self, components_reversed: Iterator[NamePathComponent]) -> bool:
+        for i, pattern_component in enumerate(reversed(self._components)):
+            try:
+                symbol_component = next(components_reversed)
+            except StopIteration:
                 return False
-        else:
-            if name_to_match != symbol_name:
+            use_substring_matching = self._substring_matching and (i == 0)
+            if not pattern_component.matches(symbol_component, use_substring_matching):
                 return False
-
-        # check for matching overload index
-        if self._overload_idx is not None:
-            if overload_idx != self._overload_idx:
+        if self._is_absolute_pattern:
+            # ensure that there are no more components in the symbol
+            try:
+                next(components_reversed)
                 return False
-
+            except StopIteration:
+                pass
         return True
 
 
@@ -311,18 +327,13 @@ class LanguageServerSymbol(Symbol, ToStringMixin):
         Get the name path of the symbol, e.g. "class/method/inner_function" or
         "class/method[1]" (overloaded method with identifying index).
         """
-        name_path = NAME_PATH_SEP.join(self.get_name_path_parts())
-        if "overload_idx" in self.symbol_root:
-            name_path += f"[{self.symbol_root['overload_idx']}]"
+        name_path = NAME_PATH_SEP.join(reversed([str(x) for x in self.iter_name_path_components_reversed()]))
         return name_path
 
-    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_name_path_components_reversed(self) -> Iterator[NamePathComponent]:
+        yield NamePathComponent(self.name, self.overload_idx)
+        for ancestor in self.iter_ancestors(up_to_symbol_kind=SymbolKind.File):
+            yield NamePathComponent(ancestor.name, ancestor.overload_idx)
 
     def iter_children(self) -> Iterator[Self]:
         for c in self.symbol_root["children"]:

test/serena/test_symbol.py

@@ -1,6 +1,6 @@
 import pytest
 
-from serena.symbol import LanguageServerSymbolRetriever, NamePathMatcher
+from serena.symbol import LanguageServerSymbolRetriever, NamePathComponent, NamePathMatcher
 from solidlsp import SolidLanguageServer
 from solidlsp.ls_config import Language
 
@@ -9,17 +9,17 @@ class TestSymbolNameMatching:
     def _create_assertion_error_message(
         self,
         name_path_pattern: str,
-        symbol_name_path_parts: list[str],
+        name_path_components: list[NamePathComponent],
         is_substring_match: bool,
         expected_result: bool,
         actual_result: bool,
     ) -> str:
         """Helper to create a detailed error message for assertions."""
-        qnp_repr = "/".join(symbol_name_path_parts)
+        qnp_repr = "/".join(map(str, name_path_components))
 
         return (
             f"Pattern '{name_path_pattern}' (substring: {is_substring_match}) vs "
-            f"Qualname parts {symbol_name_path_parts} (as '{qnp_repr}'). "
+            f"Name path components {name_path_components} (as '{qnp_repr}'). "
             f"Expected: {expected_result}, Got: {actual_result}"
         )
 
@@ -79,7 +79,8 @@ 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 = NamePathMatcher(name_path_pattern, is_substring_match).matches_components(symbol_name_path_parts, None)
+        symbol_name_path_components = [NamePathComponent(part) for part in symbol_name_path_parts]
+        result = NamePathMatcher(name_path_pattern, is_substring_match).matches_reversed_components(reversed(symbol_name_path_components))
         error_msg = self._create_assertion_error_message(name_path_pattern, symbol_name_path_parts, is_substring_match, expected, result)
         assert result == expected, error_msg
 
@@ -146,6 +147,13 @@ class TestSymbolNameMatching:
             ),
             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",
+                ["baz", "bar", "foo"],
+                False,
+                False,
+                id="A: '/bar/foo' does not match ['baz', 'bar', 'foo'] (only suffix match for abs pattern)",
+            ),
             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"),
@@ -159,24 +167,66 @@ 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 = NamePathMatcher(name_path_pattern, is_substring_match).matches_components(symbol_name_path_parts, None)
+        symbol_name_path_components = [NamePathComponent(part) for part in symbol_name_path_parts]
+        result = NamePathMatcher(name_path_pattern, is_substring_match).matches_reversed_components(reversed(symbol_name_path_components))
         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_path_pattern, symbol_name_path_parts, symbol_overload_idx, expected",
+        "name_path_pattern, symbol_name_path_components, expected",
         [
-            pytest.param("bar/foo", ["bar", "foo"], 0, True, id="R: 'bar/foo' matches ['bar', 'foo'] with overload_index=0"),
-            pytest.param("bar/foo", ["bar", "foo"], 1, True, id="R: 'bar/foo' matches ['bar', 'foo'] with overload_index=1"),
-            pytest.param("bar/foo[0]", ["bar", "foo"], 0, True, id="R: 'bar/foo[0]' matches ['bar', 'foo'] with overload_index=0"),
-            pytest.param("bar/foo[1]", ["bar", "foo"], 0, False, id="R: 'bar/foo[1]' does not match ['bar', 'foo'] with overload_index=0"),
+            pytest.param(
+                "bar/foo",
+                [NamePathComponent("bar"), NamePathComponent("foo", 0)],
+                True,
+                id="R: 'bar/foo' matches ['bar', 'foo'] with overload_index=0",
+            ),
+            pytest.param(
+                "bar/foo",
+                [NamePathComponent("bar"), NamePathComponent("foo", 1)],
+                True,
+                id="R: 'bar/foo' matches ['bar', 'foo'] with overload_index=1",
+            ),
+            pytest.param(
+                "bar/foo[0]",
+                [NamePathComponent("bar"), NamePathComponent("foo", 0)],
+                True,
+                id="R: 'bar/foo[0]' matches ['bar', 'foo'] with overload_index=0",
+            ),
+            pytest.param(
+                "bar/foo[1]",
+                [NamePathComponent("bar"), NamePathComponent("foo", 0)],
+                False,
+                id="R: 'bar/foo[1]' does not match ['bar', 'foo'] with overload_index=0",
+            ),
+            pytest.param(
+                "bar/foo", [NamePathComponent("bar", 0), NamePathComponent("foo")], True, id="R: 'bar/foo' matches ['bar[0]', 'foo']"
+            ),
+            pytest.param(
+                "bar/foo", [NamePathComponent("bar", 0), NamePathComponent("foo", 1)], True, id="R: 'bar/foo' matches ['bar[0]', 'foo[1]']"
+            ),
+            pytest.param(
+                "bar[0]/foo", [NamePathComponent("bar", 0), NamePathComponent("foo")], True, id="R: 'bar[0]/foo' matches ['bar[0]', 'foo']"
+            ),
+            pytest.param(
+                "bar[0]/foo[1]",
+                [NamePathComponent("bar", 0), NamePathComponent("foo", 1)],
+                True,
+                id="R: 'bar[0]/foo[1]' matches ['bar[0]', 'foo[1]']",
+            ),
+            pytest.param(
+                "bar[0]/foo[1]",
+                [NamePathComponent("bar", 1), NamePathComponent("foo", 0)],
+                False,
+                id="R: 'bar[0]/foo[1]' does not match ['bar[1]', 'foo[0]']",
+            ),
         ],
     )
-    def test_match_name_path_pattern_with_overload_idx(self, name_path_pattern, symbol_name_path_parts, symbol_overload_idx, expected):
+    def test_match_name_path_pattern_with_overload_idx(self, name_path_pattern, symbol_name_path_components, expected):
         """Tests matching for qualified names (e.g. 'module/class/func')."""
         matcher = NamePathMatcher(name_path_pattern, False)
-        result = matcher.matches_components(symbol_name_path_parts, symbol_overload_idx)
-        error_msg = self._create_assertion_error_message(name_path_pattern, symbol_name_path_parts, False, expected, result)
+        result = matcher.matches_reversed_components(reversed(symbol_name_path_components))
+        error_msg = self._create_assertion_error_message(name_path_pattern, symbol_name_path_components, False, expected, result)
         assert result == expected, error_msg