diff --git a/crates/ruff_python_formatter/src/builders.rs b/crates/ruff_python_formatter/src/builders.rs index e4e2909a4a6dd..c7ce681d07b69 100644 --- a/crates/ruff_python_formatter/src/builders.rs +++ b/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( diff --git a/crates/ruff_python_formatter/src/context.rs b/crates/ruff_python_formatter/src/context.rs index 515bebcd96808..4a053821150f6 100644 --- a/crates/ruff_python_formatter/src/context.rs +++ b/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, diff --git a/crates/ruff_python_formatter/src/other/f_string_element.rs b/crates/ruff_python_formatter/src/other/f_string_element.rs index eba5d90ae9443..c581413705f04 100644 --- a/crates/ruff_python_formatter/src/other/f_string_element.rs +++ b/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> 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> 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 - } -} diff --git a/crates/ruff_python_formatter/src/string/normalize.rs b/crates/ruff_python_formatter/src/string/normalize.rs index 9fe57070c4055..0047c58981989 100644 --- a/crates/ruff_python_formatter/src/string/normalize.rs +++ b/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