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.

diff --git a/CHANGES.md b/CHANGES.md
index d203896a801fba9fcd5d84a54294efef26eecfef..379906865085bb9ca9de2137f1a3cf95f8456b2d 100644 (file)
--- 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 7024c9d52b059f5dd8b7db94c2c5655619afe706..769e693ed2386659522f2f2fdda6e5516f8883fb 100644 (file)
--- 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 5ad29a8affdc5df1e42eec1c4cafa1dc4894010b..f5be2f24cf41ccf1c2d2f62f813b3894bfc3547d 100644 (file)
--- 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 2817073816eebe96c46552ba4e1f8dc4477c5a9b..1dfb54ca687d4a9a679d6180f375a931090e265b 100644 (file)
--- 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 e6a673ec537b7ecb6978efb34976f4659d094646..bccf47430a74f622392ad180bd06e4cb76e1cd1e 100644 (file)
--- 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 8d691d2f0194a998b76af56f1d6b1f69bb0c7513..2dd284f2cd6ed176f030e986f1aacaf13d4dc823 100644 (file)
--- 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 c6c811040dcac311a0a921ab5167c815fbb9b0dc..a4619b4a652e85c5cee070b28cc44cebe6270956 100644 (file)
--- 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",
 ]