Maybe parenthesize long constants and names (#6816)

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

@@ -5,7 +5,7 @@ use ruff_python_ast::{Constant, ExprConstant, Ranged};
 use ruff_text_size::{TextLen, TextRange};
 
 use crate::expression::number::{FormatComplex, FormatFloat, FormatInt};
-use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
+use crate::expression::parentheses::{should_use_best_fit, NeedsParentheses, OptionalParentheses};
 use crate::expression::string::{
     AnyString, FormatString, StringLayout, StringPrefix, StringQuotes,
 };
@@ -79,13 +79,16 @@ 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
-            } else {
-                OptionalParentheses::Multiline
-            }
+        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 if should_use_best_fit(self, context) {
+            OptionalParentheses::BestFit
         } else {
             OptionalParentheses::Never
         }
@@ -99,7 +102,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,6 +1,8 @@
 use super::string::{AnyString, FormatString};
 use crate::context::PyFormatContext;
-use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
+use memchr::memchr2;
+
+use crate::expression::parentheses::{should_use_best_fit, NeedsParentheses, OptionalParentheses};
 use crate::prelude::*;
 use crate::{FormatNodeRule, PyFormatter};
 use ruff_formatter::FormatResult;
@@ -20,8 +22,16 @@ impl NeedsParentheses for ExprFString {
     fn needs_parentheses(
         &self,
         _parent: AnyNodeRef,
-        _context: &PyFormatContext,
+        context: &PyFormatContext,
     ) -> OptionalParentheses {
-        OptionalParentheses::Multiline
+        if self.implicit_concatenated {
+            OptionalParentheses::Multiline
+        } else if memchr2(b'\n', b'\r', context.source()[self.range].as_bytes()).is_none()
+            && should_use_best_fit(self, context)
+        {
+            OptionalParentheses::BestFit
+        } else {
+            OptionalParentheses::Never
+        }
     }
 }

crates/ruff_python_formatter/src/expression/expr_name.rs

@@ -1,5 +1,5 @@
 use crate::comments::SourceComment;
-use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
+use crate::expression::parentheses::{should_use_best_fit, NeedsParentheses, OptionalParentheses};
 use crate::prelude::*;
 use crate::FormatNodeRule;
 use ruff_formatter::{write, FormatContext};
@@ -38,9 +38,13 @@ impl NeedsParentheses for ExprName {
     fn needs_parentheses(
         &self,
         _parent: AnyNodeRef,
-        _context: &PyFormatContext,
+        context: &PyFormatContext,
     ) -> OptionalParentheses {
-        OptionalParentheses::Never
+        if should_use_best_fit(self, context) {
+            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

@@ -1,5 +1,5 @@
 use ruff_formatter::prelude::tag::Condition;
-use ruff_formatter::{format_args, write, Argument, Arguments};
+use ruff_formatter::{format_args, write, Argument, Arguments, FormatContext, FormatOptions};
 use ruff_python_ast::node::AnyNodeRef;
 use ruff_python_ast::{ExpressionRef, Ranged};
 use ruff_python_trivia::{first_non_trivia_token, SimpleToken, SimpleTokenKind, SimpleTokenizer};
@@ -15,14 +15,37 @@ 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,
 }
 
+pub(super) fn should_use_best_fit<T>(value: T, context: &PyFormatContext) -> bool
+where
+    T: Ranged,
+{
+    let text_len = context.source()[value.range()].len();
+
+    // Only use best fits if:
+    // * The text is longer than 5 characters:
+    //   This is to align the behavior with `True` and `False`, that don't use best fits and are 5 characters long.
+    //   It allows to avoid [`OptionalParentheses::BestFit`] for most numbers and common identifiers like `self`.
+    //   The downside is that it can result in short values not being parenthesized if they exceed the line width.
+    //   This is considered an edge case not worth the performance penalty and IMO, breaking an identifier
+    //   of 5 characters to avoid it exceeding the line width by 1 reduces the readability.
+    // * The text is know to never fit: The text can never fit even when parenthesizing if it is longer
+    //   than the configured line width (minus indent).
+    text_len > 5 && text_len < context.options().line_width().value() as usize
+}
+
 pub(crate) trait NeedsParentheses {
     /// Determines if this object needs optional parentheses or if it is safe to omit the parentheses.
     fn needs_parentheses(

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):
+    ...
 "#
         );
     }