Fix f-string formatting in assignment statement (#14454)

## Summary

fixes: #13813

This PR fixes a bug in the formatting assignment statement when the
value is an f-string.

This is resolved by using custom best fit layouts if the f-string is (a)
not already a flat f-string (thus, cannot be multiline) and (b) is not a
multiline string (thus, cannot be flattened). So, it is used in cases
like the following:
```py
aaaaaaaaaaaaaaaaaa = f"testeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee{
    expression}moreeeeeeeeeeeeeeeee"
```
Which is (a) `FStringLayout::Multiline` and (b) not a multiline.

There are various other examples in the PR diff along with additional
explanation and context as code comments.

## Test Plan

Add multiple test cases for various scenarios.
This commit is contained in:
Dhruv Manilawala 2024-11-26 15:07:18 +05:30 committed by GitHub
parent e4cefd9bf9
commit f3dac27e9a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 2184 additions and 74 deletions

View file

@ -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)

View file

@ -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

View file

@ -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)

View file

@ -4,10 +4,12 @@ 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::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,11 @@ 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)
|| self.value.as_single().is_some_and(|f_string| {
FStringLayout::from_f_string(f_string, context.source()).is_multiline()
})
{
OptionalParentheses::Never
} else {
OptionalParentheses::BestFit

View file

@ -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