From a8ce2293d24cf5ed9286764e7856a4e215fa6af1 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Fri, 18 Aug 2023 20:08:33 -0700 Subject: [PATCH 1/3] Apply ignore logic before symlink resolution This means, for instance, that a gitignored symlink cannot affect your formatting. Fixes #3527, fixes #3826 --- CHANGES.md | 1 + src/black/__init__.py | 2 +- src/black/files.py | 20 +++++++------- tests/test_black.py | 62 ++++++++++++++++++++++++------------------- 4 files changed, 48 insertions(+), 37 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a14a55a03ac..d657aa05fca 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -19,6 +19,7 @@ ### Configuration +- Black now applies exclusion and ignore logic before resolving symlinks (#3845) ### Packaging diff --git a/src/black/__init__.py b/src/black/__init__.py index dc06eab8dd0..380d48ed815 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -628,7 +628,7 @@ def get_sources( ) -> Set[Path]: """Compute the set of files to be formatted.""" sources: Set[Path] = set() - root = ctx.obj["root"] + root: Path = ctx.obj["root"] using_default_exclude = exclude is None exclude = re_compile_maybe_verbose(DEFAULT_EXCLUDES) if exclude is None else exclude 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 8ae92172d43..32f52fc88ea 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: @@ -2387,38 +2385,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: From 3e00fb214206d0f4f6bc4a02f7d024012cd8c69d Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Fri, 18 Aug 2023 20:51:30 -0700 Subject: [PATCH 2/3] changelog --- CHANGES.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index d657aa05fca..2168c1b90ce 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -19,7 +19,8 @@ ### Configuration -- Black now applies exclusion and ignore logic before resolving symlinks (#3845) + +- Black now applies exclusion and ignore logic before resolving symlinks (#3846) ### Packaging From 2a8804cb52e0134495ec7276d10a8ac2cd1c8133 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Fri, 18 Aug 2023 20:54:12 -0700 Subject: [PATCH 3/3] fix --- src/black/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 380d48ed815..dc06eab8dd0 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -628,7 +628,7 @@ def get_sources( ) -> Set[Path]: """Compute the set of files to be formatted.""" sources: Set[Path] = set() - root: Path = ctx.obj["root"] + root = ctx.obj["root"] using_default_exclude = exclude is None exclude = re_compile_maybe_verbose(DEFAULT_EXCLUDES) if exclude is None else exclude