mirror of
https://github.com/tiennm99/serena.git
synced 2026-09-03 16:17:07 +00:00
Fixes in glob matching, extended tests
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
import fnmatch
|
||||
import logging
|
||||
import re
|
||||
from collections.abc import Callable
|
||||
@@ -6,8 +7,6 @@ from enum import StrEnum
|
||||
from typing import Any, Self
|
||||
|
||||
from joblib import Parallel, delayed
|
||||
from pathspec import PathSpec
|
||||
from pathspec.patterns.gitwildmatch import GitWildMatchPattern
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
@@ -258,6 +257,49 @@ def default_file_reader(file_path: str) -> str:
|
||||
return f.read()
|
||||
|
||||
|
||||
def glob_match(pattern: str, path: str) -> bool:
|
||||
"""
|
||||
Match a file path against a glob pattern.
|
||||
|
||||
Supports standard glob patterns:
|
||||
- * matches any number of characters except /
|
||||
- ** matches any number of directories (zero or more)
|
||||
- ? matches a single character except /
|
||||
- [seq] matches any character in seq
|
||||
|
||||
:param pattern: Glob pattern (e.g., 'src/**/*.py', '**agent.py')
|
||||
:param path: File path to match against
|
||||
:return: True if path matches pattern
|
||||
"""
|
||||
# Handle ** patterns that should match zero or more directories
|
||||
if "**" in pattern:
|
||||
# Method 1: Standard fnmatch (matches one or more directories)
|
||||
regex1 = fnmatch.translate(pattern)
|
||||
if re.match(regex1, path):
|
||||
return True
|
||||
|
||||
# Method 2: Handle zero-directory case by removing /** entirely
|
||||
# Convert "src/**/test.py" to "src/test.py"
|
||||
if "/**/" in pattern:
|
||||
zero_dir_pattern = pattern.replace("/**/", "/")
|
||||
regex2 = fnmatch.translate(zero_dir_pattern)
|
||||
if re.match(regex2, path):
|
||||
return True
|
||||
|
||||
# Method 3: Handle leading ** case by removing **/
|
||||
# Convert "**/test.py" to "test.py"
|
||||
if pattern.startswith("**/"):
|
||||
zero_dir_pattern = pattern[3:] # Remove "**/"
|
||||
regex3 = fnmatch.translate(zero_dir_pattern)
|
||||
if re.match(regex3, path):
|
||||
return True
|
||||
|
||||
return False
|
||||
else:
|
||||
# Simple pattern without **, use fnmatch directly
|
||||
return fnmatch.fnmatch(path, pattern)
|
||||
|
||||
|
||||
def search_files(
|
||||
file_paths: list[str],
|
||||
pattern: re.Pattern | str,
|
||||
@@ -281,15 +323,14 @@ def search_files(
|
||||
:return: List of MatchedConsecutiveLines objects
|
||||
"""
|
||||
# Pre-filter paths (done sequentially to avoid overhead)
|
||||
include_spec = PathSpec.from_lines(GitWildMatchPattern, [paths_include_glob]) if paths_include_glob else None
|
||||
exclude_spec = PathSpec.from_lines(GitWildMatchPattern, [paths_exclude_glob]) if paths_exclude_glob else None
|
||||
# Use proper glob matching instead of gitignore patterns
|
||||
|
||||
filtered_paths = []
|
||||
for path in file_paths:
|
||||
if include_spec and not include_spec.match_file(path):
|
||||
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
|
||||
if exclude_spec and exclude_spec.match_file(path):
|
||||
if paths_exclude_glob and glob_match(paths_exclude_glob, path):
|
||||
log.debug(f"Skipping {path}: matches exclude pattern {paths_exclude_glob}")
|
||||
continue
|
||||
filtered_paths.append(path)
|
||||
|
||||
@@ -285,6 +285,87 @@ class TestSearchFiles:
|
||||
assert result.matched_lines[0].line_content == "This line contains a match."
|
||||
assert result.matched_lines[0].match_type == LineType.MATCH
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"file_paths, pattern, paths_include_glob, paths_exclude_glob, expected_matched_files, description",
|
||||
[
|
||||
# Glob patterns that were problematic with gitignore syntax
|
||||
(
|
||||
["src/serena/agent.py", "src/serena/process_isolated_agent.py", "test/agent.py"],
|
||||
"match",
|
||||
"src/**agent.py",
|
||||
None,
|
||||
["src/serena/agent.py", "src/serena/process_isolated_agent.py"],
|
||||
"Glob: src/**agent.py should match files ending with agent.py under src/",
|
||||
),
|
||||
(
|
||||
["src/serena/agent.py", "src/serena/process_isolated_agent.py", "other/agent.py"],
|
||||
"match",
|
||||
"**agent.py",
|
||||
None,
|
||||
["src/serena/agent.py", "src/serena/process_isolated_agent.py", "other/agent.py"],
|
||||
"Glob: **agent.py should match files ending with agent.py anywhere",
|
||||
),
|
||||
(
|
||||
["dir/subdir/file.py", "dir/other/file.py", "elsewhere/file.py"],
|
||||
"match",
|
||||
"dir/**file.py",
|
||||
None,
|
||||
["dir/subdir/file.py", "dir/other/file.py"],
|
||||
"Glob: dir/**file.py should match files ending with file.py under dir/",
|
||||
),
|
||||
(
|
||||
["src/a/b/c/test.py", "src/x/test.py", "other/test.py"],
|
||||
"match",
|
||||
"src/**/test.py",
|
||||
None,
|
||||
["src/a/b/c/test.py", "src/x/test.py"],
|
||||
"Glob: src/**/test.py should match test.py files under src/ at any depth",
|
||||
),
|
||||
# Edge cases for ** patterns
|
||||
(
|
||||
["agent.py", "src/agent.py", "src/serena/agent.py"],
|
||||
"match",
|
||||
"**agent.py",
|
||||
None,
|
||||
["agent.py", "src/agent.py", "src/serena/agent.py"],
|
||||
"Glob: **agent.py should match at root and any depth",
|
||||
),
|
||||
(["file.txt", "src/file.txt"], "match", "src/**", None, ["src/file.txt"], "Glob: src/** should match everything under src/"),
|
||||
],
|
||||
ids=lambda x: x if isinstance(x, str) else "", # Use description as test ID
|
||||
)
|
||||
def test_search_files_glob_patterns(
|
||||
self, file_paths, pattern, paths_include_glob, paths_exclude_glob, expected_matched_files, description
|
||||
):
|
||||
"""
|
||||
Test glob patterns that were problematic with the previous gitignore-based implementation.
|
||||
"""
|
||||
results = search_files(
|
||||
file_paths=file_paths,
|
||||
pattern=pattern,
|
||||
file_reader=mock_reader_always_match,
|
||||
paths_include_glob=paths_include_glob,
|
||||
paths_exclude_glob=paths_exclude_glob,
|
||||
context_lines_before=0,
|
||||
context_lines_after=0,
|
||||
)
|
||||
|
||||
# Extract the source file paths from the results
|
||||
actual_matched_files = sorted([result.source_file_path for result in results if result.source_file_path])
|
||||
|
||||
# Assert that the matched files are exactly the ones expected
|
||||
assert actual_matched_files == sorted(
|
||||
expected_matched_files
|
||||
), f"Pattern '{paths_include_glob}' failed: expected {sorted(expected_matched_files)}, got {actual_matched_files}"
|
||||
|
||||
# Basic check on results structure if files were expected
|
||||
if expected_matched_files:
|
||||
assert len(results) == len(expected_matched_files)
|
||||
for result in results:
|
||||
assert len(result.matched_lines) == 1 # Mock reader returns one matching line
|
||||
assert result.matched_lines[0].line_content == "This line contains a match."
|
||||
assert result.matched_lines[0].match_type == LineType.MATCH
|
||||
|
||||
def test_search_files_no_pattern_match_in_content(self):
|
||||
"""Test that no results are returned if the pattern doesn't match the file content, even if files pass filters."""
|
||||
file_paths = ["a.py", "b.txt"]
|
||||
@@ -365,3 +446,47 @@ class TestSearchFiles:
|
||||
assert result.lines[1].match_type == LineType.MATCH
|
||||
assert result.lines[2].line_content == "Line after 1", "Incorrect 'after' context line"
|
||||
assert result.lines[2].match_type == LineType.AFTER_MATCH
|
||||
|
||||
|
||||
class TestGlobMatch:
|
||||
"""Test the glob_match function directly."""
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"pattern, path, expected",
|
||||
[
|
||||
# Basic wildcard patterns
|
||||
("*.py", "file.py", True),
|
||||
("*.py", "file.txt", False),
|
||||
("*agent.py", "agent.py", True),
|
||||
("*agent.py", "process_isolated_agent.py", True),
|
||||
("*agent.py", "agent_test.py", False),
|
||||
# Double asterisk patterns
|
||||
("**agent.py", "agent.py", True),
|
||||
("**agent.py", "src/agent.py", True),
|
||||
("**agent.py", "src/serena/agent.py", True),
|
||||
("**agent.py", "src/serena/process_isolated_agent.py", True),
|
||||
("**agent.py", "agent_test.py", False),
|
||||
# Prefix with double asterisk
|
||||
("src/**agent.py", "src/agent.py", True),
|
||||
("src/**agent.py", "src/serena/agent.py", True),
|
||||
("src/**agent.py", "src/serena/process_isolated_agent.py", True),
|
||||
("src/**agent.py", "other/agent.py", False),
|
||||
("src/**agent.py", "src/agent_test.py", False),
|
||||
# Directory patterns
|
||||
("src/**", "src/file.py", True),
|
||||
("src/**", "src/dir/file.py", True),
|
||||
("src/**", "other/file.py", False),
|
||||
# Exact matches with double asterisk
|
||||
("src/**/test.py", "src/test.py", True),
|
||||
("src/**/test.py", "src/a/b/test.py", True),
|
||||
("src/**/test.py", "src/test_file.py", False),
|
||||
# Simple patterns without asterisks
|
||||
("src/file.py", "src/file.py", True),
|
||||
("src/file.py", "src/other.py", False),
|
||||
],
|
||||
)
|
||||
def test_glob_match(self, pattern, path, expected):
|
||||
"""Test glob_match function with various patterns."""
|
||||
from src.serena.text_utils import glob_match
|
||||
|
||||
assert glob_match(pattern, path) == expected
|
||||
|
||||
Reference in New Issue
Block a user