Maybe parenthesize long constants and names

crates/ruff_formatter/src/builders.rs

@@ -2534,17 +2534,17 @@ impl<'a, Context> BestFitting<'a, Context> {
 
 impl<Context> Format<Context> for BestFitting<'_, Context> {
     fn fmt(&self, f: &mut Formatter<Context>) -> FormatResult<()> {
-        let mut buffer = VecBuffer::new(f.state_mut());
         let variants = self.variants.items();
 
         let mut formatted_variants = Vec::with_capacity(variants.len());
 
         for variant in variants {
+            let mut buffer = VecBuffer::with_capacity(8, f.state_mut());
             buffer.write_element(FormatElement::Tag(StartEntry));
             buffer.write_fmt(Arguments::from(variant))?;
             buffer.write_element(FormatElement::Tag(EndEntry));
 
-            formatted_variants.push(buffer.take_vec().into_boxed_slice());
+            formatted_variants.push(buffer.into_vec().into_boxed_slice());
         }
 
         // SAFETY: The constructor guarantees that there are always at least two variants. It's, therefore,

crates/ruff_formatter/src/format_element/document.rs

@@ -122,8 +122,12 @@ impl Document {
             expands
         }
 
-        let mut enclosing: Vec<Enclosing> = Vec::new();
-        let mut interned: FxHashMap<&Interned, bool> = FxHashMap::default();
+        let mut enclosing = Vec::with_capacity(if self.is_empty() {
+            0
+        } else {
+            self.len().ilog2() as usize
+        });
+        let mut interned = FxHashMap::default();
         propagate_expands(self, &mut enclosing, &mut interned);
     }
 

crates/ruff_python_formatter/Cargo.toml

@@ -25,6 +25,7 @@ clap = { workspace = true }
 countme = "3.0.1"
 is-macro = { workspace = true }
 itertools = { workspace = true }
+memchr = { workspace = true }
 once_cell = { workspace = true }
 rustc-hash = { workspace = true }
 serde = { workspace = true, optional = true }

crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unary.py

@@ -140,3 +140,9 @@
 # Regression: https://github.com/astral-sh/ruff/issues/5338
 if a and not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa & aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
     ...
+
+if (
+  not
+  # comment
+  a):
+    ...

crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unsplittable.py

@@ -0,0 +1,54 @@
+x = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+
+x_aa = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+xxxxx = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+
+while (
+    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+):
+    pass
+
+while aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
+    pass
+
+# Only applies in `Parenthesize::IfBreaks` positions
+raise aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+
+raise (
+    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+)
+
+raise a from aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+raise a from aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+
+# Can never apply on expression statement level
+aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+
+# Is it only relevant for items that can't break
+
+aaaaaaa = 111111111111111111111111111111111111111111111111111111111111111111111111111111
+aaaaaaa = (
+    1111111111111111111111111111111111111111111111111111111111111111111111111111111
+)
+
+aaaaaaa = """111111111111111111111111111111111111111111111111111111111111111111111111111
+1111111111111111111111111111111111111111111111111111111111111111111111111111111111111"""
+
+# Never parenthesize multiline strings
+aaaaaaa = (
+    """1111111111111111111111111111111111111111111111111111111111111111111111111111
+1111111111111111111111111111111111111111111111111111111111111111111111111111111111111"""
+)
+
+
+
+aaaaaaaa = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbb
+aaaaaaaa = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
+
+aaaaaaaa = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
+
+
+for converter in connection.ops.get_db_converters(
+    expression
+) + expression.get_db_converters(connection):
+    ...

crates/ruff_python_formatter/src/expression/expr_call.rs

@@ -69,7 +69,10 @@ impl NeedsParentheses for ExprCall {
         {
             OptionalParentheses::Multiline
         } else {
-            self.func.needs_parentheses(self.into(), context)
+            match self.func.needs_parentheses(self.into(), context) {
+                OptionalParentheses::BestFit => OptionalParentheses::Never,
+                parentheses => parentheses,
+            }
         }
     }
 }

crates/ruff_python_formatter/src/expression/expr_constant.rs

@@ -1,5 +1,5 @@
 use crate::comments::SourceComment;
-use ruff_formatter::FormatRuleWithOptions;
+use ruff_formatter::{FormatContext, FormatOptions, FormatRuleWithOptions};
 use ruff_python_ast::node::AnyNodeRef;
 use ruff_python_ast::{Constant, ExprConstant, Ranged};
 use ruff_text_size::{TextLen, TextRange};
@@ -79,15 +79,22 @@ impl NeedsParentheses for ExprConstant {
         _parent: AnyNodeRef,
         context: &PyFormatContext,
     ) -> OptionalParentheses {
-        if self.value.is_implicit_concatenated() {
-            // Don't wrap triple quoted strings
-            if is_multiline_string(self, context.source()) {
-                OptionalParentheses::Never
+        if is_multiline_string(self, context.source())
+            || self.value.is_none()
+            || self.value.is_bool()
+            || self.value.is_ellipsis()
+        {
+            OptionalParentheses::Never
+        } else if self.value.is_implicit_concatenated() {
+            OptionalParentheses::Multiline
+        } else {
+            let text_len = context.source()[self.range].len();
+
+            if text_len > 4 && text_len < context.options().line_width().value() as usize {
+                OptionalParentheses::BestFit
             } else {
-                OptionalParentheses::Multiline
+                OptionalParentheses::Never
             }
-        } else {
-            OptionalParentheses::Never
         }
     }
 }
@@ -99,7 +106,8 @@ pub(super) fn is_multiline_string(constant: &ExprConstant, source: &str) -> bool
         let quotes =
             StringQuotes::parse(&contents[TextRange::new(prefix.text_len(), contents.text_len())]);
 
-        quotes.is_some_and(StringQuotes::is_triple) && contents.contains(['\n', '\r'])
+        quotes.is_some_and(StringQuotes::is_triple)
+            && memchr::memchr2(b'\n', b'\r', contents.as_bytes()).is_some()
     } else {
         false
     }

crates/ruff_python_formatter/src/expression/expr_f_string.rs

@@ -1,9 +1,11 @@
 use super::string::{AnyString, FormatString};
 use crate::context::PyFormatContext;
+use memchr::memchr2;
+
 use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
 use crate::prelude::*;
 use crate::{FormatNodeRule, PyFormatter};
-use ruff_formatter::FormatResult;
+use ruff_formatter::{FormatContext, FormatOptions, FormatResult};
 use ruff_python_ast::node::AnyNodeRef;
 use ruff_python_ast::ExprFString;
 
@@ -20,8 +22,21 @@ impl NeedsParentheses for ExprFString {
     fn needs_parentheses(
         &self,
         _parent: AnyNodeRef,
-        _context: &PyFormatContext,
+        context: &PyFormatContext,
     ) -> OptionalParentheses {
-        OptionalParentheses::Multiline
+        if self.implicit_concatenated {
+            OptionalParentheses::Multiline
+        } else {
+            let text = &context.source()[self.range];
+
+            if memchr2(b'\n', b'\r', text.as_bytes()).is_none()
+                && text.len() > 4
+                && text.len() < context.options().line_width().value() as usize
+            {
+                OptionalParentheses::BestFit
+            } else {
+                OptionalParentheses::Never
+            }
+        }
     }
 }

crates/ruff_python_formatter/src/expression/expr_name.rs

@@ -2,7 +2,7 @@ use crate::comments::SourceComment;
 use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
 use crate::prelude::*;
 use crate::FormatNodeRule;
-use ruff_formatter::{write, FormatContext};
+use ruff_formatter::{write, FormatContext, FormatOptions};
 use ruff_python_ast::node::AnyNodeRef;
 use ruff_python_ast::ExprName;
 
@@ -38,9 +38,14 @@ impl NeedsParentheses for ExprName {
     fn needs_parentheses(
         &self,
         _parent: AnyNodeRef,
-        _context: &PyFormatContext,
+        context: &PyFormatContext,
     ) -> OptionalParentheses {
-        OptionalParentheses::Never
+        let text_len = context.source()[self.range].len();
+        if text_len > 4 && text_len < context.options().line_width().value() as usize {
+            OptionalParentheses::BestFit
+        } else {
+            OptionalParentheses::Never
+        }
     }
 }
 

crates/ruff_python_formatter/src/expression/expr_subscript.rs

@@ -101,7 +101,10 @@ impl NeedsParentheses for ExprSubscript {
             {
                 OptionalParentheses::Multiline
             } else {
-                self.value.needs_parentheses(self.into(), context)
+                match self.value.needs_parentheses(self.into(), context) {
+                    OptionalParentheses::BestFit => OptionalParentheses::Never,
+                    parentheses => parentheses,
+                }
             }
         }
     }

crates/ruff_python_formatter/src/expression/expr_unary_op.rs

@@ -3,7 +3,7 @@ use ruff_python_ast::{ExprUnaryOp, Ranged};
 use ruff_text_size::{TextLen, TextRange};
 
 use ruff_formatter::prelude::{hard_line_break, space, text};
-use ruff_formatter::{Format, FormatContext, FormatResult};
+use ruff_formatter::{Format, FormatResult};
 use ruff_python_ast::node::AnyNodeRef;
 use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
 
@@ -57,7 +57,7 @@ impl FormatNodeRule<ExprUnaryOp> for FormatExprUnaryOp {
         //  a)
         // ```
         if !leading_operand_comments.is_empty()
-            && !is_operand_parenthesized(item, f.context().source_code().as_str())
+            && !is_operand_parenthesized(item, f.context().source())
         {
             hard_line_break().fmt(f)?;
         } else if op.is_not() {

crates/ruff_python_formatter/src/expression/mod.rs

@@ -1,7 +1,7 @@
 use std::cmp::Ordering;
 
 use ruff_formatter::{
-    write, FormatOwnedWithRule, FormatRefWithRule, FormatRule, FormatRuleWithOptions,
+    format_args, write, FormatOwnedWithRule, FormatRefWithRule, FormatRule, FormatRuleWithOptions,
 };
 use ruff_python_ast as ast;
 use ruff_python_ast::node::AnyNodeRef;
@@ -220,6 +220,64 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
                     }
                 }
             },
+            OptionalParentheses::BestFit => match parenthesize {
+                Parenthesize::IfBreaksOrIfRequired => {
+                    parenthesize_if_expands(&expression.format().with_options(Parentheses::Never))
+                        .fmt(f)
+                }
+
+                Parenthesize::Optional | Parenthesize::IfRequired => {
+                    expression.format().with_options(Parentheses::Never).fmt(f)
+                }
+                Parenthesize::IfBreaks => {
+                    let group_id = f.group_id("optional_parentheses");
+                    let f = &mut WithNodeLevel::new(NodeLevel::Expression(Some(group_id)), f);
+                    let mut format_expression = expression
+                        .format()
+                        .with_options(Parentheses::Never)
+                        .memoized();
+
+                    // Don't use best fitting if it is known that the expression can never fit
+                    if format_expression.inspect(f)?.will_break() {
+                        // The group here is necessary because `format_expression` may contain IR elements
+                        // that refer to the group id
+                        group(&format_expression)
+                            .with_group_id(Some(group_id))
+                            .should_expand(true)
+                            .fmt(f)
+                    } else {
+                        // Only add parentheses if it makes the expression fit on the line.
+                        // Using the flat version as the most expanded version gives a left-to-right splitting behavior
+                        // which differs from when using regular groups, because they split right-to-left.
+                        best_fitting![
+                            // ---------------------------------------------------------------------
+                            // Variant 1:
+                            // Try to fit the expression without any parentheses
+                            group(&format_expression).with_group_id(Some(group_id)),
+                            // ---------------------------------------------------------------------
+                            // Variant 2:
+                            // Try to fit the expression by adding parentheses and indenting the expression.
+                            group(&format_args![
+                                text("("),
+                                soft_block_indent(&format_expression),
+                                text(")")
+                            ])
+                            .with_group_id(Some(group_id))
+                            .should_expand(true),
+                            // ---------------------------------------------------------------------
+                            // Variant 3: Fallback, no parentheses
+                            // Expression doesn't fit regardless of adding the parentheses. Remove the parentheses again.
+                            group(&format_expression)
+                                .with_group_id(Some(group_id))
+                                .should_expand(true)
+                        ]
+                        // Measure all lines, to avoid that the printer decides that this fits right after hitting
+                        // the `(`.
+                        .with_mode(BestFittingMode::AllLines)
+                        .fmt(f)
+                    }
+                }
+            },
             OptionalParentheses::Never => match parenthesize {
                 Parenthesize::IfBreaksOrIfRequired => {
                     parenthesize_if_expands(&expression.format().with_options(Parentheses::Never))
@@ -230,6 +288,7 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
                     expression.format().with_options(Parentheses::Never).fmt(f)
                 }
             },
+
             OptionalParentheses::Always => {
                 expression.format().with_options(Parentheses::Always).fmt(f)
             }

crates/ruff_python_formatter/src/expression/parentheses.rs

@@ -15,11 +15,16 @@ pub(crate) enum OptionalParentheses {
     /// Add parentheses if the expression expands over multiple lines
     Multiline,
 
-    /// Always set parentheses regardless if the expression breaks or if they were
+    /// Always set parentheses regardless if the expression breaks or if they are
     /// present in the source.
     Always,
 
-    /// Never add parentheses
+    /// Add parentheses if it helps to make this expression fit. Otherwise never add parentheses.
+    /// This mode should only be used for expressions that don't have their own split points, e.g. identifiers,
+    /// or constants.
+    BestFit,
+
+    /// Never add parentheses. Use it for expressions that have their own parentheses or if the expression body always spans multiple lines (multiline strings).
     Never,
 }
 

crates/ruff_python_formatter/src/lib.rs

@@ -249,12 +249,10 @@ if True:
     #[test]
     fn quick_test() {
         let src = r#"
-@MyDecorator(list = a) # fmt: skip
-# trailing comment
-class Test:
-    pass
-
-
+for converter in connection.ops.get_db_converters(
+    expression
+) + expression.get_db_converters(connection):
+    ...
 "#;
         // Tokenize once
         let mut tokens = Vec::new();
@@ -291,9 +289,10 @@ class Test:
 
         assert_eq!(
             printed.as_code(),
-            r#"while True:
-    if something.changed:
-        do.stuff()  # trailing comment
+            r#"for converter in connection.ops.get_db_converters(
+    expression
+) + expression.get_db_converters(connection):
+    ...
 "#
         );
     }