Skip to content

Commit 5b470f0

Browse files
Fix # fmt: skip ignored in deeply nested expressions (psf#4883)
1 parent 1b342ef commit 5b470f0

File tree

4 files changed

+117
-12
lines changed

4 files changed

+117
-12
lines changed

CHANGES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
magic trailing commas and intentional multiline formatting (#4865)
3131
- Fix `fix_fmt_skip_in_one_liners` crashing on `with` statements (#4853)
3232
- Fix `fix_fmt_skip_in_one_liners` crashing on annotated parameters (#4854)
33+
- Fix `# fmt: skip` behavior for deeply nested expressions (#4883)
3334

3435
### Configuration
3536

src/black/comments.py

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from black.mode import Mode, Preview
88
from black.nodes import (
99
CLOSING_BRACKETS,
10+
OPENING_BRACKETS,
1011
STANDALONE_COMMENT,
1112
STATEMENT,
1213
WHITESPACE,
@@ -645,7 +646,46 @@ def _generate_ignored_nodes_from_fmt_skip(
645646
return
646647

647648
if Preview.fix_fmt_skip_in_one_liners in mode and not prev_sibling and parent:
648-
prev_sibling = parent.prev_sibling
649+
# If the current leaf doesn't have a previous sibling, it might be deeply nested
650+
# (e.g. inside a list, function call, etc.). We need to climb up the tree
651+
# to find the previous sibling on the same line.
652+
653+
# First, find the ancestor node that starts the current line
654+
# (has a newline in its prefix, or is at the root)
655+
line_start_node = parent
656+
while line_start_node.parent is not None:
657+
# The comment itself is in the leaf's prefix, so we need to check
658+
# if the current node's prefix (before the comment) has a newline
659+
node_prefix = (
660+
line_start_node.prefix if hasattr(line_start_node, "prefix") else ""
661+
)
662+
# Skip the comment part if this is the first node (parent)
663+
if line_start_node == parent:
664+
# The parent node has the comment in its prefix, so we check
665+
# what's before the comment
666+
comment_start = node_prefix.find(comment.value)
667+
if comment_start >= 0:
668+
prefix_before_comment = node_prefix[:comment_start]
669+
if "\n" in prefix_before_comment:
670+
# There's a newline before the comment, so this node
671+
# is at the start of the line
672+
break
673+
elif "\n" in node_prefix:
674+
break
675+
elif "\n" in node_prefix:
676+
# This node starts on a new line, so it's the line start
677+
break
678+
line_start_node = line_start_node.parent
679+
680+
# Now find the prev_sibling by climbing from parent up to line_start_node
681+
curr = parent
682+
while curr is not None and curr != line_start_node.parent:
683+
if curr.prev_sibling is not None:
684+
prev_sibling = curr.prev_sibling
685+
break
686+
if curr.parent is None:
687+
break
688+
curr = curr.parent
649689

650690
if prev_sibling is not None:
651691
leaf.prefix = leaf.prefix[comment.consumed :]
@@ -746,6 +786,22 @@ def _generate_ignored_nodes_from_fmt_skip(
746786
if header_nodes:
747787
ignored_nodes = header_nodes + ignored_nodes
748788

789+
# If the leaf's parent is an atom (parenthesized expression) and we've
790+
# captured the opening bracket in our ignored_nodes, we should include
791+
# the entire atom (including the closing bracket and the leaf itself)
792+
# to avoid partial formatting
793+
if (
794+
parent is not None
795+
and parent.type == syms.atom
796+
and len(parent.children) >= 2
797+
and parent.children[0].type in OPENING_BRACKETS
798+
and parent.children[0] in ignored_nodes
799+
):
800+
# Replace the opening bracket and any other captured children of this atom
801+
# with the entire atom node
802+
ignored_nodes = [node for node in ignored_nodes if node.parent != parent]
803+
ignored_nodes.append(parent)
804+
749805
yield from ignored_nodes
750806
elif (
751807
parent is not None and parent.type == syms.suite and leaf.type == token.NEWLINE

src/black/lines.py

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -863,6 +863,10 @@ def is_line_short_enough(line: Line, *, mode: Mode, line_str: str = "") -> bool:
863863
for i, leaf in enumerate(line.leaves):
864864
if max_level_to_update == math.inf:
865865
had_comma: int | None = None
866+
# Skip multiline_string_handling logic for leaves without bracket_depth
867+
# (e.g., newly created leaves not yet processed by bracket tracker)
868+
if not hasattr(leaf, "bracket_depth"):
869+
continue
866870
if leaf.bracket_depth + 1 > len(commas):
867871
commas.append(0)
868872
elif leaf.bracket_depth + 1 < len(commas):
@@ -878,17 +882,22 @@ def is_line_short_enough(line: Line, *, mode: Mode, line_str: str = "") -> bool:
878882
# MLS was in parens with at least one comma - force split
879883
return False
880884

881-
if leaf.bracket_depth <= max_level_to_update and leaf.type == token.COMMA:
882-
# Inside brackets, ignore trailing comma
883-
# directly after MLS/MLS-containing expression
884-
ignore_ctxs: list[LN | None] = [None]
885-
ignore_ctxs += multiline_string_contexts
886-
if (line.inside_brackets or leaf.bracket_depth > 0) and (
887-
i != len(line.leaves) - 1 or leaf.prev_sibling not in ignore_ctxs
888-
):
889-
commas[leaf.bracket_depth] += 1
890-
if max_level_to_update != math.inf:
891-
max_level_to_update = min(max_level_to_update, leaf.bracket_depth)
885+
# Skip bracket-depth-dependent processing for leaves without the attribute
886+
if not hasattr(leaf, "bracket_depth"):
887+
# Still process multiline string detection below
888+
pass
889+
else:
890+
if leaf.bracket_depth <= max_level_to_update and leaf.type == token.COMMA:
891+
# Inside brackets, ignore trailing comma
892+
# directly after MLS/MLS-containing expression
893+
ignore_ctxs: list[LN | None] = [None]
894+
ignore_ctxs += multiline_string_contexts
895+
if (line.inside_brackets or leaf.bracket_depth > 0) and (
896+
i != len(line.leaves) - 1 or leaf.prev_sibling not in ignore_ctxs
897+
):
898+
commas[leaf.bracket_depth] += 1
899+
if max_level_to_update != math.inf:
900+
max_level_to_update = min(max_level_to_update, leaf.bracket_depth)
892901

893902
if is_multiline_string(leaf):
894903
if leaf.parent and (
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# flags: --preview --no-preview-line-length-1
2+
class ClassWithALongName:
3+
Constant1 = 1
4+
Constant2 = 2
5+
Constant3 = 3
6+
7+
8+
def test():
9+
if (
10+
"cond1" == "cond1"
11+
and "cond2" == "cond2"
12+
and 1 in ( # fmt: skip
13+
ClassWithALongName.Constant1,
14+
ClassWithALongName.Constant2,
15+
ClassWithALongName.Constant3,
16+
)
17+
):
18+
return True
19+
return False
20+
21+
# output
22+
23+
class ClassWithALongName:
24+
Constant1 = 1
25+
Constant2 = 2
26+
Constant3 = 3
27+
28+
29+
def test():
30+
if (
31+
"cond1" == "cond1" and "cond2" == "cond2"
32+
and 1 in ( # fmt: skip
33+
ClassWithALongName.Constant1,
34+
ClassWithALongName.Constant2,
35+
ClassWithALongName.Constant3,
36+
)
37+
):
38+
return True
39+
return False

0 commit comments

Comments
 (0)