Avoid parenthesizing unsplittable because of comments

crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/optional_parentheses_comments.py

@@ -0,0 +1,55 @@
+comment_string = "Long lines with inline comments should have their comments appended to the reformatted string's enclosing right parentheses."  # This comment gets thrown to the top.
+
+
+## ident, 83 columns, comment 89 columns
+____aaa = (
+    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvvvvvvvvvvv # ccc
+)
+
+____aaa = (
+    # leading
+    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvvvvvvvvvvv
+) # ccc
+
+## ident, 83 columns, comment 88 columns
+____aaa = (
+    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvvvvvvvvvvv # cc
+)
+
+## ident, 83 columns, comment 87 columns
+____aaa = (
+    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvvvvvvvvvvv # c
+)
+
+____[a] = (
+    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbvvvvvvvvvvv # c
+)
+
+# 88 column
+a = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa and bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
+# 89
+a = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa and bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
+
+
+def setUpTestData(cls):
+    cls.happening = (
+        Happening.objects.create()
+    )  # make sure the defaults are working (#20158)
+
+def setUpTestData(cls):
+    cls.happening = (
+        Happening.objects.create  # make sure the defaults are working (#20158)
+    )
+
+if True:
+    if True:
+        if True:
+            # Black layout
+            model.config.use_cache = (
+                False  # FSTM still requires this hack -> FSTM should probably be refactored s
+            )
+
+
+req.META[
+    "SERVER_NAME"
+] = "testservereeeeeeeeee"  # Required to have redirect work in l

crates/ruff_python_formatter/src/expression/expr_await.rs

@@ -20,7 +20,7 @@ impl FormatNodeRule<ExprAwait> for FormatExprAwait {
             [
                 token("await"),
                 space(),
-                maybe_parenthesize_expression(value, item, Parenthesize::IfBreaks)
+                maybe_parenthesize_expression(value, item, Parenthesize::IfRequired)
             ]
         )
     }

crates/ruff_python_formatter/src/expression/mod.rs

@@ -12,7 +12,9 @@ use ruff_python_trivia::CommentRanges;
 use ruff_text_size::Ranged;
 
 use crate::builders::parenthesize_if_expands;
-use crate::comments::{leading_comments, trailing_comments, LeadingDanglingTrailingComments};
+use crate::comments::{
+    leading_comments, trailing_comments, LeadingDanglingTrailingComments, SourceComment,
+};
 use crate::context::{NodeLevel, WithNodeLevel};
 use crate::expression::expr_generator_exp::is_generator_parenthesized;
 use crate::expression::expr_tuple::is_tuple_parenthesized;
@@ -374,10 +376,8 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
             return expression.format().with_options(Parentheses::Always).fmt(f);
         }
 
-        let node_comments = f
-            .context()
-            .comments()
-            .leading_dangling_trailing(*expression);
+        let comments = f.context().comments().clone();
+        let node_comments = comments.leading_dangling_trailing(*expression);
 
         // If the expression has comments, we always want to preserve the parentheses. This also
         // ensures that we correctly handle parenthesized comments, and don't need to worry about
@@ -426,15 +426,104 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
                     expression.format().with_options(Parentheses::Never).fmt(f)
                 }
                 Parenthesize::IfBreaks => {
-                    if node_comments.has_trailing() {
+                    let last_expression = parent.is_stmt_assign()
+                        || parent.is_stmt_ann_assign()
+                        || parent.is_stmt_aug_assign();
+                    // || parent.is_stmt_return();
+
+                    // TODO doesn't work on expression level. Only apply to assignments?
+                    // Test with return, await, and yield.
+                    // The problem is that `await` and `yield` already add optional parentheses.
+                    // Does await need to call `self.value.needs_parentheses()`. Or should it use another layout than `IfBreaks`?
+
+                    // Format the statements and value's trailing end of line comments:
+                    // * after the expression if the expression needs no parentheses (necessary or the `expand_parent` makes the group never fit).
+                    // * inside the parentheses if the expression exceeds the line-width.
+                    //
+                    // ```python
+                    // a = long # with_comment
+                    // b = (
+                    //     short # with_comment
+                    // )
+                    //
+                    // # formatted
+                    // a = (
+                    //     long # with comment
+                    // )
+                    // b = short # with comment
+                    // ```
+                    // This matches Black's formatting with the exception that ruff applies this style also for
+                    // attribute chains and non-fluent call expressions. See https://github.com/psf/black/issues/4001#issuecomment-1786681792
+                    //
+                    // This logic isn't implemented in [`place_comment`] by associating trailing statement comments to the expression because
+                    // doing so breaks the suite empty lines formatting that relies on trailing comments to be stored on the statement.
+                    let (inline_comments, expression_trailing_comments) = if last_expression
+                        && !(expression.is_call_expr() || expression.is_attribute_expr())
+                    // this prevents the regression
+                    // This check doesn't work for yield and await expressions.
+                    {
+                        let parent_trailing_comments = comments.trailing(*parent);
+                        let after_end_of_line = parent_trailing_comments
+                            .partition_point(|comment| comment.line_position().is_end_of_line());
+                        let (stmt_inline_comments, _) =
+                            parent_trailing_comments.split_at(after_end_of_line);
+
+                        let after_end_of_line = node_comments
+                            .trailing
+                            .partition_point(|comment| comment.line_position().is_end_of_line());
+
+                        let (expression_inline_comments, expression_trailing_comments) =
+                            node_comments.trailing.split_at(after_end_of_line);
+
+                        (
+                            OptionalParenthesesInlinedComments {
+                                expression: expression_inline_comments,
+                                statement: stmt_inline_comments,
+                            },
+                            expression_trailing_comments,
+                        )
+                    } else {
+                        (
+                            OptionalParenthesesInlinedComments::default(),
+                            node_comments.trailing,
+                        )
+                    };
+
+                    if !expression_trailing_comments.is_empty() {
                         expression.format().with_options(Parentheses::Always).fmt(f)
                     } else {
                         // The group id is necessary because the nested expressions may reference it.
                         let group_id = f.group_id("optional_parentheses");
                         let f = &mut WithNodeLevel::new(NodeLevel::Expression(Some(group_id)), f);
-                        best_fit_parenthesize(&expression.format().with_options(Parentheses::Never))
-                            .with_group_id(Some(group_id))
-                            .fmt(f)
+
+                        best_fit_parenthesize(&format_with(|f| {
+                            inline_comments.mark_formatted();
+
+                            expression
+                                .format()
+                                .with_options(Parentheses::Never)
+                                .fmt(f)?;
+
+                            if !inline_comments.is_empty() {
+                                // If the expressions exceeds the line width, format the comments in the parentheses
+                                if_group_breaks(&inline_comments)
+                                    .with_group_id(Some(group_id))
+                                    .fmt(f)?;
+                            }
+
+                            Ok(())
+                        }))
+                        .with_group_id(Some(group_id))
+                        .fmt(f)?;
+
+                        if !inline_comments.is_empty() {
+                            // If the line fits into the line width, format the comments after the parenthesized expression
+                            if_group_fits_on_line(&inline_comments)
+                                .with_group_id(Some(group_id))
+                                .fmt(f)?;
+                        }
+
+                        Ok(())
                     }
                 }
             },
@@ -1069,3 +1158,41 @@ impl From<ast::Operator> for OperatorPrecedence {
         }
     }
 }
+
+#[derive(Debug, Default)]
+struct OptionalParenthesesInlinedComments<'a> {
+    expression: &'a [SourceComment],
+    statement: &'a [SourceComment],
+}
+
+impl<'a> OptionalParenthesesInlinedComments<'a> {
+    fn is_empty(&self) -> bool {
+        self.expression.is_empty() && self.statement.is_empty()
+    }
+
+    fn iter_comments(&self) -> impl Iterator<Item = &'a SourceComment> {
+        self.expression.iter().chain(self.statement)
+    }
+
+    fn mark_formatted(&self) {
+        for comment in self.iter_comments() {
+            comment.mark_formatted();
+        }
+    }
+}
+
+impl Format<PyFormatContext<'_>> for OptionalParenthesesInlinedComments<'_> {
+    fn fmt(&self, f: &mut Formatter<PyFormatContext<'_>>) -> FormatResult<()> {
+        for comment in self.iter_comments() {
+            comment.mark_unformatted();
+        }
+
+        write!(
+            f,
+            [
+                trailing_comments(self.expression),
+                trailing_comments(self.statement)
+            ]
+        )
+    }
+}

crates/ruff_python_formatter/src/lib.rs

@@ -206,13 +206,9 @@ if True:
     #[test]
     fn quick_test() {
         let source = r#"
-def main() -> None:
-    if True:
-        some_very_long_variable_name_abcdefghijk = Foo()
-        some_very_long_variable_name_abcdefghijk = some_very_long_variable_name_abcdefghijk[
-            some_very_long_variable_name_abcdefghijk.some_very_long_attribute_name
-            == "This is a very long string abcdefghijk"
-        ]
+result = (
+                await master.api.get_albums()
+            )  # list of albums with name, artist, uri
 
 "#;
         let source_type = PySourceType::Python;

crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__remove_await_parens.py.snap

@@ -93,7 +93,7 @@ async def main():
 ```diff
 --- Black
 +++ Ruff
-@@ -21,11 +21,15 @@
+@@ -21,7 +21,9 @@
  
  # Check comments
  async def main():
@@ -103,13 +103,6 @@ async def main():
 +    )
  
  
- async def main():
--    await asyncio.sleep(1)  # Hello
-+    await (
-+        asyncio.sleep(1)  # Hello
-+    )
- 
- 
  async def main():
 ```
 
@@ -145,9 +138,7 @@ async def main():
 
 
 async def main():
-    await (
-        asyncio.sleep(1)  # Hello
-    )
+    await asyncio.sleep(1)  # Hello
 
 
 async def main():