From c31befd0d8be36ca770ab1e14b9947e938b063de Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Mon, 24 Jan 2022 20:22:43 -0800 Subject: [PATCH 1/6] Fix crash on some power hugging cases Found by the fuzzer. Repro case: python -m black -c 'importA;()<<0**0#' Unfortunately the resulting code still crashes due to an apparently unrelated stability bug. I'd like to fix the crash first. --- src/blib2to3/pytree.py | 6 +++++- tests/data/power_op_newline.py | 6 ++++++ tests/test_format.py | 7 +++++++ 3 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 tests/data/power_op_newline.py diff --git a/src/blib2to3/pytree.py b/src/blib2to3/pytree.py index bd86270b8e2..b203ce5b2ac 100644 --- a/src/blib2to3/pytree.py +++ b/src/blib2to3/pytree.py @@ -386,7 +386,8 @@ class Leaf(Base): value: Text fixers_applied: List[Any] bracket_depth: int - opening_bracket: "Leaf" + # Changed later in brackets.py + opening_bracket: Optional["Leaf"] = None used_names: Optional[Set[Text]] _prefix = "" # Whitespace and comments preceding this token in the input lineno: int = 0 # Line where this token starts in the input @@ -399,6 +400,7 @@ def __init__( context: Optional[Context] = None, prefix: Optional[Text] = None, fixers_applied: List[Any] = [], + opening_bracket: Optional["Leaf"] = None, ) -> None: """ Initializer. @@ -416,6 +418,7 @@ def __init__( self._prefix = prefix self.fixers_applied: Optional[List[Any]] = fixers_applied[:] self.children = [] + self.opening_bracket = opening_bracket def __repr__(self) -> str: """Return a canonical string representation.""" @@ -448,6 +451,7 @@ def clone(self) -> "Leaf": self.value, (self.prefix, (self.lineno, self.column)), fixers_applied=self.fixers_applied, + opening_bracket=self.opening_bracket, ) def leaves(self) -> Iterator["Leaf"]: diff --git a/tests/data/power_op_newline.py b/tests/data/power_op_newline.py new file mode 100644 index 00000000000..0e7775b0ffe --- /dev/null +++ b/tests/data/power_op_newline.py @@ -0,0 +1,6 @@ +importA;()<<0**0# + +# output + +importA +() << 0**0 # diff --git a/tests/test_format.py b/tests/test_format.py index c6c811040dc..48f14166e51 100644 --- a/tests/test_format.py +++ b/tests/test_format.py @@ -253,3 +253,10 @@ def test_python38() -> None: def test_python39() -> None: source, expected = read_data("python39") assert_format(source, expected, minimum_version=(3, 9)) + + +def test_power_op_newline() -> None: + # requires line_length=0 + source, expected = read_data("power_op_newline") + # fast=True because this exposes a different stability bug + assert_format(source, expected, mode=black.Mode(line_length=0), fast=True) From 31f94d9919c67886975b5b132e68f746af4a0d77 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Mon, 24 Jan 2022 20:27:10 -0800 Subject: [PATCH 2/6] fix mypy --- src/black/linegen.py | 2 ++ src/black/lines.py | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/black/linegen.py b/src/black/linegen.py index 9fbdfadba6a..ac60ed1986d 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -942,6 +942,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf if ( prev and prev.type == token.COMMA + and leaf.opening_bracket is not None and not is_one_tuple_between( leaf.opening_bracket, leaf, line.leaves ) @@ -969,6 +970,7 @@ def generate_trailers_to_omit(line: Line, line_length: int) -> Iterator[Set[Leaf if ( prev and prev.type == token.COMMA + and leaf.opening_bracket is not None and not is_one_tuple_between(leaf.opening_bracket, leaf, line.leaves) ): # Never omit bracket pairs with trailing commas. diff --git a/src/black/lines.py b/src/black/lines.py index c602aa69ce9..7d50f02aebc 100644 --- a/src/black/lines.py +++ b/src/black/lines.py @@ -277,7 +277,9 @@ def has_magic_trailing_comma( if self.is_import: return True - if not is_one_tuple_between(closing.opening_bracket, closing, self.leaves): + if closing.opening_bracket is not None and not is_one_tuple_between( + closing.opening_bracket, closing, self.leaves + ): return True return False From ff9c6549550c2711849c7d91efbfc23dcf195195 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Mon, 24 Jan 2022 20:40:15 -0800 Subject: [PATCH 3/6] properly run ourselves twice --- CHANGES.md | 2 ++ src/black/__init__.py | 24 ++++++++++++------------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index d203896a801..8fcd4c2c236 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 7024c9d52b0..1f501ef45f2 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 f( 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) From 5a3ca0df54a3d11a42109deac93d74f0954bd578 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Mon, 24 Jan 2022 20:42:06 -0800 Subject: [PATCH 4/6] prettier --- CHANGES.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 8fcd4c2c236..37990686508 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -40,8 +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) +- Work around bug that causes unstable formatting in some cases in the presence of the + magic trailing comma (#2807) ### Packaging From 274b874082c113351d45ad8e12d7a9b0a0eb7ab2 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Tue, 25 Jan 2022 12:41:57 -0800 Subject: [PATCH 5/6] fix fast test --- tests/data/power_op_newline.py | 6 +++++- tests/test_format.py | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/data/power_op_newline.py b/tests/data/power_op_newline.py index 0e7775b0ffe..85d434d63f6 100644 --- a/tests/data/power_op_newline.py +++ b/tests/data/power_op_newline.py @@ -3,4 +3,8 @@ # output importA -() << 0**0 # +( + () + << 0 + ** 0 +) # diff --git a/tests/test_format.py b/tests/test_format.py index 2594e5ee6e9..b0e1db5f4d2 100644 --- a/tests/test_format.py +++ b/tests/test_format.py @@ -262,4 +262,4 @@ def test_power_op_newline() -> None: # requires line_length=0 source, expected = read_data("power_op_newline") # fast=True because this exposes a different stability bug - assert_format(source, expected, mode=black.Mode(line_length=0), fast=True) + assert_format(source, expected, mode=black.Mode(line_length=0)) From 3866454442bee324facdc3f12c00c16ab2a7a59a Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Tue, 25 Jan 2022 16:02:03 -0800 Subject: [PATCH 6/6] Update tests/test_format.py --- tests/test_format.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_format.py b/tests/test_format.py index b0e1db5f4d2..88f084ea478 100644 --- a/tests/test_format.py +++ b/tests/test_format.py @@ -261,5 +261,4 @@ def test_python39() -> None: def test_power_op_newline() -> None: # requires line_length=0 source, expected = read_data("power_op_newline") - # fast=True because this exposes a different stability bug assert_format(source, expected, mode=black.Mode(line_length=0))