From 66008fda5dc07f5626e5f5d0dcefc476a9c12ab8 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Tue, 7 Nov 2023 21:29:24 +0200 Subject: [PATCH] [563] Fix standalone comments inside complex blocks crashing Black (#4016) Bracket depth is not an accurate indicator of standalone comment position inside more complex blocks because bracket depth can be virtual (in loops' and lambdas' parameter blocks) or from optional parens. Here we try to stop cumulating lines upon standalone comments in complex blocks, and try to make standalone comment processing more simple. The fundamental idea is, that if we have a standalone comment, it needs to go on its own line, so we always have to split. This is not perfect, but at least a first step. --- CHANGES.md | 1 + src/black/brackets.py | 7 ++ src/black/linegen.py | 4 +- src/black/lines.py | 27 ++++- tests/data/cases/comments_in_blocks.py | 111 ++++++++++++++++++ ..._parens_with_braces_and_square_brackets.py | 20 ++++ 6 files changed, 163 insertions(+), 7 deletions(-) create mode 100644 tests/data/cases/comments_in_blocks.py diff --git a/CHANGES.md b/CHANGES.md index c8ba83b5ae9..d30622b7786 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,6 +13,7 @@ - Fix crash on formatting bytes strings that look like docstrings (#4003) - Fix crash when whitespace followed a backslash before newline in a docstring (#4008) +- Fix standalone comments inside complex blocks crashing Black (#4016) - Fix crash on formatting code like `await (a ** b)` (#3994) diff --git a/src/black/brackets.py b/src/black/brackets.py index 85dac6edd1e..3020cc0d390 100644 --- a/src/black/brackets.py +++ b/src/black/brackets.py @@ -127,6 +127,13 @@ def mark(self, leaf: Leaf) -> None: self.maybe_increment_lambda_arguments(leaf) self.maybe_increment_for_loop_variable(leaf) + def any_open_for_or_lambda(self) -> bool: + """Return True if there is an open for or lambda expression on the line. + + See maybe_increment_for_loop_variable and maybe_increment_lambda_arguments + for details.""" + return bool(self._for_loop_depths or self._lambda_argument_depths) + def any_open_brackets(self) -> bool: """Return True if there is an yet unmatched open bracket on the line.""" return bool(self.bracket_match) diff --git a/src/black/linegen.py b/src/black/linegen.py index 30cfff3e846..e2c961d7a01 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -861,8 +861,6 @@ def _maybe_split_omitting_optional_parens( # it's not an import (optional parens are the only thing we can split on # in this case; attempting a split without them is a waste of time) and not line.is_import - # there are no standalone comments in the body - and not rhs.body.contains_standalone_comments(0) # and we can actually remove the parens and can_omit_invisible_parens(rhs, mode.line_length) ): @@ -1181,7 +1179,7 @@ def standalone_comment_split( line: Line, features: Collection[Feature], mode: Mode ) -> Iterator[Line]: """Split standalone comments from the rest of the line.""" - if not line.contains_standalone_comments(0): + if not line.contains_standalone_comments(): raise CannotSplit("Line does not have any standalone comments") current_line = Line( diff --git a/src/black/lines.py b/src/black/lines.py index 23c1a93d3d4..f0cf25ba3e7 100644 --- a/src/black/lines.py +++ b/src/black/lines.py @@ -1,6 +1,5 @@ import itertools import math -import sys from dataclasses import dataclass, field from typing import ( Callable, @@ -103,7 +102,10 @@ def append_safe(self, leaf: Leaf, preformatted: bool = False) -> None: Raises ValueError when any `leaf` is appended after a standalone comment or when a standalone comment is not the first leaf on the line. """ - if self.bracket_tracker.depth == 0: + if ( + self.bracket_tracker.depth == 0 + or self.bracket_tracker.any_open_for_or_lambda() + ): if self.is_comment: raise ValueError("cannot append to standalone comments") @@ -233,10 +235,10 @@ def is_fmt_pass_converted( leaf.fmt_pass_converted_first_leaf ) - def contains_standalone_comments(self, depth_limit: int = sys.maxsize) -> bool: + def contains_standalone_comments(self) -> bool: """If so, needs to be split before emitting.""" for leaf in self.leaves: - if leaf.type == STANDALONE_COMMENT and leaf.bracket_depth <= depth_limit: + if leaf.type == STANDALONE_COMMENT: return True return False @@ -982,6 +984,23 @@ def can_omit_invisible_parens( are too long. """ line = rhs.body + + # We need optional parens in order to split standalone comments to their own lines + # if there are no nested parens around the standalone comments + closing_bracket: Optional[Leaf] = None + for leaf in reversed(line.leaves): + if closing_bracket and leaf is closing_bracket.opening_bracket: + closing_bracket = None + if leaf.type == STANDALONE_COMMENT and not closing_bracket: + return False + if ( + not closing_bracket + and leaf.type in CLOSING_BRACKETS + and leaf.opening_bracket in line.leaves + and leaf.value + ): + closing_bracket = leaf + bt = line.bracket_tracker if not bt.delimiters: # Without delimiters the optional parentheses are useless. diff --git a/tests/data/cases/comments_in_blocks.py b/tests/data/cases/comments_in_blocks.py new file mode 100644 index 00000000000..1221139b6d8 --- /dev/null +++ b/tests/data/cases/comments_in_blocks.py @@ -0,0 +1,111 @@ +# Test cases from: +# - https://github.com/psf/black/issues/1798 +# - https://github.com/psf/black/issues/1499 +# - https://github.com/psf/black/issues/1211 +# - https://github.com/psf/black/issues/563 + +( + lambda + # a comment + : None +) + +( + lambda: + # b comment + None +) + +( + lambda + # a comment + : + # b comment + None +) + +[ + x + # Let's do this + for + # OK? + x + # Some comment + # And another + in + # One more + y +] + +return [ + (offers[offer_index], 1.0) + for offer_index, _ + # avoid returning any offers that don't match the grammar so + # that the return values here are consistent with what would be + # returned in AcceptValidHeader + in self._parse_and_normalize_offers(offers) +] + +from foo import ( + bar, + # qux +) + + +def convert(collection): + # replace all variables by integers + replacement_dict = { + variable: f"{index}" + for index, variable + # 0 is reserved as line terminator + in enumerate(collection.variables(), start=1) + } + + +{ + i: i + for i + # a comment + in range(5) +} + + +def get_subtree_proof_nodes( + chunk_index_groups: Sequence[Tuple[int, ...], ...], +) -> Tuple[int, ...]: + subtree_node_paths = ( + # We take a candidate element from each group and shift it to + # remove the bits that are not common to other group members, then + # we convert it to a tree path that all elements from this group + # have in common. + chunk_index + for chunk_index, bits_to_truncate + # Each group will contain an even "power-of-two" number of# elements. + # This tells us how many tailing bits each element has# which need to + # be truncated to get the group's common prefix. + in ((group[0], (len(group) - 1).bit_length()) for group in chunk_index_groups) + ) + return subtree_node_paths + + +if ( + # comment1 + a + # comment2 + or ( + # comment3 + ( + # comment4 + b + ) + # comment5 + and + # comment6 + c + or ( + # comment7 + d + ) + ) +): + print("Foo") diff --git a/tests/data/cases/preview_hug_parens_with_braces_and_square_brackets.py b/tests/data/cases/preview_hug_parens_with_braces_and_square_brackets.py index 51fe516add5..97b5b2e8dd1 100644 --- a/tests/data/cases/preview_hug_parens_with_braces_and_square_brackets.py +++ b/tests/data/cases/preview_hug_parens_with_braces_and_square_brackets.py @@ -152,6 +152,16 @@ def foo_square_brackets(request): foo(**{x: y for x, y in enumerate(["long long long long line","long long long long line"])}) +for foo in ["a", "b"]: + output.extend([ + individual + for + # Foobar + container in xs_by_y[foo] + # Foobar + for individual in container["nested"] + ]) + # output def foo_brackets(request): return JsonResponse({ @@ -323,3 +333,13 @@ def foo_square_brackets(request): foo(**{ x: y for x, y in enumerate(["long long long long line", "long long long long line"]) }) + +for foo in ["a", "b"]: + output.extend([ + individual + for + # Foobar + container in xs_by_y[foo] + # Foobar + for individual in container["nested"] + ])