Apply ignore logic before symlink resolution (#3846)

This means, for instance, that a gitignored symlink cannot affect your
formatting. Fixes #3527, fixes #3826

diff --git a/CHANGES.md b/CHANGES.md
index a14a55a03acb9e4c1e29bbcae1177cb503a0de92..2168c1b90ce0b0b9a1e4214746af57a3f7483045 100644 (file)
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -20,6 +20,8 @@
 
 <!-- Changes to how Black can be configured -->
 
 
 <!-- Changes to how Black can be configured -->
 

+- Black now applies exclusion and ignore logic before resolving symlinks (#3846)
+

 ### 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/files.py b/src/black/files.py
index 368e4170d4722d64ec9f8d96f1b1364467ccb15b..362898dc0fd71bdc1e19d5c589837c2d0c2a0c96 100644 (file)
--- a/src/black/files.py
+++ b/src/black/files.py
@@ -330,35 +330,37 @@ def gen_python_files(
 
     assert root.is_absolute(), f"INTERNAL ERROR: `root` must be absolute but is {root}"
     for child in paths:
 
     assert root.is_absolute(), f"INTERNAL ERROR: `root` must be absolute but is {root}"
     for child in paths:

-        normalized_path = normalize_path_maybe_ignore(child, root, report)
-        if normalized_path is None:
-            continue
+        root_relative_path = child.absolute().relative_to(root).as_posix()

 
         # First ignore files matching .gitignore, if passed
         if gitignore_dict and _path_is_ignored(
 
         # First ignore files matching .gitignore, if passed
         if gitignore_dict and _path_is_ignored(

-            normalized_path, root, gitignore_dict, report
+            root_relative_path, root, gitignore_dict, report

         ):
             continue
 
         # Then ignore with `--exclude` `--extend-exclude` and `--force-exclude` options.
         ):
             continue
 
         # Then ignore with `--exclude` `--extend-exclude` and `--force-exclude` options.

-        normalized_path = "/" + normalized_path
+        root_relative_path = "/" + root_relative_path

         if child.is_dir():
         if child.is_dir():

-            normalized_path += "/"
+            root_relative_path += "/"

 
 

-        if path_is_excluded(normalized_path, exclude):
+        if path_is_excluded(root_relative_path, exclude):

             report.path_ignored(child, "matches the --exclude regular expression")
             continue
 
             report.path_ignored(child, "matches the --exclude regular expression")
             continue
 

-        if path_is_excluded(normalized_path, extend_exclude):
+        if path_is_excluded(root_relative_path, extend_exclude):

             report.path_ignored(
                 child, "matches the --extend-exclude regular expression"
             )
             continue
 
             report.path_ignored(
                 child, "matches the --extend-exclude regular expression"
             )
             continue
 

-        if path_is_excluded(normalized_path, force_exclude):
+        if path_is_excluded(root_relative_path, force_exclude):

             report.path_ignored(child, "matches the --force-exclude regular expression")
             continue
 
             report.path_ignored(child, "matches the --force-exclude regular expression")
             continue
 

+        normalized_path = normalize_path_maybe_ignore(child, root, report)
+        if normalized_path is None:
+            continue
+

         if child.is_dir():
             # If gitignore is None, gitignore usage is disabled, while a Falsey
             # gitignore is when the directory doesn't have a .gitignore file.
         if child.is_dir():
             # If gitignore is None, gitignore usage is disabled, while a Falsey
             # gitignore is when the directory doesn't have a .gitignore file.

diff --git a/tests/test_black.py b/tests/test_black.py
index 79930fabf1f67f68d221234dc7e8dc3156379b1e..4fb6aef9bca126dff9718c151a8b7398867b2eb3 100644 (file)
--- a/tests/test_black.py
+++ b/tests/test_black.py
@@ -492,9 +492,7 @@ class BlackTestCase(BlackBaseTestCase):
         project_root = Path(THIS_DIR / "data" / "nested_gitignore_tests")
         working_directory = project_root / "root"
         target_abspath = working_directory / "child"
         project_root = Path(THIS_DIR / "data" / "nested_gitignore_tests")
         working_directory = project_root / "root"
         target_abspath = working_directory / "child"

-        target_contents = (
-            src.relative_to(working_directory) for src in target_abspath.iterdir()
-        )
+        target_contents = list(target_abspath.iterdir())

 
         def mock_n_calls(responses: List[bool]) -> Callable[[], bool]:
             def _mocked_calls() -> bool:
 
         def mock_n_calls(responses: List[bool]) -> Callable[[], bool]:
             def _mocked_calls() -> bool:


@@ -2375,38 +2373,48 @@ class TestFileCollection:
         )
 
     @pytest.mark.incompatible_with_mypyc
         )
 
     @pytest.mark.incompatible_with_mypyc

-    def test_symlink_out_of_root_directory(self) -> None:
+    def test_symlinks(self) -> None:

         path = MagicMock()
         root = THIS_DIR.resolve()
         path = MagicMock()
         root = THIS_DIR.resolve()

-        child = MagicMock()

         include = re.compile(black.DEFAULT_INCLUDES)
         exclude = re.compile(black.DEFAULT_EXCLUDES)
         report = black.Report()
         gitignore = PathSpec.from_lines("gitwildmatch", [])
         include = re.compile(black.DEFAULT_INCLUDES)
         exclude = re.compile(black.DEFAULT_EXCLUDES)
         report = black.Report()
         gitignore = PathSpec.from_lines("gitwildmatch", [])

-        # `child` should behave like a symlink which resolved path is clearly
-        # outside of the `root` directory.
-        path.iterdir.return_value = [child]
-        child.resolve.return_value = Path("/a/b/c")
-        child.as_posix.return_value = "/a/b/c"
-        try:
-            list(
-                black.gen_python_files(
-                    path.iterdir(),
-                    root,
-                    include,
-                    exclude,
-                    None,
-                    None,
-                    report,
-                    {path: gitignore},
-                    verbose=False,
-                    quiet=False,
-                )
+
+        regular = MagicMock()
+        outside_root_symlink = MagicMock()
+        ignored_symlink = MagicMock()
+
+        path.iterdir.return_value = [regular, outside_root_symlink, ignored_symlink]
+
+        regular.absolute.return_value = root / "regular.py"
+        regular.resolve.return_value = root / "regular.py"
+        regular.is_dir.return_value = False
+
+        outside_root_symlink.absolute.return_value = root / "symlink.py"
+        outside_root_symlink.resolve.return_value = Path("/nowhere")
+
+        ignored_symlink.absolute.return_value = root / ".mypy_cache" / "symlink.py"
+
+        files = list(
+            black.gen_python_files(
+                path.iterdir(),
+                root,
+                include,
+                exclude,
+                None,
+                None,
+                report,
+                {path: gitignore},
+                verbose=False,
+                quiet=False,

             )
             )

-        except ValueError as ve:
-            pytest.fail(f"`get_python_files_in_dir()` failed: {ve}")
+        )
+        assert files == [regular]
+

         path.iterdir.assert_called_once()
         path.iterdir.assert_called_once()

-        child.resolve.assert_called_once()
+        outside_root_symlink.resolve.assert_called_once()
+        ignored_symlink.resolve.assert_not_called()

 
     @patch("black.find_project_root", lambda *args: (THIS_DIR.resolve(), None))
     def test_get_sources_with_stdin(self) -> None:
 
     @patch("black.find_project_root", lambda *args: (THIS_DIR.resolve(), None))
     def test_get_sources_with_stdin(self) -> None: