From 520c7efe26ccdf4f233ea8bba44773ac7888b703 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 20 Nov 2024 16:54:33 +0530 Subject: [PATCH 01/10] Fix f-string formatting in assignment statement --- .../test/fixtures/ruff/expression/fstring.py | 35 ++++ .../src/expression/expr_bin_op.rs | 2 +- .../src/expression/expr_bytes_literal.rs | 2 +- .../src/expression/expr_compare.rs | 2 +- .../src/expression/expr_f_string.rs | 27 +-- .../src/expression/expr_string_literal.rs | 2 +- .../src/other/arguments.rs | 2 +- .../src/statement/stmt_assign.rs | 110 +++++++++- .../src/string/implicit.rs | 2 +- .../ruff_python_formatter/src/string/mod.rs | 80 +++++++- .../format@expression__fstring.py.snap | 188 +++++++++++++++++- 11 files changed, 406 insertions(+), 46 deletions(-) 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 e61467cfea580..a4d8b9b418f99 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 @@ -313,6 +313,41 @@ # comment 28 } woah {x}" +# Assignment statement + +# Even though this f-string has multiline expression, thus allowing us to break it at the +# curly braces, the f-string fits on a single line if it's moved inside the parentheses. +# We should prefer doing that instead. +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeee" + +# Same as above +xxxxxxx = f"{ + {'aaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbb', 'cccccccccccccccccccccccccc'} +}" + +# Similar to the previous example, but the f-string will exceed the line length limit, +# we shouldn't add any parentheses here. +xxxxxxx = f"{ + {'aaaaaaaaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbbbbbbbbbb', 'cccccccccccccccccccccccccc'} +}" + +# This is an implicitly concatenated f-string but it cannot be joined because otherwise +# it'll exceed the line length limit. So, the two f-strings will be inside parentheses +# instead and the inline comment should be outside the parentheses. +a = f"test{ + expression +}flat" f"can be { + joined +} togethereeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" # inline + +# Similar to the above example but this fits within the line length limit. +a = f"test{ + expression +}flat" f"can be { + joined +} togethereeeeeeeeeeeeeeeeeeeeeeeeeee" # inline + # Indentation # What should be the indentation? diff --git a/crates/ruff_python_formatter/src/expression/expr_bin_op.rs b/crates/ruff_python_formatter/src/expression/expr_bin_op.rs index 15b2b328147df..dcd4721860abc 100644 --- a/crates/ruff_python_formatter/src/expression/expr_bin_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_bin_op.rs @@ -28,7 +28,7 @@ impl NeedsParentheses for ExprBinOp { } else if let Ok(string) = StringLike::try_from(&*self.left) { // Multiline strings are guaranteed to never fit, avoid adding unnecessary parentheses if !string.is_implicit_concatenated() - && string.is_multiline(context.source()) + && string.is_multiline(context) && has_parentheses(&self.right, context).is_some() && !context.comments().has_dangling(self) && !context.comments().has(string) diff --git a/crates/ruff_python_formatter/src/expression/expr_bytes_literal.rs b/crates/ruff_python_formatter/src/expression/expr_bytes_literal.rs index 92d2af2d1f952..15c0a6e4bfdb9 100644 --- a/crates/ruff_python_formatter/src/expression/expr_bytes_literal.rs +++ b/crates/ruff_python_formatter/src/expression/expr_bytes_literal.rs @@ -40,7 +40,7 @@ impl NeedsParentheses for ExprBytesLiteral { ) -> OptionalParentheses { if self.value.is_implicit_concatenated() { OptionalParentheses::Multiline - } else if StringLike::Bytes(self).is_multiline(context.source()) { + } else if StringLike::Bytes(self).is_multiline(context) { OptionalParentheses::Never } else { OptionalParentheses::BestFit diff --git a/crates/ruff_python_formatter/src/expression/expr_compare.rs b/crates/ruff_python_formatter/src/expression/expr_compare.rs index 13f072cde51ae..0b10ccaa31534 100644 --- a/crates/ruff_python_formatter/src/expression/expr_compare.rs +++ b/crates/ruff_python_formatter/src/expression/expr_compare.rs @@ -29,7 +29,7 @@ impl NeedsParentheses for ExprCompare { } else if let Ok(string) = StringLike::try_from(&*self.left) { // Multiline strings are guaranteed to never fit, avoid adding unnecessary parentheses if !string.is_implicit_concatenated() - && string.is_multiline(context.source()) + && string.is_multiline(context) && !context.comments().has(string) && self.comparators.first().is_some_and(|right| { has_parentheses(right, context).is_some() && !context.comments().has(right) 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 dd67fd1bea0af..9994a360d4dfe 100644 --- a/crates/ruff_python_formatter/src/expression/expr_f_string.rs +++ b/crates/ruff_python_formatter/src/expression/expr_f_string.rs @@ -6,8 +6,10 @@ use crate::expression::parentheses::{ }; use crate::other::f_string::FormatFString; use crate::prelude::*; -use crate::string::implicit::FormatImplicitConcatenatedStringFlat; -use crate::string::{implicit::FormatImplicitConcatenatedString, Quoting, StringLikeExtensions}; +use crate::string::implicit::{ + FormatImplicitConcatenatedString, FormatImplicitConcatenatedStringFlat, +}; +use crate::string::{Quoting, StringLikeExtensions}; #[derive(Default)] pub struct FormatExprFString; @@ -45,26 +47,7 @@ impl NeedsParentheses for ExprFString { ) -> OptionalParentheses { if self.value.is_implicit_concatenated() { OptionalParentheses::Multiline - } - // TODO(dhruvmanila): Ideally what we want here is a new variant which - // is something like: - // - If the expression fits by just adding the parentheses, then add them and - // avoid breaking the f-string expression. So, - // ``` - // xxxxxxxxx = ( - // f"aaaaaaaaaaaa { xxxxxxx + yyyyyyyy } bbbbbbbbbbbbb" - // ) - // ``` - // - But, if the expression is too long to fit even with parentheses, then - // don't add the parentheses and instead break the expression at `soft_line_break`. - // ``` - // xxxxxxxxx = f"aaaaaaaaaaaa { - // xxxxxxxxx + yyyyyyyyyy - // } bbbbbbbbbbbbb" - // ``` - // This isn't decided yet, refer to the relevant discussion: - // https://github.com/astral-sh/ruff/discussions/9785 - else if StringLike::FString(self).is_multiline(context.source()) { + } else if StringLike::FString(self).is_multiline(context) { OptionalParentheses::Never } else { OptionalParentheses::BestFit diff --git a/crates/ruff_python_formatter/src/expression/expr_string_literal.rs b/crates/ruff_python_formatter/src/expression/expr_string_literal.rs index 9813958de52cf..429e55bad053c 100644 --- a/crates/ruff_python_formatter/src/expression/expr_string_literal.rs +++ b/crates/ruff_python_formatter/src/expression/expr_string_literal.rs @@ -53,7 +53,7 @@ impl NeedsParentheses for ExprStringLiteral { ) -> OptionalParentheses { if self.value.is_implicit_concatenated() { OptionalParentheses::Multiline - } else if StringLike::String(self).is_multiline(context.source()) { + } else if StringLike::String(self).is_multiline(context) { OptionalParentheses::Never } else { OptionalParentheses::BestFit diff --git a/crates/ruff_python_formatter/src/other/arguments.rs b/crates/ruff_python_formatter/src/other/arguments.rs index 6e50f1cf01eb1..f7eadfdb70cae 100644 --- a/crates/ruff_python_formatter/src/other/arguments.rs +++ b/crates/ruff_python_formatter/src/other/arguments.rs @@ -223,7 +223,7 @@ fn is_huggable_string_argument( arguments: &Arguments, context: &PyFormatContext, ) -> bool { - if string.is_implicit_concatenated() || !string.is_multiline(context.source()) { + if string.is_implicit_concatenated() || !string.is_multiline(context) { return false; } diff --git a/crates/ruff_python_formatter/src/statement/stmt_assign.rs b/crates/ruff_python_formatter/src/statement/stmt_assign.rs index 7ad90cd025db7..e5a0e8e9628cf 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_assign.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_assign.rs @@ -1,6 +1,7 @@ use ruff_formatter::{format_args, write, FormatError, RemoveSoftLinesBuffer}; use ruff_python_ast::{ - AnyNodeRef, Expr, ExprAttribute, ExprCall, Operator, StmtAssign, StringLike, TypeParams, + AnyNodeRef, Expr, ExprAttribute, ExprCall, ExprFString, FString, FStringPart, Operator, + StmtAssign, StringLike, TypeParams, }; use crate::builders::parenthesize_if_expands; @@ -8,6 +9,7 @@ use crate::comments::{ trailing_comments, Comments, LeadingDanglingTrailingComments, SourceComment, }; use crate::context::{NodeLevel, WithNodeLevel}; +use crate::expression::expr_f_string::f_string_quoting; use crate::expression::parentheses::{ is_expression_parenthesized, optional_parentheses, NeedsParentheses, OptionalParentheses, Parentheses, Parenthesize, @@ -16,12 +18,16 @@ use crate::expression::{ can_omit_optional_parentheses, has_own_parentheses, has_parentheses, maybe_parenthesize_expression, }; -use crate::preview::is_join_implicit_concatenated_string_enabled; +use crate::other::f_string::FormatFString; +use crate::preview::{ + is_f_string_formatting_enabled, is_join_implicit_concatenated_string_enabled, +}; use crate::statement::trailing_semicolon; use crate::string::implicit::{ FormatImplicitConcatenatedStringExpanded, FormatImplicitConcatenatedStringFlat, ImplicitConcatenatedLayout, }; +use crate::string::StringLikeExtensions; use crate::{has_skip_comment, prelude::*}; #[derive(Default)] @@ -183,6 +189,7 @@ impl Format> for FormatTargetWithEqualOperator<'_> { /// /// This logic isn't implemented in [`place_comment`] by associating trailing statement comments to the expression because /// doing so breaks the suite empty lines formatting that relies on trailing comments to be stored on the statement. +#[derive(Debug)] pub(super) enum FormatStatementsLastExpression<'a> { /// Prefers to split what's left of `value` before splitting the value. /// @@ -286,11 +293,18 @@ impl Format> for FormatStatementsLastExpression<'_> { match self { FormatStatementsLastExpression::LeftToRight { value, statement } => { let can_inline_comment = should_inline_comments(value, *statement, f.context()); - let format_implicit_flat = StringLike::try_from(*value).ok().and_then(|string| { + + let string_like = StringLike::try_from(*value).ok(); + let format_f_string_flat = + string_like.and_then(|string| FormatFStringFlat::new(string, f.context())); + let format_implicit_flat = string_like.and_then(|string| { FormatImplicitConcatenatedStringFlat::new(string, f.context()) }); - if !can_inline_comment && format_implicit_flat.is_none() { + if !can_inline_comment + && format_implicit_flat.is_none() + && format_f_string_flat.is_none() + { return maybe_parenthesize_expression( value, *statement, @@ -436,6 +450,57 @@ impl Format> for FormatStatementsLastExpression<'_> { best_fitting![single_line, joined_parenthesized, implicit_expanded] .with_mode(BestFittingMode::AllLines) .fmt(f)?; + } else if let Some(format_f_string) = format_f_string_flat { + inline_comments.mark_formatted(); + + let f_string_flat = format_with(|f| { + let mut buffer = RemoveSoftLinesBuffer::new(&mut *f); + + write!(buffer, [format_f_string]) + }) + .memoized(); + + // F-String containing an expression with a magic trailing comma, a comment, or a + // multiline debug expression should never be joined. Use the default layout. + // ```python + // aaaa = f"abcd{[ + // 1, + // 2, + // ]}" "more" + // ``` + if f_string_flat.inspect(f)?.will_break() { + inline_comments.mark_unformatted(); + + return write!( + f, + [maybe_parenthesize_expression( + value, + *statement, + Parenthesize::IfBreaks, + )] + ); + } + + let single_line = + format_with(|f| write!(f, [f_string_flat, inline_comments])); + + let joined_parenthesized = format_with(|f| { + group(&format_args![ + token("("), + soft_block_indent(&format_args![f_string_flat, inline_comments]), + token(")"), + ]) + .with_group_id(Some(group_id)) + .should_expand(true) + .fmt(f) + }); + + let format_f_string = + format_with(|f| write!(f, [format_f_string, inline_comments])); + + best_fitting![single_line, joined_parenthesized, format_f_string] + .with_mode(BestFittingMode::AllLines) + .fmt(f)?; } else { best_fit_parenthesize(&format_once(|f| { inline_comments.mark_formatted(); @@ -818,6 +883,43 @@ impl Format> for FormatStatementsLastExpression<'_> { } } +pub(crate) struct FormatFStringFlat<'a> { + expr: &'a ExprFString, + f_string: &'a FString, +} + +impl<'a> FormatFStringFlat<'a> { + pub(crate) fn new(string: StringLike<'a>, context: &PyFormatContext) -> Option { + if !is_f_string_formatting_enabled(context) { + return None; + } + + let StringLike::FString(expr) = string else { + return None; + }; + + let [FStringPart::FString(f_string)] = expr.value.as_slice() else { + return None; + }; + + if string.is_multiline(context) { + return None; + } + + Some(Self { expr, f_string }) + } +} + +impl Format> for FormatFStringFlat<'_> { + fn fmt(&self, f: &mut Formatter>) -> FormatResult<()> { + FormatFString::new( + self.f_string, + f_string_quoting(self.expr, f.context().source()), + ) + .fmt(f) + } +} + #[derive(Debug, Default)] struct OptionalParenthesesInlinedComments<'a> { expression: &'a [SourceComment], diff --git a/crates/ruff_python_formatter/src/string/implicit.rs b/crates/ruff_python_formatter/src/string/implicit.rs index 10489e1e0a5a3..0da98d9e4364d 100644 --- a/crates/ruff_python_formatter/src/string/implicit.rs +++ b/crates/ruff_python_formatter/src/string/implicit.rs @@ -154,7 +154,7 @@ impl<'a> FormatImplicitConcatenatedStringFlat<'a> { } // Multiline strings can never fit on a single line. - if !string.is_fstring() && string.is_multiline(context.source()) { + if !string.is_fstring() && string.is_multiline(context) { return None; } diff --git a/crates/ruff_python_formatter/src/string/mod.rs b/crates/ruff_python_formatter/src/string/mod.rs index eaff5efac2cf8..01eb9351ed2cd 100644 --- a/crates/ruff_python_formatter/src/string/mod.rs +++ b/crates/ruff_python_formatter/src/string/mod.rs @@ -1,16 +1,20 @@ -use memchr::memchr2; - pub(crate) use normalize::{normalize_string, NormalizedString, StringNormalizer}; use ruff_python_ast::str::Quote; +use ruff_python_ast::visitor::source_order::{ + walk_f_string_element, SourceOrderVisitor, TraversalSignal, +}; +use ruff_python_ast::AstNode; use ruff_python_ast::{ self as ast, str_prefix::{AnyStringPrefix, StringLiteralPrefix}, AnyStringFlags, StringFlags, }; +use ruff_source_file::LineRanges; use ruff_text_size::Ranged; use crate::expression::expr_f_string::f_string_quoting; use crate::prelude::*; +use crate::preview::is_f_string_formatting_enabled; use crate::QuoteStyle; pub(crate) mod docstring; @@ -90,7 +94,7 @@ impl From for QuoteStyle { pub(crate) trait StringLikeExtensions { fn quoting(&self, source: &str) -> Quoting; - fn is_multiline(&self, source: &str) -> bool; + fn is_multiline(&self, context: &PyFormatContext) -> bool; } impl StringLikeExtensions for ast::StringLike<'_> { @@ -101,15 +105,77 @@ impl StringLikeExtensions for ast::StringLike<'_> { } } - fn is_multiline(&self, source: &str) -> bool { + fn is_multiline(&self, context: &PyFormatContext) -> bool { match self { Self::String(_) | Self::Bytes(_) => self.parts().any(|part| { part.flags().is_triple_quoted() - && memchr2(b'\n', b'\r', source[self.range()].as_bytes()).is_some() + && context.source().contains_line_break(self.range()) }), - Self::FString(fstring) => { - memchr2(b'\n', b'\r', source[fstring.range].as_bytes()).is_some() + Self::FString(expr) => { + let mut visitor = FStringMultilineVisitor::new(context); + expr.visit_source_order(&mut visitor); + visitor.is_multiline + } + } + } +} + +struct FStringMultilineVisitor<'a> { + context: &'a PyFormatContext<'a>, + is_multiline: bool, +} + +impl<'a> FStringMultilineVisitor<'a> { + fn new(context: &'a PyFormatContext<'a>) -> Self { + Self { + context, + is_multiline: false, + } + } +} + +impl<'a> SourceOrderVisitor<'a> for FStringMultilineVisitor<'a> { + fn enter_node(&mut self, _node: ruff_python_ast::AnyNodeRef<'a>) -> TraversalSignal { + if self.is_multiline { + TraversalSignal::Skip + } else { + TraversalSignal::Traverse + } + } + + fn visit_string_literal(&mut self, string_literal: &'a ast::StringLiteral) { + if string_literal.flags.is_triple_quoted() + && self + .context + .source() + .contains_line_break(string_literal.range()) + { + self.is_multiline = true; + } + } + + fn visit_f_string_element(&mut self, f_string_element: &'a ast::FStringElement) { + let is_multiline = match f_string_element { + ast::FStringElement::Literal(literal) => { + self.context.source().contains_line_break(literal.range()) } + ast::FStringElement::Expression(expression) => { + if is_f_string_formatting_enabled(self.context) { + // Expressions containing comments can't be joined. + self.context.comments().contains_comments(expression.into()) + } else { + // Multiline f-string expressions can't be joined if the f-string formatting is disabled because + // the string gets inserted in verbatim preserving the newlines. + self.context + .source() + .contains_line_break(expression.range()) + } + } + }; + if is_multiline { + self.is_multiline = true; + } else { + walk_f_string_element(self, f_string_element); } } } diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap index e00cd01460af0..1f3299b3b07b1 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py -snapshot_kind: text --- ## Input ```python @@ -320,6 +319,41 @@ f"{ # comment 26 # comment 28 } woah {x}" +# Assignment statement + +# Even though this f-string has multiline expression, thus allowing us to break it at the +# curly braces, the f-string fits on a single line if it's moved inside the parentheses. +# We should prefer doing that instead. +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeee" + +# Same as above +xxxxxxx = f"{ + {'aaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbb', 'cccccccccccccccccccccccccc'} +}" + +# Similar to the previous example, but the f-string will exceed the line length limit, +# we shouldn't add any parentheses here. +xxxxxxx = f"{ + {'aaaaaaaaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbbbbbbbbbb', 'cccccccccccccccccccccccccc'} +}" + +# This is an implicitly concatenated f-string but it cannot be joined because otherwise +# it'll exceed the line length limit. So, the two f-strings will be inside parentheses +# instead and the inline comment should be outside the parentheses. +a = f"test{ + expression +}flat" f"can be { + joined +} togethereeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" # inline + +# Similar to the above example but this fits within the line length limit. +a = f"test{ + expression +}flat" f"can be { + joined +} togethereeeeeeeeeeeeeeeeeeeeeeeeeee" # inline + # Indentation # What should be the indentation? @@ -521,9 +555,9 @@ x = f"aaaaa { {'aaaaaa', 'bbbbbbb', 'ccccccccc'}:.3f} bbbbbb" # 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. -xxxxxxx = f"{ - {'aaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbbbbb', 'ccccccccccccccccccccc'} -}" +xxxxxxx = ( + f"{ {'aaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbbbbb', 'ccccccccccccccccccccc'} }" +) # And, split the expression itself because it exceeds the line length. xxxxxxx = f"{ { @@ -698,6 +732,41 @@ f"{ # comment 26 # comment 28 } woah {x}" +# Assignment statement + +# Even though this f-string has multiline expression, thus allowing us to break it at the +# curly braces, the f-string fits on a single line if it's moved inside the parentheses. +# We should prefer doing that instead. +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee" +) + +# Same as above +xxxxxxx = ( + f"{ {'aaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbb', 'cccccccccccccccccccccccccc'} }" +) + +# Similar to the previous example, but the f-string will exceed the line length limit, +# we shouldn't add any parentheses here. +xxxxxxx = f"{ + { + 'aaaaaaaaaaaaaaaaaaaaaaaaa', + 'bbbbbbbbbbbbbbbbbbbbbbbbbbb', + 'cccccccccccccccccccccccccc', + } +}" + +# This is an implicitly concatenated f-string but it cannot be joined because otherwise +# it'll exceed the line length limit. So, the two f-strings will be inside parentheses +# instead and the inline comment should be outside the parentheses. +a = ( + f"test{expression}flat" + f"can be {joined} togethereeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" +) # inline + +# Similar to the above example but this fits within the line length limit. +a = f"test{expression}flatcan be {joined} togethereeeeeeeeeeeeeeeeeeeeeeeeeee" # inline + # Indentation # What should be the indentation? @@ -1064,6 +1133,47 @@ f"{ # comment 26 # comment 28 } woah {x}" +# Assignment statement + +# Even though this f-string has multiline expression, thus allowing us to break it at the +# curly braces, the f-string fits on a single line if it's moved inside the parentheses. +# We should prefer doing that instead. +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeee" + +# Same as above +xxxxxxx = f"{ + {'aaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbb', 'cccccccccccccccccccccccccc'} +}" + +# Similar to the previous example, but the f-string will exceed the line length limit, +# we shouldn't add any parentheses here. +xxxxxxx = f"{ + {'aaaaaaaaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbbbbbbbbbb', 'cccccccccccccccccccccccccc'} +}" + +# This is an implicitly concatenated f-string but it cannot be joined because otherwise +# it'll exceed the line length limit. So, the two f-strings will be inside parentheses +# instead and the inline comment should be outside the parentheses. +a = ( + f"test{ + expression +}flat" + f"can be { + joined +} togethereeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" +) # inline + +# Similar to the above example but this fits within the line length limit. +a = ( + f"test{ + expression +}flat" + f"can be { + joined +} togethereeeeeeeeeeeeeeeeeeeeeeeeeee" +) # inline + # Indentation # What should be the indentation? @@ -1238,8 +1348,16 @@ f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccc x = f"{ # comment 13 {'x': 1, 'y': 2} = }" # But, if there's a format specifier or a conversion flag then we don't need to add -@@ -141,7 +149,11 @@ - }" +@@ -136,12 +144,16 @@ + + # 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. +-xxxxxxx = f"{ +- {'aaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbbbbb', 'ccccccccccccccccccccc'} +-}" ++xxxxxxx = ( ++ f"{ {'aaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbbbbb', 'ccccccccccccccccccccc'} }" ++) # And, split the expression itself because it exceeds the line length. xxxxxxx = f"{ - {'aaaaaaaaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbbbbbbbbbb', 'cccccccccccccccccccccccccc'} @@ -1453,7 +1571,63 @@ f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccc # comment 27 # comment 28 } woah {x}" -@@ -318,29 +330,29 @@ +@@ -314,41 +326,35 @@ + # Even though this f-string has multiline expression, thus allowing us to break it at the + # curly braces, the f-string fits on a single line if it's moved inside the parentheses. + # We should prefer doing that instead. +-aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +- expression}moreeeeeeeeeeeeeeeee" ++aaaaaaaaaaaaaaaaaa = ( ++ f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee" ++) + + # Same as above +-xxxxxxx = f"{ +- {'aaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbb', 'cccccccccccccccccccccccccc'} +-}" ++xxxxxxx = ( ++ f"{ {'aaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbb', 'cccccccccccccccccccccccccc'} }" ++) + + # Similar to the previous example, but the f-string will exceed the line length limit, + # we shouldn't add any parentheses here. + xxxxxxx = f"{ +- {'aaaaaaaaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbbbbbbbbbb', 'cccccccccccccccccccccccccc'} ++ { ++ 'aaaaaaaaaaaaaaaaaaaaaaaaa', ++ 'bbbbbbbbbbbbbbbbbbbbbbbbbbb', ++ 'cccccccccccccccccccccccccc', ++ } + }" + + # This is an implicitly concatenated f-string but it cannot be joined because otherwise + # it'll exceed the line length limit. So, the two f-strings will be inside parentheses + # instead and the inline comment should be outside the parentheses. + a = ( +- f"test{ +- expression +-}flat" +- f"can be { +- joined +-} togethereeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" ++ f"test{expression}flat" ++ f"can be {joined} togethereeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" + ) # inline + + # Similar to the above example but this fits within the line length limit. +-a = ( +- f"test{ +- expression +-}flat" +- f"can be { +- joined +-} togethereeeeeeeeeeeeeeeeeeeeeeeeeee" +-) # inline ++a = f"test{expression}flatcan be {joined} togethereeeeeeeeeeeeeeeeeeeeeeeeeee" # inline + + # Indentation + +@@ -359,29 +365,29 @@ if indent2: foo = f"""hello world hello { From a233e176eb03de395eda57b86ea5bcfc30ebeed2 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 20 Nov 2024 18:54:17 +0530 Subject: [PATCH 02/10] Handle right to left variant --- .../src/statement/stmt_assign.rs | 121 +++++++++++++++++- 1 file changed, 118 insertions(+), 3 deletions(-) diff --git a/crates/ruff_python_formatter/src/statement/stmt_assign.rs b/crates/ruff_python_formatter/src/statement/stmt_assign.rs index e5a0e8e9628cf..6d405899d446d 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_assign.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_assign.rs @@ -539,7 +539,11 @@ impl Format> for FormatStatementsLastExpression<'_> { statement, } => { let should_inline_comments = should_inline_comments(value, *statement, f.context()); - let format_implicit_flat = StringLike::try_from(*value).ok().and_then(|string| { + + let string_like = StringLike::try_from(*value).ok(); + let format_f_string_flat = + string_like.and_then(|string| FormatFStringFlat::new(string, f.context())); + let format_implicit_flat = string_like.and_then(|string| { FormatImplicitConcatenatedStringFlat::new(string, f.context()) }); @@ -547,6 +551,7 @@ impl Format> for FormatStatementsLastExpression<'_> { if !should_inline_comments && !should_non_inlineable_use_best_fit(value, *statement, f.context()) && format_implicit_flat.is_none() + && format_f_string_flat.is_none() { return write!( f, @@ -568,7 +573,10 @@ impl Format> for FormatStatementsLastExpression<'_> { let expression_comments = comments.leading_dangling_trailing(*value); // Don't inline comments for attribute and call expressions for black compatibility - let inline_comments = if should_inline_comments || format_implicit_flat.is_some() { + let inline_comments = if should_inline_comments + || format_implicit_flat.is_some() + || format_f_string_flat.is_some() + { OptionalParenthesesInlinedComments::new( &expression_comments, *statement, @@ -607,7 +615,10 @@ impl Format> for FormatStatementsLastExpression<'_> { // This is mainly a performance optimisation that avoids unnecessary memoization // and using the costly `BestFitting` layout if it is already known that only the last variant // can ever fit because the left breaks. - if format_implicit_flat.is_none() && last_target_breaks { + if format_implicit_flat.is_none() + && format_f_string_flat.is_none() + && last_target_breaks + { return write!( f, [ @@ -633,6 +644,9 @@ impl Format> for FormatStatementsLastExpression<'_> { } else { format_implicit_flat.fmt(f) } + } else if let Some(format_f_string_flat) = format_f_string_flat.as_ref() { + let mut buffer = RemoveSoftLinesBuffer::new(&mut *f); + write!(buffer, [format_f_string_flat]) } else { value.format().with_options(Parentheses::Never).fmt(f) } @@ -848,6 +862,107 @@ impl Format> for FormatStatementsLastExpression<'_> { ) }); + // This is only a perf optimisation. No point in trying all the "flat-target" + // variants if we know that the last target must break. + if last_target_breaks { + best_fitting![ + split_target_flat_value, + split_target_value_parenthesized_flat, + split_target_value_parenthesized_multiline, + ] + .with_mode(BestFittingMode::AllLines) + .fmt(f) + } else { + best_fitting![ + single_line, + flat_target_parenthesize_value, + flat_target_value_parenthesized_multiline, + split_target_flat_value, + split_target_value_parenthesized_flat, + split_target_value_parenthesized_multiline, + ] + .with_mode(BestFittingMode::AllLines) + .fmt(f) + } + } else if let Some(format_f_string) = &format_f_string_flat { + // F-String containing an expression with a magic trailing comma, a comment, or a + // multiline debug expression should never be joined. Use the default layout. + // + // ```python + // aaaa = f"abcd{[ + // 1, + // 2, + // ]}" "more" + // ``` + if format_value.inspect(f)?.will_break() { + inline_comments.mark_unformatted(); + + return write!( + f, + [ + before_operator, + space(), + operator, + space(), + maybe_parenthesize_expression( + value, + *statement, + Parenthesize::IfBreaks + ) + ] + ); + } + + let format_f_string = + format_with(|f| write!(f, [format_f_string, inline_comments])).memoized(); + + let flat_target_value_parenthesized_multiline = format_with(|f| { + write!( + f, + [ + last_target, + space(), + operator, + space(), + format_f_string, + inline_comments + ] + ) + }); + + let split_target_value_parenthesized_flat = format_with(|f| { + write!( + f, + [ + group(&last_target).should_expand(true), + space(), + operator, + space(), + token("("), + group(&soft_block_indent(&format_args![ + format_value, + inline_comments + ])) + .should_expand(true), + token(")") + ] + ) + }); + + let split_target_value_parenthesized_multiline = format_with(|f| { + write!( + f, + [ + group(&last_target).should_expand(true), + space(), + operator, + space(), + format_f_string, + inline_comments + ] + ) + }); + // This is only a perf optimisation. No point in trying all the "flat-target" // variants if we know that the last target must break. if last_target_breaks { From 9f1c77817e3854fce4b38bead1cd7b51809f6d11 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 21 Nov 2024 10:20:45 +0530 Subject: [PATCH 03/10] Documentation, rename, test cases --- crates/ruff_python_ast/src/expression.rs | 2 +- .../test/fixtures/ruff/expression/fstring.py | 34 ++++ .../src/statement/stmt_assign.rs | 159 +++++++++++++----- .../src/string/implicit.rs | 4 +- .../ruff_python_formatter/src/string/mod.rs | 15 +- .../format@expression__fstring.py.snap | 159 +++++++++++++++++- 6 files changed, 323 insertions(+), 50 deletions(-) diff --git a/crates/ruff_python_ast/src/expression.rs b/crates/ruff_python_ast/src/expression.rs index 469f12b003af4..08b4da80fa913 100644 --- a/crates/ruff_python_ast/src/expression.rs +++ b/crates/ruff_python_ast/src/expression.rs @@ -408,7 +408,7 @@ pub enum StringLike<'a> { } impl<'a> StringLike<'a> { - pub const fn is_fstring(self) -> bool { + pub const fn is_f_string(self) -> bool { matches!(self, Self::FString(_)) } 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 a4d8b9b418f99..c1d8dd9d87e60 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 @@ -332,6 +332,40 @@ {'aaaaaaaaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbbbbbbbbbb', 'cccccccccccccccccccccccccc'} }" +# Same as above but with an inline comment. The f-string should be formatted inside the +# parentheses and the comment should be part of the line inside the parentheses. +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeee" # comment + +# Similar to the previous example but this time parenthesizing doesn't work because it +# exceeds the line length. So, avoid parenthesizing this f-string. +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeee" # comment loooooooong + +# Similar to the previous example but we start with the parenthesized layout. This should +# remove the parentheses and format the f-string on a single line. This shows that the +# final layout for the formatter is same for this and the previous case. The only +# difference is that in the previous case the expression is already mulitline which means +# the formatter can break it further at the curly braces. +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee" # comment loooooooong +) + +# The following f-strings are going to break because of the trailing comma so we should +# avoid using the best fit layout and instead use the default layout. +# left-to-right +aaaa = f"aaaa {[ + 1, 2, +]} bbbb" +# right-to-left +aaaa, bbbb = f"aaaa {[ + 1, 2, +]} bbbb" + +# Using the right-to-left assignment statement variant. +aaaaaaaaaaaaaaaaaa, bbbbbbbbbbb = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeee" # comment + # This is an implicitly concatenated f-string but it cannot be joined because otherwise # it'll exceed the line length limit. So, the two f-strings will be inside parentheses # instead and the inline comment should be outside the parentheses. diff --git a/crates/ruff_python_formatter/src/statement/stmt_assign.rs b/crates/ruff_python_formatter/src/statement/stmt_assign.rs index 6d405899d446d..45ce6e31dfada 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_assign.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_assign.rs @@ -295,15 +295,15 @@ impl Format> for FormatStatementsLastExpression<'_> { let can_inline_comment = should_inline_comments(value, *statement, f.context()); let string_like = StringLike::try_from(*value).ok(); - let format_f_string_flat = - string_like.and_then(|string| FormatFStringFlat::new(string, f.context())); + let format_f_string_assignment = string_like + .and_then(|string| FormatFStringAssignment::new(string, f.context())); let format_implicit_flat = string_like.and_then(|string| { FormatImplicitConcatenatedStringFlat::new(string, f.context()) }); if !can_inline_comment && format_implicit_flat.is_none() - && format_f_string_flat.is_none() + && format_f_string_assignment.is_none() { return maybe_parenthesize_expression( value, @@ -355,7 +355,7 @@ impl Format> for FormatStatementsLastExpression<'_> { let string = flat.string(); let flat = format_with(|f| { - if string.is_fstring() { + if string.is_f_string() { let mut buffer = RemoveSoftLinesBuffer::new(&mut *f); write!(buffer, [flat]) @@ -373,7 +373,7 @@ impl Format> for FormatStatementsLastExpression<'_> { // 2, // ]}" "more" // ``` - if string.is_fstring() && flat.inspect(f)?.will_break() { + if string.is_f_string() && flat.inspect(f)?.will_break() { inline_comments.mark_unformatted(); return write!( @@ -450,7 +450,7 @@ impl Format> for FormatStatementsLastExpression<'_> { best_fitting![single_line, joined_parenthesized, implicit_expanded] .with_mode(BestFittingMode::AllLines) .fmt(f)?; - } else if let Some(format_f_string) = format_f_string_flat { + } else if let Some(format_f_string) = format_f_string_assignment { inline_comments.mark_formatted(); let f_string_flat = format_with(|f| { @@ -463,10 +463,9 @@ impl Format> for FormatStatementsLastExpression<'_> { // F-String containing an expression with a magic trailing comma, a comment, or a // multiline debug expression should never be joined. Use the default layout. // ```python - // aaaa = f"abcd{[ - // 1, - // 2, - // ]}" "more" + // aaaa = f"aaaa {[ + // 1, 2, + // ]} bbbb" // ``` if f_string_flat.inspect(f)?.will_break() { inline_comments.mark_unformatted(); @@ -481,9 +480,25 @@ impl Format> for FormatStatementsLastExpression<'_> { ); } + // Considering the following example: + // ```python + // aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + // expression}moreeeeeeeeeeeeeeeee" + // ``` + + // Flatten the f-string. + // ```python + // aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee" + // ``` let single_line = format_with(|f| write!(f, [f_string_flat, inline_comments])); + // Parenthesize the f-sring and flatten the f-string. + // ```python + // aaaaaaaaaaaaaaaaaa = ( + // f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee" + // ) + // ``` let joined_parenthesized = format_with(|f| { group(&format_args![ token("("), @@ -495,6 +510,13 @@ impl Format> for FormatStatementsLastExpression<'_> { .fmt(f) }); + // Avoid flattening or parenthesizing the f-string, keep the original + // f-string formatting. + // ```python + // aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + // expression + // }moreeeeeeeeeeeeeeeee" + // ``` let format_f_string = format_with(|f| write!(f, [format_f_string, inline_comments])); @@ -541,8 +563,8 @@ impl Format> for FormatStatementsLastExpression<'_> { let should_inline_comments = should_inline_comments(value, *statement, f.context()); let string_like = StringLike::try_from(*value).ok(); - let format_f_string_flat = - string_like.and_then(|string| FormatFStringFlat::new(string, f.context())); + let format_f_string_assignment = string_like + .and_then(|string| FormatFStringAssignment::new(string, f.context())); let format_implicit_flat = string_like.and_then(|string| { FormatImplicitConcatenatedStringFlat::new(string, f.context()) }); @@ -551,7 +573,7 @@ impl Format> for FormatStatementsLastExpression<'_> { if !should_inline_comments && !should_non_inlineable_use_best_fit(value, *statement, f.context()) && format_implicit_flat.is_none() - && format_f_string_flat.is_none() + && format_f_string_assignment.is_none() { return write!( f, @@ -575,7 +597,7 @@ impl Format> for FormatStatementsLastExpression<'_> { // Don't inline comments for attribute and call expressions for black compatibility let inline_comments = if should_inline_comments || format_implicit_flat.is_some() - || format_f_string_flat.is_some() + || format_f_string_assignment.is_some() { OptionalParenthesesInlinedComments::new( &expression_comments, @@ -616,7 +638,7 @@ impl Format> for FormatStatementsLastExpression<'_> { // and using the costly `BestFitting` layout if it is already known that only the last variant // can ever fit because the left breaks. if format_implicit_flat.is_none() - && format_f_string_flat.is_none() + && format_f_string_assignment.is_none() && last_target_breaks { return write!( @@ -634,7 +656,7 @@ impl Format> for FormatStatementsLastExpression<'_> { let format_value = format_with(|f| { if let Some(format_implicit_flat) = format_implicit_flat.as_ref() { - if format_implicit_flat.string().is_fstring() { + if format_implicit_flat.string().is_f_string() { // Remove any soft line breaks emitted by the f-string formatting. // This is important when formatting f-strings as part of an assignment right side // because `best_fit_parenthesize` will otherwise still try to break inner @@ -644,9 +666,11 @@ impl Format> for FormatStatementsLastExpression<'_> { } else { format_implicit_flat.fmt(f) } - } else if let Some(format_f_string_flat) = format_f_string_flat.as_ref() { + } else if let Some(format_f_string) = format_f_string_assignment.as_ref() { + // Similar to above, remove any soft line breaks emitted by the f-string + // formatting. let mut buffer = RemoveSoftLinesBuffer::new(&mut *f); - write!(buffer, [format_f_string_flat]) + write!(buffer, [format_f_string]) } else { value.format().with_options(Parentheses::Never).fmt(f) } @@ -745,7 +769,7 @@ impl Format> for FormatStatementsLastExpression<'_> { // 2, // ]}" "more" // ``` - if format_implicit_flat.string().is_fstring() + if format_implicit_flat.string().is_f_string() && format_value.inspect(f)?.will_break() { inline_comments.mark_unformatted(); @@ -884,17 +908,17 @@ impl Format> for FormatStatementsLastExpression<'_> { .with_mode(BestFittingMode::AllLines) .fmt(f) } - } else if let Some(format_f_string) = &format_f_string_flat { + } else if let Some(format_f_string) = &format_f_string_assignment { // F-String containing an expression with a magic trailing comma, a comment, or a // multiline debug expression should never be joined. Use the default layout. // // ```python - // aaaa = f"abcd{[ - // 1, - // 2, - // ]}" "more" + // aaaa, bbbb = f"aaaa {[ + // 1, 2, + // ]} bbbb" // ``` if format_value.inspect(f)?.will_break() { + println!("Flattened f-string will break, using the default layout"); inline_comments.mark_unformatted(); return write!( @@ -916,20 +940,15 @@ impl Format> for FormatStatementsLastExpression<'_> { let format_f_string = format_with(|f| write!(f, [format_f_string, inline_comments])).memoized(); - let flat_target_value_parenthesized_multiline = format_with(|f| { + // Keep the target flat, and use the regular f-string formatting. + let flat_target_regular_f_string = format_with(|f| { write!( f, - [ - last_target, - space(), - operator, - space(), - format_f_string, - inline_comments - ] + [last_target, space(), operator, space(), format_f_string] ) }); + // Expand the parent and parenthesize the flattened f-string. let split_target_value_parenthesized_flat = format_with(|f| { write!( f, @@ -949,7 +968,8 @@ impl Format> for FormatStatementsLastExpression<'_> { ) }); - let split_target_value_parenthesized_multiline = format_with(|f| { + // Expand the parent, and use the regular f-string formatting. + let split_target_regular_f_string = format_with(|f| { write!( f, [ @@ -958,7 +978,6 @@ impl Format> for FormatStatementsLastExpression<'_> { operator, space(), format_f_string, - inline_comments ] ) }); @@ -969,7 +988,7 @@ impl Format> for FormatStatementsLastExpression<'_> { best_fitting![ split_target_flat_value, split_target_value_parenthesized_flat, - split_target_value_parenthesized_multiline, + split_target_regular_f_string, ] .with_mode(BestFittingMode::AllLines) .fmt(f) @@ -977,10 +996,10 @@ impl Format> for FormatStatementsLastExpression<'_> { best_fitting![ single_line, flat_target_parenthesize_value, - flat_target_value_parenthesized_multiline, + flat_target_regular_f_string, split_target_flat_value, split_target_value_parenthesized_flat, - split_target_value_parenthesized_multiline, + split_target_regular_f_string, ] .with_mode(BestFittingMode::AllLines) .fmt(f) @@ -998,13 +1017,69 @@ impl Format> for FormatStatementsLastExpression<'_> { } } -pub(crate) struct FormatFStringFlat<'a> { +/// Formats an f-string that is at the value position of an assignment statement. +/// +/// This is just a wrapper around [`FormatFString`] while considering a special case when the +/// f-string is at an assignment statement's value position. +/// +/// This is necessary to prevent an instability where an f-string contains a multiline expression +/// and the f-string fits on the line, but only when it's surrounded by parentheses. +/// +/// ```python +/// aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +/// expression}moreeeeeeeeeeeeeeeee" +/// ``` +/// +/// Without the special handling, this would get formatted to: +/// ```python +/// aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +/// expression +/// }moreeeeeeeeeeeeeeeee" +/// ``` +/// +/// However, if the parentheses already existed in the source like: +/// ```python +/// aaaaaaaaaaaaaaaaaa = ( +/// f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee" +/// ) +/// ``` +/// +/// Then, it would remain unformatted because it fits on the line. This means that even in the +/// first example, the f-string should be formatted by surrounding it with parentheses. +/// +/// One might ask why not just use the `BestFit` layout in this case. Consider the following +/// example in which the f-string doesn't fit on the line even when surrounded by parentheses: +/// ```python +/// xxxxxxx = f"{ +/// {'aaaaaaaaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbbbbbbbbbb', 'cccccccccccccccccccccccccc'} +/// }" +/// ``` +/// +/// The `BestFit` layout will format this as: +/// ```python +/// xxxxxxx = ( +/// f"{ +/// { +/// 'aaaaaaaaaaaaaaaaaaaaaaaaa', +/// 'bbbbbbbbbbbbbbbbbbbbbbbbbbb', +/// 'cccccccccccccccccccccccccc', +/// } +/// }" +/// ) +/// ``` +/// +/// The reason for this is because (a) f-string already has a multiline expression thus it tries to +/// break the expression and (b) the `BestFit` layout doesn't considers the layout where the +/// multiline f-string isn't surrounded by parentheses. +struct FormatFStringAssignment<'a> { + /// The expression that contains the f-string. expr: &'a ExprFString, + /// The only f-string part in the expression. f_string: &'a FString, } -impl<'a> FormatFStringFlat<'a> { - pub(crate) fn new(string: StringLike<'a>, context: &PyFormatContext) -> Option { +impl<'a> FormatFStringAssignment<'a> { + fn new(string: StringLike<'a>, context: &PyFormatContext) -> Option { if !is_f_string_formatting_enabled(context) { return None; } @@ -1025,7 +1100,7 @@ impl<'a> FormatFStringFlat<'a> { } } -impl Format> for FormatFStringFlat<'_> { +impl Format> for FormatFStringAssignment<'_> { fn fmt(&self, f: &mut Formatter>) -> FormatResult<()> { FormatFString::new( self.f_string, diff --git a/crates/ruff_python_formatter/src/string/implicit.rs b/crates/ruff_python_formatter/src/string/implicit.rs index 0da98d9e4364d..ce9e2e4ac6409 100644 --- a/crates/ruff_python_formatter/src/string/implicit.rs +++ b/crates/ruff_python_formatter/src/string/implicit.rs @@ -104,7 +104,7 @@ impl Format> for FormatImplicitConcatenatedStringExpanded<'_ for part in self.string.parts() { let format_part = format_with(|f: &mut PyFormatter| match part { StringLikePart::String(part) => { - let kind = if self.string.is_fstring() { + let kind = if self.string.is_f_string() { #[allow(deprecated)] StringLiteralKind::InImplicitlyConcatenatedFString(quoting) } else { @@ -154,7 +154,7 @@ impl<'a> FormatImplicitConcatenatedStringFlat<'a> { } // Multiline strings can never fit on a single line. - if !string.is_fstring() && string.is_multiline(context) { + if !string.is_f_string() && string.is_multiline(context) { return None; } diff --git a/crates/ruff_python_formatter/src/string/mod.rs b/crates/ruff_python_formatter/src/string/mod.rs index 01eb9351ed2cd..c33ea2879c6d9 100644 --- a/crates/ruff_python_formatter/src/string/mod.rs +++ b/crates/ruff_python_formatter/src/string/mod.rs @@ -135,7 +135,7 @@ impl<'a> FStringMultilineVisitor<'a> { } impl<'a> SourceOrderVisitor<'a> for FStringMultilineVisitor<'a> { - fn enter_node(&mut self, _node: ruff_python_ast::AnyNodeRef<'a>) -> TraversalSignal { + fn enter_node(&mut self, _node: ast::AnyNodeRef<'a>) -> TraversalSignal { if self.is_multiline { TraversalSignal::Skip } else { @@ -164,8 +164,9 @@ impl<'a> SourceOrderVisitor<'a> for FStringMultilineVisitor<'a> { // Expressions containing comments can't be joined. self.context.comments().contains_comments(expression.into()) } else { - // Multiline f-string expressions can't be joined if the f-string formatting is disabled because - // the string gets inserted in verbatim preserving the newlines. + // Multiline f-string expressions can't be joined if the f-string formatting is + // disabled because the string gets inserted in verbatim preserving the + // newlines. self.context .source() .contains_line_break(expression.range()) @@ -175,6 +176,14 @@ impl<'a> SourceOrderVisitor<'a> for FStringMultilineVisitor<'a> { if is_multiline { self.is_multiline = true; } else { + // Continue walking the f-string elements to visit the ones in format specifiers. + // + // For example, the following should be considered multiline because the literal part + // of the format specifier contains a newline at the end (`.3f\n`): + // ```py + // x = f"hello {a + b + c + d:.3f + // } world" + // ``` walk_f_string_element(self, f_string_element); } } diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap index 1f3299b3b07b1..24b1fc270bf66 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap @@ -338,6 +338,40 @@ xxxxxxx = f"{ {'aaaaaaaaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbbbbbbbbbb', 'cccccccccccccccccccccccccc'} }" +# Same as above but with an inline comment. The f-string should be formatted inside the +# parentheses and the comment should be part of the line inside the parentheses. +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeee" # comment + +# Similar to the previous example but this time parenthesizing doesn't work because it +# exceeds the line length. So, avoid parenthesizing this f-string. +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeee" # comment loooooooong + +# Similar to the previous example but we start with the parenthesized layout. This should +# remove the parentheses and format the f-string on a single line. This shows that the +# final layout for the formatter is same for this and the previous case. The only +# difference is that in the previous case the expression is already mulitline which means +# the formatter can break it further at the curly braces. +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee" # comment loooooooong +) + +# The following f-strings are going to break because of the trailing comma so we should +# avoid using the best fit layout and instead use the default layout. +# left-to-right +aaaa = f"aaaa {[ + 1, 2, +]} bbbb" +# right-to-left +aaaa, bbbb = f"aaaa {[ + 1, 2, +]} bbbb" + +# Using the right-to-left assignment statement variant. +aaaaaaaaaaaaaaaaaa, bbbbbbbbbbb = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeee" # comment + # This is an implicitly concatenated f-string but it cannot be joined because otherwise # it'll exceed the line length limit. So, the two f-strings will be inside parentheses # instead and the inline comment should be outside the parentheses. @@ -756,6 +790,47 @@ xxxxxxx = f"{ } }" +# Same as above but with an inline comment. The f-string should be formatted inside the +# parentheses and the comment should be part of the line inside the parentheses. +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee" # comment +) + +# Similar to the previous example but this time parenthesizing doesn't work because it +# exceeds the line length. So, avoid parenthesizing this f-string. +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression +}moreeeeeeeeeeeeeeeee" # comment loooooooong + +# Similar to the previous example but we start with the parenthesized layout. This should +# remove the parentheses and format the f-string on a single line. This shows that the +# final layout for the formatter is same for this and the previous case. The only +# difference is that in the previous case the expression is already mulitline which means +# the formatter can break it further at the curly braces. +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee" # comment loooooooong + +# The following f-strings are going to break because of the trailing comma so we should +# avoid using the best fit layout and instead use the default layout. +# left-to-right +aaaa = f"aaaa { + [ + 1, + 2, + ] +} bbbb" +# right-to-left +aaaa, bbbb = f"aaaa { + [ + 1, + 2, + ] +} bbbb" + +# Using the right-to-left assignment statement variant. +aaaaaaaaaaaaaaaaaa, bbbbbbbbbbb = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee" # comment +) + # This is an implicitly concatenated f-string but it cannot be joined because otherwise # it'll exceed the line length limit. So, the two f-strings will be inside parentheses # instead and the inline comment should be outside the parentheses. @@ -1152,6 +1227,38 @@ xxxxxxx = f"{ {'aaaaaaaaaaaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbbbbbbbbbbbbb', 'cccccccccccccccccccccccccc'} }" +# Same as above but with an inline comment. The f-string should be formatted inside the +# parentheses and the comment should be part of the line inside the parentheses. +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeee" # comment + +# Similar to the previous example but this time parenthesizing doesn't work because it +# exceeds the line length. So, avoid parenthesizing this f-string. +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeee" # comment loooooooong + +# Similar to the previous example but we start with the parenthesized layout. This should +# remove the parentheses and format the f-string on a single line. This shows that the +# final layout for the formatter is same for this and the previous case. The only +# difference is that in the previous case the expression is already mulitline which means +# the formatter can break it further at the curly braces. +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee" # comment loooooooong + +# The following f-strings are going to break because of the trailing comma so we should +# avoid using the best fit layout and instead use the default layout. +# left-to-right +aaaa = f"aaaa {[ + 1, 2, +]} bbbb" +# right-to-left +aaaa, bbbb = f"aaaa {[ + 1, 2, +]} bbbb" + +# Using the right-to-left assignment statement variant. +aaaaaaaaaaaaaaaaaa, bbbbbbbbbbb = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeee" # comment + # This is an implicitly concatenated f-string but it cannot be joined because otherwise # it'll exceed the line length limit. So, the two f-strings will be inside parentheses # instead and the inline comment should be outside the parentheses. @@ -1571,7 +1678,7 @@ f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccc # comment 27 # comment 28 } woah {x}" -@@ -314,41 +326,35 @@ +@@ -314,29 +326,36 @@ # Even though this f-string has multiline expression, thus allowing us to break it at the # curly braces, the f-string fits on a single line if it's moved inside the parentheses. # We should prefer doing that instead. @@ -1600,6 +1707,54 @@ f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccc + } }" + # Same as above but with an inline comment. The f-string should be formatted inside the + # parentheses and the comment should be part of the line inside the parentheses. +-aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +- expression}moreeeeeeeeeeeeeeeee" # comment ++aaaaaaaaaaaaaaaaaa = ( ++ f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee" # comment ++) + + # Similar to the previous example but this time parenthesizing doesn't work because it + # exceeds the line length. So, avoid parenthesizing this f-string. + aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +- expression}moreeeeeeeeeeeeeeeee" # comment loooooooong ++ expression ++}moreeeeeeeeeeeeeeeee" # comment loooooooong + + # Similar to the previous example but we start with the parenthesized layout. This should + # remove the parentheses and format the f-string on a single line. This shows that the +@@ -348,39 +367,35 @@ + # The following f-strings are going to break because of the trailing comma so we should + # avoid using the best fit layout and instead use the default layout. + # left-to-right +-aaaa = f"aaaa {[ +- 1, 2, +-]} bbbb" ++aaaa = f"aaaa { ++ [ ++ 1, ++ 2, ++ ] ++} bbbb" + # right-to-left +-aaaa, bbbb = f"aaaa {[ +- 1, 2, +-]} bbbb" ++aaaa, bbbb = f"aaaa { ++ [ ++ 1, ++ 2, ++ ] ++} bbbb" + + # Using the right-to-left assignment statement variant. +-aaaaaaaaaaaaaaaaaa, bbbbbbbbbbb = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +- expression}moreeeeeeeeeeeeeeeee" # comment ++aaaaaaaaaaaaaaaaaa, bbbbbbbbbbb = ( ++ f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee" # comment ++) + # This is an implicitly concatenated f-string but it cannot be joined because otherwise # it'll exceed the line length limit. So, the two f-strings will be inside parentheses # instead and the inline comment should be outside the parentheses. @@ -1627,7 +1782,7 @@ f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccc # Indentation -@@ -359,29 +365,29 @@ +@@ -391,29 +406,29 @@ if indent2: foo = f"""hello world hello { From c1da5c6c9eada4d0527f9d9ff9226aaead63d649 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 21 Nov 2024 12:06:10 +0530 Subject: [PATCH 04/10] Ignore f-strings that cannot be multiline --- .../test/fixtures/ruff/expression/fstring.py | 19 +++++ .../src/other/f_string.rs | 4 + .../src/statement/stmt_assign.rs | 14 +++- .../format@expression__fstring.py.snap | 79 ++++++++++++++++++- 4 files changed, 111 insertions(+), 5 deletions(-) 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 c1d8dd9d87e60..f50676646fed5 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 @@ -366,6 +366,25 @@ aaaaaaaaaaaaaaaaaa, bbbbbbbbbbb = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ expression}moreeeeeeeeeeeeeeeee" # comment +# Here, the f-string layout is flat but it exceeds the line length limit. This shouldn't +# try the custom best fit layout because the f-string doesn't have any split points. +aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = ( + f"aaaaaaaaaaaaaaaaaaa {aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc} ddddddddddddddddddd" +) +# Same as above but without the parentheses to test that it gets formatted to the same +# layout as the previous example. +aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = f"aaaaaaaaaaaaaaaaaaa {aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc} ddddddddddddddddddd" + +# But, the following f-string does have a split point because of the multiline expression. +aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = ( + f"aaaaaaaaaaaaaaaaaaa { + aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc} ddddddddddddddddddd" +) +aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = ( + f"aaaaaaaaaaaaaaaaaaa { + aaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbb + cccccccccccccccccccccc + dddddddddddddddddddddddddddd} ddddddddddddddddddd" +) + # This is an implicitly concatenated f-string but it cannot be joined because otherwise # it'll exceed the line length limit. So, the two f-strings will be inside parentheses # instead and the inline comment should be outside the parentheses. diff --git a/crates/ruff_python_formatter/src/other/f_string.rs b/crates/ruff_python_formatter/src/other/f_string.rs index 409697dfadf36..067c4378d1a90 100644 --- a/crates/ruff_python_formatter/src/other/f_string.rs +++ b/crates/ruff_python_formatter/src/other/f_string.rs @@ -140,6 +140,10 @@ impl FStringLayout { } } + pub(crate) const fn is_flat(self) -> bool { + matches!(self, FStringLayout::Flat) + } + pub(crate) const fn is_multiline(self) -> bool { matches!(self, FStringLayout::Multiline) } diff --git a/crates/ruff_python_formatter/src/statement/stmt_assign.rs b/crates/ruff_python_formatter/src/statement/stmt_assign.rs index 45ce6e31dfada..de156a06f195e 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_assign.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_assign.rs @@ -18,7 +18,7 @@ use crate::expression::{ can_omit_optional_parentheses, has_own_parentheses, has_parentheses, maybe_parenthesize_expression, }; -use crate::other::f_string::FormatFString; +use crate::other::f_string::{FStringLayout, FormatFString}; use crate::preview::{ is_f_string_formatting_enabled, is_join_implicit_concatenated_string_enabled, }; @@ -493,7 +493,7 @@ impl Format> for FormatStatementsLastExpression<'_> { let single_line = format_with(|f| write!(f, [f_string_flat, inline_comments])); - // Parenthesize the f-sring and flatten the f-string. + // Parenthesize the f-string and flatten the f-string. // ```python // aaaaaaaaaaaaaaaaaa = ( // f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee" @@ -918,7 +918,6 @@ impl Format> for FormatStatementsLastExpression<'_> { // ]} bbbb" // ``` if format_value.inspect(f)?.will_break() { - println!("Flattened f-string will break, using the default layout"); inline_comments.mark_unformatted(); return write!( @@ -1092,6 +1091,15 @@ impl<'a> FormatFStringAssignment<'a> { return None; }; + // If the f-string is flat, there are no breakpoints from which it can be made multiline. + // This is the case when the f-string has no expressions or if it does then the expressions + // are flat (no newlines). + if FStringLayout::from_f_string(f_string, context.source()).is_flat() { + return None; + } + + // This checks whether the f-string is multi-line and it can *never* be flattened. Thus, + // we cannot try the flattened layout. if string.is_multiline(context) { return None; } diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap index 24b1fc270bf66..84426a0e69f72 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap @@ -372,6 +372,25 @@ aaaa, bbbb = f"aaaa {[ aaaaaaaaaaaaaaaaaa, bbbbbbbbbbb = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ expression}moreeeeeeeeeeeeeeeee" # comment +# Here, the f-string layout is flat but it exceeds the line length limit. This shouldn't +# try the custom best fit layout because the f-string doesn't have any split points. +aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = ( + f"aaaaaaaaaaaaaaaaaaa {aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc} ddddddddddddddddddd" +) +# Same as above but without the parentheses to test that it gets formatted to the same +# layout as the previous example. +aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = f"aaaaaaaaaaaaaaaaaaa {aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc} ddddddddddddddddddd" + +# But, the following f-string does have a split point because of the multiline expression. +aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = ( + f"aaaaaaaaaaaaaaaaaaa { + aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc} ddddddddddddddddddd" +) +aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = ( + f"aaaaaaaaaaaaaaaaaaa { + aaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbb + cccccccccccccccccccccc + dddddddddddddddddddddddddddd} ddddddddddddddddddd" +) + # This is an implicitly concatenated f-string but it cannot be joined because otherwise # it'll exceed the line length limit. So, the two f-strings will be inside parentheses # instead and the inline comment should be outside the parentheses. @@ -831,6 +850,28 @@ aaaaaaaaaaaaaaaaaa, bbbbbbbbbbb = ( f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee" # comment ) +# Here, the f-string layout is flat but it exceeds the line length limit. This shouldn't +# try the custom best fit layout because the f-string doesn't have any split points. +aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = ( + f"aaaaaaaaaaaaaaaaaaa {aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc} ddddddddddddddddddd" +) +# Same as above but without the parentheses to test that it gets formatted to the same +# layout as the previous example. +aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = ( + f"aaaaaaaaaaaaaaaaaaa {aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc} ddddddddddddddddddd" +) + +# But, the following f-string does have a split point because of the multiline expression. +aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = f"aaaaaaaaaaaaaaaaaaa { + aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc +} ddddddddddddddddddd" +aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = f"aaaaaaaaaaaaaaaaaaa { + aaaaaaaaaaaaaaaaaaaa + + bbbbbbbbbbbbbbbbbbbbb + + cccccccccccccccccccccc + + dddddddddddddddddddddddddddd +} ddddddddddddddddddd" + # This is an implicitly concatenated f-string but it cannot be joined because otherwise # it'll exceed the line length limit. So, the two f-strings will be inside parentheses # instead and the inline comment should be outside the parentheses. @@ -1259,6 +1300,23 @@ aaaa, bbbb = f"aaaa {[ aaaaaaaaaaaaaaaaaa, bbbbbbbbbbb = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ expression}moreeeeeeeeeeeeeeeee" # comment +# Here, the f-string layout is flat but it exceeds the line length limit. This shouldn't +# try the custom best fit layout because the f-string doesn't have any split points. +aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = ( + f"aaaaaaaaaaaaaaaaaaa {aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc} ddddddddddddddddddd" +) +# Same as above but without the parentheses to test that it gets formatted to the same +# layout as the previous example. +aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = ( + f"aaaaaaaaaaaaaaaaaaa {aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc} ddddddddddddddddddd" +) + +# But, the following f-string does have a split point because of the multiline expression. +aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = f"aaaaaaaaaaaaaaaaaaa { + aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc} ddddddddddddddddddd" +aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = f"aaaaaaaaaaaaaaaaaaa { + aaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbb + cccccccccccccccccccccc + dddddddddddddddddddddddddddd} ddddddddddddddddddd" + # This is an implicitly concatenated f-string but it cannot be joined because otherwise # it'll exceed the line length limit. So, the two f-strings will be inside parentheses # instead and the inline comment should be outside the parentheses. @@ -1724,7 +1782,7 @@ f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccc # Similar to the previous example but we start with the parenthesized layout. This should # remove the parentheses and format the f-string on a single line. This shows that the -@@ -348,39 +367,35 @@ +@@ -348,17 +367,24 @@ # The following f-strings are going to break because of the trailing comma so we should # avoid using the best fit layout and instead use the default layout. # left-to-right @@ -1755,6 +1813,23 @@ f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccc + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeee" # comment +) + # Here, the f-string layout is flat but it exceeds the line length limit. This shouldn't + # try the custom best fit layout because the f-string doesn't have any split points. +@@ -373,31 +399,25 @@ + + # But, the following f-string does have a split point because of the multiline expression. + aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = f"aaaaaaaaaaaaaaaaaaa { +- aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc} ddddddddddddddddddd" ++ aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc ++} ddddddddddddddddddd" + aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = f"aaaaaaaaaaaaaaaaaaa { +- aaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbb + cccccccccccccccccccccc + dddddddddddddddddddddddddddd} ddddddddddddddddddd" ++ aaaaaaaaaaaaaaaaaaaa ++ + bbbbbbbbbbbbbbbbbbbbb ++ + cccccccccccccccccccccc ++ + dddddddddddddddddddddddddddd ++} ddddddddddddddddddd" + # This is an implicitly concatenated f-string but it cannot be joined because otherwise # it'll exceed the line length limit. So, the two f-strings will be inside parentheses # instead and the inline comment should be outside the parentheses. @@ -1782,7 +1857,7 @@ f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccc # Indentation -@@ -391,29 +406,29 @@ +@@ -408,29 +428,29 @@ if indent2: foo = f"""hello world hello { From 308fab9e4afa959050a6c71c584200752ebeb4a9 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 21 Nov 2024 16:51:44 +0530 Subject: [PATCH 05/10] Revert `f_string` rename to keep diff minimal --- crates/ruff_python_ast/src/expression.rs | 2 +- crates/ruff_python_formatter/src/statement/stmt_assign.rs | 8 ++++---- crates/ruff_python_formatter/src/string/implicit.rs | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/ruff_python_ast/src/expression.rs b/crates/ruff_python_ast/src/expression.rs index 08b4da80fa913..469f12b003af4 100644 --- a/crates/ruff_python_ast/src/expression.rs +++ b/crates/ruff_python_ast/src/expression.rs @@ -408,7 +408,7 @@ pub enum StringLike<'a> { } impl<'a> StringLike<'a> { - pub const fn is_f_string(self) -> bool { + pub const fn is_fstring(self) -> bool { matches!(self, Self::FString(_)) } diff --git a/crates/ruff_python_formatter/src/statement/stmt_assign.rs b/crates/ruff_python_formatter/src/statement/stmt_assign.rs index de156a06f195e..c8d9e8e9db01e 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_assign.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_assign.rs @@ -355,7 +355,7 @@ impl Format> for FormatStatementsLastExpression<'_> { let string = flat.string(); let flat = format_with(|f| { - if string.is_f_string() { + if string.is_fstring() { let mut buffer = RemoveSoftLinesBuffer::new(&mut *f); write!(buffer, [flat]) @@ -373,7 +373,7 @@ impl Format> for FormatStatementsLastExpression<'_> { // 2, // ]}" "more" // ``` - if string.is_f_string() && flat.inspect(f)?.will_break() { + if string.is_fstring() && flat.inspect(f)?.will_break() { inline_comments.mark_unformatted(); return write!( @@ -656,7 +656,7 @@ impl Format> for FormatStatementsLastExpression<'_> { let format_value = format_with(|f| { if let Some(format_implicit_flat) = format_implicit_flat.as_ref() { - if format_implicit_flat.string().is_f_string() { + if format_implicit_flat.string().is_fstring() { // Remove any soft line breaks emitted by the f-string formatting. // This is important when formatting f-strings as part of an assignment right side // because `best_fit_parenthesize` will otherwise still try to break inner @@ -769,7 +769,7 @@ impl Format> for FormatStatementsLastExpression<'_> { // 2, // ]}" "more" // ``` - if format_implicit_flat.string().is_f_string() + if format_implicit_flat.string().is_fstring() && format_value.inspect(f)?.will_break() { inline_comments.mark_unformatted(); diff --git a/crates/ruff_python_formatter/src/string/implicit.rs b/crates/ruff_python_formatter/src/string/implicit.rs index ce9e2e4ac6409..0da98d9e4364d 100644 --- a/crates/ruff_python_formatter/src/string/implicit.rs +++ b/crates/ruff_python_formatter/src/string/implicit.rs @@ -104,7 +104,7 @@ impl Format> for FormatImplicitConcatenatedStringExpanded<'_ for part in self.string.parts() { let format_part = format_with(|f: &mut PyFormatter| match part { StringLikePart::String(part) => { - let kind = if self.string.is_f_string() { + let kind = if self.string.is_fstring() { #[allow(deprecated)] StringLiteralKind::InImplicitlyConcatenatedFString(quoting) } else { @@ -154,7 +154,7 @@ impl<'a> FormatImplicitConcatenatedStringFlat<'a> { } // Multiline strings can never fit on a single line. - if !string.is_f_string() && string.is_multiline(context) { + if !string.is_fstring() && string.is_multiline(context) { return None; } From 4ce0c42dac09039e86311fdb2da5558947e18188 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 21 Nov 2024 17:00:31 +0530 Subject: [PATCH 06/10] Add example to different f-string layout --- .../src/statement/stmt_assign.rs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/crates/ruff_python_formatter/src/statement/stmt_assign.rs b/crates/ruff_python_formatter/src/statement/stmt_assign.rs index c8d9e8e9db01e..4def3a127e516 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_assign.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_assign.rs @@ -939,7 +939,22 @@ impl Format> for FormatStatementsLastExpression<'_> { let format_f_string = format_with(|f| write!(f, [format_f_string, inline_comments])).memoized(); + // Considering the following initial source: + // + // ```python + // aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = ( + // f"aaaaaaaaaaaaaaaaaaa { + // aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc} ddddddddddddddddddd" + // ) + // ``` + // Keep the target flat, and use the regular f-string formatting. + // + // ```python + // aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = f"aaaaaaaaaaaaaaaaaaa { + // aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc + // } ddddddddddddddddddd" + // ``` let flat_target_regular_f_string = format_with(|f| { write!( f, @@ -948,6 +963,14 @@ impl Format> for FormatStatementsLastExpression<'_> { }); // Expand the parent and parenthesize the flattened f-string. + // + // ```python + // aaaaaaaaaaaa[ + // "bbbbbbbbbbbbbbbb" + // ] = ( + // f"aaaaaaaaaaaaaaaaaaa {aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc} ddddddddddddddddddd" + // ) + // ``` let split_target_value_parenthesized_flat = format_with(|f| { write!( f, @@ -968,6 +991,14 @@ impl Format> for FormatStatementsLastExpression<'_> { }); // Expand the parent, and use the regular f-string formatting. + // + // ```python + // aaaaaaaaaaaa[ + // "bbbbbbbbbbbbbbbb" + // ] = f"aaaaaaaaaaaaaaaaaaa { + // aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc + // } ddddddddddddddddddd" + // ``` let split_target_regular_f_string = format_with(|f| { write!( f, From 5ebbb7b9cd115542b7e2150e20faadb29f448943 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Mon, 25 Nov 2024 16:38:39 +0530 Subject: [PATCH 07/10] Address review feedback --- crates/ruff_python_ast/src/nodes.rs | 8 ++ .../test/fixtures/ruff/expression/binary.py | 6 + .../src/expression/expr_f_string.rs | 8 +- .../src/statement/stmt_assign.rs | 97 ++++++-------- .../src/string/implicit.rs | 21 +-- .../ruff_python_formatter/src/string/mod.rs | 124 +++++++----------- .../format@expression__binary.py.snap | 27 +++- 7 files changed, 132 insertions(+), 159 deletions(-) diff --git a/crates/ruff_python_ast/src/nodes.rs b/crates/ruff_python_ast/src/nodes.rs index b0eb7f3e8a423..7ceea8bfe8a3d 100644 --- a/crates/ruff_python_ast/src/nodes.rs +++ b/crates/ruff_python_ast/src/nodes.rs @@ -1273,6 +1273,14 @@ impl FStringValue { matches!(self.inner, FStringValueInner::Concatenated(_)) } + /// Returns the [`FString`] if the value is made up of a single f-string, [`None`] otherwise. + pub fn as_single(&self) -> Option<&FString> { + match &self.inner { + FStringValueInner::Single(FStringPart::FString(fstring)) => Some(fstring), + _ => None, + } + } + /// Returns a slice of all the [`FStringPart`]s contained in this value. pub fn as_slice(&self) -> &[FStringPart] { match &self.inner { diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary.py index 83c6f0ff9fc3a..818dca0bc76ac 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary.py @@ -418,3 +418,9 @@ "permissions to manage this role, or else members of this role won't receive " "a notification." ) + +# This f-string should be flattened +xxxxxxxxxxxxxxxx = f"aaaaaaaaaaaaaaaaaaaaa { + expression } bbbbbbbbbbbbbbbbbbbbbbbb" + ( + yyyyyyyyyyyyyy + zzzzzzzzzzz +) 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 9994a360d4dfe..a003a31577891 100644 --- a/crates/ruff_python_formatter/src/expression/expr_f_string.rs +++ b/crates/ruff_python_formatter/src/expression/expr_f_string.rs @@ -4,7 +4,7 @@ use ruff_text_size::TextSlice; use crate::expression::parentheses::{ in_parentheses_only_group, NeedsParentheses, OptionalParentheses, }; -use crate::other::f_string::FormatFString; +use crate::other::f_string::{FStringLayout, FormatFString}; use crate::prelude::*; use crate::string::implicit::{ FormatImplicitConcatenatedString, FormatImplicitConcatenatedStringFlat, @@ -47,7 +47,11 @@ impl NeedsParentheses for ExprFString { ) -> OptionalParentheses { if self.value.is_implicit_concatenated() { OptionalParentheses::Multiline - } else if StringLike::FString(self).is_multiline(context) { + } else if StringLike::FString(self).is_multiline(context) + || self.value.as_single().is_some_and(|f_string| { + FStringLayout::from_f_string(f_string, context.source()).is_multiline() + }) + { OptionalParentheses::Never } else { OptionalParentheses::BestFit diff --git a/crates/ruff_python_formatter/src/statement/stmt_assign.rs b/crates/ruff_python_formatter/src/statement/stmt_assign.rs index 4def3a127e516..51e355d6cdb0d 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_assign.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_assign.rs @@ -1,7 +1,7 @@ use ruff_formatter::{format_args, write, FormatError, RemoveSoftLinesBuffer}; use ruff_python_ast::{ - AnyNodeRef, Expr, ExprAttribute, ExprCall, ExprFString, FString, FStringPart, Operator, - StmtAssign, StringLike, TypeParams, + AnyNodeRef, Expr, ExprAttribute, ExprCall, FStringPart, Operator, StmtAssign, StringLike, + TypeParams, }; use crate::builders::parenthesize_if_expands; @@ -295,15 +295,15 @@ impl Format> for FormatStatementsLastExpression<'_> { let can_inline_comment = should_inline_comments(value, *statement, f.context()); let string_like = StringLike::try_from(*value).ok(); - let format_f_string_assignment = string_like - .and_then(|string| FormatFStringAssignment::new(string, f.context())); + let format_f_string = + string_like.and_then(|string| format_f_string_assignment(string, f.context())); let format_implicit_flat = string_like.and_then(|string| { FormatImplicitConcatenatedStringFlat::new(string, f.context()) }); if !can_inline_comment && format_implicit_flat.is_none() - && format_f_string_assignment.is_none() + && format_f_string.is_none() { return maybe_parenthesize_expression( value, @@ -450,7 +450,7 @@ impl Format> for FormatStatementsLastExpression<'_> { best_fitting![single_line, joined_parenthesized, implicit_expanded] .with_mode(BestFittingMode::AllLines) .fmt(f)?; - } else if let Some(format_f_string) = format_f_string_assignment { + } else if let Some(format_f_string) = format_f_string { inline_comments.mark_formatted(); let f_string_flat = format_with(|f| { @@ -563,8 +563,8 @@ impl Format> for FormatStatementsLastExpression<'_> { let should_inline_comments = should_inline_comments(value, *statement, f.context()); let string_like = StringLike::try_from(*value).ok(); - let format_f_string_assignment = string_like - .and_then(|string| FormatFStringAssignment::new(string, f.context())); + let format_f_string = + string_like.and_then(|string| format_f_string_assignment(string, f.context())); let format_implicit_flat = string_like.and_then(|string| { FormatImplicitConcatenatedStringFlat::new(string, f.context()) }); @@ -573,7 +573,7 @@ impl Format> for FormatStatementsLastExpression<'_> { if !should_inline_comments && !should_non_inlineable_use_best_fit(value, *statement, f.context()) && format_implicit_flat.is_none() - && format_f_string_assignment.is_none() + && format_f_string.is_none() { return write!( f, @@ -597,7 +597,7 @@ impl Format> for FormatStatementsLastExpression<'_> { // Don't inline comments for attribute and call expressions for black compatibility let inline_comments = if should_inline_comments || format_implicit_flat.is_some() - || format_f_string_assignment.is_some() + || format_f_string.is_some() { OptionalParenthesesInlinedComments::new( &expression_comments, @@ -637,9 +637,7 @@ impl Format> for FormatStatementsLastExpression<'_> { // This is mainly a performance optimisation that avoids unnecessary memoization // and using the costly `BestFitting` layout if it is already known that only the last variant // can ever fit because the left breaks. - if format_implicit_flat.is_none() - && format_f_string_assignment.is_none() - && last_target_breaks + if format_implicit_flat.is_none() && format_f_string.is_none() && last_target_breaks { return write!( f, @@ -666,7 +664,7 @@ impl Format> for FormatStatementsLastExpression<'_> { } else { format_implicit_flat.fmt(f) } - } else if let Some(format_f_string) = format_f_string_assignment.as_ref() { + } else if let Some(format_f_string) = format_f_string.as_ref() { // Similar to above, remove any soft line breaks emitted by the f-string // formatting. let mut buffer = RemoveSoftLinesBuffer::new(&mut *f); @@ -908,7 +906,7 @@ impl Format> for FormatStatementsLastExpression<'_> { .with_mode(BestFittingMode::AllLines) .fmt(f) } - } else if let Some(format_f_string) = &format_f_string_assignment { + } else if let Some(format_f_string) = &format_f_string { // F-String containing an expression with a magic trailing comma, a comment, or a // multiline debug expression should never be joined. Use the default layout. // @@ -947,7 +945,7 @@ impl Format> for FormatStatementsLastExpression<'_> { // aaaaaaaaa + bbbbbbbbbbb + cccccccccccccc} ddddddddddddddddddd" // ) // ``` - + // // Keep the target flat, and use the regular f-string formatting. // // ```python @@ -1101,52 +1099,39 @@ impl Format> for FormatStatementsLastExpression<'_> { /// The reason for this is because (a) f-string already has a multiline expression thus it tries to /// break the expression and (b) the `BestFit` layout doesn't considers the layout where the /// multiline f-string isn't surrounded by parentheses. -struct FormatFStringAssignment<'a> { - /// The expression that contains the f-string. - expr: &'a ExprFString, - /// The only f-string part in the expression. - f_string: &'a FString, -} - -impl<'a> FormatFStringAssignment<'a> { - fn new(string: StringLike<'a>, context: &PyFormatContext) -> Option { - if !is_f_string_formatting_enabled(context) { - return None; - } - - let StringLike::FString(expr) = string else { - return None; - }; - - let [FStringPart::FString(f_string)] = expr.value.as_slice() else { - return None; - }; +fn format_f_string_assignment<'a>( + string: StringLike<'a>, + context: &PyFormatContext, +) -> Option> { + if !is_f_string_formatting_enabled(context) { + return None; + } - // If the f-string is flat, there are no breakpoints from which it can be made multiline. - // This is the case when the f-string has no expressions or if it does then the expressions - // are flat (no newlines). - if FStringLayout::from_f_string(f_string, context.source()).is_flat() { - return None; - } + let StringLike::FString(expr) = string else { + return None; + }; - // This checks whether the f-string is multi-line and it can *never* be flattened. Thus, - // we cannot try the flattened layout. - if string.is_multiline(context) { - return None; - } + let [FStringPart::FString(f_string)] = expr.value.as_slice() else { + return None; + }; - Some(Self { expr, f_string }) + // If the f-string is flat, there are no breakpoints from which it can be made multiline. + // This is the case when the f-string has no expressions or if it does then the expressions + // are flat (no newlines). + if FStringLayout::from_f_string(f_string, context.source()).is_flat() { + return None; } -} -impl Format> for FormatFStringAssignment<'_> { - fn fmt(&self, f: &mut Formatter>) -> FormatResult<()> { - FormatFString::new( - self.f_string, - f_string_quoting(self.expr, f.context().source()), - ) - .fmt(f) + // This checks whether the f-string is multi-line and it can *never* be flattened. Thus, + // it's useless to try the flattened layout. + if string.is_multiline(context) { + return None; } + + Some(FormatFString::new( + f_string, + f_string_quoting(expr, context.source()), + )) } #[derive(Debug, Default)] diff --git a/crates/ruff_python_formatter/src/string/implicit.rs b/crates/ruff_python_formatter/src/string/implicit.rs index 0da98d9e4364d..3db5fc7f9cea3 100644 --- a/crates/ruff_python_formatter/src/string/implicit.rs +++ b/crates/ruff_python_formatter/src/string/implicit.rs @@ -154,7 +154,7 @@ impl<'a> FormatImplicitConcatenatedStringFlat<'a> { } // Multiline strings can never fit on a single line. - if !string.is_fstring() && string.is_multiline(context) { + if string.is_multiline(context) { return None; } @@ -187,25 +187,6 @@ impl<'a> FormatImplicitConcatenatedStringFlat<'a> { } if let StringLikePart::FString(fstring) = part { - if fstring.elements.iter().any(|element| match element { - // Same as for other literals. Multiline literals can't fit on a single line. - FStringElement::Literal(literal) => { - context.source().contains_line_break(literal.range()) - } - FStringElement::Expression(expression) => { - if is_f_string_formatting_enabled(context) { - // Expressions containing comments can't be joined. - context.comments().contains_comments(expression.into()) - } else { - // Multiline f-string expressions can't be joined if the f-string formatting is disabled because - // the string gets inserted in verbatim preserving the newlines. - context.source().contains_line_break(expression.range()) - } - } - }) { - return None; - } - if context.options().target_version().supports_pep_701() { if is_fstring_with_quoted_format_spec_and_debug(fstring, context) { if preserve_quotes_requirement diff --git a/crates/ruff_python_formatter/src/string/mod.rs b/crates/ruff_python_formatter/src/string/mod.rs index c33ea2879c6d9..f1141161d464c 100644 --- a/crates/ruff_python_formatter/src/string/mod.rs +++ b/crates/ruff_python_formatter/src/string/mod.rs @@ -1,9 +1,6 @@ pub(crate) use normalize::{normalize_string, NormalizedString, StringNormalizer}; use ruff_python_ast::str::Quote; -use ruff_python_ast::visitor::source_order::{ - walk_f_string_element, SourceOrderVisitor, TraversalSignal, -}; -use ruff_python_ast::AstNode; +use ruff_python_ast::StringLikePart; use ruff_python_ast::{ self as ast, str_prefix::{AnyStringPrefix, StringLiteralPrefix}, @@ -106,85 +103,52 @@ impl StringLikeExtensions for ast::StringLike<'_> { } fn is_multiline(&self, context: &PyFormatContext) -> bool { - match self { - Self::String(_) | Self::Bytes(_) => self.parts().any(|part| { + self.parts().any(|part| match part { + StringLikePart::String(_) | StringLikePart::Bytes(_) => { part.flags().is_triple_quoted() - && context.source().contains_line_break(self.range()) - }), - Self::FString(expr) => { - let mut visitor = FStringMultilineVisitor::new(context); - expr.visit_source_order(&mut visitor); - visitor.is_multiline + && context.source().contains_line_break(part.range()) } - } - } -} - -struct FStringMultilineVisitor<'a> { - context: &'a PyFormatContext<'a>, - is_multiline: bool, -} - -impl<'a> FStringMultilineVisitor<'a> { - fn new(context: &'a PyFormatContext<'a>) -> Self { - Self { - context, - is_multiline: false, - } - } -} - -impl<'a> SourceOrderVisitor<'a> for FStringMultilineVisitor<'a> { - fn enter_node(&mut self, _node: ast::AnyNodeRef<'a>) -> TraversalSignal { - if self.is_multiline { - TraversalSignal::Skip - } else { - TraversalSignal::Traverse - } - } - - fn visit_string_literal(&mut self, string_literal: &'a ast::StringLiteral) { - if string_literal.flags.is_triple_quoted() - && self - .context - .source() - .contains_line_break(string_literal.range()) - { - self.is_multiline = true; - } - } - - fn visit_f_string_element(&mut self, f_string_element: &'a ast::FStringElement) { - let is_multiline = match f_string_element { - ast::FStringElement::Literal(literal) => { - self.context.source().contains_line_break(literal.range()) - } - ast::FStringElement::Expression(expression) => { - if is_f_string_formatting_enabled(self.context) { - // Expressions containing comments can't be joined. - self.context.comments().contains_comments(expression.into()) - } else { - // Multiline f-string expressions can't be joined if the f-string formatting is - // disabled because the string gets inserted in verbatim preserving the - // newlines. - self.context - .source() - .contains_line_break(expression.range()) + StringLikePart::FString(f_string) => { + fn contains_line_break_or_comments( + elements: &ast::FStringElements, + context: &PyFormatContext, + ) -> bool { + elements.iter().any(|element| match element { + ast::FStringElement::Literal(literal) => { + context.source().contains_line_break(literal.range()) + } + ast::FStringElement::Expression(expression) => { + if is_f_string_formatting_enabled(context) { + // Expressions containing comments can't be joined. + // + // Format specifiers needs to be checked as well. For example, the + // following should be considered multiline because the literal + // part of the format specifier contains a newline at the end + // (`.3f\n`): + // + // ```py + // x = f"hello {a + b + c + d:.3f + // } world" + // ``` + context.comments().contains_comments(expression.into()) + || expression.format_spec.as_deref().is_some_and(|spec| { + contains_line_break_or_comments(&spec.elements, context) + }) + } else { + // Multiline f-string expressions can't be joined if the f-string + // formatting is disabled because the string gets inserted in + // verbatim preserving the newlines. + // + // We don't need to check format specifiers here because the + // expression range already includes them. + context.source().contains_line_break(expression.range()) + } + } + }) } + + contains_line_break_or_comments(&f_string.elements, context) } - }; - if is_multiline { - self.is_multiline = true; - } else { - // Continue walking the f-string elements to visit the ones in format specifiers. - // - // For example, the following should be considered multiline because the literal part - // of the format specifier contains a newline at the end (`.3f\n`): - // ```py - // x = f"hello {a + b + c + d:.3f - // } world" - // ``` - walk_f_string_element(self, f_string_element); - } + }) } } diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__binary.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__binary.py.snap index 82d426ac7025c..5f2072ff10794 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__binary.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__binary.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary.py -snapshot_kind: text --- ## Input ```python @@ -425,6 +424,12 @@ if True: "permissions to manage this role, or else members of this role won't receive " "a notification." ) + +# This f-string should be flattened +xxxxxxxxxxxxxxxx = f"aaaaaaaaaaaaaaaaaaaaa { + expression } bbbbbbbbbbbbbbbbbbbbbbbb" + ( + yyyyyyyyyyyyyy + zzzzzzzzzzz +) ``` ## Output @@ -897,4 +902,24 @@ if True: "permissions to manage this role, or else members of this role won't receive " "a notification." ) + +# This f-string should be flattened +xxxxxxxxxxxxxxxx = f"aaaaaaaaaaaaaaaaaaaaa { + expression } bbbbbbbbbbbbbbbbbbbbbbbb" + (yyyyyyyyyyyyyy + zzzzzzzzzzz) +``` + + +## Preview changes +```diff +--- Stable ++++ Preview +@@ -468,5 +468,6 @@ + ) + + # This f-string should be flattened +-xxxxxxxxxxxxxxxx = f"aaaaaaaaaaaaaaaaaaaaa { +- expression } bbbbbbbbbbbbbbbbbbbbbbbb" + (yyyyyyyyyyyyyy + zzzzzzzzzzz) ++xxxxxxxxxxxxxxxx = f"aaaaaaaaaaaaaaaaaaaaa {expression} bbbbbbbbbbbbbbbbbbbbbbbb" + ( ++ yyyyyyyyyyyyyy + zzzzzzzzzzz ++) ``` From 0ec4627aa6c405143f53415a82f99325328f8976 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Mon, 25 Nov 2024 20:41:12 +0530 Subject: [PATCH 08/10] Add more test cases with multiline expression --- .../test/fixtures/ruff/expression/fstring.py | 84 +++++ .../format@expression__fstring.py.snap | 305 +++++++++++++++++- 2 files changed, 386 insertions(+), 3 deletions(-) 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 f50676646fed5..e6f64b17277f2 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 @@ -401,6 +401,90 @@ joined } togethereeeeeeeeeeeeeeeeeeeeeeeeeee" # inline +# The following test cases are adopted from implicit string concatenation but for a +# single f-string instead. + +# Don't inline f-strings that contain expressions that are guaranteed to split, e.g. because of a magic trailing comma +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +[a,] +}moreeeeeeeeeeeeeeeeeeee" # comment + +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +[a,] +}moreeeeeeeeeeeeeeeeeeee" # comment +) + +aaaaa[aaaaaaaaaaa] = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +[a,] +}moreeeeeeeeeeeeeeeeeeee" # comment + +aaaaa[aaaaaaaaaaa] = (f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +[a,] +}moreeeeeeeeeeeeeeeeeeee" # comment +) + +# Don't inline f-strings that contain commented expressions +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{[ + a # comment + ]}moreeeeeeeeeeeeeeeeeetest" # comment +) + +aaaaa[aaaaaaaaaaa] = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{[ + a # comment + ]}moreeeeeeeeeeeeeeeeeetest" # comment +) + +# Don't inline f-strings with multiline debug expressions or format specifiers +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{ + a=}moreeeeeeeeeeeeeeeeeetest" # comment +) + +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{a + + b=}moreeeeeeeeeeeeeeeeeetest" # comment +) + +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{a + =}moreeeeeeeeeeeeeeeeeetest" # comment +) + +aaaaa[aaaaaaaaaaa] = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{ + a=}moreeeeeeeeeeeeeeeeeetest" # comment +) + +aaaaa[aaaaaaaaaaa] = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{a + =}moreeeeeeeeeeeeeeeeeetest" # comment +) + +# This is not a multiline f-string even though it has a newline after the format specifier. +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{ + a:.3f + }moreeeeeeeeeeeeeeeeeetest" # comment + +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{ + a:.3f + }moreeeeeeeeeeeeeeeeeetest" # comment +) + +# The newline is only considered when it's a tripled-quoted f-string. +aaaaaaaaaaaaaaaaaa = f"""testeeeeeeeeeeeeeeeeeeeeeeeee{ + a:.3f + }moreeeeeeeeeeeeeeeeeetest""" # comment + +aaaaaaaaaaaaaaaaaa = ( + f"""testeeeeeeeeeeeeeeeeeeeeeeeee{ + a:.3f + }moreeeeeeeeeeeeeeeeeetest""" # comment +) + # Indentation # What should be the indentation? diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap index 84426a0e69f72..c503c10afa15f 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap @@ -407,6 +407,90 @@ a = f"test{ joined } togethereeeeeeeeeeeeeeeeeeeeeeeeeee" # inline +# The following test cases are adopted from implicit string concatenation but for a +# single f-string instead. + +# Don't inline f-strings that contain expressions that are guaranteed to split, e.g. because of a magic trailing comma +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +[a,] +}moreeeeeeeeeeeeeeeeeeee" # comment + +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +[a,] +}moreeeeeeeeeeeeeeeeeeee" # comment +) + +aaaaa[aaaaaaaaaaa] = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +[a,] +}moreeeeeeeeeeeeeeeeeeee" # comment + +aaaaa[aaaaaaaaaaa] = (f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +[a,] +}moreeeeeeeeeeeeeeeeeeee" # comment +) + +# Don't inline f-strings that contain commented expressions +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{[ + a # comment + ]}moreeeeeeeeeeeeeeeeeetest" # comment +) + +aaaaa[aaaaaaaaaaa] = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{[ + a # comment + ]}moreeeeeeeeeeeeeeeeeetest" # comment +) + +# Don't inline f-strings with multiline debug expressions or format specifiers +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{ + a=}moreeeeeeeeeeeeeeeeeetest" # comment +) + +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{a + + b=}moreeeeeeeeeeeeeeeeeetest" # comment +) + +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{a + =}moreeeeeeeeeeeeeeeeeetest" # comment +) + +aaaaa[aaaaaaaaaaa] = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{ + a=}moreeeeeeeeeeeeeeeeeetest" # comment +) + +aaaaa[aaaaaaaaaaa] = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{a + =}moreeeeeeeeeeeeeeeeeetest" # comment +) + +# This is not a multiline f-string even though it has a newline after the format specifier. +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{ + a:.3f + }moreeeeeeeeeeeeeeeeeetest" # comment + +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{ + a:.3f + }moreeeeeeeeeeeeeeeeeetest" # comment +) + +# The newline is only considered when it's a tripled-quoted f-string. +aaaaaaaaaaaaaaaaaa = f"""testeeeeeeeeeeeeeeeeeeeeeeeee{ + a:.3f + }moreeeeeeeeeeeeeeeeeetest""" # comment + +aaaaaaaaaaaaaaaaaa = ( + f"""testeeeeeeeeeeeeeeeeeeeeeeeee{ + a:.3f + }moreeeeeeeeeeeeeeeeeetest""" # comment +) + # Indentation # What should be the indentation? @@ -883,6 +967,79 @@ a = ( # Similar to the above example but this fits within the line length limit. a = f"test{expression}flatcan be {joined} togethereeeeeeeeeeeeeeeeeeeeeeeeeee" # inline +# The following test cases are adopted from implicit string concatenation but for a +# single f-string instead. + +# Don't inline f-strings that contain expressions that are guaranteed to split, e.g. because of a magic trailing comma +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + [ + a, + ] +}moreeeeeeeeeeeeeeeeeeee" # comment + +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + [ + a, + ] +}moreeeeeeeeeeeeeeeeeeee" # comment + +aaaaa[aaaaaaaaaaa] = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + [ + a, + ] +}moreeeeeeeeeeeeeeeeeeee" # comment + +aaaaa[aaaaaaaaaaa] = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + [ + a, + ] +}moreeeeeeeeeeeeeeeeeeee" # comment + +# Don't inline f-strings that contain commented expressions +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{ + [ + a # comment + ] +}moreeeeeeeeeeeeeeeeeetest" # comment + +aaaaa[aaaaaaaaaaa] = f"testeeeeeeeeeeeeeeeeeeeeeeeee{ + [ + a # comment + ] +}moreeeeeeeeeeeeeeeeeetest" # comment + +# Don't inline f-strings with multiline debug expressions or format specifiers +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{ + a=}moreeeeeeeeeeeeeeeeeetest" # comment + +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{a + + b=}moreeeeeeeeeeeeeeeeeetest" # comment + +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{a + =}moreeeeeeeeeeeeeeeeeetest" # comment + +aaaaa[aaaaaaaaaaa] = f"testeeeeeeeeeeeeeeeeeeeeeeeee{ + a=}moreeeeeeeeeeeeeeeeeetest" # comment + +aaaaa[aaaaaaaaaaa] = f"testeeeeeeeeeeeeeeeeeeeeeeeee{a + =}moreeeeeeeeeeeeeeeeeetest" # comment + +# This is not a multiline f-string even though it has a newline after the format specifier. +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{a:.3f}moreeeeeeeeeeeeeeeeeetest" # comment +) + +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeee{a:.3f}moreeeeeeeeeeeeeeeeeetest" # comment +) + +# The newline is only considered when it's a tripled-quoted f-string. +aaaaaaaaaaaaaaaaaa = f"""testeeeeeeeeeeeeeeeeeeeeeeeee{a:.3f + }moreeeeeeeeeeeeeeeeeetest""" # comment + +aaaaaaaaaaaaaaaaaa = f"""testeeeeeeeeeeeeeeeeeeeeeeeee{a:.3f + }moreeeeeeeeeeeeeeeeeetest""" # comment + # Indentation # What should be the indentation? @@ -1339,6 +1496,69 @@ a = ( } togethereeeeeeeeeeeeeeeeeeeeeeeeeee" ) # inline +# The following test cases are adopted from implicit string concatenation but for a +# single f-string instead. + +# Don't inline f-strings that contain expressions that are guaranteed to split, e.g. because of a magic trailing comma +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +[a,] +}moreeeeeeeeeeeeeeeeeeee" # comment + +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +[a,] +}moreeeeeeeeeeeeeeeeeeee" # comment + +aaaaa[aaaaaaaaaaa] = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +[a,] +}moreeeeeeeeeeeeeeeeeeee" # comment + +aaaaa[aaaaaaaaaaa] = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +[a,] +}moreeeeeeeeeeeeeeeeeeee" # comment + +# Don't inline f-strings that contain commented expressions +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{[ + a # comment + ]}moreeeeeeeeeeeeeeeeeetest" # comment + +aaaaa[aaaaaaaaaaa] = f"testeeeeeeeeeeeeeeeeeeeeeeeee{[ + a # comment + ]}moreeeeeeeeeeeeeeeeeetest" # comment + +# Don't inline f-strings with multiline debug expressions or format specifiers +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{ + a=}moreeeeeeeeeeeeeeeeeetest" # comment + +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{a + + b=}moreeeeeeeeeeeeeeeeeetest" # comment + +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{a + =}moreeeeeeeeeeeeeeeeeetest" # comment + +aaaaa[aaaaaaaaaaa] = f"testeeeeeeeeeeeeeeeeeeeeeeeee{ + a=}moreeeeeeeeeeeeeeeeeetest" # comment + +aaaaa[aaaaaaaaaaa] = f"testeeeeeeeeeeeeeeeeeeeeeeeee{a + =}moreeeeeeeeeeeeeeeeeetest" # comment + +# This is not a multiline f-string even though it has a newline after the format specifier. +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{ + a:.3f + }moreeeeeeeeeeeeeeeeeetest" # comment + +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{ + a:.3f + }moreeeeeeeeeeeeeeeeeetest" # comment + +# The newline is only considered when it's a tripled-quoted f-string. +aaaaaaaaaaaaaaaaaa = f"""testeeeeeeeeeeeeeeeeeeeeeeeee{ + a:.3f + }moreeeeeeeeeeeeeeeeeetest""" # comment + +aaaaaaaaaaaaaaaaaa = f"""testeeeeeeeeeeeeeeeeeeeeeeeee{ + a:.3f + }moreeeeeeeeeeeeeeeeeetest""" # comment + # Indentation # What should be the indentation? @@ -1815,7 +2035,7 @@ f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccc # Here, the f-string layout is flat but it exceeds the line length limit. This shouldn't # try the custom best fit layout because the f-string doesn't have any split points. -@@ -373,31 +399,25 @@ +@@ -373,60 +399,66 @@ # But, the following f-string does have a split point because of the multiline expression. aaaaaaaaaaaa["bbbbbbbbbbbbbbbb"] = f"aaaaaaaaaaaaaaaaaaa { @@ -1855,9 +2075,88 @@ f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccc -) # inline +a = f"test{expression}flatcan be {joined} togethereeeeeeeeeeeeeeeeeeeeeeeeeee" # inline - # Indentation + # The following test cases are adopted from implicit string concatenation but for a + # single f-string instead. -@@ -408,29 +428,29 @@ + # Don't inline f-strings that contain expressions that are guaranteed to split, e.g. because of a magic trailing comma + aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +-[a,] ++ [ ++ a, ++ ] + }moreeeeeeeeeeeeeeeeeeee" # comment + + aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +-[a,] ++ [ ++ a, ++ ] + }moreeeeeeeeeeeeeeeeeeee" # comment + + aaaaa[aaaaaaaaaaa] = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +-[a,] ++ [ ++ a, ++ ] + }moreeeeeeeeeeeeeeeeeeee" # comment + + aaaaa[aaaaaaaaaaa] = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +-[a,] ++ [ ++ a, ++ ] + }moreeeeeeeeeeeeeeeeeeee" # comment + + # Don't inline f-strings that contain commented expressions +-aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{[ ++aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{ ++ [ + a # comment +- ]}moreeeeeeeeeeeeeeeeeetest" # comment ++ ] ++}moreeeeeeeeeeeeeeeeeetest" # comment + +-aaaaa[aaaaaaaaaaa] = f"testeeeeeeeeeeeeeeeeeeeeeeeee{[ ++aaaaa[aaaaaaaaaaa] = f"testeeeeeeeeeeeeeeeeeeeeeeeee{ ++ [ + a # comment +- ]}moreeeeeeeeeeeeeeeeeetest" # comment ++ ] ++}moreeeeeeeeeeeeeeeeeetest" # comment + + # Don't inline f-strings with multiline debug expressions or format specifiers + aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{ +@@ -445,21 +477,19 @@ + =}moreeeeeeeeeeeeeeeeeetest" # comment + + # This is not a multiline f-string even though it has a newline after the format specifier. +-aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{ +- a:.3f +- }moreeeeeeeeeeeeeeeeeetest" # comment ++aaaaaaaaaaaaaaaaaa = ( ++ f"testeeeeeeeeeeeeeeeeeeeeeeeee{a:.3f}moreeeeeeeeeeeeeeeeeetest" # comment ++) + +-aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{ +- a:.3f +- }moreeeeeeeeeeeeeeeeeetest" # comment ++aaaaaaaaaaaaaaaaaa = ( ++ f"testeeeeeeeeeeeeeeeeeeeeeeeee{a:.3f}moreeeeeeeeeeeeeeeeeetest" # comment ++) + + # The newline is only considered when it's a tripled-quoted f-string. +-aaaaaaaaaaaaaaaaaa = f"""testeeeeeeeeeeeeeeeeeeeeeeeee{ +- a:.3f ++aaaaaaaaaaaaaaaaaa = f"""testeeeeeeeeeeeeeeeeeeeeeeeee{a:.3f + }moreeeeeeeeeeeeeeeeeetest""" # comment + +-aaaaaaaaaaaaaaaaaa = f"""testeeeeeeeeeeeeeeeeeeeeeeeee{ +- a:.3f ++aaaaaaaaaaaaaaaaaa = f"""testeeeeeeeeeeeeeeeeeeeeeeeee{a:.3f + }moreeeeeeeeeeeeeeeeeetest""" # comment + + # Indentation +@@ -471,29 +501,29 @@ if indent2: foo = f"""hello world hello { From 47bd32059ceb333f851389502ee6e68f311e38d9 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Tue, 26 Nov 2024 12:35:55 +0530 Subject: [PATCH 09/10] Add test cases where f-string is in other statement positions --- crates/ruff_python_ast/src/nodes.rs | 3 +- .../test/fixtures/ruff/expression/fstring.py | 143 ++++ .../src/other/arguments.rs | 11 +- .../format@expression__fstring.py.snap | 715 +++++++++++++++++- 4 files changed, 867 insertions(+), 5 deletions(-) diff --git a/crates/ruff_python_ast/src/nodes.rs b/crates/ruff_python_ast/src/nodes.rs index 7ceea8bfe8a3d..f210c08b70dec 100644 --- a/crates/ruff_python_ast/src/nodes.rs +++ b/crates/ruff_python_ast/src/nodes.rs @@ -1273,7 +1273,8 @@ impl FStringValue { matches!(self.inner, FStringValueInner::Concatenated(_)) } - /// Returns the [`FString`] if the value is made up of a single f-string, [`None`] otherwise. + /// Returns the single [`FString`] if the f-string isn't implicitly concatenated, [`None`] + /// otherwise. pub fn as_single(&self) -> Option<&FString> { match &self.inner { FStringValueInner::Single(FStringPart::FString(fstring)) => Some(fstring), 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 e6f64b17277f2..9da1aa403ee9d 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 @@ -485,6 +485,149 @@ }moreeeeeeeeeeeeeeeeeetest""" # comment ) +# Remove the parentheses here +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{[a, b, + # comment + ]}moee" # comment +) +# ... but not here because of the ownline comment +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{[a, b, + ]}moee" + # comment +) + +# F-strings in other positions + +if f"aaaaaaaaaaa {ttttteeeeeeeeest} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": pass + +if ( + f"aaaaaaaaaaa {ttttteeeeeeeeest} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + }" +): pass + +if f"aaaaaaaaaaa {ttttteeeeeeeeest} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": pass + +if f"aaaaaaaaaaa {ttttteeeeeeeeest} more { # comment + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": pass + +if f"aaaaaaaaaaa {[ttttteeeeeeeeest,]} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": + pass + +if ( + f"aaaaaaaaaaa {[ttttteeeeeeeeest,]} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + }" +): + pass + +if f"aaaaaaaaaaa {[ttttteeeeeeeeest,]} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": + pass + +# For loops +for a in f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeeeeee": + pass + +for a in f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee": + pass + +for a in f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee": + pass + +for a in ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeee" +): + pass + +# With statements +with f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeeeeee": + pass + +with f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee": + pass + +with f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee": + pass + +with ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeee" +): + pass + +# Assert statements +assert f"aaaaaaaaa{ + expression}bbbbbbbbbbbb", f"cccccccccc{ + expression}dddddddddd" + +assert f"aaaaaaaaa{expression}bbbbbbbbbbbb", f"cccccccccccccccc{ + expression}dddddddddddddddd" + +assert f"aaaaaaaaa{expression}bbbbbbbbbbbb", f"cccccccccccccccc{expression}dddddddddddddddd" + +assert f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{ + expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", f"ccccccc{expression}dddddddddd" + +assert f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", f"ccccccc{expression}dddddddddd" + +assert f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{ + expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", f"ccccccccccccccccccccc { + expression} dddddddddddddddddddddddddd" + +assert f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", f"cccccccccccccccccccccccccccccccc {expression} ddddddddddddddddddddddddddddddddddddd" + +# F-strings as a single argument to a call expression to test whether it's huggable or not. +call(f"{ + testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee +}") + +call(f"{ + testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee +}") + +call(f"{ # comment + testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee +}") + +call(f"""aaaaaaaaaaaaaaaa bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee}""") + +call(f"""aaaaaaaaaaaaaaaa bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + }""") + +call(f"""aaaaaaaaaaaaaaaa + bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + }""") + +call(f"""aaaaaaaaaaaaaaaa + bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee # comment + }""") + +call( + f"""aaaaaaaaaaaaaaaa + bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee # comment + }""" +) + +call(f"{ + aaaaaa + + '''test + more''' +}") + # Indentation # What should be the indentation? diff --git a/crates/ruff_python_formatter/src/other/arguments.rs b/crates/ruff_python_formatter/src/other/arguments.rs index f7eadfdb70cae..8dc079fce97ff 100644 --- a/crates/ruff_python_formatter/src/other/arguments.rs +++ b/crates/ruff_python_formatter/src/other/arguments.rs @@ -1,5 +1,5 @@ use ruff_formatter::{write, FormatContext}; -use ruff_python_ast::{ArgOrKeyword, Arguments, Expr, StringLike}; +use ruff_python_ast::{ArgOrKeyword, Arguments, Expr, StringFlags, StringLike}; use ruff_python_trivia::{PythonWhitespace, SimpleTokenKind, SimpleTokenizer}; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; @@ -137,6 +137,7 @@ fn is_single_argument_parenthesized(argument: &Expr, call_end: TextSize, source: false } + /// Returns `true` if the arguments can hug directly to the enclosing parentheses in the call, as /// in Black's `hug_parens_with_braces_and_square_brackets` preview style behavior. /// @@ -223,7 +224,13 @@ fn is_huggable_string_argument( arguments: &Arguments, context: &PyFormatContext, ) -> bool { - if string.is_implicit_concatenated() || !string.is_multiline(context) { + if string.is_implicit_concatenated() + || !string.is_multiline(context) + || !string + .parts() + .next() + .is_some_and(|part| part.flags().is_triple_quoted()) + { return false; } diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap index c503c10afa15f..d4665022334bc 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__fstring.py.snap @@ -491,6 +491,149 @@ aaaaaaaaaaaaaaaaaa = ( }moreeeeeeeeeeeeeeeeeetest""" # comment ) +# Remove the parentheses here +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{[a, b, + # comment + ]}moee" # comment +) +# ... but not here because of the ownline comment +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{[a, b, + ]}moee" + # comment +) + +# F-strings in other positions + +if f"aaaaaaaaaaa {ttttteeeeeeeeest} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": pass + +if ( + f"aaaaaaaaaaa {ttttteeeeeeeeest} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + }" +): pass + +if f"aaaaaaaaaaa {ttttteeeeeeeeest} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": pass + +if f"aaaaaaaaaaa {ttttteeeeeeeeest} more { # comment + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": pass + +if f"aaaaaaaaaaa {[ttttteeeeeeeeest,]} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": + pass + +if ( + f"aaaaaaaaaaa {[ttttteeeeeeeeest,]} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + }" +): + pass + +if f"aaaaaaaaaaa {[ttttteeeeeeeeest,]} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": + pass + +# For loops +for a in f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeeeeee": + pass + +for a in f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee": + pass + +for a in f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee": + pass + +for a in ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeee" +): + pass + +# With statements +with f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeeeeee": + pass + +with f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee": + pass + +with f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee": + pass + +with ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeee" +): + pass + +# Assert statements +assert f"aaaaaaaaa{ + expression}bbbbbbbbbbbb", f"cccccccccc{ + expression}dddddddddd" + +assert f"aaaaaaaaa{expression}bbbbbbbbbbbb", f"cccccccccccccccc{ + expression}dddddddddddddddd" + +assert f"aaaaaaaaa{expression}bbbbbbbbbbbb", f"cccccccccccccccc{expression}dddddddddddddddd" + +assert f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{ + expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", f"ccccccc{expression}dddddddddd" + +assert f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", f"ccccccc{expression}dddddddddd" + +assert f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{ + expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", f"ccccccccccccccccccccc { + expression} dddddddddddddddddddddddddd" + +assert f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", f"cccccccccccccccccccccccccccccccc {expression} ddddddddddddddddddddddddddddddddddddd" + +# F-strings as a single argument to a call expression to test whether it's huggable or not. +call(f"{ + testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee +}") + +call(f"{ + testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee +}") + +call(f"{ # comment + testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee +}") + +call(f"""aaaaaaaaaaaaaaaa bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee}""") + +call(f"""aaaaaaaaaaaaaaaa bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + }""") + +call(f"""aaaaaaaaaaaaaaaa + bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + }""") + +call(f"""aaaaaaaaaaaaaaaa + bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee # comment + }""") + +call( + f"""aaaaaaaaaaaaaaaa + bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee # comment + }""" +) + +call(f"{ + aaaaaa + + '''test + more''' +}") + # Indentation # What should be the indentation? @@ -1040,6 +1183,185 @@ aaaaaaaaaaaaaaaaaa = f"""testeeeeeeeeeeeeeeeeeeeeeeeee{a:.3f aaaaaaaaaaaaaaaaaa = f"""testeeeeeeeeeeeeeeeeeeeeeeeee{a:.3f }moreeeeeeeeeeeeeeeeeetest""" # comment +# Remove the parentheses here +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + [ + a, + b, + # comment + ] +}moee" # comment +# ... but not here because of the ownline comment +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + [ + a, + b, + ] + }moee" + # comment +) + +# F-strings in other positions + +if f"aaaaaaaaaaa {ttttteeeeeeeeest} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": + pass + +if f"aaaaaaaaaaa {ttttteeeeeeeeest} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": + pass + +if f"aaaaaaaaaaa {ttttteeeeeeeeest} more {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa}": + pass + +if f"aaaaaaaaaaa {ttttteeeeeeeeest} more { # comment + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": + pass + +if f"aaaaaaaaaaa { + [ + ttttteeeeeeeeest, + ] +} more {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa}": + pass + +if f"aaaaaaaaaaa { + [ + ttttteeeeeeeeest, + ] +} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": + pass + +if f"aaaaaaaaaaa { + [ + ttttteeeeeeeeest, + ] +} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": + pass + +# For loops +for a in f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeee": + pass + +for a in ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee" +): + pass + +for a in f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression +}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee": + pass + +for a in ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeee" +): + pass + +# With statements +with f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeee": + pass + +with ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee" +): + pass + +with f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression +}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee": + pass + +with ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeee" +): + pass + +# Assert statements +assert f"aaaaaaaaa{expression}bbbbbbbbbbbb", f"cccccccccc{expression}dddddddddd" + +assert f"aaaaaaaaa{expression}bbbbbbbbbbbb", f"cccccccccccccccc{ + expression +}dddddddddddddddd" + +assert f"aaaaaaaaa{expression}bbbbbbbbbbbb", ( + f"cccccccccccccccc{expression}dddddddddddddddd" +) + +assert f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{ + expression +}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", f"ccccccc{expression}dddddddddd" + +assert ( + f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" +), f"ccccccc{expression}dddddddddd" + +assert f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{ + expression +}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", f"ccccccccccccccccccccc { + expression +} dddddddddddddddddddddddddd" + +assert ( + f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" +), ( + f"cccccccccccccccccccccccccccccccc {expression} ddddddddddddddddddddddddddddddddddddd" +) + +# F-strings as a single argument to a call expression to test whether it's huggable or not. +call(f"{testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee}") + +call( + f"{testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee}" +) + +call( + f"{ # comment + testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + }" +) + +call( + f"""aaaaaaaaaaaaaaaa bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee}""" +) + +call( + f"""aaaaaaaaaaaaaaaa bbbbbbbbbb { + testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + }""" +) + +call(f"""aaaaaaaaaaaaaaaa + bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee}""") + +call(f"""aaaaaaaaaaaaaaaa + bbbbbbbbbb { + testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee # comment +}""") + +call( + f"""aaaaaaaaaaaaaaaa + bbbbbbbbbb { + testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee # comment + }""" +) + +call( + f"{ + aaaaaa + + '''test + more''' + }" +) + # Indentation # What should be the indentation? @@ -1559,6 +1881,167 @@ aaaaaaaaaaaaaaaaaa = f"""testeeeeeeeeeeeeeeeeeeeeeeeee{ a:.3f }moreeeeeeeeeeeeeeeeeetest""" # comment +# Remove the parentheses here +aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{[a, b, + # comment + ]}moee" # comment +# ... but not here because of the ownline comment +aaaaaaaaaaaaaaaaaa = ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{[a, b, + ]}moee" + # comment +) + +# F-strings in other positions + +if f"aaaaaaaaaaa {ttttteeeeeeeeest} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": + pass + +if f"aaaaaaaaaaa {ttttteeeeeeeeest} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + }": + pass + +if f"aaaaaaaaaaa {ttttteeeeeeeeest} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": + pass + +if f"aaaaaaaaaaa {ttttteeeeeeeeest} more { # comment + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": + pass + +if f"aaaaaaaaaaa {[ttttteeeeeeeeest,]} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": + pass + +if f"aaaaaaaaaaa {[ttttteeeeeeeeest,]} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + }": + pass + +if f"aaaaaaaaaaa {[ttttteeeeeeeeest,]} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +}": + pass + +# For loops +for a in f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeeeeee": + pass + +for a in ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee" +): + pass + +for a in f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee": + pass + +for a in ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeee" +): + pass + +# With statements +with f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeeeeee": + pass + +with ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee" +): + pass + +with f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ + expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee": + pass + +with ( + f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeee" +): + pass + +# Assert statements +assert f"aaaaaaaaa{ + expression}bbbbbbbbbbbb", f"cccccccccc{ + expression}dddddddddd" + +assert f"aaaaaaaaa{expression}bbbbbbbbbbbb", f"cccccccccccccccc{ + expression}dddddddddddddddd" + +assert ( + f"aaaaaaaaa{expression}bbbbbbbbbbbb" +), f"cccccccccccccccc{expression}dddddddddddddddd" + +assert f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{ + expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", ( + f"ccccccc{expression}dddddddddd" +) + +assert ( + f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" +), f"ccccccc{expression}dddddddddd" + +assert f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{ + expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", f"ccccccccccccccccccccc { + expression} dddddddddddddddddddddddddd" + +assert f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", f"cccccccccccccccccccccccccccccccc {expression} ddddddddddddddddddddddddddddddddddddd" + +# F-strings as a single argument to a call expression to test whether it's huggable or not. +call( + f"{ + testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee +}" +) + +call( + f"{ + testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee +}" +) + +call( + f"{ # comment + testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee +}" +) + +call( + f"""aaaaaaaaaaaaaaaa bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee}""" +) + +call(f"""aaaaaaaaaaaaaaaa bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + }""") + +call(f"""aaaaaaaaaaaaaaaa + bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + }""") + +call(f"""aaaaaaaaaaaaaaaa + bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee # comment + }""") + +call( + f"""aaaaaaaaaaaaaaaa + bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee # comment + }""" +) + +call( + f"{ + aaaaaa + + '''test + more''' +}" +) + # Indentation # What should be the indentation? @@ -2126,7 +2609,7 @@ f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccc # Don't inline f-strings with multiline debug expressions or format specifiers aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeee{ -@@ -445,21 +477,19 @@ +@@ -445,31 +477,37 @@ =}moreeeeeeeeeeeeeeeeeetest" # comment # This is not a multiline f-string even though it has a newline after the format specifier. @@ -2155,8 +2638,236 @@ f"aaaaaaaaaaaaaaaaaaaaaaaaaa {10**27} bbbbbbbbbbbbbbbbbbbbbbbbbb ccccccccccccccc +aaaaaaaaaaaaaaaaaa = f"""testeeeeeeeeeeeeeeeeeeeeeeeee{a:.3f }moreeeeeeeeeeeeeeeeeetest""" # comment + # Remove the parentheses here +-aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{[a, b, +- # comment +- ]}moee" # comment ++aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ ++ [ ++ a, ++ b, ++ # comment ++ ] ++}moee" # comment + # ... but not here because of the ownline comment + aaaaaaaaaaaaaaaaaa = ( +- f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{[a, b, +- ]}moee" ++ f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ ++ [ ++ a, ++ b, ++ ] ++ }moee" + # comment + ) + +@@ -481,13 +519,11 @@ + pass + + if f"aaaaaaaaaaa {ttttteeeeeeeeest} more { +- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +- }": ++ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ++}": + pass + +-if f"aaaaaaaaaaa {ttttteeeeeeeeest} more { +- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +-}": ++if f"aaaaaaaaaaa {ttttteeeeeeeeest} more {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa}": + pass + + if f"aaaaaaaaaaa {ttttteeeeeeeeest} more { # comment +@@ -495,24 +531,33 @@ + }": + pass + +-if f"aaaaaaaaaaa {[ttttteeeeeeeeest,]} more { +- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +-}": ++if f"aaaaaaaaaaa { ++ [ ++ ttttteeeeeeeeest, ++ ] ++} more {aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa}": + pass + +-if f"aaaaaaaaaaa {[ttttteeeeeeeeest,]} more { +- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +- }": ++if f"aaaaaaaaaaa { ++ [ ++ ttttteeeeeeeeest, ++ ] ++} more { ++ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ++}": + pass + +-if f"aaaaaaaaaaa {[ttttteeeeeeeeest,]} more { ++if f"aaaaaaaaaaa { ++ [ ++ ttttteeeeeeeeest, ++ ] ++} more { + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + }": + pass + + # For loops +-for a in f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +- expression}moreeeeeeeeeeeeeeeeeeee": ++for a in f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeee": + pass + + for a in ( +@@ -521,7 +566,8 @@ + pass + + for a in f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +- expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee": ++ expression ++}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee": + pass + + for a in ( +@@ -530,8 +576,7 @@ + pass + + # With statements +-with f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +- expression}moreeeeeeeeeeeeeeeeeeee": ++with f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{expression}moreeeeeeeeeeeeeeeeeeee": + pass + + with ( +@@ -540,7 +585,8 @@ + pass + + with f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{ +- expression}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee": ++ expression ++}moreeeeeeeeeeeeeeeeeeeeeeeeeeeeee": + pass + + with ( +@@ -549,78 +595,80 @@ + pass + + # Assert statements +-assert f"aaaaaaaaa{ +- expression}bbbbbbbbbbbb", f"cccccccccc{ +- expression}dddddddddd" ++assert f"aaaaaaaaa{expression}bbbbbbbbbbbb", f"cccccccccc{expression}dddddddddd" + + assert f"aaaaaaaaa{expression}bbbbbbbbbbbb", f"cccccccccccccccc{ +- expression}dddddddddddddddd" ++ expression ++}dddddddddddddddd" + +-assert ( +- f"aaaaaaaaa{expression}bbbbbbbbbbbb" +-), f"cccccccccccccccc{expression}dddddddddddddddd" ++assert f"aaaaaaaaa{expression}bbbbbbbbbbbb", ( ++ f"cccccccccccccccc{expression}dddddddddddddddd" ++) + + assert f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{ +- expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", ( +- f"ccccccc{expression}dddddddddd" +-) ++ expression ++}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", f"ccccccc{expression}dddddddddd" + + assert ( + f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + ), f"ccccccc{expression}dddddddddd" + + assert f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{ +- expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", f"ccccccccccccccccccccc { +- expression} dddddddddddddddddddddddddd" ++ expression ++}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", f"ccccccccccccccccccccc { ++ expression ++} dddddddddddddddddddddddddd" + +-assert f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", f"cccccccccccccccccccccccccccccccc {expression} ddddddddddddddddddddddddddddddddddddd" ++assert ( ++ f"aaaaaaaaaaaaaaaaaaaaaaaaaaa{expression}bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" ++), ( ++ f"cccccccccccccccccccccccccccccccc {expression} ddddddddddddddddddddddddddddddddddddd" ++) + + # F-strings as a single argument to a call expression to test whether it's huggable or not. +-call( +- f"{ +- testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee +-}" +-) ++call(f"{testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee}") + + call( +- f"{ +- testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee +-}" ++ f"{testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee}" + ) + + call( + f"{ # comment +- testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee +-}" ++ testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee ++ }" + ) + + call( + f"""aaaaaaaaaaaaaaaa bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee}""" + ) + +-call(f"""aaaaaaaaaaaaaaaa bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee +- }""") ++call( ++ f"""aaaaaaaaaaaaaaaa bbbbbbbbbb { ++ testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee ++ }""" ++) + + call(f"""aaaaaaaaaaaaaaaa +- bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee +- }""") ++ bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee}""") + + call(f"""aaaaaaaaaaaaaaaa +- bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee # comment +- }""") ++ bbbbbbbbbb { ++ testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee # comment ++}""") + + call( + f"""aaaaaaaaaaaaaaaa +- bbbbbbbbbb {testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee # comment +- }""" ++ bbbbbbbbbb { ++ testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee # comment ++ }""" + ) + + call( + f"{ +- aaaaaa +- + '''test ++ aaaaaa ++ + '''test + more''' +-}" ++ }" + ) + # Indentation -@@ -471,29 +501,29 @@ +@@ -632,29 +680,29 @@ if indent2: foo = f"""hello world hello { From 0587004ce997957b5834351946799acd64071470 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Tue, 26 Nov 2024 12:44:35 +0530 Subject: [PATCH 10/10] Consider debug text for `is_multline` --- crates/ruff_python_formatter/src/string/mod.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/ruff_python_formatter/src/string/mod.rs b/crates/ruff_python_formatter/src/string/mod.rs index f1141161d464c..cc6cc7766cb90 100644 --- a/crates/ruff_python_formatter/src/string/mod.rs +++ b/crates/ruff_python_formatter/src/string/mod.rs @@ -1,3 +1,4 @@ +use memchr::memchr2; pub(crate) use normalize::{normalize_string, NormalizedString, StringNormalizer}; use ruff_python_ast::str::Quote; use ruff_python_ast::StringLikePart; @@ -134,13 +135,19 @@ impl StringLikeExtensions for ast::StringLike<'_> { || expression.format_spec.as_deref().is_some_and(|spec| { contains_line_break_or_comments(&spec.elements, context) }) + || expression.debug_text.as_ref().is_some_and(|debug_text| { + memchr2(b'\n', b'\r', debug_text.leading.as_bytes()) + .is_some() + || memchr2(b'\n', b'\r', debug_text.trailing.as_bytes()) + .is_some() + }) } else { // Multiline f-string expressions can't be joined if the f-string // formatting is disabled because the string gets inserted in // verbatim preserving the newlines. // - // We don't need to check format specifiers here because the - // expression range already includes them. + // We don't need to check format specifiers or debug text here + // because the expression range already includes them. context.source().contains_line_break(expression.range()) } }