From 32dd9ecb2e9dec8b29c07726d5713ed5b4c36547 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Tue, 25 Jan 2022 15:58:58 -0800 Subject: [PATCH] properly run ourselves twice (#2807) The previous run-twice logic only affected the stability checks but not the output. Now, we actually output the twice-formatted code. --- CHANGES.md | 2 + src/black/__init__.py | 29 +++++++------- tests/data/trailing_comma_optional_parens1.py | 12 ++++++ tests/data/trailing_comma_optional_parens2.py | 16 +++++++- tests/data/trailing_comma_optional_parens3.py | 18 ++++++++- tests/test_black.py | 39 ------------------- tests/test_format.py | 3 ++ 7 files changed, 65 insertions(+), 54 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index d203896..3799068 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -40,6 +40,8 @@ - Enable Python 3.10+ by default, without any extra need to specify `--target-version=py310`. (#2758) - Make passing `SRC` or `--code` mandatory and mutually exclusive (#2804) +- Work around bug that causes unstable formatting in some cases in the presence of the + magic trailing comma (#2807) ### Packaging diff --git a/src/black/__init__.py b/src/black/__init__.py index 7024c9d..769e693 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -968,17 +968,7 @@ def check_stability_and_equivalence( content differently. """ assert_equivalent(src_contents, dst_contents) - - # Forced second pass to work around optional trailing commas (becoming - # forced trailing commas on pass 2) interacting differently with optional - # parentheses. Admittedly ugly. - dst_contents_pass2 = format_str(dst_contents, mode=mode) - if dst_contents != dst_contents_pass2: - dst_contents = dst_contents_pass2 - assert_equivalent(src_contents, dst_contents, pass_num=2) - assert_stable(src_contents, dst_contents, mode=mode) - # Note: no need to explicitly call `assert_stable` if `dst_contents` was - # the same as `dst_contents_pass2`. + assert_stable(src_contents, dst_contents, mode=mode) def format_file_contents(src_contents: str, *, fast: bool, mode: Mode) -> FileContent: @@ -1108,7 +1098,7 @@ def format_ipynb_string(src_contents: str, *, fast: bool, mode: Mode) -> FileCon raise NothingChanged -def format_str(src_contents: str, *, mode: Mode) -> FileContent: +def format_str(src_contents: str, *, mode: Mode) -> str: """Reformat a string and return new contents. `mode` determines formatting options, such as how many characters per line are @@ -1138,6 +1128,16 @@ def format_str(src_contents: str, *, mode: Mode) -> FileContent: hey """ + dst_contents = _format_str_once(src_contents, mode=mode) + # Forced second pass to work around optional trailing commas (becoming + # forced trailing commas on pass 2) interacting differently with optional + # parentheses. Admittedly ugly. + if src_contents != dst_contents: + return _format_str_once(dst_contents, mode=mode) + return dst_contents + + +def _format_str_once(src_contents: str, *, mode: Mode) -> str: src_node = lib2to3_parse(src_contents.lstrip(), mode.target_versions) dst_contents = [] future_imports = get_future_imports(src_node) @@ -1367,7 +1367,10 @@ def assert_equivalent(src: str, dst: str, *, pass_num: int = 1) -> None: def assert_stable(src: str, dst: str, mode: Mode) -> None: """Raise AssertionError if `dst` reformats differently the second time.""" - newdst = format_str(dst, mode=mode) + # We shouldn't call format_str() here, because that formats the string + # twice and may hide a bug where we bounce back and forth between two + # versions. + newdst = _format_str_once(dst, mode=mode) if dst != newdst: log = dump_to_file( str(mode), diff --git a/tests/data/trailing_comma_optional_parens1.py b/tests/data/trailing_comma_optional_parens1.py index 5ad29a8..f5be2f2 100644 --- a/tests/data/trailing_comma_optional_parens1.py +++ b/tests/data/trailing_comma_optional_parens1.py @@ -1,3 +1,15 @@ if e1234123412341234.winerror not in (_winapi.ERROR_SEM_TIMEOUT, _winapi.ERROR_PIPE_BUSY) or _check_timeout(t): + pass + +# output + +if ( + e1234123412341234.winerror + not in ( + _winapi.ERROR_SEM_TIMEOUT, + _winapi.ERROR_PIPE_BUSY, + ) + or _check_timeout(t) +): pass \ No newline at end of file diff --git a/tests/data/trailing_comma_optional_parens2.py b/tests/data/trailing_comma_optional_parens2.py index 2817073..1dfb54c 100644 --- a/tests/data/trailing_comma_optional_parens2.py +++ b/tests/data/trailing_comma_optional_parens2.py @@ -1,3 +1,17 @@ if (e123456.get_tk_patchlevel() >= (8, 6, 0, 'final') or (8, 5, 8) <= get_tk_patchlevel() < (8, 6)): - pass \ No newline at end of file + pass + +# output + +if ( + e123456.get_tk_patchlevel() >= (8, 6, 0, "final") + or ( + 8, + 5, + 8, + ) + <= get_tk_patchlevel() + < (8, 6) +): + pass diff --git a/tests/data/trailing_comma_optional_parens3.py b/tests/data/trailing_comma_optional_parens3.py index e6a673e..bccf474 100644 --- a/tests/data/trailing_comma_optional_parens3.py +++ b/tests/data/trailing_comma_optional_parens3.py @@ -5,4 +5,20 @@ if True: "qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweas " + "qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqwegqweasdzxcqweasdzxc.", "qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqwe", - ) % {"reported_username": reported_username, "report_reason": report_reason} \ No newline at end of file + ) % {"reported_username": reported_username, "report_reason": report_reason} + + +# output + + +if True: + if True: + if True: + return _( + "qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweas " + + "qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqwegqweasdzxcqweasdzxc.", + "qweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqweasdzxcqwe", + ) % { + "reported_username": reported_username, + "report_reason": report_reason, + } \ No newline at end of file diff --git a/tests/test_black.py b/tests/test_black.py index 8d691d2..2dd284f 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -228,45 +228,6 @@ class BlackTestCase(BlackBaseTestCase): black.assert_equivalent(source, actual) black.assert_stable(source, actual, black.FileMode()) - @unittest.expectedFailure - @patch("black.dump_to_file", dump_to_stderr) - def test_trailing_comma_optional_parens_stability1(self) -> None: - source, _expected = read_data("trailing_comma_optional_parens1") - actual = fs(source) - black.assert_stable(source, actual, DEFAULT_MODE) - - @unittest.expectedFailure - @patch("black.dump_to_file", dump_to_stderr) - def test_trailing_comma_optional_parens_stability2(self) -> None: - source, _expected = read_data("trailing_comma_optional_parens2") - actual = fs(source) - black.assert_stable(source, actual, DEFAULT_MODE) - - @unittest.expectedFailure - @patch("black.dump_to_file", dump_to_stderr) - def test_trailing_comma_optional_parens_stability3(self) -> None: - source, _expected = read_data("trailing_comma_optional_parens3") - actual = fs(source) - black.assert_stable(source, actual, DEFAULT_MODE) - - @patch("black.dump_to_file", dump_to_stderr) - def test_trailing_comma_optional_parens_stability1_pass2(self) -> None: - source, _expected = read_data("trailing_comma_optional_parens1") - actual = fs(fs(source)) # this is what `format_file_contents` does with --safe - black.assert_stable(source, actual, DEFAULT_MODE) - - @patch("black.dump_to_file", dump_to_stderr) - def test_trailing_comma_optional_parens_stability2_pass2(self) -> None: - source, _expected = read_data("trailing_comma_optional_parens2") - actual = fs(fs(source)) # this is what `format_file_contents` does with --safe - black.assert_stable(source, actual, DEFAULT_MODE) - - @patch("black.dump_to_file", dump_to_stderr) - def test_trailing_comma_optional_parens_stability3_pass2(self) -> None: - source, _expected = read_data("trailing_comma_optional_parens3") - actual = fs(fs(source)) # this is what `format_file_contents` does with --safe - black.assert_stable(source, actual, DEFAULT_MODE) - def test_pep_572_version_detection(self) -> None: source, _ = read_data("pep_572") root = black.lib2to3_parse(source) diff --git a/tests/test_format.py b/tests/test_format.py index c6c8110..a4619b4 100644 --- a/tests/test_format.py +++ b/tests/test_format.py @@ -52,6 +52,9 @@ SIMPLE_CASES: List[str] = [ "remove_parens", "slices", "string_prefixes", + "trailing_comma_optional_parens1", + "trailing_comma_optional_parens2", + "trailing_comma_optional_parens3", "tricky_unicode_symbols", "tupleassign", ] -- 2.39.5