From 993df0c25c090b76e64c69c868f261159c769a88 Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Mon, 30 Jun 2025 19:40:05 +0200 Subject: [PATCH 1/6] Bugfix: match_path for gitignores was not working with anchored patterns --- src/serena/agent.py | 1 + src/serena/util/file_system.py | 15 +++++++++++---- src/solidlsp/ls.py | 15 +-------------- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/serena/agent.py b/src/serena/agent.py index 7da88e1..0098e83 100644 --- a/src/serena/agent.py +++ b/src/serena/agent.py @@ -1563,6 +1563,7 @@ class FindFileTool(Tool): recursive=True, is_ignored_dir=self.agent.path_is_gitignored, is_ignored_file=is_ignored_file, + relative_to=self.get_project_root(), ) result = json.dumps({"files": files}) diff --git a/src/serena/util/file_system.py b/src/serena/util/file_system.py index 4e88a9a..f16b46a 100644 --- a/src/serena/util/file_system.py +++ b/src/serena/util/file_system.py @@ -276,12 +276,19 @@ class GitignoreParser: self._load_gitignore_files() -def match_path(path: str, path_spec: PathSpec) -> bool: - path = os.path.abspath(path) - normalized_path = str(path).replace(os.path.sep, "/") +def match_path(relative_path: str, path_spec: PathSpec) -> bool: + normalized_path = str(relative_path).replace(os.path.sep, "/") + + # We can have patterns like /src/..., which would only match corresponding paths from the repo root + # Unfortunately, pathspec can't know whether a relative path is relative to the repo root or not, + # so it will never match src/... + # The fix is to just always assume that the input path is relative to the repo root and to + # prefix it with /. + if not normalized_path.startswith("/"): + normalized_path = "/" + normalized_path # pathspec can't handle the matching of directories if they don't end with a slash! # see https://github.com/cpburnz/python-pathspec/issues/89 - if os.path.isdir(normalized_path) and not normalized_path.endswith("/"): + if os.path.isdir(relative_path) and not normalized_path.endswith("/"): normalized_path = normalized_path + "/" return path_spec.match_file(normalized_path) diff --git a/src/solidlsp/ls.py b/src/solidlsp/ls.py index aa3879b..f77ed99 100644 --- a/src/solidlsp/ls.py +++ b/src/solidlsp/ls.py @@ -318,20 +318,7 @@ class SolidLanguageServer(ABC): if self.is_ignored_dirname(part): return True - # Use pathspec for gitignore-style pattern matching - # Normalize path separators for pathspec (it expects forward slashes) - normalized_path = str(rel_path).replace(os.path.sep, "/") - - # pathspec can't handle the matching of directories if they don't end with a slash! - # see https://github.com/cpburnz/python-pathspec/issues/89 - if os.path.isdir(os.path.join(self.repository_root_path, normalized_path)) and not normalized_path.endswith("/"): - normalized_path = normalized_path + "/" - - # Use the pathspec matcher to check if the path matches any ignore pattern - if self.get_ignore_spec().match_file(normalized_path): - return True - - return False + return match_path(relative_path, self.get_ignore_spec()) def _shutdown(self, timeout: float = 5.0): """ From 81e0a324a5dc7fbba90d93c222ac679c90b387c3 Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Mon, 30 Jun 2025 19:43:17 +0200 Subject: [PATCH 2/6] Possibility to restrict paths to search for The LLM also tries passing the relative_path param to the tool --- scripts/demo_run_tools.py | 12 +++-- src/serena/agent.py | 33 +++++++----- .../language_servers/solargraph/solargraph.py | 1 + src/solidlsp/ls.py | 54 ++++++++++++------- 4 files changed, 63 insertions(+), 37 deletions(-) diff --git a/scripts/demo_run_tools.py b/scripts/demo_run_tools.py index 8822db7..7679372 100644 --- a/scripts/demo_run_tools.py +++ b/scripts/demo_run_tools.py @@ -20,10 +20,16 @@ class InMemorySerenaConfig(SerenaConfigBase): if __name__ == "__main__": - agent = SerenaAgent(project=REPO_ROOT) + agent = SerenaAgent(project=REPO_ROOT, serena_config=InMemorySerenaConfig()) # apply a tool find_refs_tool = agent.get_tool(FindReferencingSymbolsTool) - print("Finding the symbol 'SyncLanguageServer'\n") - result = agent.execute_task(lambda: find_refs_tool.apply(name_path="SolidLanguageServer", relative_path="src/solidlsp/ls.py")) + find_file_tool = agent.get_tool(FindFileTool) + search_pattern_tool = agent.get_tool(SearchForPatternTool) + + result = agent.execute_task( + lambda: search_pattern_tool.apply( + ".*Pyright.*|.*Omnisharp.*", restrict_search_to_code_files=False, relative_path="src/solidlsp/ls.py" + ) + ) pprint(json.loads(result)) diff --git a/src/serena/agent.py b/src/serena/agent.py index 0098e83..25d2944 100644 --- a/src/serena/agent.py +++ b/src/serena/agent.py @@ -2178,6 +2178,7 @@ class SearchForPatternTool(Tool): context_lines_after: int = 0, paths_include_glob: str | None = None, paths_exclude_glob: str | None = None, + relative_path: str = "", restrict_search_to_code_files: bool = False, max_answer_chars: int = _DEFAULT_MAX_ANSWER_LENGTH, ) -> str: @@ -2191,6 +2192,8 @@ class SearchForPatternTool(Tool): :param context_lines_after: Number of lines of context to include after each match :param paths_include_glob: optional glob pattern specifying files to include in the search; if not provided, search globally. :param paths_exclude_glob: optional glob pattern specifying files to exclude from the search (takes precedence over paths_include_glob). + :param relative_path: only subpaths of this path (relative to the repo root) will be analyzed. If a path to a single + file is passed, only that will be searched. The path must exist, otherwise a FileNotFoundError will be raised. :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 @@ -2203,34 +2206,36 @@ class SearchForPatternTool(Tool): which is why it is the default. :return: A JSON object mapping file paths to lists of matched consecutive lines (with context, if requested). """ - # this was previously a kwarg and was true by default - # However, the LLM doesn't really know which files are taken into account by the language server - # and which onees + abs_path = os.path.join(self.get_project_root(), relative_path) + if not os.path.exists(abs_path): + raise FileNotFoundError(f"Relative path {relative_path} does not exist.") + if restrict_search_to_code_files: matches = self.language_server.search_files_for_pattern( pattern=pattern, + relative_path=relative_path, context_lines_before=context_lines_before, context_lines_after=context_lines_after, paths_include_glob=paths_include_glob, paths_exclude_glob=paths_exclude_glob, ) else: - # we walk through all files in the project starting from the root - project_root = self.get_project_root() - rel_paths_to_search = [] - for root, dirs, files in os.walk(project_root): - # don't explore ignored dirs - dirs[:] = [d for d in dirs if not self.agent.path_is_gitignored(os.path.join(root, d))] - for file in files: - file_path = os.path.join(root, file) - if not self.agent.path_is_gitignored(file_path): - relative_path = os.path.relpath(file_path, project_root) - rel_paths_to_search.append(relative_path) + if os.path.isfile(abs_path): + rel_paths_to_search = [relative_path] + else: + dirs, rel_paths_to_search = scan_directory( + path=abs_path, + recursive=True, + is_ignored_dir=self.agent.path_is_gitignored, + is_ignored_file=self.agent.path_is_gitignored, + relative_to=self.get_project_root(), + ) # TODO (maybe): not super efficient to walk through the files again and filter if glob patterns are provided # but it probably never matters and this version required no further refactoring matches = search_files( rel_paths_to_search, pattern, + root_path=self.get_project_root(), paths_include_glob=paths_include_glob, paths_exclude_glob=paths_exclude_glob, ) diff --git a/src/solidlsp/language_servers/solargraph/solargraph.py b/src/solidlsp/language_servers/solargraph/solargraph.py index 2eea3ec..171cc88 100644 --- a/src/solidlsp/language_servers/solargraph/solargraph.py +++ b/src/solidlsp/language_servers/solargraph/solargraph.py @@ -10,6 +10,7 @@ import pathlib import stat import subprocess import threading + from overrides import override from solidlsp.ls import SolidLanguageServer diff --git a/src/solidlsp/ls.py b/src/solidlsp/ls.py index f77ed99..a4d8f92 100644 --- a/src/solidlsp/ls.py +++ b/src/solidlsp/ls.py @@ -5,7 +5,6 @@ import logging import os import pathlib import pickle -import re import subprocess import threading from abc import ABC, abstractmethod @@ -20,6 +19,7 @@ import pathspec import tqdm from serena.text_utils import MatchedConsecutiveLines, search_files +from serena.util.file_system import match_path from solidlsp import ls_types from solidlsp.ls_config import Language, LanguageServerConfig from solidlsp.ls_exceptions import LanguageServerException @@ -697,11 +697,13 @@ class SolidLanguageServer(ABC): for ref in references ] - def retrieve_full_file_content(self, relative_file_path: str) -> str: + def retrieve_full_file_content(self, file_path: str) -> str: """ Retrieve the full content of the given file. """ - with self.open_file(relative_file_path) as file_data: + if os.path.isabs(file_path): + file_path = os.path.relpath(file_path, self.repository_root_path) + with self.open_file(file_path) as file_data: return file_data.contents def retrieve_content_around_line( @@ -1188,8 +1190,11 @@ class SolidLanguageServer(ABC): symbol_body = symbol_body[symbol_start_column:] return symbol_body - def request_parsed_files(self) -> list[str]: - """Retrieves relative paths of all files analyzed by the Language Server.""" + def request_parsed_files(self, relative_path: str = "") -> list[str]: + """Retrieves relative paths of all files analyzed by the Language Server. + + :param relative_path: will only retrieve files that are subpaths of this. + """ if not self.server_started: self.logger.log( "request_parsed_files called before Language Server started", @@ -1197,23 +1202,30 @@ class SolidLanguageServer(ABC): ) raise LanguageServerException("Language Server not started") rel_file_paths = [] - for root, dirs, files in os.walk(self.repository_root_path, followlinks=True): - dirs[:] = [d for d in dirs if not self.is_ignored_path(os.path.join(root, d))] - for file in files: - rel_file_path = os.path.relpath(os.path.join(root, file), start=self.repository_root_path) - try: - if not self.is_ignored_path(rel_file_path): - rel_file_paths.append(rel_file_path) - except FileNotFoundError: - self.logger.log( - f"File {rel_file_path} not found (possibly due it being a symlink), skipping it in request_parsed_files", - logging.WARNING, - ) - return rel_file_paths + start_path = os.path.join(self.repository_root_path, relative_path) + if not os.path.exists(start_path): + raise FileNotFoundError(f"Relative path {start_path} not found.") + if os.path.isfile(start_path): + return [relative_path] + else: + for root, dirs, files in os.walk(start_path, followlinks=True): + dirs[:] = [d for d in dirs if not self.is_ignored_path(os.path.join(root, d))] + for file in files: + rel_file_path = os.path.relpath(os.path.join(root, file), start=self.repository_root_path) + try: + if not self.is_ignored_path(rel_file_path): + rel_file_paths.append(rel_file_path) + except FileNotFoundError: + self.logger.log( + f"File {rel_file_path} not found (possibly due it being a symlink), skipping it in request_parsed_files", + logging.WARNING, + ) + return rel_file_paths def search_files_for_pattern( self, - pattern: re.Pattern | str, + pattern: str, + relative_path: str = "", context_lines_before: int = 0, context_lines_after: int = 0, paths_include_glob: str | None = None, @@ -1223,6 +1235,7 @@ class SolidLanguageServer(ABC): Search for a pattern across all files analyzed by the Language Server. :param pattern: Regular expression pattern to search for, either as a compiled Pattern or string + :param relative_path: :param context_lines_before: Number of lines of context to include before each match :param context_lines_after: Number of lines of context to include after each match :param paths_include_glob: Glob pattern to filter which files to include in the search @@ -1232,11 +1245,12 @@ class SolidLanguageServer(ABC): if isinstance(pattern, str): pattern = re.compile(pattern) - relative_file_paths = self.request_parsed_files() + relative_file_paths = self.request_parsed_files(relative_path=relative_path) return search_files( relative_file_paths, pattern, file_reader=self.retrieve_full_file_content, + root_path=self.repository_root_path, context_lines_before=context_lines_before, context_lines_after=context_lines_after, paths_include_glob=paths_include_glob, From 4d3281b7e781229a8d13f75a6fa01c7ec7bdd22a Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Mon, 30 Jun 2025 21:17:28 +0200 Subject: [PATCH 3/6] Simplify search_text interface and handling of patterns --- scripts/demo_run_tools.py | 4 +- src/serena/text_utils.py | 110 ++++++++++++++------------------- src/solidlsp/ls.py | 3 - test/serena/test_text_utils.py | 20 ++---- 4 files changed, 55 insertions(+), 82 deletions(-) diff --git a/scripts/demo_run_tools.py b/scripts/demo_run_tools.py index 7679372..12d8479 100644 --- a/scripts/demo_run_tools.py +++ b/scripts/demo_run_tools.py @@ -28,8 +28,6 @@ if __name__ == "__main__": search_pattern_tool = agent.get_tool(SearchForPatternTool) result = agent.execute_task( - lambda: search_pattern_tool.apply( - ".*Pyright.*|.*Omnisharp.*", restrict_search_to_code_files=False, relative_path="src/solidlsp/ls.py" - ) + lambda: search_pattern_tool.apply("\n[^\n]*?Pyright", restrict_search_to_code_files=False, relative_path="src/solidlsp/ls.py") ) pprint(json.loads(result)) diff --git a/src/serena/text_utils.py b/src/serena/text_utils.py index 1b9deb2..4be99d7 100644 --- a/src/serena/text_utils.py +++ b/src/serena/text_utils.py @@ -1,5 +1,6 @@ import fnmatch import logging +import os import re from collections.abc import Callable from dataclasses import dataclass, field @@ -111,8 +112,29 @@ class MatchedConsecutiveLines: return cls(lines=text_lines, source_file_path=source_file_path) +def glob_to_regex(glob_pat: str) -> str: + regex_parts: list[str] = [] + i = 0 + while i < len(glob_pat): + ch = glob_pat[i] + if ch == "*": + regex_parts.append(".*") + elif ch == "?": + regex_parts.append(".") + elif ch == "\\": + i += 1 + if i < len(glob_pat): + regex_parts.append(re.escape(glob_pat[i])) + else: + regex_parts.append("\\") + else: + regex_parts.append(re.escape(ch)) + i += 1 + return "".join(regex_parts) + + def search_text( - pattern: str | re.Pattern[str], + pattern: str, content: str | None = None, source_file_path: str | None = None, allow_multiline_match: bool = False, @@ -123,20 +145,18 @@ def search_text( """ Search for a pattern in text content. Supports both regex and glob-like patterns. - Args: - pattern: Pattern to search for (regex or glob-like pattern) - content: The text content to search. May be None if source_file_path is provided. - source_file_path: Optional path to the source file. If content is None, - this has to be passed and the file will be read. - allow_multiline_match: Whether to search across multiple lines. Currently, the default - option (False) is very inefficient, so it is recommended to set this to True. - context_lines_before: Number of context lines to include before matches - context_lines_after: Number of context lines to include after matches - is_glob: If True, pattern is treated as a glob-like pattern (e.g., "*.py", "test_??.py") - and will be converted to regex internally + :param pattern: Pattern to search for (regex or glob-like pattern) + :param content: The text content to search. May be None if source_file_path is provided. + :param source_file_path: Optional path to the source file. If content is None, + this has to be passed and the file will be read. + :param allow_multiline_match: Whether to search across multiple lines. Currently, the default + option (False) is very inefficient, so it is recommended to set this to True. + :param context_lines_before: Number of context lines to include before matches + :param context_lines_after: Number of context lines to include after matches + :param is_glob: If True, pattern is treated as a glob-like pattern (e.g., "*.py", "test_??.py") + and will be converted to regex internally - Returns: - List of TextSearchMatch objects + :return: List of `TextSearchMatch` objects :raises: ValueError if the pattern is not valid @@ -149,52 +169,15 @@ def search_text( raise ValueError("Pass either content or source_file_path") matches = [] - - # Convert pattern to a compiled regex if it's a string - if is_glob and isinstance(pattern, str): - # Convert glob pattern with optional backslash escaping to regex - def glob_to_regex(glob_pat: str) -> str: - regex_parts: list[str] = [] - i = 0 - while i < len(glob_pat): - ch = glob_pat[i] - if ch == "*": - regex_parts.append(".*") - elif ch == "?": - regex_parts.append(".") - elif ch == "\\": - i += 1 - if i < len(glob_pat): - regex_parts.append(re.escape(glob_pat[i])) - else: - regex_parts.append("\\") - else: - regex_parts.append(re.escape(ch)) - i += 1 - return "".join(regex_parts) - - escaped_pattern = glob_to_regex(pattern) - # For glob patterns, don't anchor with ^ and $ to allow partial line matches - compiled_pattern = re.compile(escaped_pattern) - elif isinstance(pattern, str): - try: - compiled_pattern = re.compile(pattern) - except re.error as e: - raise ValueError(f"Invalid regex pattern: {e}") from e - else: - # Pattern is already a compiled regex - compiled_pattern = pattern - - # Split the content into lines for processing lines = content.splitlines() total_lines = len(lines) + # Convert pattern to a compiled regex if it's a string + if is_glob: + pattern = glob_to_regex(pattern) if allow_multiline_match: # For multiline matches, we need to use the DOTALL flag to make '.' match newlines - if isinstance(pattern, str): - # If we've compiled the pattern ourselves, we need to recompile with DOTALL - pattern_str = compiled_pattern.pattern - compiled_pattern = re.compile(pattern_str, re.DOTALL) + compiled_pattern = re.compile(pattern, re.DOTALL) # Search across the entire content as a single string for match in compiled_pattern.finditer(content): start_pos = match.start() @@ -225,7 +208,8 @@ def search_text( else: # TODO: extremely inefficient! Since we currently don't use this option in SerenaAgent or LanguageServer, # it is not urgent to fix, but should be either improved or the option should be removed. - # Search line by line + # Search line by line, normal compile without DOTALL + compiled_pattern = re.compile(pattern) for i, line in enumerate(lines): line_num = i + 1 if compiled_pattern.search(line): @@ -304,8 +288,9 @@ def glob_match(pattern: str, path: str) -> bool: def search_files( - file_paths: list[str], - pattern: re.Pattern | str, + relative_file_paths: list[str], + pattern: str, + root_path: str = "", file_reader: Callable[[str], str] = default_file_reader, context_lines_before: int = 0, context_lines_after: int = 0, @@ -315,8 +300,9 @@ def search_files( """ Search for a pattern in a list of files. - :param file_paths: List of files in which to search + :param relative_file_paths: List of relative file paths in which to search :param pattern: Pattern to search for + :param root_path: Root path to resolve relative paths against (by default, current working directory). :param file_reader: Function to read a file, by default will just use os.open. All files that can't be read by it will be skipped. :param context_lines_before: Number of context lines to include before matches @@ -327,9 +313,8 @@ def search_files( """ # Pre-filter paths (done sequentially to avoid overhead) # Use proper glob matching instead of gitignore patterns - filtered_paths = [] - for path in file_paths: + for path in relative_file_paths: if paths_include_glob and not glob_match(paths_include_glob, path): log.debug(f"Skipping {path}: does not match include pattern {paths_include_glob}") continue @@ -343,7 +328,8 @@ def search_files( def process_single_file(path: str) -> dict[str, Any]: """Process a single file - this function will be parallelized.""" try: - file_content = file_reader(path) + abs_path = os.path.join(root_path, path) + file_content = file_reader(abs_path) search_results = search_text( pattern, content=file_content, diff --git a/src/solidlsp/ls.py b/src/solidlsp/ls.py index a4d8f92..77d35ba 100644 --- a/src/solidlsp/ls.py +++ b/src/solidlsp/ls.py @@ -1242,9 +1242,6 @@ class SolidLanguageServer(ABC): :param paths_exclude_glob: Glob pattern to filter which files to exclude from the search. Takes precedence over paths_include_glob. :return: List of matched consecutive lines with context """ - if isinstance(pattern, str): - pattern = re.compile(pattern) - relative_file_paths = self.request_parsed_files(relative_path=relative_path) return search_files( relative_file_paths, diff --git a/test/serena/test_text_utils.py b/test/serena/test_text_utils.py index 86ff975..d122f6a 100644 --- a/test/serena/test_text_utils.py +++ b/test/serena/test_text_utils.py @@ -214,14 +214,6 @@ class TestSearchText: assert len(matches) == 0 - def test_search_text_invalid_regex(self): - """Test searching with an invalid regex pattern raises ValueError.""" - content = "def example(): pass" - - # Search with an invalid regex pattern (unmatched parenthesis) - with pytest.raises(ValueError): - search_text("example(", content=content) - # Mock file reader that always returns matching content def mock_reader_always_match(file_path: str) -> str: @@ -262,7 +254,7 @@ class TestSearchFiles: Test the include/exclude glob filtering logic in search_files using PathSpec patterns. """ results = search_files( - file_paths=file_paths, + relative_file_paths=file_paths, pattern=pattern, file_reader=mock_reader_always_match, paths_include_glob=paths_include_glob, @@ -341,7 +333,7 @@ class TestSearchFiles: Test glob patterns that were problematic with the previous gitignore-based implementation. """ results = search_files( - file_paths=file_paths, + relative_file_paths=file_paths, pattern=pattern, file_reader=mock_reader_always_match, paths_include_glob=paths_include_glob, @@ -371,7 +363,7 @@ class TestSearchFiles: file_paths = ["a.py", "b.txt"] pattern = "non_existent_pattern_in_mock_content" # This won't match mock_reader_always_match content results = search_files( - file_paths=file_paths, + relative_file_paths=file_paths, pattern=pattern, file_reader=mock_reader_always_match, # Content is "This line contains a match." paths_include_glob=None, # Both files would pass filters @@ -393,10 +385,10 @@ class TestSearchFiles: return "No values here." file_paths = ["a.py", "b.py", "c.txt"] - pattern = re.compile(r"value=(\d+)") # Regex pattern to find numbers after 'value=' + pattern = r"value=(\d+)" results = search_files( - file_paths=file_paths, + relative_file_paths=file_paths, pattern=pattern, file_reader=specific_mock_reader, paths_include_glob="*.py", # Only include .py files @@ -426,7 +418,7 @@ class TestSearchFiles: pattern = "MATCH HERE" results = search_files( - file_paths=file_paths, + relative_file_paths=file_paths, pattern=pattern, file_reader=context_mock_reader, paths_include_glob="*.txt", # Only include .txt files From c2c7d35e9998c3a673d24a90ee0ed5a04b6645c9 Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Tue, 1 Jul 2025 12:20:27 +0200 Subject: [PATCH 4/6] Extended docstring of SearchForPatternTool The LM had a hard time doing good searches. --- scripts/demo_run_tools.py | 7 ++++++- src/serena/agent.py | 42 +++++++++++++++++++++++++++++++-------- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/scripts/demo_run_tools.py b/scripts/demo_run_tools.py index 12d8479..e3c9286 100644 --- a/scripts/demo_run_tools.py +++ b/scripts/demo_run_tools.py @@ -28,6 +28,11 @@ if __name__ == "__main__": search_pattern_tool = agent.get_tool(SearchForPatternTool) result = agent.execute_task( - lambda: search_pattern_tool.apply("\n[^\n]*?Pyright", restrict_search_to_code_files=False, relative_path="src/solidlsp/ls.py") + lambda: search_pattern_tool.apply( + r"def request_parsed_files.*?\).*?\)", + restrict_search_to_code_files=False, + relative_path="src/solidlsp", + paths_include_glob="**/ls.py", + ) ) pprint(json.loads(result)) diff --git a/src/serena/agent.py b/src/serena/agent.py index 25d2944..522b6c2 100644 --- a/src/serena/agent.py +++ b/src/serena/agent.py @@ -2173,7 +2173,7 @@ class SearchForPatternTool(Tool): def apply( self, - pattern: str, + substring_pattern: str, context_lines_before: int = 0, context_lines_after: int = 0, paths_include_glob: str | None = None, @@ -2183,17 +2183,43 @@ class SearchForPatternTool(Tool): max_answer_chars: int = _DEFAULT_MAX_ANSWER_LENGTH, ) -> str: """ - Search for a pattern in the project. You can select whether all files or only code files should be searched. + Offers a flexible search for arbitrary patterns in the codebase, including the + possibility to search in non-code files. Generally, symbolic operations like find_symbol or find_referencing_symbols should be preferred if you know which symbols you are looking for. - :param pattern: Regular expression pattern to search for, either as a compiled Pattern or string + Pattern Matching Logic: + For each match, the returned result will contain the full lines where the + substring pattern is found, as well as optionally some lines before and after it. The pattern will be compiled with + DOTALL, meaning that the dot will match all characters including newlines. + This also means that it never makes sense to have .* at the beginning or end of the pattern, + but it may make sense to have it in the middle for complex patterns. + If a pattern matches multiple lines, all those lines will be part of the match. + Be careful to not use greedy quantifiers unnecessarily, it is usually better to use non-greedy quantifiers like .*? to avoid + matching too much content. + + File Selection Logic: + The files in which the search is performed can be restricted very flexibly. + Using `restrict_search_to_code_files` is useful if you are only interested in code symbols (i.e., those + symbols that can be manipulated with symbolic tools like find_symbol). + You can also restrict the search to a specific file or directory, + and provide glob patterns to include or exclude certain files on top of that. + The globs are matched against relative file paths from the project root (not to the `relative_path` parameter that + is used to further restrict the search). + Smartly combining the various restrictions allows you to perform very targeted searches. + + + :param substring_pattern: Regular expression for a substring pattern to search for :param context_lines_before: Number of lines of context to include before each match :param context_lines_after: Number of lines of context to include after each match - :param paths_include_glob: optional glob pattern specifying files to include in the search; if not provided, search globally. - :param paths_exclude_glob: optional glob pattern specifying files to exclude from the search (takes precedence over paths_include_glob). + :param paths_include_glob: optional glob pattern specifying files to include in the search. + Matches against relative file paths from the project root (e.g., "*.py", "src/**/*.ts"). + Only matches files, not directories. + :param paths_exclude_glob: optional glob pattern specifying files to exclude from the search. + Matches against relative file paths from the project root (e.g., "*test*", "**/*_generated.py"). + Takes precedence over paths_include_glob. Only matches files, not directories. :param relative_path: only subpaths of this path (relative to the repo root) will be analyzed. If a path to a single - file is passed, only that will be searched. The path must exist, otherwise a FileNotFoundError will be raised. + file is passed, only that will be searched. The path must exist, otherwise a `FileNotFoundError` is raised. :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 @@ -2212,7 +2238,7 @@ class SearchForPatternTool(Tool): if restrict_search_to_code_files: matches = self.language_server.search_files_for_pattern( - pattern=pattern, + pattern=substring_pattern, relative_path=relative_path, context_lines_before=context_lines_before, context_lines_after=context_lines_after, @@ -2234,7 +2260,7 @@ class SearchForPatternTool(Tool): # but it probably never matters and this version required no further refactoring matches = search_files( rel_paths_to_search, - pattern, + substring_pattern, root_path=self.get_project_root(), paths_include_glob=paths_include_glob, paths_exclude_glob=paths_exclude_glob, From 790ca6eac4896f0db2e008f001768ddeef5ca47b Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Tue, 1 Jul 2025 14:05:53 +0200 Subject: [PATCH 5/6] Further fixes in path matching (consider the root path for isdir check) --- src/serena/agent.py | 2 +- src/serena/util/file_system.py | 20 +++++++++++++++----- src/solidlsp/ls.py | 2 +- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/serena/agent.py b/src/serena/agent.py index 522b6c2..aa78640 100644 --- a/src/serena/agent.py +++ b/src/serena/agent.py @@ -865,7 +865,7 @@ class SerenaAgent: if len(relative_path.parts) > 0 and relative_path.parts[0] == ".git": return True - return match_path(str(relative_path), self.ignore_spec) + return match_path(str(relative_path), self.ignore_spec, root_path=self.get_project_root()) def validate_relative_path(self, relative_path: str) -> None: """ diff --git a/src/serena/util/file_system.py b/src/serena/util/file_system.py index f16b46a..26a5943 100644 --- a/src/serena/util/file_system.py +++ b/src/serena/util/file_system.py @@ -92,14 +92,14 @@ class GitignoreSpec: """Initialize the PathSpec from patterns.""" self.pathspec = PathSpec.from_lines(pathspec.patterns.GitWildMatchPattern, self.patterns) - def matches(self, path: str) -> bool: + def matches(self, relative_path: str) -> bool: """ Check if the given path matches any pattern in this gitignore spec. - :param path: Path to check (should be relative to repo root) + :param relative_path: Path to check (should be relative to repo root) :return: True if path matches any pattern """ - return self.pathspec.match_file(path) + return match_path(relative_path, self.pathspec, root_path=os.path.dirname(self.file_path)) class GitignoreParser: @@ -276,7 +276,16 @@ class GitignoreParser: self._load_gitignore_files() -def match_path(relative_path: str, path_spec: PathSpec) -> bool: +def match_path(relative_path: str, path_spec: PathSpec, root_path: str = "") -> bool: + """ + Match a relative path against a given pathspec. Just pathspec.match_file() is not enough, + we need to do some massaging to fix issues with pathspec matching. + + :param relative_path: relative path to match against the pathspec + :param path_spec: the pathspec to match against + :param root_path: the root path from which the relative path is derived + :return: + """ normalized_path = str(relative_path).replace(os.path.sep, "/") # We can have patterns like /src/..., which would only match corresponding paths from the repo root @@ -289,6 +298,7 @@ def match_path(relative_path: str, path_spec: PathSpec) -> bool: # pathspec can't handle the matching of directories if they don't end with a slash! # see https://github.com/cpburnz/python-pathspec/issues/89 - if os.path.isdir(relative_path) and not normalized_path.endswith("/"): + abs_path = os.path.abspath(os.path.join(root_path, relative_path)) + if os.path.isdir(abs_path) and not normalized_path.endswith("/"): normalized_path = normalized_path + "/" return path_spec.match_file(normalized_path) diff --git a/src/solidlsp/ls.py b/src/solidlsp/ls.py index 77d35ba..4b8163c 100644 --- a/src/solidlsp/ls.py +++ b/src/solidlsp/ls.py @@ -318,7 +318,7 @@ class SolidLanguageServer(ABC): if self.is_ignored_dirname(part): return True - return match_path(relative_path, self.get_ignore_spec()) + return match_path(relative_path, self.get_ignore_spec(), root_path=self.repository_root_path) def _shutdown(self, timeout: float = 5.0): """ From 6c670f54bd7a9c65bb1df7e344068a25304d8d8a Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Tue, 1 Jul 2025 14:23:22 +0200 Subject: [PATCH 6/6] Fix in clojure tests: skipif needs a boolean, not a string --- test/solidlsp/clojure/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/solidlsp/clojure/__init__.py b/test/solidlsp/clojure/__init__.py index ed57686..e5cb3bb 100644 --- a/test/solidlsp/clojure/__init__.py +++ b/test/solidlsp/clojure/__init__.py @@ -3,12 +3,12 @@ from pathlib import Path from solidlsp.language_servers.clojure_lsp.clojure_lsp import verify_clojure_cli -def _test_clojure_cli() -> str | bool: +def _test_clojure_cli() -> bool: try: verify_clojure_cli() return False except (FileNotFoundError, RuntimeError) as e: - return str(e) + return True CLI_FAIL = _test_clojure_cli()