From ce3d055626283bd149dd561691a9feb095a43325 Mon Sep 17 00:00:00 2001 From: RedGuy12 <61329810+RedGuy12@users.noreply.github.com> Date: Thu, 22 Feb 2024 12:20:50 -0600 Subject: [PATCH 1/7] fix: Don't merge comments while splitting delimiters Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com> --- CHANGES.md | 2 ++ src/black/linegen.py | 11 +++++++++-- tests/data/cases/split_delimiter_comments.py | 18 ++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 tests/data/cases/split_delimiter_comments.py diff --git a/CHANGES.md b/CHANGES.md index bcf6eb44fdb..e28730a3b5f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,8 @@ +- Don't move comments along with delimiters, which could cause crashes (#4248) + ### Preview style diff --git a/src/black/linegen.py b/src/black/linegen.py index cc8e41dfb20..0006f564b82 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -1194,8 +1194,11 @@ def append_to_line(leaf: Leaf) -> Iterator[Line]: for leaf_idx, leaf in enumerate(line.leaves): yield from append_to_line(leaf) - for comment_after in line.comments_after(leaf): - yield from append_to_line(comment_after) + previous_leaf = line.leaves[leaf_idx - 1] if leaf_idx > 0 else None + previous_priority = previous_leaf and bt.delimiters.get(id(previous_leaf)) + if previous_priority != delimiter_priority: + for comment_after in line.comments_after(leaf): + yield from append_to_line(comment_after) lowest_depth = min(lowest_depth, leaf.bracket_depth) if leaf.bracket_depth == lowest_depth: @@ -1215,6 +1218,10 @@ def append_to_line(leaf: Leaf) -> Iterator[Line]: leaf_priority = bt.delimiters.get(id(leaf)) if leaf_priority == delimiter_priority: + if leaf_idx + 1 < len(line.leaves): + for comment_after in line.comments_after(line.leaves[leaf_idx + 1]): + yield from append_to_line(comment_after) + yield current_line current_line = Line( diff --git a/tests/data/cases/split_delimiter_comments.py b/tests/data/cases/split_delimiter_comments.py new file mode 100644 index 00000000000..11f29e4fcec --- /dev/null +++ b/tests/data/cases/split_delimiter_comments.py @@ -0,0 +1,18 @@ +a = ( + 1 + # type: ignore + 2 # type: ignore +) +a = ( + 1 # type: ignore + + 2 # type: ignore +) + +# output +a = ( + 1 # type: ignore + + 2 # type: ignore +) +a = ( + 1 # type: ignore + + 2 # type: ignore +) From 0ff7c9add33e7dd1c7e5a9454bc874cb1d6b082f Mon Sep 17 00:00:00 2001 From: RedGuy12 <61329810+RedGuy12@users.noreply.github.com> Date: Mon, 26 Feb 2024 11:18:52 -0600 Subject: [PATCH 2/7] fix: Don't change behavior of string delimiters; Minor refactors Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com> --- src/black/linegen.py | 38 +++++++++++--------- tests/data/cases/split_delimiter_comments.py | 11 ++++++ 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/black/linegen.py b/src/black/linegen.py index 0006f564b82..27cb7e19698 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -12,6 +12,7 @@ from black.brackets import ( COMMA_PRIORITY, DOT_PRIORITY, + STRING_PRIORITY, get_leaves_inside_matching_brackets, max_delimiter_priority_in_atom, ) @@ -1190,26 +1191,29 @@ def append_to_line(leaf: Leaf) -> Iterator[Line]: ) current_line.append(leaf) + def append_comments(leaf: Leaf) -> Iterator[Line]: + for comment_after in line.comments_after(leaf): + yield from append_to_line(comment_after) + last_non_comment_leaf = _get_last_non_comment_leaf(line) for leaf_idx, leaf in enumerate(line.leaves): yield from append_to_line(leaf) - previous_leaf = line.leaves[leaf_idx - 1] if leaf_idx > 0 else None - previous_priority = previous_leaf and bt.delimiters.get(id(previous_leaf)) - if previous_priority != delimiter_priority: - for comment_after in line.comments_after(leaf): - yield from append_to_line(comment_after) + previous_priority = leaf_idx > 0 and bt.delimiters.get( + id(line.leaves[leaf_idx - 1]) + ) + if ( + delimiter_priority == STRING_PRIORITY + or previous_priority != delimiter_priority + ): + yield from append_comments(leaf) lowest_depth = min(lowest_depth, leaf.bracket_depth) - if leaf.bracket_depth == lowest_depth: + if trailing_comma_safe and leaf.bracket_depth == lowest_depth: if is_vararg(leaf, within={syms.typedargslist}): - trailing_comma_safe = ( - trailing_comma_safe and Feature.TRAILING_COMMA_IN_DEF in features - ) + trailing_comma_safe = Feature.TRAILING_COMMA_IN_DEF in features elif is_vararg(leaf, within={syms.arglist, syms.argument}): - trailing_comma_safe = ( - trailing_comma_safe and Feature.TRAILING_COMMA_IN_CALL in features - ) + trailing_comma_safe = Feature.TRAILING_COMMA_IN_CALL in features if last_leaf.type == STANDALONE_COMMENT and leaf_idx == last_non_comment_leaf: current_line = _safe_add_trailing_comma( @@ -1218,15 +1222,17 @@ def append_to_line(leaf: Leaf) -> Iterator[Line]: leaf_priority = bt.delimiters.get(id(leaf)) if leaf_priority == delimiter_priority: - if leaf_idx + 1 < len(line.leaves): - for comment_after in line.comments_after(line.leaves[leaf_idx + 1]): - yield from append_to_line(comment_after) + if ( + leaf_idx + 1 < len(line.leaves) + and delimiter_priority != STRING_PRIORITY + ): + yield from append_comments(line.leaves[leaf_idx + 1]) yield current_line - current_line = Line( mode=line.mode, depth=line.depth, inside_brackets=line.inside_brackets ) + if current_line: current_line = _safe_add_trailing_comma( trailing_comma_safe, delimiter_priority, current_line diff --git a/tests/data/cases/split_delimiter_comments.py b/tests/data/cases/split_delimiter_comments.py index 11f29e4fcec..d2f5b91fe7c 100644 --- a/tests/data/cases/split_delimiter_comments.py +++ b/tests/data/cases/split_delimiter_comments.py @@ -6,6 +6,12 @@ 1 # type: ignore + 2 # type: ignore ) +bad_split3 = ( + "What if we have inline comments on " # First Comment + "each line of a bad split? In that " # Second Comment + "case, we should just leave it alone." # Third Comment +) + # output a = ( @@ -16,3 +22,8 @@ 1 # type: ignore + 2 # type: ignore ) +bad_split3 = ( + "What if we have inline comments on " # First Comment + "each line of a bad split? In that " # Second Comment + "case, we should just leave it alone." # Third Comment +) From 0c41832a50738c4c1f67c96969a763717c0dd09c Mon Sep 17 00:00:00 2001 From: RedGuy12 <61329810+RedGuy12@users.noreply.github.com> Date: Mon, 26 Feb 2024 11:31:04 -0600 Subject: [PATCH 3/7] another attempt at lowering complexity Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com> --- src/black/linegen.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/black/linegen.py b/src/black/linegen.py index 27cb7e19698..67f56fd1496 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -1136,6 +1136,20 @@ def _get_last_non_comment_leaf(line: Line) -> Optional[int]: return None +def _can_add_trailing_comma(leaf: Leaf, features: Collection[Feature]): + if ( + is_vararg(leaf, within={syms.typedargslist}) + and Feature.TRAILING_COMMA_IN_DEF in features + ): + return True + if ( + is_vararg(leaf, within={syms.arglist, syms.argument}) + and Feature.TRAILING_COMMA_IN_CALL in features + ): + return True + return False + + def _safe_add_trailing_comma(safe: bool, delimiter_priority: int, line: Line) -> Line: if ( safe @@ -1210,10 +1224,7 @@ def append_comments(leaf: Leaf) -> Iterator[Line]: lowest_depth = min(lowest_depth, leaf.bracket_depth) if trailing_comma_safe and leaf.bracket_depth == lowest_depth: - if is_vararg(leaf, within={syms.typedargslist}): - trailing_comma_safe = Feature.TRAILING_COMMA_IN_DEF in features - elif is_vararg(leaf, within={syms.arglist, syms.argument}): - trailing_comma_safe = Feature.TRAILING_COMMA_IN_CALL in features + trailing_comma_safe = _can_add_trailing_comma(leaf, features) if last_leaf.type == STANDALONE_COMMENT and leaf_idx == last_non_comment_leaf: current_line = _safe_add_trailing_comma( From efd45cf9c2c5cde9f646900f2adc225fa4e978cc Mon Sep 17 00:00:00 2001 From: RedGuy12 <61329810+RedGuy12@users.noreply.github.com> Date: Tue, 27 Feb 2024 11:13:52 -0600 Subject: [PATCH 4/7] third time's the charm Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com> --- src/black/linegen.py | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/src/black/linegen.py b/src/black/linegen.py index 67f56fd1496..813ed78ead4 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -1136,18 +1136,14 @@ def _get_last_non_comment_leaf(line: Line) -> Optional[int]: return None -def _can_add_trailing_comma(leaf: Leaf, features: Collection[Feature]): - if ( - is_vararg(leaf, within={syms.typedargslist}) - and Feature.TRAILING_COMMA_IN_DEF in features - ): - return True - if ( - is_vararg(leaf, within={syms.arglist, syms.argument}) - and Feature.TRAILING_COMMA_IN_CALL in features - ): - return True - return False +def _can_add_trailing_comma( + leaf: Leaf, features: Collection[Feature] +) -> bool: + if is_vararg(leaf, within={syms.typedargslist}): + return Feature.TRAILING_COMMA_IN_DEF in features + if is_vararg(leaf, within={syms.arglist, syms.argument}): + return Feature.TRAILING_COMMA_IN_CALL in features + return True def _safe_add_trailing_comma(safe: bool, delimiter_priority: int, line: Line) -> Line: @@ -1171,10 +1167,9 @@ def delimiter_split( If the appropriate Features are given, the split will add trailing commas also in function signatures and calls that contain `*` and `**`. """ - try: - last_leaf = line.leaves[-1] - except IndexError: + if len(line.leaves) == 0: raise CannotSplit("Line empty") from None + last_leaf = line.leaves[-1] bt = line.bracket_tracker try: @@ -1182,9 +1177,11 @@ def delimiter_split( except ValueError: raise CannotSplit("No delimiters found") from None - if delimiter_priority == DOT_PRIORITY: - if bt.delimiter_count_with_priority(delimiter_priority) == 1: - raise CannotSplit("Splitting a single attribute from its owner looks wrong") + if ( + delimiter_priority == DOT_PRIORITY + and bt.delimiter_count_with_priority(delimiter_priority) == 1 + ): + raise CannotSplit("Splitting a single attribute from its owner looks wrong") current_line = Line( mode=line.mode, depth=line.depth, inside_brackets=line.inside_brackets From 36072e0535e204c5ffdfed65717f816e21f4b7c8 Mon Sep 17 00:00:00 2001 From: RedGuy12 <61329810+RedGuy12@users.noreply.github.com> Date: Tue, 27 Feb 2024 11:27:55 -0600 Subject: [PATCH 5/7] format Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com> --- src/black/linegen.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/black/linegen.py b/src/black/linegen.py index 813ed78ead4..7506e6228e9 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -1136,9 +1136,7 @@ def _get_last_non_comment_leaf(line: Line) -> Optional[int]: return None -def _can_add_trailing_comma( - leaf: Leaf, features: Collection[Feature] -) -> bool: +def _can_add_trailing_comma(leaf: Leaf, features: Collection[Feature]) -> bool: if is_vararg(leaf, within={syms.typedargslist}): return Feature.TRAILING_COMMA_IN_DEF in features if is_vararg(leaf, within={syms.arglist, syms.argument}): From 54042b1fcbbff31014dc525d57d2dcfc1fadc065 Mon Sep 17 00:00:00 2001 From: RedGuy12 <61329810+RedGuy12@users.noreply.github.com> Date: Wed, 28 Feb 2024 09:41:46 -0600 Subject: [PATCH 6/7] Ignore commas too Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com> --- src/black/linegen.py | 9 +++++--- tests/data/cases/split_delimiter_comments.py | 22 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/black/linegen.py b/src/black/linegen.py index 7506e6228e9..fbce091da0e 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -1156,6 +1156,9 @@ def _safe_add_trailing_comma(safe: bool, delimiter_priority: int, line: Line) -> return line +MIGRATE_COMMENT_DELIMITERS = {STRING_PRIORITY, COMMA_PRIORITY} + + @dont_increase_indentation def delimiter_split( line: Line, features: Collection[Feature], mode: Mode @@ -1212,8 +1215,8 @@ def append_comments(leaf: Leaf) -> Iterator[Line]: id(line.leaves[leaf_idx - 1]) ) if ( - delimiter_priority == STRING_PRIORITY - or previous_priority != delimiter_priority + previous_priority != delimiter_priority + or delimiter_priority in MIGRATE_COMMENT_DELIMITERS ): yield from append_comments(leaf) @@ -1230,7 +1233,7 @@ def append_comments(leaf: Leaf) -> Iterator[Line]: if leaf_priority == delimiter_priority: if ( leaf_idx + 1 < len(line.leaves) - and delimiter_priority != STRING_PRIORITY + and delimiter_priority not in MIGRATE_COMMENT_DELIMITERS ): yield from append_comments(line.leaves[leaf_idx + 1]) diff --git a/tests/data/cases/split_delimiter_comments.py b/tests/data/cases/split_delimiter_comments.py index d2f5b91fe7c..ea29f7c034f 100644 --- a/tests/data/cases/split_delimiter_comments.py +++ b/tests/data/cases/split_delimiter_comments.py @@ -11,6 +11,17 @@ "each line of a bad split? In that " # Second Comment "case, we should just leave it alone." # Third Comment ) +parametrize( + ( + {}, + {}, + ), + ( # foobar + {}, + {}, + ), +) + # output @@ -27,3 +38,14 @@ "each line of a bad split? In that " # Second Comment "case, we should just leave it alone." # Third Comment ) +parametrize( + ( + {}, + {}, + ), + ( # foobar + {}, + {}, + ), +) + From 54a89f5074013a346469b97f9459ddca528c2ebc Mon Sep 17 00:00:00 2001 From: RedGuy12 <61329810+RedGuy12@users.noreply.github.com> Date: Wed, 28 Feb 2024 09:49:46 -0600 Subject: [PATCH 7/7] chore: Revert unrelated refactors Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com> --- src/black/linegen.py | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/black/linegen.py b/src/black/linegen.py index fbce091da0e..79580ce0e7d 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -1136,14 +1136,6 @@ def _get_last_non_comment_leaf(line: Line) -> Optional[int]: return None -def _can_add_trailing_comma(leaf: Leaf, features: Collection[Feature]) -> bool: - if is_vararg(leaf, within={syms.typedargslist}): - return Feature.TRAILING_COMMA_IN_DEF in features - if is_vararg(leaf, within={syms.arglist, syms.argument}): - return Feature.TRAILING_COMMA_IN_CALL in features - return True - - def _safe_add_trailing_comma(safe: bool, delimiter_priority: int, line: Line) -> Line: if ( safe @@ -1168,9 +1160,10 @@ def delimiter_split( If the appropriate Features are given, the split will add trailing commas also in function signatures and calls that contain `*` and `**`. """ - if len(line.leaves) == 0: + try: + last_leaf = line.leaves[-1] + except IndexError: raise CannotSplit("Line empty") from None - last_leaf = line.leaves[-1] bt = line.bracket_tracker try: @@ -1178,11 +1171,9 @@ def delimiter_split( except ValueError: raise CannotSplit("No delimiters found") from None - if ( - delimiter_priority == DOT_PRIORITY - and bt.delimiter_count_with_priority(delimiter_priority) == 1 - ): - raise CannotSplit("Splitting a single attribute from its owner looks wrong") + if delimiter_priority == DOT_PRIORITY: + if bt.delimiter_count_with_priority(delimiter_priority) == 1: + raise CannotSplit("Splitting a single attribute from its owner looks wrong") current_line = Line( mode=line.mode, depth=line.depth, inside_brackets=line.inside_brackets @@ -1221,8 +1212,15 @@ def append_comments(leaf: Leaf) -> Iterator[Line]: yield from append_comments(leaf) lowest_depth = min(lowest_depth, leaf.bracket_depth) - if trailing_comma_safe and leaf.bracket_depth == lowest_depth: - trailing_comma_safe = _can_add_trailing_comma(leaf, features) + if leaf.bracket_depth == lowest_depth: + if is_vararg(leaf, within={syms.typedargslist}): + trailing_comma_safe = ( + trailing_comma_safe and Feature.TRAILING_COMMA_IN_DEF in features + ) + elif is_vararg(leaf, within={syms.arglist, syms.argument}): + trailing_comma_safe = ( + trailing_comma_safe and Feature.TRAILING_COMMA_IN_CALL in features + ) if last_leaf.type == STANDALONE_COMMENT and leaf_idx == last_non_comment_leaf: current_line = _safe_add_trailing_comma(