More in progress work

crates/ruff_formatter/src/builders.rs

@@ -1454,7 +1454,7 @@ impl<Context> std::fmt::Debug for Group<'_, Context> {
 /// layout doesn't exceed the line width too, in which case it falls back to the flat layout.
 ///
 /// This IR is identical to the following [`best_fitting`] layout but is implemented as custom IR for
-/// best performance.
+/// better performance.
 ///
 /// ```rust
 /// # use ruff_formatter::prelude::*;

crates/ruff_python_formatter/src/expression/expr_bytes_literal.rs

@@ -1,3 +1,5 @@
+use ruff_formatter::FormatRuleWithOptions;
+use ruff_formatter::GroupId;
 use ruff_python_ast::AnyNodeRef;
 use ruff_python_ast::ExprBytesLiteral;
 
@@ -8,15 +10,38 @@ use crate::prelude::*;
 use crate::string::{AnyString, FormatImplicitConcatenatedString};
 
 #[derive(Default)]
-pub struct FormatExprBytesLiteral;
+pub struct FormatExprBytesLiteral {
+    layout: ExprBytesLiteralLayout,
+}
+
+#[derive(Default)]
+pub struct ExprBytesLiteralLayout {
+    pub implicit_group_id: Option<GroupId>,
+}
+
+impl FormatRuleWithOptions<ExprBytesLiteral, PyFormatContext<'_>> for FormatExprBytesLiteral {
+    type Options = ExprBytesLiteralLayout;
+
+    fn with_options(mut self, options: Self::Options) -> Self {
+        self.layout = options;
+        self
+    }
+}
 
 impl FormatNodeRule<ExprBytesLiteral> for FormatExprBytesLiteral {
     fn fmt_fields(&self, item: &ExprBytesLiteral, f: &mut PyFormatter) -> FormatResult<()> {
         let ExprBytesLiteral { value, .. } = item;
 
         match value.as_slice() {
             [bytes_literal] => bytes_literal.format().fmt(f),
-            _ => in_parentheses_only_group(&FormatImplicitConcatenatedString::new(item)).fmt(f),
+            _ => match self.layout.implicit_group_id {
+                Some(group_id) => group(&FormatImplicitConcatenatedString::new(item))
+                    .with_group_id(Some(group_id))
+                    .fmt(f),
+                None => {
+                    in_parentheses_only_group(&FormatImplicitConcatenatedString::new(item)).fmt(f)
+                }
+            },
         }
     }
 }

crates/ruff_python_formatter/src/expression/expr_f_string.rs

@@ -1,3 +1,4 @@
+use ruff_formatter::{FormatRuleWithOptions, GroupId};
 use ruff_python_ast::{AnyNodeRef, ExprFString};
 use ruff_source_file::Locator;
 use ruff_text_size::Ranged;
@@ -10,7 +11,23 @@ use crate::prelude::*;
 use crate::string::{AnyString, FormatImplicitConcatenatedString, Quoting};
 
 #[derive(Default)]
-pub struct FormatExprFString;
+pub struct FormatExprFString {
+    layout: ExprFStringLayout,
+}
+
+#[derive(Default)]
+pub struct ExprFStringLayout {
+    pub implicit_group_id: Option<GroupId>,
+}
+
+impl FormatRuleWithOptions<ExprFString, PyFormatContext<'_>> for FormatExprFString {
+    type Options = ExprFStringLayout;
+
+    fn with_options(mut self, options: Self::Options) -> Self {
+        self.layout = options;
+        self
+    }
+}
 
 impl FormatNodeRule<ExprFString> for FormatExprFString {
     fn fmt_fields(&self, item: &ExprFString, f: &mut PyFormatter) -> FormatResult<()> {
@@ -21,7 +38,14 @@ impl FormatNodeRule<ExprFString> for FormatExprFString {
                 FormatFStringPart::new(f_string_part, f_string_quoting(item, f.context().locator()))
                     .fmt(f)
             }
-            _ => in_parentheses_only_group(&FormatImplicitConcatenatedString::new(item)).fmt(f),
+            _ => match self.layout.implicit_group_id {
+                Some(group_id) => group(&FormatImplicitConcatenatedString::new(item))
+                    .with_group_id(Some(group_id))
+                    .fmt(f),
+                None => {
+                    in_parentheses_only_group(&FormatImplicitConcatenatedString::new(item)).fmt(f)
+                }
+            },
         }
     }
 }
@@ -32,8 +56,9 @@ impl NeedsParentheses for ExprFString {
         _parent: AnyNodeRef,
         context: &PyFormatContext,
     ) -> OptionalParentheses {
-        if self.value.is_implicit_concatenated() {
-            OptionalParentheses::Multiline
+        // if self.value.is_implicit_concatenated() {
+        //     OptionalParentheses::Multiline
+        // } else
         // 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
@@ -52,7 +77,9 @@ impl NeedsParentheses for ExprFString {
         //   ```
         // This isn't decided yet, refer to the relevant discussion:
         // https://github.com/astral-sh/ruff/discussions/9785
-        } else if AnyString::FString(self).is_multiline(context.source()) {
+        if !self.value.is_implicit_concatenated()
+            && AnyString::FString(self).is_multiline(context.source())
+        {
             OptionalParentheses::Never
         } else {
             OptionalParentheses::BestFit

crates/ruff_python_formatter/src/expression/expr_string_literal.rs

@@ -1,4 +1,4 @@
-use ruff_formatter::FormatRuleWithOptions;
+use ruff_formatter::{FormatRuleWithOptions, GroupId};
 use ruff_python_ast::{AnyNodeRef, ExprStringLiteral};
 
 use crate::expression::parentheses::{
@@ -10,14 +10,29 @@ use crate::string::{AnyString, FormatImplicitConcatenatedString};
 
 #[derive(Default)]
 pub struct FormatExprStringLiteral {
-    kind: StringLiteralKind,
+    layout: ExprStringLiteralLayout,
+}
+
+#[derive(Default)]
+pub struct ExprStringLiteralLayout {
+    pub kind: StringLiteralKind,
+    pub implicit_group_id: Option<GroupId>,
+}
+
+impl ExprStringLiteralLayout {
+    pub const fn docstring() -> Self {
+        Self {
+            kind: StringLiteralKind::Docstring,
+            implicit_group_id: None,
+        }
+    }
 }
 
 impl FormatRuleWithOptions<ExprStringLiteral, PyFormatContext<'_>> for FormatExprStringLiteral {
-    type Options = StringLiteralKind;
+    type Options = ExprStringLiteralLayout;
 
     fn with_options(mut self, options: Self::Options) -> Self {
-        self.kind = options;
+        self.layout = options;
         self
     }
 }
@@ -27,15 +42,23 @@ impl FormatNodeRule<ExprStringLiteral> for FormatExprStringLiteral {
         let ExprStringLiteral { value, .. } = item;
 
         match value.as_slice() {
-            [string_literal] => string_literal.format().with_options(self.kind).fmt(f),
+            [string_literal] => string_literal
+                .format()
+                .with_options(self.layout.kind)
+                .fmt(f),
             _ => {
                 // This is just a sanity check because [`DocstringStmt::try_from_statement`]
                 // ensures that the docstring is a *single* string literal.
-                assert!(!self.kind.is_docstring());
+                assert!(!self.layout.kind.is_docstring());
 
-                in_parentheses_only_group(&FormatImplicitConcatenatedString::new(item))
+                match self.layout.implicit_group_id {
+                    Some(group_id) => group(&FormatImplicitConcatenatedString::new(item))
+                        .with_group_id(Some(group_id))
+                        .fmt(f),
+                    None => in_parentheses_only_group(&FormatImplicitConcatenatedString::new(item))
+                        .fmt(f),
+                }
             }
-            .fmt(f),
         }
     }
 }
@@ -48,9 +71,10 @@ impl NeedsParentheses for ExprStringLiteral {
     ) -> OptionalParentheses {
         // TODO: This is complicated. We should only return multiline if we *know* that
         //   it won't fit because of how assignment works.
-        if self.value.is_implicit_concatenated() {
-            OptionalParentheses::Multiline
-        } else if AnyString::String(self).is_multiline(context.source()) {
+        // if self.value.is_implicit_concatenated() {
+        //     OptionalParentheses::Multiline
+        // } else
+        if AnyString::String(self).is_multiline(context.source()) {
             OptionalParentheses::Never
         } else {
             OptionalParentheses::BestFit

crates/ruff_python_formatter/src/expression/mod.rs

@@ -768,15 +768,18 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> {
             Expr::StringLiteral(ast::ExprStringLiteral { value, .. })
                 if value.is_implicit_concatenated() =>
             {
-                self.update_max_precedence(OperatorPrecedence::String);
+                // FIXME make this a preview only change
+                // self.update_max_precedence(OperatorPrecedence::String);
+                return;
             }
             Expr::BytesLiteral(ast::ExprBytesLiteral { value, .. })
                 if value.is_implicit_concatenated() =>
             {
-                self.update_max_precedence(OperatorPrecedence::String);
+                // self.update_max_precedence(OperatorPrecedence::String);
+                return;
             }
             Expr::FString(ast::ExprFString { value, .. }) if value.is_implicit_concatenated() => {
-                self.update_max_precedence(OperatorPrecedence::String);
+                // self.update_max_precedence(OperatorPrecedence::String);
                 return;
             }
 
@@ -1255,7 +1258,7 @@ pub(crate) fn is_splittable_expression(expr: &Expr, context: &PyFormatContext) -
 
         // String like literals can expand if they are implicit concatenated.
         Expr::FString(fstring) => fstring.value.is_implicit_concatenated(),
-        Expr::StringLiteral(string) => string.value.is_implicit_concatenated(),
+        Expr::StringLiteral(string) => false,
         Expr::BytesLiteral(bytes) => bytes.value.is_implicit_concatenated(),
 
         // Expressions that have no split points per se, but they contain nested sub expressions that might expand.

crates/ruff_python_formatter/src/other/arguments.rs

@@ -223,6 +223,8 @@ fn is_huggable_string_argument(
     arguments: &Arguments,
     context: &PyFormatContext,
 ) -> bool {
+    // TODO: Implicit concatenated could become regular string. Although not if it is multiline. So that should be fine?
+    //   Double check
     if string.is_implicit_concatenated() || !string.is_multiline(context.source()) {
         return false;
     }

crates/ruff_python_formatter/src/pattern/mod.rs

@@ -289,18 +289,22 @@ impl<'a> CanOmitOptionalParenthesesVisitor<'a> {
 
             Pattern::MatchValue(value) => match &*value.value {
                 Expr::StringLiteral(string) => {
-                    self.update_max_precedence(OperatorPrecedence::String, string.value.len());
+                    // TODO update?
+
+                    // self.update_max_precedence(OperatorPrecedence::String, string.value.len());
                 }
                 Expr::BytesLiteral(bytes) => {
-                    self.update_max_precedence(OperatorPrecedence::String, bytes.value.len());
+                    // TODO update?
+                    // self.update_max_precedence(OperatorPrecedence::String, bytes.value.len());
                 }
                 // F-strings are allowed according to python's grammar but fail with a syntax error at runtime.
                 // That's why we need to support them for formatting.
                 Expr::FString(string) => {
-                    self.update_max_precedence(
-                        OperatorPrecedence::String,
-                        string.value.as_slice().len(),
-                    );
+                    // TODO update?
+                    // self.update_max_precedence(
+                    //     OperatorPrecedence::String,
+                    //     string.value.as_slice().len(),
+                    // );
                 }
 
                 Expr::NumberLiteral(_) | Expr::Attribute(_) | Expr::UnaryOp(_) => {