diff --git a/CHANGES.md b/CHANGES.md index 7c91fd036cf..df77f3908ea 100644 --- a/CHANGES.md +++ b/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 diff --git a/src/black/comments.py b/src/black/comments.py index fd133c1f43a..c4c8f799b97 100644 --- a/src/black/comments.py +++ b/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 diff --git a/tests/data/cases/fmtskip_multiple_in_clause.py b/tests/data/cases/fmtskip_multiple_in_clause.py new file mode 100644 index 00000000000..7a47e907ebd --- /dev/null +++ b/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 diff --git a/tests/data/cases/fmtskip_multiple_strings.py b/tests/data/cases/fmtskip_multiple_strings.py new file mode 100644 index 00000000000..3ba70a63c40 --- /dev/null +++ b/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 +}