From dea81b7ad5cfa04c3572771c34af823449d0a8f3 Mon Sep 17 00:00:00 2001 From: Thiago Bellini Ribeiro Date: Fri, 13 Nov 2020 12:26:07 -0300 Subject: [PATCH] Provide a stdin-filename to allow stdin to respect force-exclude rules (#1780) * Provide a stdin-filename to allow stdin to respect exclude/force-exclude rules This will allow automatic tools to enforce the project's exclude/force-exclude rules even if they pass the file through stdin to update its buffer. This is a similar solution to --stdin-display-name in flake8. * Update src/black/__init__.py Co-authored-by: Richard Si <63936253+ichard26@users.noreply.github.com> * --stdin-filename should only respect --exclude-filename * Update README with the new --stdin-filename option * Write some tests for the new stdin-filename functionality * Apply suggestions from code review Co-authored-by: Hugo van Kemenade * Force stdin output when we asked for stdin even if the file exists * Add an entry in the changelog regarding --stdin-filename * Reduce disk reads if possible Co-authored-by: Richard Si <63936253+ichard26@users.noreply.github.com> * Check for is_stdin and p.is_file before checking for p.is_dir() Co-authored-by: Richard Si <63936253+ichard26@users.noreply.github.com> Co-authored-by: Hugo van Kemenade --- README.md | 5 ++ docs/change_log.md | 3 + src/black/__init__.py | 67 +++++++++++++----- tests/test_black.py | 159 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 217 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index d34ce14..a557a6f 100644 --- a/README.md +++ b/README.md @@ -135,6 +135,11 @@ Options: matching this regex will be excluded even when they are passed explicitly as arguments. + --stdin-filename TEXT The name of the file when passing it through + stdin. Useful to make sure Black will respect + --force-exclude option on some editors that + rely on using stdin. + -q, --quiet Don't emit non-error messages to stderr. Errors are still emitted; silence those with 2>/dev/null. diff --git a/docs/change_log.md b/docs/change_log.md index 1ee35a4..e6afefb 100644 --- a/docs/change_log.md +++ b/docs/change_log.md @@ -23,6 +23,9 @@ - Added support for PEP 614 relaxed decorator syntax on python 3.9 (#1711) +- Added `--stdin-filename` argument to allow stdin to respect `--force-exclude` rules. + Works very alike to flake8's `--stdin-display-name` (#1780) + ### 20.8b1 #### _Packaging_ diff --git a/src/black/__init__.py b/src/black/__init__.py index 44edeb0..4869057 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -68,6 +68,7 @@ DEFAULT_LINE_LENGTH = 88 DEFAULT_EXCLUDES = r"/(\.direnv|\.eggs|\.git|\.hg|\.mypy_cache|\.nox|\.tox|\.venv|\.svn|_build|buck-out|build|dist)/" # noqa: B950 DEFAULT_INCLUDES = r"\.pyi?$" CACHE_DIR = Path(user_cache_dir("black", version=__version__)) +STDIN_PLACEHOLDER = "__BLACK_STDIN_FILENAME__" STRING_PREFIX_CHARS: Final = "furbFURB" # All possible string prefix characters. @@ -457,6 +458,15 @@ def target_version_option_callback( "excluded even when they are passed explicitly as arguments." ), ) +@click.option( + "--stdin-filename", + type=str, + help=( + "The name of the file when passing it through stdin. Useful to make " + "sure Black will respect --force-exclude option on some " + "editors that rely on using stdin." + ), +) @click.option( "-q", "--quiet", @@ -516,6 +526,7 @@ def main( include: str, exclude: str, force_exclude: Optional[str], + stdin_filename: Optional[str], src: Tuple[str, ...], config: Optional[str], ) -> None: @@ -548,6 +559,7 @@ def main( exclude=exclude, force_exclude=force_exclude, report=report, + stdin_filename=stdin_filename, ) path_empty( @@ -587,6 +599,7 @@ def get_sources( exclude: str, force_exclude: Optional[str], report: "Report", + stdin_filename: Optional[str], ) -> Set[Path]: """Compute the set of files to be formatted.""" try: @@ -613,22 +626,14 @@ def get_sources( gitignore = get_gitignore(root) for s in src: - p = Path(s) - if p.is_dir(): - sources.update( - gen_python_files( - p.iterdir(), - root, - include_regex, - exclude_regex, - force_exclude_regex, - report, - gitignore, - ) - ) - elif s == "-": - sources.add(p) - elif p.is_file(): + if s == "-" and stdin_filename: + p = Path(stdin_filename) + is_stdin = True + else: + p = Path(s) + is_stdin = False + + if is_stdin or p.is_file(): normalized_path = normalize_path_maybe_ignore(p, root, report) if normalized_path is None: continue @@ -643,6 +648,23 @@ def get_sources( report.path_ignored(p, "matches the --force-exclude regular expression") continue + if is_stdin: + p = Path(f"{STDIN_PLACEHOLDER}{str(p)}") + + sources.add(p) + elif p.is_dir(): + sources.update( + gen_python_files( + p.iterdir(), + root, + include_regex, + exclude_regex, + force_exclude_regex, + report, + gitignore, + ) + ) + elif s == "-": sources.add(p) else: err(f"invalid path: {s}") @@ -670,7 +692,18 @@ def reformat_one( """ try: changed = Changed.NO - if not src.is_file() and str(src) == "-": + + if str(src) == "-": + is_stdin = True + elif str(src).startswith(STDIN_PLACEHOLDER): + is_stdin = True + # Use the original name again in case we want to print something + # to the user + src = Path(str(src)[len(STDIN_PLACEHOLDER) :]) + else: + is_stdin = False + + if is_stdin: if format_stdin_to_stdout(fast=fast, write_back=write_back, mode=mode): changed = Changed.YES else: diff --git a/tests/test_black.py b/tests/test_black.py index b0cf6ed..a688c87 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -1336,10 +1336,169 @@ class BlackTestCase(BlackBaseTestCase): exclude=exclude, force_exclude=None, report=report, + stdin_filename=None, ) ) self.assertEqual(sorted(expected), sorted(sources)) + @patch("black.find_project_root", lambda *args: THIS_DIR.resolve()) + def test_get_sources_with_stdin(self) -> None: + include = "" + exclude = r"/exclude/|a\.py" + src = "-" + report = black.Report() + expected = [Path("-")] + sources = list( + black.get_sources( + ctx=FakeContext(), + src=(src,), + quiet=True, + verbose=False, + include=include, + exclude=exclude, + force_exclude=None, + report=report, + stdin_filename=None, + ) + ) + self.assertEqual(sorted(expected), sorted(sources)) + + @patch("black.find_project_root", lambda *args: THIS_DIR.resolve()) + def test_get_sources_with_stdin_filename(self) -> None: + include = "" + exclude = r"/exclude/|a\.py" + src = "-" + report = black.Report() + stdin_filename = str(THIS_DIR / "data/collections.py") + expected = [Path(f"__BLACK_STDIN_FILENAME__{stdin_filename}")] + sources = list( + black.get_sources( + ctx=FakeContext(), + src=(src,), + quiet=True, + verbose=False, + include=include, + exclude=exclude, + force_exclude=None, + report=report, + stdin_filename=stdin_filename, + ) + ) + self.assertEqual(sorted(expected), sorted(sources)) + + @patch("black.find_project_root", lambda *args: THIS_DIR.resolve()) + def test_get_sources_with_stdin_filename_and_exclude(self) -> None: + # Exclude shouldn't exclude stdin_filename since it is mimicing the + # file being passed directly. This is the same as + # test_exclude_for_issue_1572 + path = THIS_DIR / "data" / "include_exclude_tests" + include = "" + exclude = r"/exclude/|a\.py" + src = "-" + report = black.Report() + stdin_filename = str(path / "b/exclude/a.py") + expected = [Path(f"__BLACK_STDIN_FILENAME__{stdin_filename}")] + sources = list( + black.get_sources( + ctx=FakeContext(), + src=(src,), + quiet=True, + verbose=False, + include=include, + exclude=exclude, + force_exclude=None, + report=report, + stdin_filename=stdin_filename, + ) + ) + self.assertEqual(sorted(expected), sorted(sources)) + + @patch("black.find_project_root", lambda *args: THIS_DIR.resolve()) + def test_get_sources_with_stdin_filename_and_force_exclude(self) -> None: + # Force exclude should exclude the file when passing it through + # stdin_filename + path = THIS_DIR / "data" / "include_exclude_tests" + include = "" + force_exclude = r"/exclude/|a\.py" + src = "-" + report = black.Report() + stdin_filename = str(path / "b/exclude/a.py") + sources = list( + black.get_sources( + ctx=FakeContext(), + src=(src,), + quiet=True, + verbose=False, + include=include, + exclude="", + force_exclude=force_exclude, + report=report, + stdin_filename=stdin_filename, + ) + ) + self.assertEqual([], sorted(sources)) + + def test_reformat_one_with_stdin(self) -> None: + with patch( + "black.format_stdin_to_stdout", + return_value=lambda *args, **kwargs: black.Changed.YES, + ) as fsts: + report = MagicMock() + path = Path("-") + black.reformat_one( + path, + fast=True, + write_back=black.WriteBack.YES, + mode=DEFAULT_MODE, + report=report, + ) + fsts.assert_called_once() + report.done.assert_called_with(path, black.Changed.YES) + + def test_reformat_one_with_stdin_filename(self) -> None: + with patch( + "black.format_stdin_to_stdout", + return_value=lambda *args, **kwargs: black.Changed.YES, + ) as fsts: + report = MagicMock() + p = "foo.py" + path = Path(f"__BLACK_STDIN_FILENAME__{p}") + expected = Path(p) + black.reformat_one( + path, + fast=True, + write_back=black.WriteBack.YES, + mode=DEFAULT_MODE, + report=report, + ) + fsts.assert_called_once() + # __BLACK_STDIN_FILENAME__ should have been striped + report.done.assert_called_with(expected, black.Changed.YES) + + def test_reformat_one_with_stdin_and_existing_path(self) -> None: + with patch( + "black.format_stdin_to_stdout", + return_value=lambda *args, **kwargs: black.Changed.YES, + ) as fsts: + report = MagicMock() + # Even with an existing file, since we are forcing stdin, black + # should output to stdout and not modify the file inplace + p = Path(str(THIS_DIR / "data/collections.py")) + # Make sure is_file actually returns True + self.assertTrue(p.is_file()) + path = Path(f"__BLACK_STDIN_FILENAME__{p}") + expected = Path(p) + black.reformat_one( + path, + fast=True, + write_back=black.WriteBack.YES, + mode=DEFAULT_MODE, + report=report, + ) + fsts.assert_called_once() + # __BLACK_STDIN_FILENAME__ should have been striped + report.done.assert_called_with(expected, black.Changed.YES) + def test_gitignore_exclude(self) -> None: path = THIS_DIR / "data" / "include_exclude_tests" include = re.compile(r"\.pyi?$") -- 2.39.5