From df50fee7fd85018f8db462774512a83031f00322 Mon Sep 17 00:00:00 2001 From: Shantanu <12621235+hauntsaninja@users.noreply.github.com> Date: Wed, 6 Sep 2023 21:06:07 -0700 Subject: [PATCH] Apply ignore logic before symlink resolution (#3846) This means, for instance, that a gitignored symlink cannot affect your formatting. Fixes #3527, fixes #3826 --- CHANGES.md | 2 ++ src/black/files.py | 20 ++++++++------- tests/test_black.py | 62 +++++++++++++++++++++++++-------------------- 3 files changed, 48 insertions(+), 36 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a14a55a03ac..2168c1b90ce 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -20,6 +20,8 @@ +- Black now applies exclusion and ignore logic before resolving symlinks (#3846) + ### Packaging diff --git a/src/black/files.py b/src/black/files.py index 368e4170d47..362898dc0fd 100644 --- 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: - 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( - normalized_path, root, gitignore_dict, report + root_relative_path, root, gitignore_dict, report ): 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(): - 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 - 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 - 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 + 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. diff --git a/tests/test_black.py b/tests/test_black.py index 79930fabf1f..4fb6aef9bca 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -492,9 +492,7 @@ def test_false_positive_symlink_output_issue_3384(self) -> None: 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: @@ -2375,38 +2373,48 @@ def test_extend_exclude(self) -> None: ) @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() - child = MagicMock() 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() - 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: