Apply .gitignore correctly in every source entry (#3336)

When passing multiple src directories, the root gitignore was only
applied to the first processed source. The reason is that, in the
first source, exclude is `None`, but then the value gets overridden by
`re_compile_maybe_verbose(DEFAULT_EXCLUDES)`, so in the next iteration
where the source is a directory, the condition is not met and sets the
value of `gitignore` to `None`.

To fix this problem, we store a boolean indicating if `exclude` is
`None` and set the value of `exclude` to its default value if that's
the case. This makes sure that the flow enters the correct condition on
following iterations and also keeps the original value if the condition
is not met.

Also, the value of `gitignore` is initialized as `None` and overriden
if necessary. The value of `root_gitignore` is always calculated to
avoid using additional variables (at the small cost of additional
computations).

Signed-off-by: Antonio Ossa Guerra <aaossa@uc.cl>

diff --git a/CHANGES.md b/CHANGES.md
index 1dcd7f09b3c67c440493f45d2d4f1afede6dddd3..a1071a8ec7c05340719b5bc3ea8e525260876098 100644 (file)
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -22,6 +22,9 @@
 
 <!-- Changes to how Black can be configured -->
 
 
 <!-- Changes to how Black can be configured -->
 

+- Fix incorrectly ignoring .gitignore presence when more than one source directory is
+  specified (#3336)
+

 ### Packaging
 
 <!-- Changes to how Black is packaged, such as dependency requirements -->
 ### Packaging
 
 <!-- Changes to how Black is packaged, such as dependency requirements -->

diff --git a/src/black/__init__.py b/src/black/__init__.py
index 94592278c315d35093f34ddce43eddd866605eea..6c8d346858312ce4403782f92bd36af758b2d273 100644 (file)
--- a/src/black/__init__.py
+++ b/src/black/__init__.py
@@ -30,6 +30,7 @@ from typing import (
 import click
 from click.core import ParameterSource
 from mypy_extensions import mypyc_attr
 import click
 from click.core import ParameterSource
 from mypy_extensions import mypyc_attr

+from pathspec import PathSpec

 from pathspec.patterns.gitwildmatch import GitWildMatchPatternError
 
 from _black_version import version as __version__
 from pathspec.patterns.gitwildmatch import GitWildMatchPatternError
 
 from _black_version import version as __version__


@@ -627,6 +628,11 @@ def get_sources(
     sources: Set[Path] = set()
     root = ctx.obj["root"]
 
     sources: Set[Path] = set()
     root = ctx.obj["root"]
 

+    exclude_is_None = exclude is None
+    exclude = re_compile_maybe_verbose(DEFAULT_EXCLUDES) if exclude is None else exclude
+    gitignore = None  # type: Optional[PathSpec]
+    root_gitignore = get_gitignore(root)
+

     for s in src:
         if s == "-" and stdin_filename:
             p = Path(stdin_filename)
     for s in src:
         if s == "-" and stdin_filename:
             p = Path(stdin_filename)


@@ -660,16 +666,14 @@ def get_sources(
 
             sources.add(p)
         elif p.is_dir():
 
             sources.add(p)
         elif p.is_dir():

-            if exclude is None:
-                exclude = re_compile_maybe_verbose(DEFAULT_EXCLUDES)
-                gitignore = get_gitignore(root)
+            if exclude_is_None:

                 p_gitignore = get_gitignore(p)
                 # No need to use p's gitignore if it is identical to root's gitignore
                 # (i.e. root and p point to the same directory).
                 p_gitignore = get_gitignore(p)
                 # No need to use p's gitignore if it is identical to root's gitignore
                 # (i.e. root and p point to the same directory).

-                if gitignore != p_gitignore:
-                    gitignore += p_gitignore
-            else:
-                gitignore = None
+                if root_gitignore == p_gitignore:
+                    gitignore = root_gitignore
+                else:
+                    gitignore = root_gitignore + p_gitignore

             sources.update(
                 gen_python_files(
                     p.iterdir(),
             sources.update(
                 gen_python_files(
                     p.iterdir(),

diff --git a/tests/data/gitignore_used_on_multiple_sources/.gitignore b/tests/data/gitignore_used_on_multiple_sources/.gitignore
new file mode 100644 (file)
index 0000000..2987e7b
--- /dev/null
+++ b/tests/data/gitignore_used_on_multiple_sources/.gitignore
@@ -0,0 +1 @@
+a.py

diff --git a/tests/data/gitignore_used_on_multiple_sources/dir1/a.py b/tests/data/gitignore_used_on_multiple_sources/dir1/a.py
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/tests/data/gitignore_used_on_multiple_sources/dir1/b.py b/tests/data/gitignore_used_on_multiple_sources/dir1/b.py
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/tests/data/gitignore_used_on_multiple_sources/dir2/a.py b/tests/data/gitignore_used_on_multiple_sources/dir2/a.py
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/tests/data/gitignore_used_on_multiple_sources/dir2/b.py b/tests/data/gitignore_used_on_multiple_sources/dir2/b.py
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/tests/test_black.py b/tests/test_black.py
index 9256698ea2792eb49ba559febe803afde73b9d60..784eb0dc9ad8bdacaa9c818d642dd98be16dcb64 100644 (file)
--- a/tests/test_black.py
+++ b/tests/test_black.py
@@ -1998,6 +1998,17 @@ class TestFileCollection:
         ctx.obj["root"] = base
         assert_collected_sources(src, expected, ctx=ctx, extend_exclude=r"/exclude/")
 
         ctx.obj["root"] = base
         assert_collected_sources(src, expected, ctx=ctx, extend_exclude=r"/exclude/")
 

+    def test_gitignore_used_on_multiple_sources(self) -> None:
+        root = Path(DATA_DIR / "gitignore_used_on_multiple_sources")
+        expected = [
+            root / "dir1" / "b.py",
+            root / "dir2" / "b.py",
+        ]
+        ctx = FakeContext()
+        ctx.obj["root"] = root
+        src = [root / "dir1", root / "dir2"]
+        assert_collected_sources(src, expected, ctx=ctx)
+

     @patch("black.find_project_root", lambda *args: (THIS_DIR.resolve(), None))
     def test_exclude_for_issue_1572(self) -> None:
         # Exclude shouldn't touch files that were explicitly given to Black through the
     @patch("black.find_project_root", lambda *args: (THIS_DIR.resolve(), None))
     def test_exclude_for_issue_1572(self) -> None:
         # Exclude shouldn't touch files that were explicitly given to Black through the