Use BestFits for non-fluent attribute chains (#6817)

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

@@ -52,3 +52,16 @@ for converter in connection.ops.get_db_converters(
     expression
 ) + expression.get_db_converters(connection):
     ...
+
+
+aaa = (
+    bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb # awkward comment
+)
+
+def test():
+    m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyField(Person, blank=True)
+
+    m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyFieldAttributeChainField
+
+m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyField(Person, blank=True)
+m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyFieldAttributeChainFieeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeld

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

@@ -66,6 +66,13 @@ def foo():
         1
     )
 
+    yield (
+        "#   * Make sure each ForeignKey and OneToOneField has `on_delete` set "
+        "to the desired behavior"
+    )
+
+    yield aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + ccccccccccccccccccccccccccccccccccccccccccccccccccccccc
+
 yield ("Cache key will cause errors if used with memcached: %r " "(longer than %s)" % (
     key,
     MEMCACHE_MAX_KEY_LENGTH,

crates/ruff_python_formatter/src/expression/expr_call.rs

@@ -1,5 +1,6 @@
 use crate::expression::CallChainLayout;
-use ruff_formatter::FormatRuleWithOptions;
+
+use ruff_formatter::{format_args, write, FormatRuleWithOptions};
 use ruff_python_ast::node::AnyNodeRef;
 use ruff_python_ast::{Expr, ExprCall};
 
@@ -31,15 +32,11 @@ impl FormatNodeRule<ExprCall> for FormatExprCall {
 
         let call_chain_layout = self.call_chain_layout.apply_in_node(item, f);
 
-        let fmt_inner = format_with(|f| {
+        let fmt_func = format_with(|f| match func.as_ref() {
-            match func.as_ref() {
+            Expr::Attribute(expr) => expr.format().with_options(call_chain_layout).fmt(f),
-                Expr::Attribute(expr) => expr.format().with_options(call_chain_layout).fmt(f)?,
+            Expr::Call(expr) => expr.format().with_options(call_chain_layout).fmt(f),
-                Expr::Call(expr) => expr.format().with_options(call_chain_layout).fmt(f)?,
+            Expr::Subscript(expr) => expr.format().with_options(call_chain_layout).fmt(f),
-                Expr::Subscript(expr) => expr.format().with_options(call_chain_layout).fmt(f)?,
+            _ => func.format().fmt(f),
-                _ => func.format().fmt(f)?,
-            }
-
-            arguments.format().fmt(f)
         });
 
         // Allow to indent the parentheses while
@@ -51,9 +48,9 @@ impl FormatNodeRule<ExprCall> for FormatExprCall {
         if call_chain_layout == CallChainLayout::Fluent
             && self.call_chain_layout == CallChainLayout::Default
         {
-            group(&fmt_inner).fmt(f)
+            group(&format_args![fmt_func, arguments.format()]).fmt(f)
         } else {
-            fmt_inner.fmt(f)
+            write!(f, [fmt_func, arguments.format()])
         }
     }
 }
@@ -69,10 +66,7 @@ impl NeedsParentheses for ExprCall {
         {
             OptionalParentheses::Multiline
         } else {
-            match self.func.needs_parentheses(self.into(), context) {
+            self.func.needs_parentheses(self.into(), context)
-                OptionalParentheses::BestFit => OptionalParentheses::Never,
-                parentheses => parentheses,
-            }
         }
     }
 }

crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__trailing_commas_in_leading_parts.py.snap

@@ -56,6 +56,18 @@ assert xxxxxxxxx.xxxxxxxxx.xxxxxxxxx(
  
  
  # Example from https://github.com/psf/black/issues/3229
+@@ -43,8 +41,6 @@
+ )
+ 
+ # Regression test for https://github.com/psf/black/issues/3414.
+-assert xxxxxxxxx.xxxxxxxxx.xxxxxxxxx(
+-    xxxxxxxxx
+-).xxxxxxxxxxxxxxxxxx(), (
+-    "xxx {xxxxxxxxx} xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+-)
++assert (
++    xxxxxxxxx.xxxxxxxxx.xxxxxxxxx(xxxxxxxxx).xxxxxxxxxxxxxxxxxx()
++), "xxx {xxxxxxxxx} xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
 ```
 
 ## Ruff Output
@@ -104,11 +116,9 @@ assert (
 )
 
 # Regression test for https://github.com/psf/black/issues/3414.
-assert xxxxxxxxx.xxxxxxxxx.xxxxxxxxx(
+assert (
-    xxxxxxxxx
+    xxxxxxxxx.xxxxxxxxx.xxxxxxxxx(xxxxxxxxx).xxxxxxxxxxxxxxxxxx()
-).xxxxxxxxxxxxxxxxxx(), (
+), "xxx {xxxxxxxxx} xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
-    "xxx {xxxxxxxxx} xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
-)
 ```
 
 ## Black Output

crates/ruff_python_formatter/tests/snapshots/format@expression__unsplittable.py.snap

@@ -58,6 +58,19 @@ for converter in connection.ops.get_db_converters(
     expression
 ) + expression.get_db_converters(connection):
     ...
+
+
+aaa = (
+    bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb # awkward comment
+)
+
+def test():
+    m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyField(Person, blank=True)
+
+    m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyFieldAttributeChainField
+
+m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyField(Person, blank=True)
+m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyFieldAttributeChainFieeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeld
 ```
 
 ## Output
@@ -120,6 +133,25 @@ for converter in connection.ops.get_db_converters(
     expression
 ) + expression.get_db_converters(connection):
     ...
+
+
+aaa = bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb  # awkward comment
+
+
+def test():
+    m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = (
+        models.ManyToManyField(Person, blank=True)
+    )
+
+    m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = (
+        models.ManyToManyFieldAttributeChainField
+    )
+
+
+m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = (
+    models.ManyToManyField(Person, blank=True)
+)
+m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyFieldAttributeChainFieeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeld
 ```
 
 

crates/ruff_python_formatter/tests/snapshots/format@expression__yield.py.snap

@@ -72,6 +72,13 @@ def foo():
         1
     )
 
+    yield (
+        "#   * Make sure each ForeignKey and OneToOneField has `on_delete` set "
+        "to the desired behavior"
+    )
+
+    yield aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + ccccccccccccccccccccccccccccccccccccccccccccccccccccccc
+
 yield ("Cache key will cause errors if used with memcached: %r " "(longer than %s)" % (
     key,
     MEMCACHE_MAX_KEY_LENGTH,
@@ -168,6 +175,17 @@ def foo():
         )
     )
 
+    yield (
+        "#   * Make sure each ForeignKey and OneToOneField has `on_delete` set "
+        "to the desired behavior"
+    )
+
+    yield (
+        aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+        + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
+        + ccccccccccccccccccccccccccccccccccccccccccccccccccccccc
+    )
+
 
 yield (
     "Cache key will cause errors if used with memcached: %r "