From 5fee09a35f1944cbfc4f109816a59046aabf6497 Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Fri, 30 May 2025 20:30:16 +0200 Subject: [PATCH] Respect all gitignores when parsing project for finding language --- src/multilspy/language_server.py | 2 +- src/serena/agent.py | 2 +- src/serena/util/file_system.py | 236 ++++++++++++++- src/serena/util/inspection.py | 23 +- test/serena/util/test_file_system.py | 411 +++++++++++++++++++++++++++ 5 files changed, 651 insertions(+), 23 deletions(-) create mode 100644 test/serena/util/test_file_system.py diff --git a/src/multilspy/language_server.py b/src/multilspy/language_server.py index fab22e52..83f1fde8 100644 --- a/src/multilspy/language_server.py +++ b/src/multilspy/language_server.py @@ -1190,7 +1190,7 @@ class LanguageServer: # Explanation for the + "/" part: # 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 - dirs[:] = [d for d in dirs if not self.is_ignored_path(os.path.join(root, d))] + 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.join(root, file) if not self.is_ignored_path(rel_file_path): diff --git a/src/serena/agent.py b/src/serena/agent.py index cc7fce32..2c703ceb 100644 --- a/src/serena/agent.py +++ b/src/serena/agent.py @@ -1052,7 +1052,7 @@ class FindFileTool(Tool): Finds files in the given relative paths """ - def apply(self, file_mask: str, relative_path: str | None = None) -> str: + def apply(self, file_mask: str, relative_path: str) -> str: """ Finds files matching the given file mask within the given relative path diff --git a/src/serena/util/file_system.py b/src/serena/util/file_system.py index 2a733177..e7f773af 100644 --- a/src/serena/util/file_system.py +++ b/src/serena/util/file_system.py @@ -1,5 +1,17 @@ import os from collections.abc import Callable +from dataclasses import dataclass, field +from typing import NamedTuple + +import pathspec +from pathspec import PathSpec + + +class ScanResult(NamedTuple): + """Result of scanning a directory.""" + + directories: list[str] + files: list[str] def scan_directory( @@ -8,7 +20,7 @@ def scan_directory( relative_to: str | None = None, is_ignored_dir: Callable[[str], bool] = lambda x: False, is_ignored_file: Callable[[str], bool] = lambda x: False, -) -> tuple[list[str], list[str]]: +) -> ScanResult: """ :param path: the path to scan :param recursive: whether to recursively scan subdirectories @@ -39,14 +51,228 @@ def scan_directory( if not is_ignored_dir(entry_path): directories.append(result_path) if recursive: - sub_dirs, sub_files = scan_directory( + sub_result = scan_directory( entry_path, recursive=True, relative_to=relative_to, is_ignored_dir=is_ignored_dir, is_ignored_file=is_ignored_file, ) - files.extend(sub_files) - directories.extend(sub_dirs) + files.extend(sub_result.files) + directories.extend(sub_result.directories) - return directories, files + return ScanResult(directories, files) + + +def find_all_non_ignored_files(repo_root: str) -> list[str]: + """ + Find all non-ignored files in the repository, respecting all gitignore files in the repository. + + :param repo_root: The root directory of the repository + :return: A list of all non-ignored files in the repository + """ + gitignore_parser = GitignoreParser(repo_root) + _, files = scan_directory(repo_root, recursive=True) + return [file for file in files if not gitignore_parser.should_ignore(file)] + + +@dataclass +class GitignoreSpec: + """ + Represents a single gitignore file and its parsed patterns. + + :param file_path: Path to the gitignore file + :param patterns: List of adjusted patterns from the gitignore file + :param pathspec: Compiled PathSpec object for pattern matching + """ + + file_path: str + patterns: list[str] = field(default_factory=list) + pathspec: PathSpec = field(init=False) + + def __post_init__(self) -> None: + """Initialize the PathSpec from patterns.""" + self.pathspec = PathSpec.from_lines(pathspec.patterns.GitWildMatchPattern, self.patterns) + + def matches(self, 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) + :return: True if path matches any pattern + """ + return self.pathspec.match_file(path) + + +class GitignoreParser: + """ + Parser for gitignore files in a repository. + + This class handles parsing multiple gitignore files throughout a repository + and provides methods to check if paths should be ignored. + """ + + def __init__(self, repo_root: str) -> None: + """ + Initialize the parser for a repository. + + :param repo_root: Root directory of the repository + """ + self.repo_root = os.path.abspath(repo_root) + self.ignore_specs: list[GitignoreSpec] = [] + self._load_gitignore_files() + + def _load_gitignore_files(self) -> None: + """Load all gitignore files from the repository.""" + gitignore_files = self._find_gitignore_files() + + for gitignore_file in gitignore_files: + spec = self._create_ignore_spec(gitignore_file) + if spec.patterns: # Only add non-empty specs + self.ignore_specs.append(spec) + + def _find_gitignore_files(self) -> list[str]: + """ + Find all .gitignore files in the repository. + + :return: List of absolute paths to .gitignore files + """ + gitignore_files = [] + + for root, dirs, files in os.walk(self.repo_root): + # Skip .git directory + if ".git" in dirs: + dirs.remove(".git") + + if ".gitignore" in files: + gitignore_path = os.path.join(root, ".gitignore") + gitignore_files.append(gitignore_path) + + return gitignore_files + + def _create_ignore_spec(self, gitignore_file_path: str) -> GitignoreSpec: + """ + Create a GitignoreSpec from a single gitignore file. + + :param gitignore_file_path: Path to the .gitignore file + :return: GitignoreSpec object for the gitignore patterns + """ + try: + with open(gitignore_file_path, encoding="utf-8") as f: + content = f.read() + except (OSError, UnicodeDecodeError): + # If we can't read the file, return an empty spec + return GitignoreSpec(gitignore_file_path, []) + + gitignore_dir = os.path.dirname(gitignore_file_path) + patterns = self._parse_gitignore_content(content, gitignore_dir) + + return GitignoreSpec(gitignore_file_path, patterns) + + def _parse_gitignore_content(self, content: str, gitignore_dir: str) -> list[str]: + """ + Parse gitignore content and adjust patterns based on the gitignore file location. + + :param content: Content of the .gitignore file + :param gitignore_dir: Directory containing the .gitignore file (absolute path) + :return: List of adjusted patterns + """ + patterns = [] + + # Get the relative path from repo root to the gitignore directory + rel_dir = os.path.relpath(gitignore_dir, self.repo_root) + if rel_dir == ".": + rel_dir = "" + + for line in content.splitlines(): + # Strip trailing whitespace (but preserve leading whitespace for now) + line = line.rstrip() + + # Skip empty lines and comments + if not line or line.lstrip().startswith("#"): + continue + + # Handle escaped characters at the beginning + if line.startswith(("\\#", "\\!")): + line = line[1:] + + # Store whether this is a negation pattern + is_negation = line.startswith("!") + if is_negation: + line = line[1:] + + # Strip leading/trailing whitespace after removing negation + line = line.strip() + + if not line: + continue + + # Determine if pattern is anchored to the gitignore directory + is_anchored = "/" in line[:-1] or line.startswith("/") + + # Remove leading slash for processing + if line.startswith("/"): + line = line[1:] + + # Adjust pattern based on gitignore file location + if rel_dir: + if is_anchored: + # Anchored patterns are relative to the gitignore directory + adjusted_pattern = os.path.join(rel_dir, line) + else: + # Non-anchored patterns can match anywhere below the gitignore directory + # We need to preserve this behavior + if line.startswith("**/"): + adjusted_pattern = line + else: + # Add the directory prefix but also allow matching in subdirectories + adjusted_pattern = os.path.join(rel_dir, "**", line) + else: + adjusted_pattern = line + + # Re-add negation if needed + if is_negation: + adjusted_pattern = "!" + adjusted_pattern + + # Normalize path separators to forward slashes (gitignore uses forward slashes) + adjusted_pattern = adjusted_pattern.replace(os.sep, "/") + + patterns.append(adjusted_pattern) + + return patterns + + def should_ignore(self, path: str) -> bool: + """ + Check if a path should be ignored based on the gitignore rules. + + :param path: Path to check (absolute or relative to repo_root) + :return: True if the path should be ignored, False otherwise + """ + # Convert to relative path from repo root + if os.path.isabs(path): + rel_path = os.path.relpath(path, self.repo_root) + else: + rel_path = path + + # Normalize path separators + rel_path = rel_path.replace(os.sep, "/") + + # Check against each ignore spec + for spec in self.ignore_specs: + if spec.matches(rel_path): + return True + + return False + + def get_ignore_specs(self) -> list[GitignoreSpec]: + """ + Get all loaded gitignore specs. + + :return: List of GitignoreSpec objects + """ + return self.ignore_specs + + def reload(self) -> None: + """Reload all gitignore files from the repository.""" + self.ignore_specs.clear() + self._load_gitignore_files() diff --git a/src/serena/util/inspection.py b/src/serena/util/inspection.py index 850518e0..509da8e1 100644 --- a/src/serena/util/inspection.py +++ b/src/serena/util/inspection.py @@ -1,12 +1,15 @@ +import logging import os from collections.abc import Generator from typing import TypeVar from multilspy.multilspy_config import Language -from serena.util.file_system import scan_directory +from serena.util.file_system import find_all_non_ignored_files T = TypeVar("T") +log = logging.getLogger(__name__) + def iter_subclasses(cls: type[T], recursive: bool = True) -> Generator[type[T], None, None]: """Iterate over all subclasses of a class. If recursive is True, also iterate over all subclasses of all subclasses.""" @@ -16,27 +19,15 @@ def iter_subclasses(cls: type[T], recursive: bool = True) -> Generator[type[T], yield from iter_subclasses(subclass, recursive) -def determine_programming_language_composition(repo_path: str) -> dict[str, float]: +def determine_programming_language_composition(repo_path: str, rel_path_to_gitignore: str = ".gitignore") -> dict[str, float]: """ Determine the programming language composition of a repository. :param repo_path: Path to the repository to analyze + :return: Dictionary mapping language names to percentages of files matching each language """ - if not os.path.exists(repo_path): - raise FileNotFoundError(f"Repository path does not exist: {repo_path}") - - if not os.path.isdir(repo_path): - raise ValueError(f"Repository path is not a directory: {repo_path}") - - # Scan all files in the repository recursively - _, all_files = scan_directory( - path=repo_path, - recursive=True, - relative_to=repo_path, - is_ignored_dir=lambda dirname: os.path.basename(dirname).startswith("."), # Ignore hidden directories - is_ignored_file=lambda filename: os.path.basename(filename).startswith("."), # Ignore hidden files - ) + all_files = find_all_non_ignored_files(repo_path) if not all_files: return {} diff --git a/test/serena/util/test_file_system.py b/test/serena/util/test_file_system.py new file mode 100644 index 00000000..48d983a4 --- /dev/null +++ b/test/serena/util/test_file_system.py @@ -0,0 +1,411 @@ +import os +import shutil +import tempfile +from pathlib import Path + +# Assuming the gitignore parser code is in a module named 'gitignore_parser' +from serena.util.file_system import GitignoreParser, GitignoreSpec + + +class TestGitignoreParser: + """Test class for GitignoreParser functionality.""" + + def setup_method(self): + """Set up test environment before each test method.""" + # Create a temporary directory for testing + self.test_dir = tempfile.mkdtemp() + self.repo_path = Path(self.test_dir) + + # Create test repository structure + self._create_repo_structure() + + def teardown_method(self): + """Clean up test environment after each test method.""" + # Remove the temporary directory + shutil.rmtree(self.test_dir) + + def _create_repo_structure(self): + """ + Create a test repository structure with multiple gitignore files. + + Structure: + repo/ + ├── .gitignore + ├── file1.txt + ├── test.log + ├── src/ + │ ├── .gitignore + │ ├── main.py + │ ├── test.log + │ └── build/ + │ └── output.o + └── docs/ + ├── .gitignore + ├── api.md + └── temp/ + └── draft.md + """ + # Create directories + (self.repo_path / "src").mkdir() + (self.repo_path / "src" / "build").mkdir() + (self.repo_path / "docs").mkdir() + (self.repo_path / "docs" / "temp").mkdir() + + # Create files + (self.repo_path / "file1.txt").touch() + (self.repo_path / "test.log").touch() + (self.repo_path / "src" / "main.py").touch() + (self.repo_path / "src" / "test.log").touch() + (self.repo_path / "src" / "build" / "output.o").touch() + (self.repo_path / "docs" / "api.md").touch() + (self.repo_path / "docs" / "temp" / "draft.md").touch() + + # Create root .gitignore + root_gitignore = self.repo_path / ".gitignore" + root_gitignore.write_text( + """# Root gitignore +*.log +/build/ +""" + ) + + # Create src/.gitignore + src_gitignore = self.repo_path / "src" / ".gitignore" + src_gitignore.write_text( + """# Source gitignore +*.o +build/ +!important.log +""" + ) + + # Create docs/.gitignore + docs_gitignore = self.repo_path / "docs" / ".gitignore" + docs_gitignore.write_text( + """# Docs gitignore +temp/ +*.tmp +""" + ) + + def test_initialization(self): + """Test GitignoreParser initialization.""" + parser = GitignoreParser(str(self.repo_path)) + + assert parser.repo_root == str(self.repo_path.absolute()) + assert len(parser.get_ignore_specs()) == 3 + + def test_find_gitignore_files(self): + """Test finding all gitignore files in repository.""" + parser = GitignoreParser(str(self.repo_path)) + + # Get file paths from specs + gitignore_files = [spec.file_path for spec in parser.get_ignore_specs()] + + # Convert to relative paths for easier testing + rel_paths = [os.path.relpath(f, self.repo_path) for f in gitignore_files] + rel_paths.sort() + + assert len(rel_paths) == 3 + assert ".gitignore" in rel_paths + assert os.path.join("src", ".gitignore") in rel_paths + assert os.path.join("docs", ".gitignore") in rel_paths + + def test_parse_patterns_root_directory(self): + """Test parsing gitignore patterns in root directory.""" + # Create a simple test case with only root gitignore + test_dir = self.repo_path / "test_root" + test_dir.mkdir() + + gitignore = test_dir / ".gitignore" + gitignore.write_text( + """*.log +build/ +/temp.txt +""" + ) + + parser = GitignoreParser(str(test_dir)) + specs = parser.get_ignore_specs() + + assert len(specs) == 1 + patterns = specs[0].patterns + + assert "*.log" in patterns + assert "build/" in patterns + assert "temp.txt" in patterns + + def test_parse_patterns_subdirectory(self): + """Test parsing gitignore patterns in subdirectory.""" + # Create a test case with subdirectory gitignore + test_dir = self.repo_path / "test_sub" + test_dir.mkdir() + subdir = test_dir / "src" + subdir.mkdir() + + gitignore = subdir / ".gitignore" + gitignore.write_text( + """*.o +/build/ +test.log +""" + ) + + parser = GitignoreParser(str(test_dir)) + specs = parser.get_ignore_specs() + + assert len(specs) == 1 + patterns = specs[0].patterns + + # Non-anchored pattern should get ** prefix + assert "src/**/*.o" in patterns + # Anchored pattern should not get ** prefix + assert "src/build/" in patterns + # Non-anchored pattern without slash + assert "src/**/test.log" in patterns + + def test_should_ignore_root_patterns(self): + """Test ignoring files based on root .gitignore.""" + parser = GitignoreParser(str(self.repo_path)) + + # Files that should be ignored + assert parser.should_ignore("test.log") + assert parser.should_ignore(str(self.repo_path / "test.log")) + + # Files that should NOT be ignored + assert not parser.should_ignore("file1.txt") + assert not parser.should_ignore("src/main.py") + + def test_should_ignore_subdirectory_patterns(self): + """Test ignoring files based on subdirectory .gitignore files.""" + parser = GitignoreParser(str(self.repo_path)) + + # .o files in src should be ignored + assert parser.should_ignore("src/build/output.o") + + # build/ directory in src should be ignored + assert parser.should_ignore("src/build/") + + # temp/ directory in docs should be ignored + assert parser.should_ignore("docs/temp/draft.md") + + # But temp/ outside docs should not be ignored by docs/.gitignore + assert not parser.should_ignore("temp/file.txt") + + def test_anchored_vs_non_anchored_patterns(self): + """Test the difference between anchored and non-anchored patterns.""" + # Create new test structure + test_dir = self.repo_path / "test_anchored" + test_dir.mkdir() + (test_dir / "src").mkdir() + (test_dir / "src" / "subdir").mkdir() + (test_dir / "src" / "subdir" / "deep").mkdir() + + # Create src/.gitignore with both anchored and non-anchored patterns + gitignore = test_dir / "src" / ".gitignore" + gitignore.write_text( + """/temp.txt +data.json +""" + ) + + # Create test files + (test_dir / "src" / "temp.txt").touch() + (test_dir / "src" / "data.json").touch() + (test_dir / "src" / "subdir" / "temp.txt").touch() + (test_dir / "src" / "subdir" / "data.json").touch() + (test_dir / "src" / "subdir" / "deep" / "data.json").touch() + + parser = GitignoreParser(str(test_dir)) + + # Anchored pattern /temp.txt should only match in src/ + assert parser.should_ignore("src/temp.txt") + assert not parser.should_ignore("src/subdir/temp.txt") + + # Non-anchored pattern data.json should match anywhere under src/ + assert parser.should_ignore("src/data.json") + assert parser.should_ignore("src/subdir/data.json") + assert parser.should_ignore("src/subdir/deep/data.json") + + def test_negation_patterns(self): + """Test negation patterns are parsed correctly.""" + test_dir = self.repo_path / "test_negation" + test_dir.mkdir() + + gitignore = test_dir / ".gitignore" + gitignore.write_text( + """*.log +!important.log +!src/keep.log +""" + ) + + parser = GitignoreParser(str(test_dir)) + specs = parser.get_ignore_specs() + + assert len(specs) == 1 + patterns = specs[0].patterns + + assert "*.log" in patterns + assert "!important.log" in patterns + assert "!src/keep.log" in patterns + + def test_comments_and_empty_lines(self): + """Test that comments and empty lines are ignored.""" + test_dir = self.repo_path / "test_comments" + test_dir.mkdir() + + gitignore = test_dir / ".gitignore" + gitignore.write_text( + """# This is a comment +*.log + +# Another comment + # Indented comment + +build/ +""" + ) + + parser = GitignoreParser(str(test_dir)) + specs = parser.get_ignore_specs() + + assert len(specs) == 1 + patterns = specs[0].patterns + + assert len(patterns) == 2 + assert "*.log" in patterns + assert "build/" in patterns + + def test_escaped_characters(self): + """Test escaped special characters.""" + test_dir = self.repo_path / "test_escaped" + test_dir.mkdir() + + gitignore = test_dir / ".gitignore" + gitignore.write_text( + """\\#not-a-comment.txt +\\!not-negation.txt +""" + ) + + parser = GitignoreParser(str(test_dir)) + specs = parser.get_ignore_specs() + + assert len(specs) == 1 + patterns = specs[0].patterns + + assert "#not-a-comment.txt" in patterns + assert "!not-negation.txt" in patterns + + def test_glob_patterns(self): + """Test various glob patterns work correctly.""" + test_dir = self.repo_path / "test_glob" + test_dir.mkdir() + + gitignore = test_dir / ".gitignore" + gitignore.write_text( + """*.pyc +**/*.tmp +src/*.o +!src/important.o +[Tt]est* +""" + ) + + # Create test files + (test_dir / "src").mkdir() + (test_dir / "src" / "nested").mkdir() + (test_dir / "file.pyc").touch() + (test_dir / "src" / "file.pyc").touch() + (test_dir / "file.tmp").touch() + (test_dir / "src" / "nested" / "file.tmp").touch() + (test_dir / "src" / "file.o").touch() + (test_dir / "src" / "important.o").touch() + (test_dir / "Test.txt").touch() + (test_dir / "test.log").touch() + + parser = GitignoreParser(str(test_dir)) + + # *.pyc should match everywhere + assert parser.should_ignore("file.pyc") + assert parser.should_ignore("src/file.pyc") + + # **/*.tmp should match all .tmp files + assert parser.should_ignore("file.tmp") + assert parser.should_ignore("src/nested/file.tmp") + + # src/*.o should only match .o files directly in src/ + assert parser.should_ignore("src/file.o") + + # Character class patterns + assert parser.should_ignore("Test.txt") + assert parser.should_ignore("test.log") + + def test_empty_gitignore(self): + """Test handling of empty gitignore files.""" + test_dir = self.repo_path / "test_empty" + test_dir.mkdir() + + gitignore = test_dir / ".gitignore" + gitignore.write_text("") + + parser = GitignoreParser(str(test_dir)) + + # Should not crash and should return empty list + assert len(parser.get_ignore_specs()) == 0 + + def test_malformed_gitignore(self): + """Test handling of malformed gitignore content.""" + test_dir = self.repo_path / "test_malformed" + test_dir.mkdir() + + gitignore = test_dir / ".gitignore" + gitignore.write_text( + """# Only comments and empty lines + +# More comments + + """ + ) + + parser = GitignoreParser(str(test_dir)) + + # Should handle gracefully + assert len(parser.get_ignore_specs()) == 0 + + def test_reload(self): + """Test reloading gitignore files.""" + test_dir = self.repo_path / "test_reload" + test_dir.mkdir() + + # Create initial gitignore + gitignore = test_dir / ".gitignore" + gitignore.write_text("*.log") + + parser = GitignoreParser(str(test_dir)) + assert len(parser.get_ignore_specs()) == 1 + assert parser.should_ignore("test.log") + + # Modify gitignore + gitignore.write_text("*.tmp") + + # Without reload, should still use old patterns + assert parser.should_ignore("test.log") + assert not parser.should_ignore("test.tmp") + + # After reload, should use new patterns + parser.reload() + assert not parser.should_ignore("test.log") + assert parser.should_ignore("test.tmp") + + def test_gitignore_spec_matches(self): + """Test GitignoreSpec.matches method.""" + spec = GitignoreSpec("/path/to/.gitignore", ["*.log", "build/", "!important.log"]) + + assert spec.matches("test.log") + assert spec.matches("build/output.o") + assert spec.matches("src/test.log") + + # Note: Negation patterns in pathspec work differently than in git + # This is a limitation of the pathspec library