Simplify magic trailing comma logic

crates/ruff_python_formatter/src/builders.rs

@@ -1,7 +1,7 @@
 use ruff_formatter::{write, Argument, Arguments};
 use ruff_text_size::{Ranged, TextRange, TextSize};
 
-use crate::context::{NodeLevel, WithNodeLevel};
+use crate::context::{FStringState, NodeLevel, WithNodeLevel};
 use crate::other::commas::has_magic_trailing_comma;
 use crate::prelude::*;
 
@@ -205,6 +205,15 @@ impl<'fmt, 'ast, 'buf> JoinCommaSeparatedBuilder<'fmt, 'ast, 'buf> {
     }
 
     pub(crate) fn finish(&mut self) -> FormatResult<()> {
+        // If the formatter is inside an f-string expression element, and the layout
+        // is flat, then we don't need to add a trailing comma.
+        if let FStringState::InsideExpressionElement(context) = self.fmt.context().f_string_state()
+        {
+            if context.layout().is_flat() {
+                return Ok(());
+            }
+        }
+
         self.result.and_then(|()| {
             if let Some(last_end) = self.entries.position() {
                 let magic_trailing_comma = has_magic_trailing_comma(

crates/ruff_python_formatter/src/context.rs

@@ -1,5 +1,6 @@
 use crate::comments::Comments;
-use crate::string::{QuoteChar, StringQuotes};
+use crate::other::f_string::FStringContext;
+use crate::string::QuoteChar;
 use crate::PyFormatOptions;
 use ruff_formatter::{Buffer, FormatContext, GroupId, IndentWidth, SourceCode};
 use ruff_source_file::Locator;
@@ -97,12 +98,6 @@ impl<'a> PyFormatContext<'a> {
         self.f_string_state = f_string_state;
     }
 
-    /// Returns a new context with the given set of options.
-    pub(crate) fn with_options(mut self, options: PyFormatOptions) -> Self {
-        self.options = options;
-        self
-    }
-
     /// Returns `true` if preview mode is enabled.
     pub(crate) const fn is_preview(&self) -> bool {
         self.options.preview().is_enabled()
@@ -137,8 +132,8 @@ pub(crate) enum FStringState {
     /// The formatter is inside an f-string expression element i.e., between the
     /// curly brace in `f"foo {x}"`.
     ///
-    /// The containing `StringQuotes` is the surrounding f-string quote information.
-    InsideExpressionElement(StringQuotes),
+    /// The containing `FStringContext` is the surrounding f-string context.
+    InsideExpressionElement(FStringContext),
     /// The formatter is outside an f-string.
     #[default]
     Outside,

crates/ruff_python_formatter/src/other/f_string_element.rs

@@ -1,17 +1,13 @@
 use std::borrow::Cow;
-use std::num::NonZeroU16;
 
 use ruff_formatter::{format_args, write, Buffer, RemoveSoftLinesBuffer};
-use ruff_python_ast::visitor::preorder::{PreorderVisitor, TraversalSignal};
 use ruff_python_ast::{
-    self as ast, AnyNodeRef, ConversionFlag, Expr, FStringElement, FStringExpressionElement,
-    FStringLiteralElement,
+    ConversionFlag, Expr, FStringElement, FStringExpressionElement, FStringLiteralElement,
 };
 use ruff_text_size::Ranged;
 
 use crate::comments::{dangling_open_parenthesis_comments, trailing_comments};
 use crate::context::{FStringState, NodeLevel, WithFStringState, WithNodeLevel};
-use crate::options::MagicTrailingComma;
 use crate::prelude::*;
 use crate::preview::is_hex_codes_in_unicode_sequences_enabled;
 use crate::string::normalize_string;
@@ -173,47 +169,13 @@ impl Format<PyFormatContext<'_>> for FormatFStringExpressionElement<'_> {
                     _ => None,
                 };
 
-                bracket_spacing.fmt(f)?;
-
-                // Update the context to be inside the f-string.
+                // Update the context to be inside the f-string expression element.
                 let f = &mut WithFStringState::new(
-                    FStringState::InsideExpressionElement(self.context.quotes()),
+                    FStringState::InsideExpressionElement(self.context),
                     f,
                 );
 
-                // If we're going to remove the soft line breaks, then there's a chance
-                // that there will be trailing commas in the formatted expression. For
-                // example, if the expression is a collection which exceeds the line length:
-                //
-                // ```python
-                // xxxxxxx = f"aaaaaaaa {['aaaaaaaaaaaaa', 'bbbbbbbbbbbb', 'cccccccccccc', 'dddddddddddd']} aaaaaaaaaa"
-                // ```
-                //
-                // Currently, it's difficult to remove that conditionally, because the
-                // context would need to be passed down to all the expressions and the
-                // magic trailing comma builder would need to be updated to handle this.
-                //
-                // So, we'll manually format the expression with the maximum line width
-                // and disabling the magic trailing comma. This will ensure that even if
-                // a trailing comma was added by the user, they're removed. This is expensive
-                // so we've implemented some heuristics to avoid this in cases where the
-                // expression can't contain a trailing comma.
-                if self.context.layout().is_flat() && {
-                    let visitor = &mut CanContainTrailingCommaVisitor::default();
-                    visitor.visit_expr(expression);
-                    visitor.can_have_trailing_comma
-                } {
-                    let options = f
-                        .options()
-                        .clone()
-                        .with_line_width(NonZeroU16::MAX.into())
-                        .with_magic_trailing_comma(MagicTrailingComma::Ignore);
-                    let context = f.context().clone().with_options(options);
-                    let formatted = crate::format!(context, [expression.format()])?;
-                    text(formatted.print()?.as_code()).fmt(f)?;
-                } else {
-                    expression.format().fmt(f)?;
-                }
+                write!(f, [bracket_spacing, expression.format()])?;
 
                 // Conversion comes first, then the format spec.
                 match conversion {
@@ -280,42 +242,3 @@ impl Format<PyFormatContext<'_>> for FormatFStringExpressionElement<'_> {
         }
     }
 }
-
-/// A visitor to check if an expression can contain a trailing comma.
-#[derive(Default)]
-struct CanContainTrailingCommaVisitor {
-    can_have_trailing_comma: bool,
-}
-
-impl<'a> PreorderVisitor<'a> for CanContainTrailingCommaVisitor {
-    fn enter_node(&mut self, node: AnyNodeRef<'a>) -> TraversalSignal {
-        match node {
-            AnyNodeRef::ExprList(ast::ExprList { elts, .. })
-            | AnyNodeRef::ExprTuple(ast::ExprTuple { elts, .. })
-            | AnyNodeRef::ExprSet(ast::ExprSet { elts, .. }) => {
-                if !elts.is_empty() {
-                    self.can_have_trailing_comma = true;
-                    return TraversalSignal::Skip;
-                }
-            }
-            AnyNodeRef::ExprDict(ast::ExprDict { keys, values, .. }) => {
-                if !keys.is_empty() || !values.is_empty() {
-                    self.can_have_trailing_comma = true;
-                    return TraversalSignal::Skip;
-                }
-            }
-            AnyNodeRef::Arguments(arguments) => {
-                if !arguments.is_empty() {
-                    self.can_have_trailing_comma = true;
-                    return TraversalSignal::Skip;
-                }
-            }
-            _ => (),
-        }
-
-        // Any other expression with a trailing comma, assuming that it's a
-        // valid syntax, is basically a tuple. So, we need to traverse into
-        // it to check the inner expressions.
-        TraversalSignal::Traverse
-    }
-}

crates/ruff_python_formatter/src/string/normalize.rs

@@ -103,7 +103,7 @@ impl StringNormalizer {
             self.preferred_quote_style
         };
 
-        let quoting = if let FStringState::InsideExpressionElement(quotes) = self.f_string_state {
+        let quoting = if let FStringState::InsideExpressionElement(context) = self.f_string_state {
             // If we're inside an f-string, we need to make sure to preserve the
             // existing quotes unless we're inside a triple-quoted f-string and
             // the inner string itself isn't triple-quoted. For example:
@@ -118,7 +118,7 @@ impl StringNormalizer {
             // The reason to preserve the quotes is based on the assumption that
             // the original f-string is valid in terms of quoting, and we don't
             // want to change that to make it invalid.
-            if (quotes.is_triple() && !string.quotes().is_triple())
+            if (context.quotes().is_triple() && !string.quotes().is_triple())
                 || self.target_version.supports_pep_701()
             {
                 self.quoting