diff --git a/crates/ruff_python_formatter/src/comments/mod.rs b/crates/ruff_python_formatter/src/comments/mod.rs index fdc5ea2190..6792adfcd2 100644 --- a/crates/ruff_python_formatter/src/comments/mod.rs +++ b/crates/ruff_python_formatter/src/comments/mod.rs @@ -858,4 +858,18 @@ a = ( assert_debug_snapshot!(comments.debug(test_case.source_code)); } + + #[test] + fn while_trailing_end_of_line_comment() { + let source = r#"while True: + if something.changed: + do.stuff() # trailing comment +"#; + + let test_case = CommentsTestCase::from_code(source); + + let comments = test_case.to_comments(); + + assert_debug_snapshot!(comments.debug(test_case.source_code)); + } } diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index f689d7c127..23bc961c5f 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -18,6 +18,7 @@ pub(super) fn place_comment<'a>( .or_else(|comment| handle_match_comment(comment, locator)) .or_else(|comment| handle_in_between_bodies_comment(comment, locator)) .or_else(|comment| handle_trailing_body_comment(comment, locator)) + .or_else(handle_trailing_end_of_line_body_comment) .or_else(|comment| handle_positional_only_arguments_separator_comment(comment, locator)) .or_else(|comment| { handle_trailing_binary_expression_left_or_operator_comment(comment, locator) @@ -401,6 +402,41 @@ fn handle_trailing_body_comment<'a>( } } +/// Handles end of line comments of the last statement in an indented body: +/// +/// ```python +/// while True: +/// if something.changed: +/// do.stuff() # trailing comment +/// ``` +fn handle_trailing_end_of_line_body_comment(comment: DecoratedComment<'_>) -> CommentPlacement<'_> { + // Must be an end of line comment + if comment.text_position().is_own_line() { + return CommentPlacement::Default(comment); + } + + // Must be *after* a statement + let Some(preceding) = comment.preceding_node() else { + return CommentPlacement::Default(comment); + }; + + // Recursively get the last child of statements with a body. + let last_children = std::iter::successors(last_child_in_body(preceding), |parent| { + last_child_in_body(*parent) + }); + + if let Some(last_child) = last_children.last() { + CommentPlacement::trailing(last_child, comment) + } else { + // End of line comment of a statement that has no body. This is not what we're looking for. + // ```python + // a # trailing comment + // b + // ``` + CommentPlacement::Default(comment) + } +} + /// Attaches comments for the positional-only arguments separator `/` as trailing comments to the /// enclosing [`Arguments`] node. /// diff --git a/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__while_trailing_end_of_line_comment.snap b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__while_trailing_end_of_line_comment.snap new file mode 100644 index 0000000000..41e845c4b4 --- /dev/null +++ b/crates/ruff_python_formatter/src/comments/snapshots/ruff_python_formatter__comments__tests__while_trailing_end_of_line_comment.snap @@ -0,0 +1,21 @@ +--- +source: crates/ruff_python_formatter/src/comments/mod.rs +expression: comments.debug(test_case.source_code) +--- +{ + Node { + kind: StmtExpr, + range: 46..56, + source: `do.stuff()`, + }: { + "leading": [], + "dangling": [], + "trailing": [ + SourceComment { + text: "# trailing comment", + position: EndOfLine, + formatted: false, + }, + ], + }, +} diff --git a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__comments5_py.snap b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__comments5_py.snap index df1348d88f..7087a8c711 100644 --- a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__comments5_py.snap +++ b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__comments5_py.snap @@ -89,12 +89,13 @@ if __name__ == "__main__": @@ -1,11 +1,6 @@ while True: if something.changed: - do.stuff() # trailing comment +- do.stuff() # trailing comment - # Comment belongs to the `if` block. - # This one belongs to the `while` block. - - # Should this one, too? I guess so. - ++ do.stuff() # This one is properly standalone now. for i in range(100): @@ -161,7 +162,7 @@ if __name__ == "__main__": ```py while True: if something.changed: - do.stuff() # trailing comment + do.stuff() # This one is properly standalone now. for i in range(100): diff --git a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__comments6_py.snap b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__comments6_py.snap index 24247fc646..6181d6489b 100644 --- a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__comments6_py.snap +++ b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__comments6_py.snap @@ -168,12 +168,14 @@ aaaaaaaaaaaaa, bbbbbbbbb = map(list, map(itertools.chain.from_iterable, zip(*ite def f( a, # type: int b, # type: int -@@ -67,22 +57,16 @@ +@@ -66,23 +56,17 @@ + + element + another_element + another_element_with_long_name - ) # type: int +- ) # type: int - - ++ ) def f( x, # not a type comment y, # type: int @@ -272,7 +274,7 @@ def f( + element + another_element + another_element_with_long_name - ) # type: int + ) def f( x, # not a type comment y, # type: int diff --git a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__fmtonoff_py.snap b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__fmtonoff_py.snap index 362982ef50..1766ef2a6b 100644 --- a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__fmtonoff_py.snap +++ b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__fmtonoff_py.snap @@ -369,7 +369,7 @@ d={'a':1, + (1, 2, 3, 4), + (5, 6, 7, 8), + (9, 10, 11, 12) -+ } # yapf: disable ++ } cfg.rule( - "Default", - "address", @@ -544,7 +544,7 @@ def single_literal_yapf_disable(): (1, 2, 3, 4), (5, 6, 7, 8), (9, 10, 11, 12) - } # yapf: disable + } cfg.rule( "Default", "address", xxxx_xxxx=["xxx-xxxxxx-xxxxxxxxxx"], diff --git a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__fmtskip6_py.snap b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__fmtskip6_py.snap new file mode 100644 index 0000000000..46b8920531 --- /dev/null +++ b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__fmtskip6_py.snap @@ -0,0 +1,49 @@ +--- +source: crates/ruff_python_formatter/src/lib.rs +expression: snapshot +input_file: crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/fmtskip6.py +--- +## Input + +```py +class A: + def f(self): + for line in range(10): + if True: + pass # fmt: skip +``` + +## Black Differences + +```diff +--- Black ++++ Ruff +@@ -2,4 +2,4 @@ + def f(self): + for line in range(10): + if True: +- pass # fmt: skip ++ pass +``` + +## Ruff Output + +```py +class A: + def f(self): + for line in range(10): + if True: + pass +``` + +## Black Output + +```py +class A: + def f(self): + for line in range(10): + if True: + pass # fmt: skip +``` + + diff --git a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__remove_await_parens_py.snap b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__remove_await_parens_py.snap index 62096f8534..603cab0375 100644 --- a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__remove_await_parens_py.snap +++ b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__remove_await_parens_py.snap @@ -143,7 +143,7 @@ async def main(): - + await ( + asyncio.sleep(1) -+ ) # Hello ++ ) # Long lines async def main(): - await asyncio.gather( @@ -251,7 +251,7 @@ async def main(): async def main(): await ( asyncio.sleep(1) - ) # Hello + ) # Long lines async def main(): await asyncio.gather(asyncio.sleep(1), asyncio.sleep(1), asyncio.sleep(1), asyncio.sleep(1), asyncio.sleep(1), asyncio.sleep(1), asyncio.sleep(1))