Repository: fpgmaas/deptry (language: python)

Issue:
`.gitignore` from parent git repository is used while it shouldn't
My setup may be not exactly standard, but `deptry` is the only tool that has a problem with it:

Up in my filesystem hierarchy there lingers a Git repository which ignores everything (`*`) and only a few files/directories are excluded from this catch all ignore. This works 100% fine with VSCode Git, command line Git and every other Git related tool I can think of.

Only `deptry` refuses to find any file with this setup, as it somehow climbs up in hierarchy to find my `.gitignore`->
```
Scanning 0 file...
```
Is there a way to fix this gracefully?



Discussion / hints:
Agree that we should avoid reading `.gitignore` from parents directories outside the path where `deptry` is invoked. We could disable [parents](https://docs.rs/ignore/0.4.23/ignore/struct.WalkBuilder.html#method.parents) when constructing the directory iterator [here](https://github.com/fpgmaas/deptry/blob/5f407ad2638f60ddd2a8999010d649f8fd4ad986/src/python_file_finder.rs#L62-L68).

But testing a bit the changes, this made me realise that we probably don't handle `.gitignore` properly today. If we disable reading parents, given the following structure:

```
.
├── requirements.txt
└── src
    └── foo.py
```
where `.gitignore` contains `*`.

```console
$ deptry .
[...]
Scanning 0 file...
[...]
```

```console
$ deptry src
[...]
Scanning 1 file...
[...]
```

Reading `.gitignore` file is relative to the directory passed as a positional argument, which is probably not what we want. Seems to me that reading the file from the location `deptry` itself is invoked would make more sense, although we should likely also respect `.gitignore` in sub-directories (which is probably something that the library we use does automatically).
Tiny addition: There must be some kind of clever library at work, as even if I move the git ignore patterns from `.gitignore` to `.git/info/exclude`, they still get found in the git repository up in the parent hierarchy.

Fix patch:
diff --git a/src/python_file_finder.rs b/src/python_file_finder.rs
index 493df2a0..f3d909d0 100644
--- a/src/python_file_finder.rs
+++ b/src/python_file_finder.rs
@@ -61,9 +61,8 @@ fn build_walker(
 
     walk_builder
         .types(build_types(ignore_notebooks).unwrap())
+        .standard_filters(use_git_ignore)
         .hidden(false)
-        .git_ignore(use_git_ignore)
-        .require_git(false)
         .filter_entry(move |entry| entry_satisfies_predicate(entry, re.as_ref()))
         .build()
 }


Test patch:
diff --git a/tests/fixtures/pep_621_project/.gitignore b/tests/fixtures/pep_621_project/.ignore
similarity index 100%
rename from tests/fixtures/pep_621_project/.gitignore
rename to tests/fixtures/pep_621_project/.ignore
diff --git a/tests/fixtures/project_using_namespace/.gitignore b/tests/fixtures/project_using_namespace/.ignore
similarity index 100%
rename from tests/fixtures/project_using_namespace/.gitignore
rename to tests/fixtures/project_using_namespace/.ignore
diff --git a/tests/fixtures/project_with_gitignore/.gitignore b/tests/fixtures/project_with_gitignore/.gitignore
index 044d1c59..24837bd2 100644
--- a/tests/fixtures/project_with_gitignore/.gitignore
+++ b/tests/fixtures/project_with_gitignore/.gitignore
@@ -1,1 +1,3 @@
+/build
 foobar.py
+/src/barfoo.py
diff --git a/tests/fixtures/project_with_gitignore/src/.gitignore b/tests/fixtures/project_with_gitignore/src/.gitignore
new file mode 100644
index 00000000..ac77925d
--- /dev/null
+++ b/tests/fixtures/project_with_gitignore/src/.gitignore
@@ -0,0 +1,2 @@
+/baz.py
+/src/bar.py
diff --git a/tests/fixtures/project_with_gitignore/src/barfoo.py b/tests/fixtures/project_with_gitignore/src/barfoo.py
new file mode 100644
index 00000000..88af17dd
--- /dev/null
+++ b/tests/fixtures/project_with_gitignore/src/barfoo.py
@@ -0,0 +1,1 @@
+import hello
diff --git a/tests/fixtures/project_with_gitignore/src/baz.py b/tests/fixtures/project_with_gitignore/src/baz.py
new file mode 100644
index 00000000..b05e2854
--- /dev/null
+++ b/tests/fixtures/project_with_gitignore/src/baz.py
@@ -0,0 +1,1 @@
+import hej
diff --git a/tests/fixtures/project_with_src_directory/.gitignore b/tests/fixtures/project_with_src_directory/.gitignore
deleted file mode 100644
index 891399da..00000000
--- a/tests/fixtures/project_with_src_directory/.gitignore
+++ /dev/null
@@ -1,1 +0,0 @@
-src/this_file_is_gitignored.py
diff --git a/tests/fixtures/project_with_src_directory/.ignore b/tests/fixtures/project_with_src_directory/.ignore
new file mode 100644
index 00000000..c9dd1c7d
--- /dev/null
+++ b/tests/fixtures/project_with_src_directory/.ignore
@@ -0,0 +1,1 @@
+src/this_file_is_ignored.py
diff --git a/tests/fixtures/project_with_src_directory/src/this_file_is_gitignored.py b/tests/fixtures/project_with_src_directory/src/this_file_is_ignored.py
similarity index 100%
rename from tests/fixtures/project_with_src_directory/src/this_file_is_gitignored.py
rename to tests/fixtures/project_with_src_directory/src/this_file_is_ignored.py
diff --git a/tests/functional/cli/test_cli_gitignore.py b/tests/functional/cli/test_cli_gitignore.py
index 9e815a33..ce78d064 100644
--- a/tests/functional/cli/test_cli_gitignore.py
+++ b/tests/functional/cli/test_cli_gitignore.py
@@ -14,9 +14,13 @@
 
 
 @pytest.mark.xdist_group(name=Project.GITIGNORE)
-def test_cli_gitignore_is_used(pip_venv_factory: PipVenvFactory) -> None:
+def test_cli_gitignore_used(pip_venv_factory: PipVenvFactory) -> None:
     with pip_venv_factory(Project.GITIGNORE) as virtual_env:
         issue_report = f"{uuid.uuid4()}.json"
+
+        # Simulate the fact that the project is a git repository.
+        Path(".git").mkdir(exist_ok=True)
+
         result = virtual_env.run(f"deptry . -o {issue_report}")
 
         assert result.returncode == 1
@@ -61,9 +65,64 @@ def test_cli_gitignore_is_used(pip_venv_factory: PipVenvFactory) -> None:
 
 
 @pytest.mark.xdist_group(name=Project.GITIGNORE)
-def test_cli_gitignore_is_not_used(pip_venv_factory: PipVenvFactory) -> None:
+def test_cli_gitignore_used_for_non_root_directory(pip_venv_factory: PipVenvFactory) -> None:
     with pip_venv_factory(Project.GITIGNORE) as virtual_env:
         issue_report = f"{uuid.uuid4()}.json"
+
+        # Simulate the fact that the project is a git repository.
+        Path(".git").mkdir(exist_ok=True)
+
+        result = virtual_env.run(f"deptry src -o {issue_report}")
+
+        assert result.returncode == 1
+        assert get_issues_report(Path(issue_report)) == [
+            {
+                "error": {
+                    "code": "DEP002",
+                    "message": "'requests' defined as a dependency but not used in the codebase",
+                },
+                "module": "requests",
+                "location": {
+                    "file": str(Path("pyproject.toml")),
+                    "line": None,
+                    "column": None,
+                },
+            },
+            {
+                "error": {
+                    "code": "DEP002",
+                    "message": "'mypy' defined as a dependency but not used in the codebase",
+                },
+                "module": "mypy",
+                "location": {
+                    "file": str(Path("pyproject.toml")),
+                    "line": None,
+                    "column": None,
+                },
+            },
+            {
+                "error": {
+                    "code": "DEP002",
+                    "message": "'pytest' defined as a dependency but not used in the codebase",
+                },
+                "module": "pytest",
+                "location": {
+                    "file": str(Path("pyproject.toml")),
+                    "line": None,
+                    "column": None,
+                },
+            },
+        ]
+
+
+@pytest.mark.xdist_group(name=Project.GITIGNORE)
+def test_cli_gitignore_not_used_when_using_exclude(pip_venv_factory: PipVenvFactory) -> None:
+    with pip_venv_factory(Project.GITIGNORE) as virtual_env:
+        issue_report = f"{uuid.uuid4()}.json"
+
+        # Simulate the fact that the project is a git repository.
+        Path(".git").mkdir(exist_ok=True)
+
         result = virtual_env.run(f"deptry . --exclude build/|src/bar.py -o {issue_report}")
 
         assert result.returncode == 1
@@ -104,4 +163,28 @@ def test_cli_gitignore_is_not_used(pip_venv_factory: PipVenvFactory) -> None:
                     "column": None,
                 },
             },
+            {
+                "error": {
+                    "code": "DEP001",
+                    "message": "'hello' imported but missing from the dependency definitions",
+                },
+                "module": "hello",
+                "location": {
+                    "file": str(Path("src/barfoo.py")),
+                    "line": 1,
+                    "column": 8,
+                },
+            },
+            {
+                "error": {
+                    "code": "DEP001",
+                    "message": "'hej' imported but missing from the dependency definitions",
+                },
+                "module": "hej",
+                "location": {
+                    "file": str(Path("src/baz.py")),
+                    "line": 1,
+                    "column": 8,
+                },
+            },
         ]
diff --git a/tests/unit/test_python_file_finder.py b/tests/unit/test_python_file_finder.py
index d777c035..6b0af65a 100644
--- a/tests/unit/test_python_file_finder.py
+++ b/tests/unit/test_python_file_finder.py
@@ -174,3 +174,110 @@ def test_duplicates_are_removed(tmp_path: Path) -> None:
         files = get_all_python_files_in((Path(), Path()), exclude=(), extend_exclude=(), using_default_exclude=False)
 
         assert sorted(files) == [Path("dir/subdir/file1.py")]
+
+
+def test_gitignore_used_in_git_project(tmp_path: Path) -> None:
+    """Test that gitignore files are respected when project uses git."""
+    git_project_path = tmp_path / "git_project"
+    git_project_path.mkdir()
+
+    # Simulate the presence of a gitignore outside the git project, that should not be used.
+    with (tmp_path / ".gitignore").open("w") as f:
+        f.write("*")
+
+    with run_within_dir(tmp_path / git_project_path):
+        # Simulate the fact that the project is a git repository.
+        Path(".git").mkdir()
+
+        create_files([
+            Path("file1.py"),
+            Path("file2.py"),
+            Path("file3.py"),
+            Path("dir1/file.py"),
+            Path("dir2/file2.py"),
+            Path("dir3/file3.py"),
+            Path("dir3/file4.py"),
+        ])
+
+        with Path(".gitignore").open("w") as f:
+            f.write("""/file1.py
+/dir1/file.py
+file2.py""")
+
+        with Path("dir3/.gitignore").open("w") as f:
+            f.write("/file3.py")
+
+        files = get_all_python_files_in((Path(),), exclude=(), extend_exclude=(), using_default_exclude=True)
+
+        assert sorted(files) == [
+            Path("dir3/file4.py"),
+            Path("file3.py"),
+        ]
+
+
+def test_gitignore_ignored_when_not_using_default_exclude(tmp_path: Path) -> None:
+    """Test that gitignore files are ignored when project uses git but does not use default exclude."""
+    git_project_path = tmp_path / "git_project"
+    git_project_path.mkdir()
+
+    # Simulate the presence of a gitignore outside the git project, that should not be used.
+    with (tmp_path / ".gitignore").open("w") as f:
+        f.write("*")
+
+    with run_within_dir(tmp_path / git_project_path):
+        # Simulate the fact that the project is a git repository.
+        Path(".git").mkdir()
+
+        create_files([
+            Path("file1.py"),
+            Path("file2.py"),
+            Path("file3.py"),
+            Path("dir1/file.py"),
+            Path("dir2/file2.py"),
+            Path("dir3/file3.py"),
+            Path("dir3/file4.py"),
+        ])
+
+        with Path(".gitignore").open("w") as f:
+            f.write("""/file1.py
+/dir1/file.py
+file2.py""")
+
+        with Path("dir3/.gitignore").open("w") as f:
+            f.write("/file3.py")
+
+        files = get_all_python_files_in(
+            (Path(),),
+            exclude=("file3.py",),
+            extend_exclude=(),
+            using_default_exclude=False,
+        )
+
+        assert sorted(files) == [
+            Path("dir1/file.py"),
+            Path("dir2/file2.py"),
+            Path("dir3/file3.py"),
+            Path("dir3/file4.py"),
+            Path("file1.py"),
+            Path("file2.py"),
+        ]
+
+
+def test_gitignore_ignored_in_non_git_project(tmp_path: Path) -> None:
+    """Test that gitignore files are ignored when project does not use git."""
+    with run_within_dir(tmp_path):
+        create_files([
+            Path("file1.py"),
+            Path("file2.py"),
+        ])
+
+        # This will be ignored, since project is not a git project.
+        with Path(".gitignore").open("w") as f:
+            f.write("/file1.py")
+
+        files = get_all_python_files_in((Path(),), exclude=(), extend_exclude=(), using_default_exclude=True)
+
+        assert sorted(files) == [
+            Path("file1.py"),
+            Path("file2.py"),
+        ]