From d97b7898b34b67eb3c6839998920e17ac8c77908 Mon Sep 17 00:00:00 2001 From: Antonio Ossa-Guerra Date: Fri, 11 Nov 2022 22:05:36 -0300 Subject: [PATCH] Remove whitespaces of whitespace-only files (#3348) Currently, empty and whitespace-only (with or without newlines) are not modified. In some discussions (issues and pull requests) consensus was to reformat whitespace-only files to empty or single-character files, preserving line endings when possible. With that said, this commit introduces the following behaviors: * Empty files are left as is * Whitespace-only files (no newline) reformat into empty files * Whitespace-only files (1 or more newlines) reformat into a single newline character To implement these changes, we moved the initial check at `format_file_contents` that raises `NothingChanged` if the source (with no whitespaces) is an empty string. In the case of *.ipynb files, `format_ipynb_string` checks a similar condition and removed whitespaces. In the case of Python files, `format_str_once` includes a check on the output that returns the correct newline character if possible or an empty string otherwise. Signed-off-by: Antonio Ossa Guerra --- CHANGES.md | 2 + src/black/__init__.py | 12 ++++- tests/data/preview/whitespace.py | 6 +++ tests/test_black.py | 87 +++++++++++++++++++++++++++++++- 4 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 tests/data/preview/whitespace.py diff --git a/CHANGES.md b/CHANGES.md index 4d1887f2842..992b3800405 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -15,6 +15,8 @@ - Enforce empty lines before classes and functions with sticky leading comments (#3302) +- Reformat empty and whitespace-only files as either an empty file (if no newline is + present) or as a single newline character (if a newline is present) (#3348) - Implicitly concatenated strings used as function args are now wrapped inside parentheses (#3307) - Correctly handle trailing commas that are inside a line's leading non-nested parens diff --git a/src/black/__init__.py b/src/black/__init__.py index 2786861e9e0..7d7ddbe0930 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -917,7 +917,7 @@ def format_file_contents(src_contents: str, *, fast: bool, mode: Mode) -> FileCo valid by calling :func:`assert_equivalent` and :func:`assert_stable` on it. `mode` is passed to :func:`format_str`. """ - if not src_contents.strip(): + if not mode.preview and not src_contents.strip(): raise NothingChanged if mode.is_ipynb: @@ -1014,6 +1014,9 @@ def format_ipynb_string(src_contents: str, *, fast: bool, mode: Mode) -> FileCon Operate cell-by-cell, only on code cells, only for Python notebooks. If the ``.ipynb`` originally had a trailing newline, it'll be preserved. """ + if mode.preview and not src_contents: + raise NothingChanged + trailing_newline = src_contents[-1] == "\n" modified = False nb = json.loads(src_contents) @@ -1106,6 +1109,13 @@ def _format_str_once(src_contents: str, *, mode: Mode) -> str: dst_contents = [] for block in dst_blocks: dst_contents.extend(block.all_lines()) + if mode.preview and not dst_contents: + # Use decode_bytes to retrieve the correct source newline (CRLF or LF), + # and check if normalized_content has more than one line + normalized_content, _, newline = decode_bytes(src_contents.encode("utf-8")) + if "\n" in normalized_content: + return newline + return "" return "".join(dst_contents) diff --git a/tests/data/preview/whitespace.py b/tests/data/preview/whitespace.py new file mode 100644 index 00000000000..a319c0117b1 --- /dev/null +++ b/tests/data/preview/whitespace.py @@ -0,0 +1,6 @@ + + + + + +# output diff --git a/tests/test_black.py b/tests/test_black.py index a43f05e083b..dda10555c97 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -25,6 +25,7 @@ List, Optional, Sequence, + Type, TypeVar, Union, ) @@ -153,6 +154,34 @@ def test_empty_ff(self) -> None: os.unlink(tmp_file) self.assertFormatEqual(expected, actual) + @patch("black.dump_to_file", dump_to_stderr) + def test_one_empty_line(self) -> None: + mode = black.Mode(preview=True) + for nl in ["\n", "\r\n"]: + source = expected = nl + assert_format(source, expected, mode=mode) + + def test_one_empty_line_ff(self) -> None: + mode = black.Mode(preview=True) + for nl in ["\n", "\r\n"]: + expected = nl + tmp_file = Path(black.dump_to_file(nl)) + if system() == "Windows": + # Writing files in text mode automatically uses the system newline, + # but in this case we don't want this for testing reasons. See: + # https://github.com/psf/black/pull/3348 + with open(tmp_file, "wb") as f: + f.write(nl.encode("utf-8")) + try: + self.assertFalse( + ff(tmp_file, mode=mode, write_back=black.WriteBack.YES) + ) + with open(tmp_file, "rb") as f: + actual = f.read().decode("utf8") + finally: + os.unlink(tmp_file) + self.assertFormatEqual(expected, actual) + def test_experimental_string_processing_warns(self) -> None: self.assertWarns( black.mode.Deprecated, black.Mode, experimental_string_processing=True @@ -971,8 +1000,8 @@ def err(msg: str, **kwargs: Any) -> None: ) def test_format_file_contents(self) -> None: - empty = "" mode = DEFAULT_MODE + empty = "" with self.assertRaises(black.NothingChanged): black.format_file_contents(empty, mode=mode, fast=False) just_nl = "\n" @@ -990,6 +1019,17 @@ def test_format_file_contents(self) -> None: black.format_file_contents(invalid, mode=mode, fast=False) self.assertEqual(str(e.exception), "Cannot parse: 1:7: return if you can") + mode = black.Mode(preview=True) + just_crlf = "\r\n" + with self.assertRaises(black.NothingChanged): + black.format_file_contents(just_crlf, mode=mode, fast=False) + just_whitespace_nl = "\n\t\n \n\t \n \t\n\n" + actual = black.format_file_contents(just_whitespace_nl, mode=mode, fast=False) + self.assertEqual("\n", actual) + just_whitespace_crlf = "\r\n\t\r\n \r\n\t \r\n \t\r\n\r\n" + actual = black.format_file_contents(just_whitespace_crlf, mode=mode, fast=False) + self.assertEqual("\r\n", actual) + def test_endmarker(self) -> None: n = black.lib2to3_parse("\n") self.assertEqual(n.type, black.syms.file_input) @@ -1281,8 +1321,51 @@ def test_reformat_one_with_stdin_and_existing_path(self) -> None: report.done.assert_called_with(expected, black.Changed.YES) def test_reformat_one_with_stdin_empty(self) -> None: + cases = [ + ("", ""), + ("\n", "\n"), + ("\r\n", "\r\n"), + (" \t", ""), + (" \t\n\t ", "\n"), + (" \t\r\n\t ", "\r\n"), + ] + + def _new_wrapper( + output: io.StringIO, io_TextIOWrapper: Type[io.TextIOWrapper] + ) -> Callable[[Any, Any], io.TextIOWrapper]: + def get_output(*args: Any, **kwargs: Any) -> io.TextIOWrapper: + if args == (sys.stdout.buffer,): + # It's `format_stdin_to_stdout()` calling `io.TextIOWrapper()`, + # return our mock object. + return output + # It's something else (i.e. `decode_bytes()`) calling + # `io.TextIOWrapper()`, pass through to the original implementation. + # See discussion in https://github.com/psf/black/pull/2489 + return io_TextIOWrapper(*args, **kwargs) + + return get_output + + mode = black.Mode(preview=True) + for content, expected in cases: + output = io.StringIO() + io_TextIOWrapper = io.TextIOWrapper + + with patch("io.TextIOWrapper", _new_wrapper(output, io_TextIOWrapper)): + try: + black.format_stdin_to_stdout( + fast=True, + content=content, + write_back=black.WriteBack.YES, + mode=mode, + ) + except io.UnsupportedOperation: + pass # StringIO does not support detach + assert output.getvalue() == expected + + # An empty string is the only test case for `preview=False` output = io.StringIO() - with patch("io.TextIOWrapper", lambda *args, **kwargs: output): + io_TextIOWrapper = io.TextIOWrapper + with patch("io.TextIOWrapper", _new_wrapper(output, io_TextIOWrapper)): try: black.format_stdin_to_stdout( fast=True,