Use LineSuffix.reserved_width in ruff_python_formatter

crates/ruff_formatter/src/builders.rs

@@ -434,7 +434,8 @@ fn debug_assert_no_newlines(text: &str) {
     debug_assert!(!text.contains('\r'), "The content '{text}' contains an unsupported '\\r' line terminator character but text must only use line feeds '\\n' as line separator. Use '\\n' instead of '\\r' and '\\r\\n' to insert a line break in strings.");
 }
 
-/// Pushes some content to the end of the current line
+/// Pushes some content to the end of the current line. Provide a reserved width in
+/// order to include the line suffix content during measurement.
 ///
 /// ## Examples
 ///
@@ -445,7 +446,7 @@ fn debug_assert_no_newlines(text: &str) {
 /// fn main() -> FormatResult<()> {
 /// let elements = format!(SimpleFormatContext::default(), [
 ///     text("a"),
-///     line_suffix(&text("c")),
+///     line_suffix(&text("c"), 0),
 ///     text("b")
 /// ])?;
 ///
@@ -457,23 +458,25 @@ fn debug_assert_no_newlines(text: &str) {
 /// # }
 /// ```
 #[inline]
-pub fn line_suffix<Content, Context>(inner: &Content) -> LineSuffix<Context>
+pub fn line_suffix<Content, Context>(inner: &Content, reserved_width: u32) -> LineSuffix<Context>
 where
     Content: Format<Context>,
 {
     LineSuffix {
         content: Argument::new(inner),
+        reserved_width,
     }
 }
 
 #[derive(Copy, Clone)]
 pub struct LineSuffix<'a, Context> {
     content: Argument<'a, Context>,
+    reserved_width: u32,
 }
 
 impl<Context> Format<Context> for LineSuffix<'_, Context> {
     fn fmt(&self, f: &mut Formatter<Context>) -> FormatResult<()> {
-        f.write_element(FormatElement::Tag(StartLineSuffix));
+        f.write_element(FormatElement::Tag(StartLineSuffix(self.reserved_width)));
         Arguments::from(&self.content).fmt(f)?;
         f.write_element(FormatElement::Tag(EndLineSuffix));
 
@@ -500,7 +503,7 @@ impl<Context> std::fmt::Debug for LineSuffix<'_, Context> {
 /// # fn  main() -> FormatResult<()> {
 /// let elements = format!(SimpleFormatContext::default(), [
 ///     text("a"),
-///     line_suffix(&text("c")),
+///     line_suffix(&text("c"), 0),
 ///     text("b"),
 ///     line_suffix_boundary(),
 ///     text("d")

crates/ruff_formatter/src/format_element/document.rs

@@ -458,8 +458,16 @@ impl Format<IrFormatContext<'_>> for &[FormatElement] {
                             )?;
                         }
 
-                        StartLineSuffix => {
-                            write!(f, [text("line_suffix(")])?;
+                        StartLineSuffix(reserved_width) => {
+                            write!(
+                                f,
+                                [
+                                    text("line_suffix("),
+                                    dynamic_text(&std::format!("{reserved_width:?}"), None),
+                                    text(","),
+                                    space(),
+                                ]
+                            )?;
                         }
 
                         StartVerbatim(_) => {
@@ -671,7 +679,7 @@ impl FormatElements for [FormatElement] {
             match element {
                 // Line suffix
                 // Ignore if any of its content breaks
-                FormatElement::Tag(Tag::StartLineSuffix | Tag::StartFitsExpanded(_)) => {
+                FormatElement::Tag(Tag::StartLineSuffix(_) | Tag::StartFitsExpanded(_)) => {
                     ignore_depth += 1;
                 }
                 FormatElement::Tag(Tag::EndLineSuffix | Tag::EndFitsExpanded) => {

crates/ruff_formatter/src/format_element/tag.rs

@@ -64,7 +64,7 @@ pub enum Tag {
     EndEntry,
 
     /// Delay the printing of its content until the next line break
-    StartLineSuffix,
+    StartLineSuffix(u32),
     EndLineSuffix,
 
     /// A token that tracks tokens/nodes that are printed as verbatim.
@@ -96,7 +96,7 @@ impl Tag {
                 | Tag::StartIndentIfGroupBreaks(_)
                 | Tag::StartFill
                 | Tag::StartEntry
-                | Tag::StartLineSuffix
+                | Tag::StartLineSuffix(_)
                 | Tag::StartVerbatim(_)
                 | Tag::StartLabelled(_)
                 | Tag::StartFitsExpanded(_)
@@ -122,7 +122,7 @@ impl Tag {
             StartIndentIfGroupBreaks(_) | EndIndentIfGroupBreaks => TagKind::IndentIfGroupBreaks,
             StartFill | EndFill => TagKind::Fill,
             StartEntry | EndEntry => TagKind::Entry,
-            StartLineSuffix | EndLineSuffix => TagKind::LineSuffix,
+            StartLineSuffix(_) | EndLineSuffix => TagKind::LineSuffix,
             StartVerbatim(_) | EndVerbatim => TagKind::Verbatim,
             StartLabelled(_) | EndLabelled => TagKind::Labelled,
             StartFitsExpanded { .. } | EndFitsExpanded => TagKind::FitsExpanded,

crates/ruff_formatter/src/printer/mod.rs

@@ -233,7 +233,8 @@ impl<'a> Printer<'a> {
                 stack.push(TagKind::IndentIfGroupBreaks, args);
             }
 
-            FormatElement::Tag(StartLineSuffix) => {
+            FormatElement::Tag(StartLineSuffix(reserved_width)) => {
+                self.state.line_width += reserved_width;
                 self.state
                     .line_suffixes
                     .extend(args, queue.iter_content(TagKind::LineSuffix));
@@ -1184,7 +1185,11 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
                 }
             }
 
-            FormatElement::Tag(StartLineSuffix) => {
+            FormatElement::Tag(StartLineSuffix(reserved_width)) => {
+                self.state.line_width += reserved_width;
+                if self.state.line_width > self.options().print_width.into() {
+                    return Ok(Fits::No);
+                }
                 self.queue.skip_content(TagKind::LineSuffix);
                 self.state.has_line_suffix = true;
             }
@@ -1720,7 +1725,7 @@ two lines`,
                 text("]")
             ]),
             text(";"),
-            line_suffix(&format_args![space(), text("// trailing")])
+            line_suffix(&format_args![space(), text("// trailing")], 0)
         ]);
 
         assert_eq!(printed.as_code(), "[1, 2, 3]; // trailing");

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

@@ -66,3 +66,5 @@
 h1 = ((((1, 2))))
 h2 = ((((1, "qweiurpoiqwurepqiurpqirpuqoiwrupqoirupqoirupqoiurpqiorupwqiourpqurpqurpqurpqurpqurpqurüqurqpuriq"))))
 h3 = 1, "qweiurpoiqwurepqiurpqirpuqoiwrupqoirupqoirupqoiurpqiorupwqiourpqurpqurpqurpqurpqurpqurüqurqpuriq"
+
+i1 = ("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",)  # This should break

crates/ruff_python_formatter/src/comments/format.rs

@@ -1,9 +1,10 @@
 use ruff_python_ast::Ranged;
 use ruff_text_size::{TextLen, TextRange, TextSize};
 
-use ruff_formatter::{format_args, write, FormatError, SourceCode};
+use ruff_formatter::{format_args, write, FormatError, FormatOptions, SourceCode};
 use ruff_python_ast::node::{AnyNodeRef, AstNode};
 use ruff_python_trivia::{lines_after, lines_after_ignoring_trivia, lines_before};
+use unicode_width::UnicodeWidthChar;
 
 use crate::comments::{CommentLinePosition, SourceComment};
 use crate::context::NodeLevel;
@@ -151,18 +152,33 @@ impl Format<PyFormatContext<'_>> for FormatTrailingComments<'_> {
                 write!(
                     f,
                     [
-                        line_suffix(&format_args![
-                            empty_lines(lines_before_comment),
-                            format_comment(trailing)
-                        ]),
+                        line_suffix(
+                            &format_args![
+                                empty_lines(lines_before_comment),
+                                format_comment(trailing)
+                            ],
+                            0 // Reserving width isn't necessary because we don't split comments and the empty lines expand any enclosing group.
+                        ),
                         expand_parent()
                     ]
                 )?;
             } else {
+                // A trailing comment at the end of a line has a reserved width to consider during line measurement.
+                // ```python
+                // tup = (
+                //     "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+                // )  # Some comment
+                // ```
                 write!(
                     f,
                     [
-                        line_suffix(&format_args![space(), space(), format_comment(trailing)]),
+                        line_suffix(
+                            &format_args![space(), space(), format_comment(trailing)],
+                            measure_text(
+                                &f.context().source()[slice.range()],
+                                f.options().tab_width().value()
+                            ) + 2 // Account for two added spaces
+                        ),
                         expand_parent()
                     ]
                 )?;
@@ -264,10 +280,18 @@ impl Format<PyFormatContext<'_>> for FormatDanglingOpenParenthesisComments<'_> {
                 "Expected dangling comment to be at the end of the line"
             );
 
+            let slice = comment.slice();
             write!(
                 f,
                 [
-                    line_suffix(&format_args!(space(), space(), format_comment(comment))),
+                    line_suffix(
+                        &format_args!(space(), space(), format_comment(comment)),
+                        // Marking the comment as a line suffix with reserved width is safe since we expect the comment to be end of line.
+                        measure_text(
+                            &f.context().source()[slice.range()],
+                            f.options().tab_width().value()
+                        ) + 2 // Account for two added spaces
+                    ),
                     expand_parent()
                 ]
             )?;
@@ -374,3 +398,18 @@ impl Format<PyFormatContext<'_>> for FormatEmptyLines {
         }
     }
 }
+
+/// Helper for measuring text.
+fn measure_text(text: &str, tab_width: u32) -> u32 {
+    let mut width = 0;
+    for c in text.chars() {
+        let char_width = match c {
+            '\t' => tab_width,
+            // SAFETY: A u32 is sufficient to format files <= 4GB
+            #[allow(clippy::cast_possible_truncation)]
+            c => c.width().unwrap_or(0) as u32,
+        };
+        width += char_width;
+    }
+    width
+}

crates/ruff_python_formatter/src/expression/mod.rs

@@ -247,10 +247,13 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
                     if format_expression.inspect(f)?.will_break() {
                         // The group here is necessary because `format_expression` may contain IR elements
                         // that refer to the group id
-                        group(&format_expression)
-                            .with_group_id(Some(group_id))
-                            .should_expand(true)
-                            .fmt(f)
+                        group(&format_args![
+                            text("("),
+                            soft_block_indent(&format_expression),
+                            text(")")
+                        ])
+                        .with_group_id(Some(group_id))
+                        .fmt(f)
                     } else {
                         // Only add parentheses if it makes the expression fit on the line.
                         // Using the flat version as the most expanded version gives a left-to-right splitting behavior

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

@@ -156,7 +156,7 @@ aaaaaaaaaaaaa, bbbbbbbbb = map(list, map(itertools.chain.from_iterable, zip(*ite
      )
  
  
-@@ -108,11 +112,18 @@
+@@ -108,11 +112,20 @@
      "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
  )
  
@@ -176,7 +176,10 @@ aaaaaaaaaaaaa, bbbbbbbbb = map(list, map(itertools.chain.from_iterable, zip(*ite
 +    ],  # type: ignore
  )
  
- aaaaaaaaaaaaa, bbbbbbbbb = map(list, map(itertools.chain.from_iterable, zip(*items)))  # type: ignore[arg-type]
+-aaaaaaaaaaaaa, bbbbbbbbb = map(list, map(itertools.chain.from_iterable, zip(*items)))  # type: ignore[arg-type]
++aaaaaaaaaaaaa, bbbbbbbbb = map(
++    list, map(itertools.chain.from_iterable, zip(*items))
++)  # type: ignore[arg-type]
 ```
 
 ## Ruff Output
@@ -310,7 +313,9 @@ call_to_some_function_asdf(
     ],  # type: ignore
 )
 
-aaaaaaaaaaaaa, bbbbbbbbb = map(list, map(itertools.chain.from_iterable, zip(*items)))  # type: ignore[arg-type]
+aaaaaaaaaaaaa, bbbbbbbbb = map(
+    list, map(itertools.chain.from_iterable, zip(*items))
+)  # type: ignore[arg-type]
 ```
 
 ## Black Output

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

@@ -300,7 +300,18 @@ last_call()
  )  # note: no trailing comma pre-3.6
  call(*gidgets[:2])
  call(a, *gidgets[:2])
-@@ -328,13 +329,18 @@
+@@ -142,7 +143,9 @@
+ xxxx_xxxxx_xxxx_xxx: Callable[..., List[SomeClass]] = classmethod(  # type: ignore
+     sync(async_xxxx_xxx_xxxx_xxxxx_xxxx_xxx.__func__)
+ )
+-xxxx_xxx_xxxx_xxxxx_xxxx_xxx: Callable[..., List[SomeClass]] = classmethod(  # type: ignore
++xxxx_xxx_xxxx_xxxxx_xxxx_xxx: Callable[
++    ..., List[SomeClass]
++] = classmethod(  # type: ignore
+     sync(async_xxxx_xxx_xxxx_xxxxx_xxxx_xxx.__func__)
+ )
+ xxxx_xxx_xxxx_xxxxx_xxxx_xxx: Callable[..., List[SomeClass]] = classmethod(
+@@ -328,13 +331,18 @@
  ):
      return True
  if (
@@ -322,7 +333,7 @@ last_call()
      ^ aaaaaaaa.i << aaaaaaaa.k >> aaaaaaaa.l**aaaaaaaa.m // aaaaaaaa.n
  ):
      return True
-@@ -342,7 +348,8 @@
+@@ -342,7 +350,8 @@
      ~aaaaaaaaaaaaaaaa.a
      + aaaaaaaaaaaaaaaa.b
      - aaaaaaaaaaaaaaaa.c * aaaaaaaaaaaaaaaa.d @ aaaaaaaaaaaaaaaa.e
@@ -482,7 +493,9 @@ very_long_variable_name_filters: t.List[
 xxxx_xxxxx_xxxx_xxx: Callable[..., List[SomeClass]] = classmethod(  # type: ignore
     sync(async_xxxx_xxx_xxxx_xxxxx_xxxx_xxx.__func__)
 )
-xxxx_xxx_xxxx_xxxxx_xxxx_xxx: Callable[..., List[SomeClass]] = classmethod(  # type: ignore
+xxxx_xxx_xxxx_xxxxx_xxxx_xxx: Callable[
+    ..., List[SomeClass]
+] = classmethod(  # type: ignore
     sync(async_xxxx_xxx_xxxx_xxxxx_xxxx_xxx.__func__)
 )
 xxxx_xxx_xxxx_xxxxx_xxxx_xxx: Callable[..., List[SomeClass]] = classmethod(