From 35f2aaa5ff8e56c634eda30a73f14c834f940d6a Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Mon, 16 Jun 2025 17:46:52 +0200 Subject: [PATCH] Fixes in glob matching, extended tests --- src/serena/text_utils.py | 53 ++++++++++++-- test/serena/test_text_utils.py | 125 +++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 6 deletions(-) diff --git a/src/serena/text_utils.py b/src/serena/text_utils.py index 75cc7dd..4f40aa0 100644 --- a/src/serena/text_utils.py +++ b/src/serena/text_utils.py @@ -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) diff --git a/test/serena/test_text_utils.py b/test/serena/test_text_utils.py index 860ce6a..86ff975 100644 --- a/test/serena/test_text_utils.py +++ b/test/serena/test_text_utils.py @@ -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