From 92c1d74e5886935812724ac5aa9eecd8b3eea342 Mon Sep 17 00:00:00 2001 From: Nikhil172913832 Date: Thu, 27 Nov 2025 12:02:22 +0530 Subject: [PATCH 1/4] Add trailing newlines to STANDALONE_COMMENT nodes to prevent parsing errors --- CHANGES.md | 2 + src/black/comments.py | 11 +++++ .../data/cases/fmtskip_multiple_in_clause.py | 43 +++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 tests/data/cases/fmtskip_multiple_in_clause.py diff --git a/CHANGES.md b/CHANGES.md index 128d84b8ff3..fe7956cbdda 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 + (#4872) ### Preview style diff --git a/src/black/comments.py b/src/black/comments.py index de1831e4d7c..cdc49dc9cdc 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -409,6 +409,17 @@ def _handle_regular_fmt_block( else: standalone_comment_prefix = prefix[:previous_consumed] + "\n" * comment.newlines + # Ensure STANDALONE_COMMENT nodes have trailing newlines when stringified + # This prevents multiple fmt: skip comments from being concatenated on one line + for node in ignored_nodes: + if isinstance(node, Leaf) and node.type == STANDALONE_COMMENT: + if not node.value.endswith("\n"): + node.value += "\n" + elif isinstance(node, Node): + for leaf in node.leaves(): + if leaf.type == STANDALONE_COMMENT and not leaf.value.endswith("\n"): + leaf.value += "\n" + hidden_value = "".join(str(n) for n in ignored_nodes) comment_lineno = leaf.lineno - comment.newlines 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..bd7ae1991fb --- /dev/null +++ b/tests/data/cases/fmtskip_multiple_in_clause.py @@ -0,0 +1,43 @@ +# Issue 4731: 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 + + +# Issue 4731: 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 From 348b097bbc378a9d009b19903f15b5f755194b1d Mon Sep 17 00:00:00 2001 From: Nikhil172913832 Date: Tue, 2 Dec 2025 14:01:32 +0530 Subject: [PATCH 2/4] Add test cases and revise the approach for issue #4873 --- CHANGES.md | 4 +- src/black/comments.py | 61 ++++++++++++--- .../data/cases/fmtskip_multiple_in_clause.py | 4 +- tests/data/cases/fmtskip_multiple_strings.py | 75 +++++++++++++++++++ 4 files changed, 130 insertions(+), 14 deletions(-) create mode 100644 tests/data/cases/fmtskip_multiple_strings.py diff --git a/CHANGES.md b/CHANGES.md index b78b712f048..ba43a352124 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -17,8 +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 - (#4872) +- 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 3e2634f204c..430d447f17d 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -339,12 +339,11 @@ 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 ): continue @@ -415,16 +414,38 @@ def _handle_regular_fmt_block( # 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: - if not node.value.endswith("\n"): - node.value += "\n" + # 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 leaf in node.leaves(): - if leaf.type == STANDALONE_COMMENT and not leaf.value.endswith("\n"): - leaf.value += "\n" - - hidden_value = "".join(str(n) for n in ignored_nodes) + # 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): @@ -661,9 +682,29 @@ 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 index bd7ae1991fb..7a47e907ebd 100644 --- a/tests/data/cases/fmtskip_multiple_in_clause.py +++ b/tests/data/cases/fmtskip_multiple_in_clause.py @@ -1,4 +1,4 @@ -# Issue 4731: Multiple fmt: skip in multi-part if-clause +# Multiple fmt: skip in multi-part if-clause class ClassWithALongName: Constant1 = 1 Constant2 = 2 @@ -22,7 +22,7 @@ def test(): # output -# Issue 4731: Multiple fmt: skip in multi-part if-clause +# Multiple fmt: skip in multi-part if-clause class ClassWithALongName: Constant1 = 1 Constant2 = 2 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 +} From 3dab11c5907a09204871195ca1d9a975e7908871 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 2 Dec 2025 08:32:47 +0000 Subject: [PATCH 3/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- CHANGES.md | 4 ++-- src/black/comments.py | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index ba43a352124..df77f3908ea 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -17,8 +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 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 430d447f17d..9c05a8c920c 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -438,13 +438,13 @@ def stringify_node(n: LN) -> str: 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 @@ -682,19 +682,19 @@ 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()) 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)) From 5545c44ae046f95fb435cbfaeb8086524760a5c3 Mon Sep 17 00:00:00 2001 From: Nikhil172913832 Date: Tue, 2 Dec 2025 14:26:08 +0530 Subject: [PATCH 4/4] formatting changes required as per the ci failure --- src/black/comments.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index 9c05a8c920c..c4c8f799b97 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -341,9 +341,8 @@ def convert_one_fmt_off_pair( for leaf in node.leaves(): # 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") + if leaf.type == STANDALONE_COMMENT and hasattr( + leaf, "fmt_pass_converted_first_leaf" ): continue @@ -423,8 +422,11 @@ def _handle_regular_fmt_block( 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()) + # 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: @@ -691,7 +693,8 @@ def _generate_ignored_nodes_from_fmt_skip( 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 + # This can happen when STANDALONE_COMMENT nodes are inserted + # during processing if id(next_node) in seen_nodes: break