Respect all gitignores when parsing project for finding language

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):

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
 

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()

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 {}

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