From c046388bbbebdb6b2e269ec84ed22b1bd97ea204 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 2 Nov 2023 00:30:58 +0200 Subject: [PATCH 1/8] Test cases --- tests/data/cases/comments_in_blocks.py | 88 +++++++++++++++++++ ..._parens_with_braces_and_square_brackets.py | 20 +++++ 2 files changed, 108 insertions(+) create mode 100644 tests/data/cases/comments_in_blocks.py diff --git a/tests/data/cases/comments_in_blocks.py b/tests/data/cases/comments_in_blocks.py new file mode 100644 index 00000000000..876c5d791eb --- /dev/null +++ b/tests/data/cases/comments_in_blocks.py @@ -0,0 +1,88 @@ +# 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 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"] + ]) From bd3754c42b4600ffdab8150bc949fa7d16012100 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 2 Nov 2023 01:22:03 +0200 Subject: [PATCH 2/8] Fix standalone comment processing 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. --- src/black/linegen.py | 4 +--- src/black/lines.py | 30 ++++++++++++++++++++++++++---- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/black/linegen.py b/src/black/linegen.py index 121c6e314fe..0d4b04eb720 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 a73c429e3d9..94ad9734512 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,11 @@ 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._for_loop_depths + or self.bracket_tracker._lambda_argument_depths + ): if self.is_comment: raise ValueError("cannot append to standalone comments") @@ -233,10 +236,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 @@ -981,6 +984,25 @@ 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 inside + 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: + if closing_bracket: + break + 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. From e42e19d086414881adc11b20ce376871f5a90f8b Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 2 Nov 2023 01:25:43 +0200 Subject: [PATCH 3/8] Add changelog line --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index f365f1c239b..2ee7d918bec 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,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 (#XXXX) ### Preview style From 4e4cbbc5e97369f3867d4053cd7b3fa50800e20d Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 2 Nov 2023 01:33:37 +0200 Subject: [PATCH 4/8] Update PR number --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 2ee7d918bec..223cc2694b0 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,7 +10,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 (#XXXX) +- Fix standalone comments inside complex blocks crashing Black (#4016) ### Preview style From 624cf9fc0d64e107102b082da8fcf31337cb8d4e Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 2 Nov 2023 02:05:58 +0200 Subject: [PATCH 5/8] Add test case --- tests/data/cases/comments_in_blocks.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/data/cases/comments_in_blocks.py b/tests/data/cases/comments_in_blocks.py index 876c5d791eb..1221139b6d8 100644 --- a/tests/data/cases/comments_in_blocks.py +++ b/tests/data/cases/comments_in_blocks.py @@ -86,3 +86,26 @@ def get_subtree_proof_nodes( 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") From a86c13abec68c990ba762c37601dcd4a799bd14e Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 2 Nov 2023 02:06:47 +0200 Subject: [PATCH 6/8] Have to consider all standalone comments --- src/black/lines.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/black/lines.py b/src/black/lines.py index 94ad9734512..6d35d48155d 100644 --- a/src/black/lines.py +++ b/src/black/lines.py @@ -986,15 +986,14 @@ def can_omit_invisible_parens( line = rhs.body # We need optional parens in order to split standalone comments to their own lines - # if there are no nested parens inside + # 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: - if closing_bracket: - break - return False + if not closing_bracket: + return False if ( not closing_bracket and leaf.type in CLOSING_BRACKETS From 20fd2d2da7519752b76fb7a2332a7554dc9803ef Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Thu, 2 Nov 2023 08:04:57 +0200 Subject: [PATCH 7/8] Combine ifs --- src/black/lines.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/black/lines.py b/src/black/lines.py index 6d35d48155d..1878303f6c9 100644 --- a/src/black/lines.py +++ b/src/black/lines.py @@ -991,9 +991,8 @@ def can_omit_invisible_parens( for leaf in reversed(line.leaves): if closing_bracket and leaf is closing_bracket.opening_bracket: closing_bracket = None - if leaf.type == STANDALONE_COMMENT: - if not closing_bracket: - return False + if leaf.type == STANDALONE_COMMENT and not closing_bracket: + return False if ( not closing_bracket and leaf.type in CLOSING_BRACKETS From 4dc71708927bcfb1337561c8af33cafe66289672 Mon Sep 17 00:00:00 2001 From: Henri Holopainen Date: Tue, 7 Nov 2023 09:42:48 +0200 Subject: [PATCH 8/8] Wrap access to internals in a function --- src/black/brackets.py | 7 +++++++ src/black/lines.py | 3 +-- 2 files changed, 8 insertions(+), 2 deletions(-) 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/lines.py b/src/black/lines.py index 1878303f6c9..0257fe74aca 100644 --- a/src/black/lines.py +++ b/src/black/lines.py @@ -104,8 +104,7 @@ def append_safe(self, leaf: Leaf, preformatted: bool = False) -> None: """ if ( self.bracket_tracker.depth == 0 - or self.bracket_tracker._for_loop_depths - or self.bracket_tracker._lambda_argument_depths + or self.bracket_tracker.any_open_for_or_lambda() ): if self.is_comment: raise ValueError("cannot append to standalone comments")