diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py index 017d243f1f08e..6675e47563d13 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py +++ b/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}'}"}" diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index e8a3376b18b74..556287b64b736 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/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(_) diff --git a/crates/ruff_python_formatter/src/expression/expr_f_string.rs b/crates/ruff_python_formatter/src/expression/expr_f_string.rs index 088978e3c4f5e..fb2c0e0b0400b 100644 --- a/crates/ruff_python_formatter/src/expression/expr_f_string.rs +++ b/crates/ruff_python_formatter/src/expression/expr_f_string.rs @@ -21,7 +21,7 @@ impl FormatNodeRule 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 } } } diff --git a/crates/ruff_python_formatter/src/other/f_string.rs b/crates/ruff_python_formatter/src/other/f_string.rs index 1685ab2520af9..758002ad6a830 100644 --- a/crates/ruff_python_formatter/src/other/f_string.rs +++ b/crates/ruff_python_formatter/src/other/f_string.rs @@ -51,6 +51,9 @@ impl Format> 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, 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 939402bdf2d6c..66dc81fe8ea88 100644 --- a/crates/ruff_python_formatter/src/other/f_string_element.rs +++ b/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> 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> 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> 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> 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) } } } diff --git a/crates/ruff_python_formatter/src/preview.rs b/crates/ruff_python_formatter/src/preview.rs index ef85c0e1062b1..1c5917008189c 100644 --- a/crates/ruff_python_formatter/src/preview.rs +++ b/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() }