Update

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

@@ -62,3 +62,105 @@
 x = f'''a{""}b'''
 y = f'''c{1}d"""e'''
 z = f'''a{""}b''' f'''c{1}d"""e'''
+
+# F-String formatting test cases
+
+# Expression which does not exceed the line length limit
+x = f"{a}"
+x = f"{
+    a = }"
+x = f"{ # comment
+    a }"
+x = f"{ # comment
+    a = }"
+
+# Expression which exceeds the line length limit
+x = f"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa { "bbbbbbbbbbbbbbbbbbbbbbbbbbbbb" } ccccccccccccccc"
+x = f"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa { "bbbbbbbbbbbbbbbbbbbbbbbbbbbbb" = } ccccccccccccccc"
+x = f"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa { # comment
+                                             "bbbbbbbbbbbbbbbbbbbbbbbbbbbbb" } ccccccccccccccc"
+x = f"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa { # comment
+                                             "bbbbbbbbbbbbbbbbbbbbbbbbbbbbb" = } ccccccccccccccc"
+
+# Multiple larger expressions which exceeds the line length limit. Here, we need to decide
+# whether to split at the first or second expression. This should work similarly to the
+# assignment statement formatting where we split from left to right.
+x = f"aaaaaaaaaaaa { bbbbbbbbbbbbbb } cccccccccccccccccccc { ddddddddddddddd } eeeeeeeeeeeeee"
+
+# But, in this case, we would split at the first expression because there's already a
+# comment which splits it.
+x = f"aaaaaaaaaaaa { bbbbbbbbbbbbbb # comment
+                    } cccccccccccccccccccc { ddddddddddddddd } eeeeeeeeeeeeee"
+
+# Here, the expression part itself starts with a curly brace so we need to add an extra
+# space between the opening curly brace and the expression.
+x = f"{ {'x': 1, 'y': 2} }"
+x = f"{ {'x': 1, 'y': 2} = }"
+x = f"{  # comment
+    {'x': 1, 'y': 2} }"
+x = f"{  # comment
+    {'x': 1, 'y': 2} = }"
+
+# But, in this case, we would split the expression itself because it exceeds the line
+# length limit so we need not add the extra space.
+x = f"{ {'aaaaaaaaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbbbbbbbbbbb', 'cccccccccccccccccccccccccc'} }"
+
+# Comments
+
+# comment 0
+f"
+{ # comment 1
+    # comment 2
+    foo # comment 3
+    # comment 4
+}"  # comment 5
+
+# Conversion flags
+#
+# This is not a valid Python code because of the additional whitespace between the `!`
+# and conversion type. But, our parser isn't strict about this. This should probably be
+# removed once we have a strict parser.
+x = f"aaaaaaaaa { x !  r }"
+
+# Even in the case of debug expresions, we only need to preserve the whitespace within
+# the expression part of the replacement field.
+x = f"aaaaaaaaa { x    !  r = }"
+
+# Combine conversion flags with format specifiers
+x = f"{x   =   !  s
+         :>0
+
+         }"
+# Well, this is new. There can be a comment after the format specifier but only if it's
+# on it's own line. Refer to https://github.com/astral-sh/ruff/pull/7787 for more details.
+# We'll format is as trailing comments.
+x = f"{x  !s
+         :>0
+         # comment
+         }"
+
+x = f"""
+{              # dangling comment 1
+ x =   :.0{y # dangling comment 2
+           }f}"""
+
+# Here, the debug expression is in a nested f-string so we should start preserving
+# whitespaces from that point onwards. This means we should format the outer f-string.
+x = f"""{"foo " +    # comment 1
+    f"{   x =
+
+       }"    # comment 2
+ }
+        """
+
+# Mix of various features.
+f"{  # dangling comment 1
+    foo # after foo
+   :>{
+          x # after x
+          }
+    # dangling comment 2
+    # dangling comment 3
+} woah {x}"
+
+x = f"{f"{f'{x}'}"}"

crates/ruff_python_formatter/src/comments/placement.rs

@@ -290,7 +290,11 @@ fn handle_enclosed_comment<'a>(
         }
         AnyNodeRef::FString(fstring) => CommentPlacement::dangling(fstring, comment),
         AnyNodeRef::FStringExpressionElement(_) => {
-            handle_bracketed_end_of_line_comment(comment, locator)
+            if let Some(AnyNodeRef::FStringExpressionElement(_)) = comment.preceding_node() {
+                CommentPlacement::dangling(comment.enclosing_node(), comment)
+            } else {
+                handle_bracketed_end_of_line_comment(comment, locator)
+            }
         }
         AnyNodeRef::ExprList(_)
         | AnyNodeRef::ExprSet(_)

crates/ruff_python_formatter/src/expression/expr_f_string.rs

@@ -21,7 +21,7 @@ impl FormatNodeRule<ExprFString> for FormatExprFString {
         match value.as_slice() {
             [f_string_part] => FormatFStringPart::new(
                 f_string_part,
-                if is_pep_701_enabled(f) {
+                if is_pep_701_enabled(f.context()) {
                     // In preview mode, if the target version supports PEP 701, then
                     // we can re-use the same quotes inside the f-string.
                     Quoting::CanChange
@@ -51,14 +51,12 @@ impl NeedsParentheses for ExprFString {
     fn needs_parentheses(
         &self,
         _parent: AnyNodeRef,
-        context: &PyFormatContext,
+        _context: &PyFormatContext,
     ) -> OptionalParentheses {
         if self.value.is_implicit_concatenated() {
             OptionalParentheses::Multiline
-        } else if AnyString::FString(self).is_multiline(context.source()) {
-            OptionalParentheses::Never
         } else {
-            OptionalParentheses::BestFit
+            OptionalParentheses::Never
         }
     }
 }

crates/ruff_python_formatter/src/other/f_string.rs

@@ -51,6 +51,9 @@ impl Format<PyFormatContext<'_>> for FormatFString<'_> {
 
         let string = StringPart::from_source(self.value.range(), &locator);
 
+        // TODO: I think we can skip `choose_quotes` and instead just use the
+        // preferred quotes directly from the options, maybe except for when
+        // formatting inside a docstring.
         let quotes = choose_quotes(
             &string,
             &locator,

crates/ruff_python_formatter/src/other/f_string_element.rs

@@ -6,10 +6,12 @@ use ruff_python_ast::{
 };
 use ruff_text_size::Ranged;
 
+use crate::comments::trailing_comments;
 use crate::expression::parentheses::parenthesized;
 use crate::prelude::*;
 use crate::preview::is_hex_codes_in_unicode_sequences_enabled;
 use crate::string::normalize_string;
+use crate::verbatim::suppressed_node;
 
 use super::f_string::FStringContext;
 
@@ -62,7 +64,7 @@ impl Format<PyFormatContext<'_>> for FormatFStringLiteralElement<'_> {
         );
         match &normalized {
             Cow::Borrowed(_) => source_text_slice(self.element.range()).fmt(f),
-            Cow::Owned(normalized) => text(normalized, Some(self.element.start())).fmt(f),
+            Cow::Owned(normalized) => text(normalized).fmt(f),
         }
     }
 }
@@ -80,15 +82,6 @@ impl<'a> FormatFStringExpressionElement<'a> {
 
 impl Format<PyFormatContext<'_>> for FormatFStringExpressionElement<'_> {
     fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> {
-        // Notes for implementing f-string formatting for 3.12 or later:
-        //
-        // - Currently, if there are comments in the f-string, we abort formatting
-        //   and fall back to the string normalization. If comments are supported,
-        //   then they need to be handled separately when debug text is present.
-        //   This is because the comments are present in the debug text in the raw
-        //   form. One solution would be to mark all comments as formatted and
-        //   add the debug text as it is.
-
         let FStringExpressionElement {
             expression,
             debug_text,
@@ -97,80 +90,73 @@ impl Format<PyFormatContext<'_>> for FormatFStringExpressionElement<'_> {
             ..
         } = self.element;
 
-        // If an expression starts with a `{`, we need to add a space before the
-        // curly brace to avoid turning it into a literal curly with `{{`.
-        //
-        // For example,
-        // ```python
-        // f"{ {'x': 1, 'y': 2} }"
-        // #  ^                ^
-        // ```
-        //
-        // We need to preserve the space highlighted by `^`.
-        let line_break_or_space = if debug_text.is_some() {
-            None
+        let comments = f.context().comments().clone();
+
+        if let Some(debug_text) = debug_text {
+            token("{").fmt(f)?;
+
+            // If debug text is present in a f-string, we'll mark all of the comments
+            // in this f-string as formatted.
+            comments.mark_verbatim_node_comments_formatted(self.element.into());
+
+            write!(
+                f,
+                [
+                    text(&debug_text.leading),
+                    // TODO: preserve the parentheses
+                    suppressed_node(&**expression),
+                    text(&debug_text.trailing),
+                ]
+            )?;
+
+            // Even if debug text is present, any whitespace between the
+            // conversion flag and the format spec doesn't need to be preserved.
+            match conversion {
+                ConversionFlag::Str => text("!s").fmt(f)?,
+                ConversionFlag::Ascii => text("!a").fmt(f)?,
+                ConversionFlag::Repr => text("!r").fmt(f)?,
+                ConversionFlag::None => (),
+            }
+
+            if let Some(format_spec) = format_spec.as_deref() {
+                write!(f, [token(":"), suppressed_node(format_spec)])?;
+            }
+
+            token("}").fmt(f)
         } else {
-            match expression.as_ref() {
-                Expr::Dict(_) | Expr::DictComp(_) | Expr::Set(_) | Expr::SetComp(_) => {
-                    // This should either be a line break or a space to avoid adding
-                    // a leading space to the expression in case the expression
-                    // breaks over multiple lines.
+            let dangling_item_comments = comments.dangling(self.element);
+            let (dangling_open_parentheses_comments, trailing_format_spec_comments) =
+                dangling_item_comments.split_at(
+                    dangling_item_comments
+                        .partition_point(|comment| comment.start() < expression.start()),
+                );
+
+            let item = format_with(|f| {
+                let line_break_or_space = match expression.as_ref() {
+                    // If an expression starts with a `{`, we need to add a space before the
+                    // curly brace to avoid turning it into a literal curly with `{{`.
                     //
-                    // This is especially important when there's a comment present.
                     // For example,
-                    //
                     // ```python
-                    // f"something {
-                    //     # comment
-                    //     {'a': 1, 'b': 2}
-                    // } ending"
+                    // f"{ {'x': 1, 'y': 2} }"
+                    // #  ^                ^
                     // ```
                     //
-                    // If we would unconditionally add a space, there would be a
-                    // trailing space before the comment.
-                    Some(soft_line_break_or_space())
-                }
-                _ => None,
-            }
-        };
-
-        let conversion_text = match conversion {
-            ConversionFlag::Str => Some(text("!s", None)),
-            ConversionFlag::Ascii => Some(text("!a", None)),
-            ConversionFlag::Repr => Some(text("!r", None)),
-            ConversionFlag::None => None,
-        };
-
-        let inner =
-            &format_with(|f| {
-                if let Some(debug_text) = debug_text {
-                    text(&debug_text.leading, None).fmt(f)?;
-                }
-
-                write!(
-                    f,
-                    [
-                        line_break_or_space,
-                        expression.format(),
-                        // The extra whitespace isn't strictly required for the
-                        // ending curly brace but it's here for symmetry.
-                        //
-                        // For example, the following is valid:
-                        // ```python
-                        // f"{ {'a': 1}}"
-                        // ```
-                        //
-                        // But, the following looks better:
-                        // ```python
-                        // f"{ {'a': 1} }"
-                        // ```
-                        line_break_or_space,
-                        conversion_text,
-                    ]
-                )?;
-
-                if let Some(debug_text) = debug_text {
-                    text(&debug_text.trailing, None).fmt(f)?;
+                    // We need to preserve the space highlighted by `^`.
+                    Expr::Dict(_) | Expr::DictComp(_) | Expr::Set(_) | Expr::SetComp(_) => {
+                        Some(soft_line_break_or_space())
+                    }
+                    _ => None,
+                };
+
+                write!(f, [line_break_or_space, expression.format()])?;
+
+                // Conversion comes first, then the format spec.
+                match conversion {
+                    ConversionFlag::Str => text("!s").fmt(f)?,
+                    ConversionFlag::Ascii => text("!a").fmt(f)?,
+                    ConversionFlag::Repr => text("!r").fmt(f)?,
+                    ConversionFlag::None => (),
                 }
 
                 if let Some(format_spec) = format_spec.as_deref() {
@@ -182,16 +168,22 @@ impl Format<PyFormatContext<'_>> for FormatFStringExpressionElement<'_> {
                                 }))
                                 .finish()
                         });
-                    write!(f, [token(":"), elements])?;
+                    write!(
+                        f,
+                        [
+                            token(":"),
+                            elements,
+                            trailing_comments(trailing_format_spec_comments)
+                        ]
+                    )?;
                 }
 
-                Ok(())
+                line_break_or_space.fmt(f)
             });
 
-        if self.context.quotes().is_triple() {
-            parenthesized("{", inner, "}").fmt(f)
-        } else {
-            write!(f, [token("{"), inner, token("}")])
+            parenthesized("{", &item, "}")
+                .with_dangling_comments(dangling_open_parentheses_comments)
+                .fmt(f)
         }
     }
 }

crates/ruff_python_formatter/src/preview.rs

@@ -4,7 +4,9 @@
 //! to stable. The challenge with directly using [`is_preview`](PyFormatContext::is_preview) is that it is unclear
 //! for which specific feature this preview check is for. Having named functions simplifies the promotion:
 //! Simply delete the function and let Rust tell you which checks you have to remove.
-use crate::{PyFormatContext, PyFormatter};
+use ruff_formatter::FormatContext;
+
+use crate::PyFormatContext;
 
 /// Returns `true` if the [`fix_power_op_line_length`](https://github.com/astral-sh/ruff/issues/8938) preview style is enabled.
 pub(crate) const fn is_fix_power_op_line_length_enabled(context: &PyFormatContext) -> bool {
@@ -83,6 +85,6 @@ pub(crate) const fn is_format_module_docstring_enabled(context: &PyFormatContext
 }
 
 /// Returns `true` if the [`PEP 701`](https://github.com/astral-sh/ruff/issues/7594) preview style is enabled.
-pub(crate) const fn is_pep_701_enabled(f: &PyFormatter) -> bool {
-    f.context.is_preview() && f.options.target_version().supports_pep_701()
+pub(crate) fn is_pep_701_enabled(context: &PyFormatContext) -> bool {
+    context.is_preview() && context.options().target_version().supports_pep_701()
 }