Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make --exclude only apply to recursively found files #1591

Merged
merged 3 commits into from
Aug 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 65 additions & 34 deletions src/black/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -583,9 +583,7 @@ def get_sources(
root = find_project_root(src)
sources: Set[Path] = set()
path_empty(src, "No Path provided. Nothing to do 😴", quiet, verbose, ctx)
exclude_regexes = [exclude_regex]
if force_exclude_regex is not None:
exclude_regexes.append(force_exclude_regex)
gitignore = get_gitignore(root)

for s in src:
p = Path(s)
Expand All @@ -595,19 +593,30 @@ def get_sources(
p.iterdir(),
root,
include_regex,
exclude_regexes,
exclude_regex,
force_exclude_regex,
report,
get_gitignore(root),
gitignore,
)
)
elif s == "-":
sources.add(p)
elif p.is_file():
sources.update(
gen_python_files(
[p], root, None, exclude_regexes, report, get_gitignore(root)
)
)
normalized_path = normalize_path_maybe_ignore(p, root, report)
if normalized_path is None:
continue

normalized_path = "/" + normalized_path
# Hard-exclude any files that matches the `--force-exclude` regex.
if force_exclude_regex:
force_exclude_match = force_exclude_regex.search(normalized_path)
else:
force_exclude_match = None
if force_exclude_match and force_exclude_match.group(0):
report.path_ignored(p, "matches the --force-exclude regular expression")
continue

sources.add(p)
else:
err(f"invalid path: {s}")
return sources
Expand Down Expand Up @@ -5759,60 +5768,82 @@ def get_gitignore(root: Path) -> PathSpec:
return PathSpec.from_lines("gitwildmatch", lines)


def normalize_path_maybe_ignore(
path: Path, root: Path, report: "Report"
) -> Optional[str]:
"""Normalize `path`. May return `None` if `path` was ignored.

`report` is where "path ignored" output goes.
"""
try:
normalized_path = path.resolve().relative_to(root).as_posix()
except OSError as e:
report.path_ignored(path, f"cannot be read because {e}")
return None

except ValueError:
if path.is_symlink():
report.path_ignored(path, f"is a symbolic link that points outside {root}")
return None

raise

return normalized_path


def gen_python_files(
paths: Iterable[Path],
root: Path,
include: Optional[Pattern[str]],
exclude_regexes: Iterable[Pattern[str]],
exclude: Pattern[str],
force_exclude: Optional[Pattern[str]],
report: "Report",
gitignore: PathSpec,
) -> Iterator[Path]:
"""Generate all files under `path` whose paths are not excluded by the
`exclude` regex, but are included by the `include` regex.
`exclude_regex` or `force_exclude` regexes, but are included by the `include` regex.

Symbolic links pointing outside of the `root` directory are ignored.

`report` is where output about exclusions goes.
"""
assert root.is_absolute(), f"INTERNAL ERROR: `root` must be absolute but is {root}"
for child in paths:
# Then ignore with `exclude` option.
try:
normalized_path = child.resolve().relative_to(root).as_posix()
except OSError as e:
report.path_ignored(child, f"cannot be read because {e}")
normalized_path = normalize_path_maybe_ignore(child, root, report)
if normalized_path is None:
continue
except ValueError:
if child.is_symlink():
report.path_ignored(
child, f"is a symbolic link that points outside {root}"
)
continue

raise

# First ignore files matching .gitignore
if gitignore.match_file(normalized_path):
report.path_ignored(child, "matches the .gitignore file content")
continue

# Then ignore with `--exclude` and `--force-exclude` options.
normalized_path = "/" + normalized_path
if child.is_dir():
normalized_path += "/"

is_excluded = False
for exclude in exclude_regexes:
exclude_match = exclude.search(normalized_path) if exclude else None
if exclude_match and exclude_match.group(0):
report.path_ignored(child, "matches the --exclude regular expression")
is_excluded = True
break
if is_excluded:
exclude_match = exclude.search(normalized_path) if exclude else None
if exclude_match and exclude_match.group(0):
report.path_ignored(child, "matches the --exclude regular expression")
continue

force_exclude_match = (
force_exclude.search(normalized_path) if force_exclude else None
)
if force_exclude_match and force_exclude_match.group(0):
report.path_ignored(child, "matches the --force-exclude regular expression")
continue

if child.is_dir():
yield from gen_python_files(
child.iterdir(), root, include, exclude_regexes, report, gitignore
child.iterdir(),
root,
include,
exclude,
force_exclude,
report,
gitignore,
)

elif child.is_file():
Expand Down
63 changes: 47 additions & 16 deletions tests/test_black.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,20 @@ def skip_if_exception(e: str) -> Iterator[None]:
raise


class FakeContext(click.Context):
"""A fake click Context for when calling functions that need it."""

def __init__(self) -> None:
self.default_map: Dict[str, Any] = {}


class FakeParameter(click.Parameter):
"""A fake click Parameter for when calling functions that need it."""

def __init__(self) -> None:
pass


class BlackRunner(CliRunner):
"""Modify CliRunner so that stderr is not merged with stdout.

Expand Down Expand Up @@ -1551,7 +1565,32 @@ def test_include_exclude(self) -> None:
this_abs = THIS_DIR.resolve()
sources.extend(
black.gen_python_files(
path.iterdir(), this_abs, include, [exclude], report, gitignore
path.iterdir(), this_abs, include, exclude, None, report, gitignore
)
)
self.assertEqual(sorted(expected), sorted(sources))

@patch("black.find_project_root", lambda *args: THIS_DIR.resolve())
def test_exclude_for_issue_1572(self) -> None:
# Exclude shouldn't touch files that were explicitly given to Black through the
# CLI. Exclude is supposed to only apply to the recursive discovery of files.
# https://github.com/psf/black/issues/1572
path = THIS_DIR / "data" / "include_exclude_tests"
include = ""
exclude = r"/exclude/|a\.py"
src = str(path / "b/exclude/a.py")
report = black.Report()
expected = [Path(path / "b/exclude/a.py")]
sources = list(
black.get_sources(
ctx=FakeContext(),
src=(src,),
quiet=True,
verbose=False,
include=include,
exclude=exclude,
force_exclude=None,
report=report,
)
)
self.assertEqual(sorted(expected), sorted(sources))
Expand All @@ -1572,7 +1611,7 @@ def test_gitignore_exclude(self) -> None:
this_abs = THIS_DIR.resolve()
sources.extend(
black.gen_python_files(
path.iterdir(), this_abs, include, [exclude], report, gitignore
path.iterdir(), this_abs, include, exclude, None, report, gitignore
)
)
self.assertEqual(sorted(expected), sorted(sources))
Expand Down Expand Up @@ -1600,7 +1639,8 @@ def test_empty_include(self) -> None:
path.iterdir(),
this_abs,
empty,
[re.compile(black.DEFAULT_EXCLUDES)],
re.compile(black.DEFAULT_EXCLUDES),
None,
report,
gitignore,
)
Expand All @@ -1627,7 +1667,8 @@ def test_empty_exclude(self) -> None:
path.iterdir(),
this_abs,
re.compile(black.DEFAULT_INCLUDES),
[empty],
empty,
None,
report,
gitignore,
)
Expand Down Expand Up @@ -1684,7 +1725,7 @@ def test_symlink_out_of_root_directory(self) -> None:
try:
list(
black.gen_python_files(
path.iterdir(), root, include, exclude, report, gitignore
path.iterdir(), root, include, exclude, None, report, gitignore
)
)
except ValueError as ve:
Expand All @@ -1698,7 +1739,7 @@ def test_symlink_out_of_root_directory(self) -> None:
with self.assertRaises(ValueError):
list(
black.gen_python_files(
path.iterdir(), root, include, exclude, report, gitignore
path.iterdir(), root, include, exclude, None, report, gitignore
)
)
path.iterdir.assert_called()
Expand Down Expand Up @@ -1777,16 +1818,6 @@ def test_parse_pyproject_toml(self) -> None:

def test_read_pyproject_toml(self) -> None:
test_toml_file = THIS_DIR / "test.toml"

# Fake a click context and parameter so mypy stays happy
class FakeContext(click.Context):
def __init__(self) -> None:
self.default_map: Dict[str, Any] = {}

class FakeParameter(click.Parameter):
def __init__(self) -> None:
pass

fake_ctx = FakeContext()
black.read_pyproject_toml(
fake_ctx, FakeParameter(), str(test_toml_file),
Expand Down