From 18fb88486d434dbde9b2a9c98f008a71cf5d941d Mon Sep 17 00:00:00 2001 From: Antonio Ossa-Guerra Date: Wed, 18 Jan 2023 23:38:27 -0300 Subject: [PATCH] Fix false symlink detection claims in verbose output (#3385) When trying to format a project from the outside, the verbose output shows says that there are symbolic links that points outside of the project, but displays the wrong project path, meaning that these messages are false positives. This bug is triggered when the command is executed from outside a project on a folder inside it, causing an inconsistency between the path to the detected project root and the relative path to the target contents. The fix is to normalize the target path using the project root before processing the sources, which removes the presence of the incorrect messages. --- The test attemps to emulate the behavior of the CLI as closely as posible by patching some `pathlib.Path` methods and passing certain reference paths to the context object and `black.get_sources`. Before the associated fix was introduced, this test failed because some of the captured files reported the presence of a symlink due to an incorrectly formated path. The test also asserts that only a single file is reported as ignored, which is part of the expected behavior. Signed-off-by: Antonio Ossa Guerra --- CHANGES.md | 2 ++ src/black/__init__.py | 3 ++- tests/test_black.py | 47 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 97b68b9..313536e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -66,6 +66,8 @@ - Verbose logging now shows the values of `pyproject.toml` configuration variables (#3392) +- Fix false symlink detection messages in verbose output due to using an incorrect + relative path to the project root (#3385) ### _Blackd_ diff --git a/src/black/__init__.py b/src/black/__init__.py index 9f44722..5d35c80 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -673,10 +673,11 @@ def get_sources( sources.add(p) elif p.is_dir(): + p = root / normalize_path_maybe_ignore(p, ctx.obj["root"], report) if using_default_exclude: gitignore = { root: root_gitignore, - root / p: get_gitignore(p), + p: get_gitignore(p), } sources.update( gen_python_files( diff --git a/tests/test_black.py b/tests/test_black.py index dda1055..44d6172 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -475,6 +475,53 @@ class BlackTestCase(BlackBaseTestCase): self.assertFormatEqual(contents_spc, fs(contents_spc)) self.assertFormatEqual(contents_spc, fs(contents_tab)) + def test_false_positive_symlink_output_issue_3384(self) -> None: + # Emulate the behavior when using the CLI (`black ./child --verbose`), which + # involves patching some `pathlib.Path` methods. In particular, `is_dir` is + # patched only on its first call: when checking if "./child" is a directory it + # should return True. The "./child" folder exists relative to the cwd when + # running from CLI, but fails when running the tests because cwd is different + 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() + ) + + def mock_n_calls(responses: List[bool]) -> Callable[[], bool]: + def _mocked_calls() -> bool: + if responses: + return responses.pop(0) + return False + + return _mocked_calls + + with patch("pathlib.Path.iterdir", return_value=target_contents), patch( + "pathlib.Path.cwd", return_value=working_directory + ), patch("pathlib.Path.is_dir", side_effect=mock_n_calls([True])): + ctx = FakeContext() + ctx.obj["root"] = project_root + report = MagicMock(verbose=True) + black.get_sources( + ctx=ctx, + src=("./child",), + quiet=False, + verbose=True, + include=DEFAULT_INCLUDE, + exclude=None, + report=report, + extend_exclude=None, + force_exclude=None, + stdin_filename=None, + ) + assert not any( + mock_args[1].startswith("is a symbolic link that points outside") + for _, mock_args, _ in report.path_ignored.mock_calls + ), "A symbolic link was reported." + report.path_ignored.assert_called_once_with( + Path("child", "b.py"), "matches a .gitignore file content" + ) + def test_report_verbose(self) -> None: report = Report(verbose=True) out_lines = [] -- 2.39.5