Skip to content

Commit

Permalink
Apply ignore logic before symlink resolution (#3846)
Browse files Browse the repository at this point in the history
This means, for instance, that a gitignored symlink cannot affect your
formatting. Fixes #3527, fixes #3826
  • Loading branch information
hauntsaninja committed Sep 7, 2023
1 parent 58f1bf6 commit df50fee
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 36 deletions.
2 changes: 2 additions & 0 deletions CHANGES.md
Expand Up @@ -20,6 +20,8 @@

<!-- 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 -->
Expand Down
20 changes: 11 additions & 9 deletions src/black/files.py
Expand Up @@ -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.
Expand Down
62 changes: 35 additions & 27 deletions tests/test_black.py
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit df50fee

Please sign in to comment.