Call chain formatting in fluent style (#6151)

Implement fluent style/call chains. See the `call_chains.py` formatting for examples. This isn't fully like black because in `raise A from B` they allow `A` breaking can influence the formatting of `B` even if it is already multiline. Similarity index: | project | main | PR | |--------------|-------|-------| | build | ??? | 0.753 | | django | 0.991 | 0.998 | | transformers | 0.993 | 0.994 | | typeshed | 0.723 | 0.723 | | warehouse | 0.978 | 0.994 | | zulip | 0.992 | 0.994 | Call chain formatting is affected by #627, but i'm cutting scope here. Closes #5343 **Test Plan**: * Added a dedicated call chains test file * The ecosystem checks found some bugs * I manually check django and zulip formatting --------- Co-authored-by: Micha Reiser <[email protected]>
astral-sh · Aug 4, 2023 · 99baad1 · 99baad1
1 parent 35bdbe4
commit 99baad1
Show file tree

Hide file tree

Showing 16 changed files with 917 additions and 517 deletions.
diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/call_chains.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/call_chains.py
@@ -0,0 +1,157 @@
+# Test cases for call chains and optional parentheses, with and without fluent style
+
+raise OsError("") from a.aaaaa(
+    aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa
+).a(aaaa)
+
+raise OsError(
+    "sökdjffffsldkfjlhsakfjhalsökafhsöfdahsödfjösaaksjdllllllllllllll"
+) from a.aaaaa(
+    aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa
+).a(
+    aaaa
+)
+
+a1 = Blog.objects.filter(entry__headline__contains="Lennon").filter(
+    entry__pub_date__year=2008
+)
+
+a2 = Blog.objects.filter(
+    entry__headline__contains="Lennon",
+).filter(
+    entry__pub_date__year=2008,
+)
+
+raise OsError("") from (
+    Blog.objects.filter(
+        entry__headline__contains="Lennon",
+    )
+    .filter(
+        entry__pub_date__year=2008,
+    )
+    .filter(
+        entry__pub_date__year=2008,
+    )
+)
+
+raise OsError("sökdjffffsldkfjlhsakfjhalsökafhsöfdahsödfjösaaksjdllllllllllllll") from (
+    Blog.objects.filter(
+        entry__headline__contains="Lennon",
+    )
+    .filter(
+        entry__pub_date__year=2008,
+    )
+    .filter(
+        entry__pub_date__year=2008,
+    )
+)
+
+# Break only after calls and indexing
+b1 = (
+    session.query(models.Customer.id)
+    .filter(
+        models.Customer.account_id == account_id, models.Customer.email == email_address
+    )
+    .count()
+)
+
+b2 = (
+    Blog.objects.filter(
+        entry__headline__contains="Lennon",
+    )
+    .limit_results[:10]
+    .filter(
+        entry__pub_date__month=10,
+    )
+)
+
+# Nested call chains
+c1 = (
+    Blog.objects.filter(
+        entry__headline__contains="Lennon",
+    ).filter(
+        entry__pub_date__year=2008,
+    )
+    + Blog.objects.filter(
+        entry__headline__contains="McCartney",
+    )
+    .limit_results[:10]
+    .filter(
+        entry__pub_date__year=2010,
+    )
+).all()
+
+# Test different cases with trailing end of line comments:
+# * fluent style, fits: no parentheses -> ignore the expand_parent
+# * fluent style, doesn't fit: break all soft line breaks
+# * default, fits: no parentheses
+# * default, doesn't fit: parentheses but no soft line breaks
+
+# Fits, either style
+d11 = x.e().e().e() #
+d12 = (x.e().e().e()) #
+d13 = (
+    x.e() #
+    .e()
+    .e()
+)
+
+# Doesn't fit, default
+d2 = (
+    x.e().esadjkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkfsdddd()  #
+)
+
+# Doesn't fit, fluent style
+d3 = (
+    x.e()  #
+    .esadjkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk()
+    .esadjkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk()
+)
+
+# Don't drop the bin op parentheses
+e1 = (1 + 2).w().t()
+e2 = (1 + 2)().w().t()
+e3 = (1 + 2)[1].w().t()
+
+# Treat preserved parentheses correctly
+f1 = (b().c()).d(1,)
+f2 = b().c().d(1,)
+f3 = (b).c().d(1,)
+f4 = (a)(b).c(1,)
+f5 = (a.b()).c(1,)
+
+# Indent in the parentheses without breaking
+g1 = (
+    queryset.distinct().order_by(field.name).values_list(field_name_flat_long_long=True)
+)
+
+# Fluent style in subexpressions
+if (
+    not a()
+    .b()
+    .cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc()
+):
+    pass
+h2 = (
+    bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
+    + ccccccccccccccccccccccccc()
+    .dddddddddddddddddddddd()
+    .eeeeeeeeee()
+    .ffffffffffffffffffffff()
+)
+
+# Parentheses aren't allowed on statement level, don't use fluent style here
+if True:
+    (alias).filter(content_typeold_content_type).update(
+        content_typenew_contesadfasfdant_type
+    )
+
+zero(
+    one,
+).two(
+    three,
+).four(
+    five,
+)
+
+
diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/call.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/call.py
@@ -52,7 +52,7 @@ def f(*args, **kwargs):
     hey_this_is_a_very_long_call=1, it_has_funny_attributes_asdf_asdf=1, too_long_for_the_line=1, really=True
 )
 
-# TODO(konstin): Call chains/fluent interface (https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#call-chains)
+# Call chains/fluent interface (https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#call-chains)
 result = (
     session.query(models.Customer.id)
     .filter(

diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs
@@ -1016,15 +1016,15 @@ fn handle_attribute_comment<'a>(
             .contains(comment.slice().start())
     );
     if comment.line_position().is_end_of_line() {
-        // Attach to node with b
+        // Attach as trailing comment to a. The specific placement is only relevant for fluent style
         // ```python
         // x322 = (
         //     a
         //     . # end-of-line dot comment 2
         //     b
         // )
         // ```
-        CommentPlacement::trailing(comment.enclosing_node(), comment)
+        CommentPlacement::trailing(attribute.value.as_ref(), comment)
     } else {
         CommentPlacement::dangling(attribute, comment)
     }

diff --git a/crates/ruff_python_formatter/src/expression/expr_attribute.rs b/crates/ruff_python_formatter/src/expression/expr_attribute.rs
@@ -1,15 +1,28 @@
-use ruff_python_ast::{Constant, Expr, ExprAttribute, ExprConstant};
-
-use ruff_formatter::write;
+use ruff_formatter::{write, FormatRuleWithOptions};
 use ruff_python_ast::node::AnyNodeRef;
+use ruff_python_ast::{Constant, Expr, ExprAttribute, ExprConstant};
 
 use crate::comments::{leading_comments, trailing_comments};
-use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses, Parentheses};
+use crate::expression::parentheses::{
+    is_expression_parenthesized, NeedsParentheses, OptionalParentheses, Parentheses,
+};
+use crate::expression::CallChainLayout;
 use crate::prelude::*;
 use crate::FormatNodeRule;
 
 #[derive(Default)]
-pub struct FormatExprAttribute;
+pub struct FormatExprAttribute {
+    call_chain_layout: CallChainLayout,
+}
+
+impl FormatRuleWithOptions<ExprAttribute, PyFormatContext<'_>> for FormatExprAttribute {
+    type Options = CallChainLayout;
+
+    fn with_options(mut self, options: Self::Options) -> Self {
+        self.call_chain_layout = options;
+        self
+    }
+}
 
 impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
     fn fmt_fields(&self, item: &ExprAttribute, f: &mut PyFormatter) -> FormatResult<()> {
@@ -20,6 +33,8 @@ impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
             ctx: _,
         } = item;
 
+        let call_chain_layout = self.call_chain_layout.apply_in_node(item, f);
+
         let needs_parentheses = matches!(
             value.as_ref(),
             Expr::Constant(ExprConstant {
@@ -37,11 +52,36 @@ impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
 
         if needs_parentheses {
             value.format().with_options(Parentheses::Always).fmt(f)?;
-        } else if let Expr::Attribute(expr_attribute) = value.as_ref() {
-            // We're in a attribute chain (`a.b.c`). The outermost node adds parentheses if
-            // required, the inner ones don't need them so we skip the `Expr` formatting that
-            // normally adds the parentheses.
-            expr_attribute.format().fmt(f)?;
+        } else if call_chain_layout == CallChainLayout::Fluent {
+            match value.as_ref() {
+                Expr::Attribute(expr) => {
+                    expr.format().with_options(call_chain_layout).fmt(f)?;
+                }
+                Expr::Call(expr) => {
+                    expr.format().with_options(call_chain_layout).fmt(f)?;
+                    if call_chain_layout == CallChainLayout::Fluent {
+                        // Format the dot on its own line
+                        soft_line_break().fmt(f)?;
+                    }
+                }
+                Expr::Subscript(expr) => {
+                    expr.format().with_options(call_chain_layout).fmt(f)?;
+                    if call_chain_layout == CallChainLayout::Fluent {
+                        // Format the dot on its own line
+                        soft_line_break().fmt(f)?;
+                    }
+                }
+                _ => {
+                    // This matches [`CallChainLayout::from_expression`]
+                    if is_expression_parenthesized(value.as_ref().into(), f.context().source()) {
+                        value.format().with_options(Parentheses::Always).fmt(f)?;
+                        // Format the dot on its own line
+                        soft_line_break().fmt(f)?;
+                    } else {
+                        value.format().fmt(f)?;
+                    }
+                }
+            }
         } else {
             value.format().fmt(f)?;
         }
@@ -50,16 +90,51 @@ impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
             hard_line_break().fmt(f)?;
         }
 
-        write!(
-            f,
-            [
-                text("."),
-                trailing_comments(trailing_dot_comments),
-                (!leading_attribute_comments.is_empty()).then_some(hard_line_break()),
-                leading_comments(leading_attribute_comments),
-                attr.format()
-            ]
-        )
+        if call_chain_layout == CallChainLayout::Fluent {
+            // Fluent style has line breaks before the dot
+            // ```python
+            // blogs3 = (
+            //     Blog.objects.filter(
+            //         entry__headline__contains="Lennon",
+            //     )
+            //     .filter(
+            //         entry__pub_date__year=2008,
+            //     )
+            //     .filter(
+            //         entry__pub_date__year=2008,
+            //     )
+            // )
+            // ```
+            write!(
+                f,
+                [
+                    (!leading_attribute_comments.is_empty()).then_some(hard_line_break()),
+                    leading_comments(leading_attribute_comments),
+                    text("."),
+                    trailing_comments(trailing_dot_comments),
+                    attr.format()
+                ]
+            )
+        } else {
+            // Regular style
+            // ```python
+            // blogs2 = Blog.objects.filter(
+            //     entry__headline__contains="Lennon",
+            // ).filter(
+            //     entry__pub_date__year=2008,
+            // )
+            // ```
+            write!(
+                f,
+                [
+                    text("."),
+                    trailing_comments(trailing_dot_comments),
+                    (!leading_attribute_comments.is_empty()).then_some(hard_line_break()),
+                    leading_comments(leading_attribute_comments),
+                    attr.format()
+                ]
+            )
+        }
     }
 
     fn fmt_dangling_comments(
@@ -79,7 +154,11 @@ impl NeedsParentheses for ExprAttribute {
         context: &PyFormatContext,
     ) -> OptionalParentheses {
         // Checks if there are any own line comments in an attribute chain (a.b.c).
-        if context
+        if CallChainLayout::from_expression(self.into(), context.source())
+            == CallChainLayout::Fluent
+        {
+            OptionalParentheses::Multiline
+        } else if context
             .comments()
             .dangling_comments(self)
             .iter()

diff --git a/crates/ruff_python_formatter/src/expression/expr_bin_op.rs b/crates/ruff_python_formatter/src/expression/expr_bin_op.rs
@@ -5,9 +5,7 @@ use ruff_python_ast::{
 };
 use smallvec::SmallVec;
 
-use ruff_formatter::{
-    format_args, write, FormatOwnedWithRule, FormatRefWithRule, FormatRuleWithOptions,
-};
+use ruff_formatter::{format_args, write, FormatOwnedWithRule, FormatRefWithRule};
 use ruff_python_ast::node::{AnyNodeRef, AstNode};
 use ruff_python_ast::str::is_implicit_concatenation;
 
@@ -19,23 +17,11 @@ use crate::expression::parentheses::{
     NeedsParentheses, OptionalParentheses,
 };
 use crate::expression::string::StringLayout;
-use crate::expression::Parentheses;
 use crate::prelude::*;
 use crate::FormatNodeRule;
 
 #[derive(Default)]
-pub struct FormatExprBinOp {
-    parentheses: Option<Parentheses>,
-}
-
-impl FormatRuleWithOptions<ExprBinOp, PyFormatContext<'_>> for FormatExprBinOp {
-    type Options = Option<Parentheses>;
-
-    fn with_options(mut self, options: Self::Options) -> Self {
-        self.parentheses = options;
-        self
-    }
-}
+pub struct FormatExprBinOp;
 
 impl FormatNodeRule<ExprBinOp> for FormatExprBinOp {
     fn fmt_fields(&self, item: &ExprBinOp, f: &mut PyFormatter) -> FormatResult<()> {