diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/if.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/if.py index 266b2da8a7..61e3bc130b 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/if.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/if.py @@ -103,3 +103,15 @@ else: if True: print("a") # 1 elif True: print("b") # 2 else: print("c") # 3 + +# Regression test for https://github.com/astral-sh/ruff/issues/5337 +if parent_body: + if current_body: + child_in_body() + last_child_in_current_body() # may or may not have children on its own +# a + # b + # c + # d + # e + #f diff --git a/crates/ruff_python_formatter/src/comments/mod.rs b/crates/ruff_python_formatter/src/comments/mod.rs index ae855c82b6..238665f9b7 100644 --- a/crates/ruff_python_formatter/src/comments/mod.rs +++ b/crates/ruff_python_formatter/src/comments/mod.rs @@ -600,8 +600,13 @@ def test(x, y): def other(y, z): if y == z: pass - # Trailing `if` comment - # Trailing `other` function comment + # Trailing `pass` comment + # Trailing `if` statement comment + +class Test: + def func(): + pass + # Trailing `func` function comment test(10, 20) "#; diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index a231a54096..81035bf7dd 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -451,12 +451,11 @@ fn handle_own_line_comment_after_branch<'a>( return CommentPlacement::Default(comment); } - let mut parent_body = None; - let mut current_body = Some(preceding_node); - let mut last_child_in_current_body = last_child; + let mut parent = None; + let mut last_child_in_parent = last_child; loop { - let child_indentation = indentation(locator, &last_child_in_current_body) + let child_indentation = indentation(locator, &last_child_in_parent) .unwrap_or_default() .len(); @@ -465,15 +464,16 @@ fn handle_own_line_comment_after_branch<'a>( // if parent_body: // if current_body: // child_in_body() - // last_child_in_current_body # may or may not have children on its own + // last_child_in_current_body # may or may not have children on its own // # less: Comment belongs to the parent block. - // # less + // # less: Comment belongs to the parent block. // # equal: The comment belongs to this block. - // # greater - // # greater: The comment belongs to the inner block. + // # greater (but less in the next iteration) + // # greater: The comment belongs to the inner block. + // ``` match comment_indentation.cmp(&child_indentation) { Ordering::Less => { - return if let Some(parent_block) = parent_body { + return if let Some(parent_block) = parent { // Comment belongs to the parent block. CommentPlacement::trailing(parent_block, comment) } else { @@ -488,14 +488,13 @@ fn handle_own_line_comment_after_branch<'a>( } Ordering::Equal => { // The comment belongs to this block. - return CommentPlacement::trailing(last_child_in_current_body, comment); + return CommentPlacement::trailing(last_child_in_parent, comment); } Ordering::Greater => { - if let Some(nested_child) = last_child_in_body(last_child_in_current_body) { + if let Some(nested_child) = last_child_in_body(last_child_in_parent) { // The comment belongs to the inner block. - parent_body = current_body; - current_body = Some(last_child_in_current_body); - last_child_in_current_body = nested_child; + parent = Some(last_child_in_parent); + last_child_in_parent = nested_child; } else { // The comment is overindented, we assign it to the most indented child we have. // ```python @@ -503,7 +502,7 @@ fn handle_own_line_comment_after_branch<'a>( // pass // # comment // ``` - return CommentPlacement::trailing(last_child_in_current_body, comment); + return CommentPlacement::trailing(last_child_in_parent, comment); } } } @@ -1346,7 +1345,7 @@ where right.is_some_and(|right| left.ptr_eq(right.into())) } -/// The last child of the last branch, if the node hs multiple branches. +/// The last child of the last branch, if the node has multiple branches. fn last_child_in_body(node: AnyNodeRef) -> Option { let body = match node { AnyNodeRef::StmtFunctionDef(ast::StmtFunctionDef { body, .. }) @@ -1504,9 +1503,7 @@ mod tests { ); assert_eq!( - max_empty_lines( - "# trailing comment\n\n# own line comment\n\n\n# an other own line comment\n# block" - ), + max_empty_lines("# trailing comment\n\n# own line comment\n\n\n# an other own line comment\n# block"), 2 ); diff --git a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_function_comment.snap b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_function_comment.snap index 005e4fcf8d..9dd6d289d4 100644 --- a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_function_comment.snap +++ b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__trailing_function_comment.snap @@ -34,15 +34,15 @@ expression: comments.debug(test_case.source_code) ], }, Node { - kind: StmtFunctionDef, - range: 193..237, - source: `def other(y, z):⏎`, + kind: StmtIf, + range: 214..237, + source: `if y == z:⏎`, }: { "leading": [], "dangling": [], "trailing": [ SourceComment { - text: "# Trailing `other` function comment", + text: "# Trailing `if` statement comment", position: OwnLine, formatted: false, }, @@ -57,7 +57,22 @@ expression: comments.debug(test_case.source_code) "dangling": [], "trailing": [ SourceComment { - text: "# Trailing `if` comment", + text: "# Trailing `pass` comment", + position: OwnLine, + formatted: false, + }, + ], + }, + Node { + kind: StmtFunctionDef, + range: 333..357, + source: `def func():⏎`, + }: { + "leading": [], + "dangling": [], + "trailing": [ + SourceComment { + text: "# Trailing `func` function comment", position: OwnLine, formatted: false, }, diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__if.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__if.py.snap index 10ab45b872..9c459694dd 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__if.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__if.py.snap @@ -109,6 +109,18 @@ else: if True: print("a") # 1 elif True: print("b") # 2 else: print("c") # 3 + +# Regression test for https://github.com/astral-sh/ruff/issues/5337 +if parent_body: + if current_body: + child_in_body() + last_child_in_current_body() # may or may not have children on its own +# a + # b + # c + # d + # e + #f ``` ## Output @@ -224,6 +236,18 @@ elif True: print("b") # 2 else: print("c") # 3 + +# Regression test for https://github.com/astral-sh/ruff/issues/5337 +if parent_body: + if current_body: + child_in_body() + last_child_in_current_body() # may or may not have children on its own + # e + # f + # c + # d +# a +# b ```