Fix crash when multiple # fmt: skip comments are used in multi-part if-clause (#4731)

CHANGES.md

@@ -17,6 +17,8 @@
 
 - Fix bug where comments preceding `# fmt: off`/`# fmt: on` blocks were incorrectly
   removed, particularly affecting Jupytext's `# %% [markdown]` comments (#4845)
+- Fix crash when multiple `# fmt: skip` comments are used in a multi-part if-clause, on
+  string literals, or on dictionary entries with long lines (#4872)
 - Fix possible crash when `fmt: ` directives aren't on the top level (#4856)
 
 ### Preview style

src/black/comments.py

@@ -339,12 +339,10 @@ def convert_one_fmt_off_pair(
     Returns True if a pair was converted.
     """
     for leaf in node.leaves():
-        # Skip STANDALONE_COMMENT nodes that were created by fmt:off/on processing
+        # Skip STANDALONE_COMMENT nodes that were created by fmt:off/on/skip processing
         # to avoid reprocessing them in subsequent iterations
-        if (
-            leaf.type == STANDALONE_COMMENT
-            and hasattr(leaf, "fmt_pass_converted_first_leaf")
-            and leaf.fmt_pass_converted_first_leaf is None
+        if leaf.type == STANDALONE_COMMENT and hasattr(
+            leaf, "fmt_pass_converted_first_leaf"
         ):
             continue
 
@@ -413,7 +411,43 @@ def _handle_regular_fmt_block(
     else:
         standalone_comment_prefix = prefix[:previous_consumed] + "\n" * comment.newlines
 
-    hidden_value = "".join(str(n) for n in ignored_nodes)
+    # Ensure STANDALONE_COMMENT nodes have trailing newlines when stringified
+    # This prevents multiple fmt: skip comments from being concatenated on one line
+    parts = []
+    for node in ignored_nodes:
+        if isinstance(node, Leaf) and node.type == STANDALONE_COMMENT:
+            # Add newline after STANDALONE_COMMENT Leaf
+            node_str = str(node)
+            if not node_str.endswith("\n"):
+                node_str += "\n"
+            parts.append(node_str)
+        elif isinstance(node, Node):
+            # For nodes that might contain STANDALONE_COMMENT leaves,
+            # we need custom stringify
+            has_standalone = any(
+                leaf.type == STANDALONE_COMMENT for leaf in node.leaves()
+            )
+            if has_standalone:
+                # Stringify node with STANDALONE_COMMENT leaves having trailing newlines
+                def stringify_node(n: LN) -> str:
+                    if isinstance(n, Leaf):
+                        if n.type == STANDALONE_COMMENT:
+                            result = n.prefix + n.value
+                            if not result.endswith("\n"):
+                                result += "\n"
+                            return result
+                        return str(n)
+                    else:
+                        # For nested nodes, recursively process children
+                        return "".join(stringify_node(child) for child in n.children)
+
+                parts.append(stringify_node(node))
+            else:
+                parts.append(str(node))
+        else:
+            parts.append(str(node))
+
+    hidden_value = "".join(parts)
     comment_lineno = leaf.lineno - comment.newlines
 
     if contains_fmt_directive(comment.value, FMT_OFF):
@@ -650,9 +684,30 @@ def _generate_ignored_nodes_from_fmt_skip(
         ignored_nodes = [current_node]
         if current_node.prev_sibling is None and current_node.parent is not None:
             current_node = current_node.parent
+
+        # Track seen nodes to detect cycles that can occur after tree modifications
+        seen_nodes = {id(current_node)}
+
         while "\n" not in current_node.prefix and current_node.prev_sibling is not None:
             leaf_nodes = list(current_node.prev_sibling.leaves())
-            current_node = leaf_nodes[-1] if leaf_nodes else current_node
+            next_node = leaf_nodes[-1] if leaf_nodes else current_node
+
+            # Detect infinite loop - if we've seen this node before, stop
+            # This can happen when STANDALONE_COMMENT nodes are inserted
+            # during processing
+            if id(next_node) in seen_nodes:
+                break
+
+            current_node = next_node
+            seen_nodes.add(id(current_node))
+
+            # Stop if we encounter a STANDALONE_COMMENT created by fmt processing
+            if (
+                isinstance(current_node, Leaf)
+                and current_node.type == STANDALONE_COMMENT
+                and hasattr(current_node, "fmt_pass_converted_first_leaf")
+            ):
+                break
 
             if (
                 current_node.type in CLOSING_BRACKETS

tests/data/cases/fmtskip_multiple_in_clause.py

@@ -0,0 +1,43 @@
+# Multiple fmt: skip in multi-part if-clause
+class ClassWithALongName:
+    Constant1 = 1
+    Constant2 = 2
+    Constant3 = 3
+
+
+def test():
+    if (
+        "cond1" == "cond1"
+        and "cond2" == "cond2"
+        and 1 in (
+            ClassWithALongName.Constant1,
+            ClassWithALongName.Constant2,
+            ClassWithALongName.Constant3, # fmt: skip
+        ) # fmt: skip
+    ):
+        return True
+    return False
+
+
+# output
+
+
+# Multiple fmt: skip in multi-part if-clause
+class ClassWithALongName:
+    Constant1 = 1
+    Constant2 = 2
+    Constant3 = 3
+
+
+def test():
+    if (
+        "cond1" == "cond1"
+        and "cond2" == "cond2"
+        and 1 in (
+            ClassWithALongName.Constant1,
+            ClassWithALongName.Constant2,
+            ClassWithALongName.Constant3, # fmt: skip
+        ) # fmt: skip
+    ):
+        return True
+    return False

tests/data/cases/fmtskip_multiple_strings.py

@@ -0,0 +1,75 @@
+# Multiple fmt: skip on string literals
+a = (
+    "this should "  # fmt: skip
+    "be fine"
+)
+
+b = (
+    "this is "  # fmt: skip
+    "not working"  # fmt: skip
+)
+
+c = (
+    "and neither "  # fmt: skip
+    "is this "  # fmt: skip
+    "working"
+)
+
+d = (
+    "nor "
+    "is this "  # fmt: skip
+    "working"  # fmt: skip
+)
+
+e = (
+    "and this "  # fmt: skip
+    "is definitely "
+    "not working"  # fmt: skip
+)
+
+# Dictionary entries with fmt: skip (covers issue with long lines)
+hotkeys = {
+    "editor:swap-line-down": [{"key": "ArrowDown", "modifiers": ["Alt", "Mod"]}],  # fmt: skip
+    "editor:swap-line-up":   [{"key": "ArrowUp",   "modifiers": ["Alt", "Mod"]}],  # fmt: skip
+    "editor:toggle-source":  [{"key": "S",         "modifiers": ["Alt", "Mod"]}],  # fmt: skip
+}
+
+
+# output
+
+
+# Multiple fmt: skip on string literals
+a = (
+    "this should "  # fmt: skip
+    "be fine"
+)
+
+b = (
+    "this is "  # fmt: skip
+    "not working"  # fmt: skip
+)
+
+c = (
+    "and neither "  # fmt: skip
+    "is this "  # fmt: skip
+    "working"
+)
+
+d = (
+    "nor "
+    "is this "  # fmt: skip
+    "working"  # fmt: skip
+)
+
+e = (
+    "and this "  # fmt: skip
+    "is definitely "
+    "not working"  # fmt: skip
+)
+
+# Dictionary entries with fmt: skip (covers issue with long lines)
+hotkeys = {
+    "editor:swap-line-down": [{"key": "ArrowDown", "modifiers": ["Alt", "Mod"]}],  # fmt: skip
+    "editor:swap-line-up":   [{"key": "ArrowUp",   "modifiers": ["Alt", "Mod"]}],  # fmt: skip
+    "editor:toggle-source":  [{"key": "S",         "modifiers": ["Alt", "Mod"]}],  # fmt: skip
+}