Merge pull request #263 from oraios/fix_file_ignores_and_pattern_search

Fix file ignores and pattern search
This commit is contained in:
Michael Panchenko
2025-07-01 14:23:53 +02:00
committed by GitHub
7 changed files with 181 additions and 147 deletions
+12 -3
View File
@@ -20,10 +20,19 @@ 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(
r"def request_parsed_files.*?\).*?\)",
restrict_search_to_code_files=False,
relative_path="src/solidlsp",
paths_include_glob="**/ls.py",
)
)
pprint(json.loads(result))
+54 -22
View File
@@ -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:
"""
@@ -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})
@@ -2172,24 +2173,53 @@ 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,
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:
"""
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` 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
@@ -2202,34 +2232,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,
pattern=substring_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,
substring_pattern,
root_path=self.get_project_root(),
paths_include_glob=paths_include_glob,
paths_exclude_glob=paths_exclude_glob,
)
+48 -62
View File
@@ -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,
+24 -7
View File
@@ -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,12 +276,29 @@ 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, 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
# 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("/"):
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)
+35 -37
View File
@@ -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
@@ -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(), root_path=self.repository_root_path)
def _shutdown(self, timeout: float = 5.0):
"""
@@ -710,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(
@@ -1201,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",
@@ -1210,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,
@@ -1236,20 +1235,19 @@ 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
: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_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,
+6 -14
View File
@@ -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
+2 -2
View File
@@ -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()