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 new file mode 100644 index 0000000000000..67b702c7055ac --- /dev/null +++ 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 index f5198c62ca652..a158a2ec4dfdc 100644 --- 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 index ddd0080cc36d2..14988291df099 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1021,7 +1021,7 @@ 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 @@ -1029,7 +1029,7 @@ fn handle_attribute_comment<'a>( // 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 index a577ae88d0610..f734a5fc5bef6 100644 --- 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> for FormatExprAttribute { + type Options = CallChainLayout; + + fn with_options(mut self, options: Self::Options) -> Self { + self.call_chain_layout = options; + self + } +} impl FormatNodeRule for FormatExprAttribute { fn fmt_fields(&self, item: &ExprAttribute, f: &mut PyFormatter) -> FormatResult<()> { @@ -20,6 +33,8 @@ impl FormatNodeRule 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 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 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 index af24f6ea4afba..1a59bdb051236 100644 --- 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, -} - -impl FormatRuleWithOptions> for FormatExprBinOp { - type Options = Option; - - fn with_options(mut self, options: Self::Options) -> Self { - self.parentheses = options; - self - } -} +pub struct FormatExprBinOp; impl FormatNodeRule for FormatExprBinOp { fn fmt_fields(&self, item: &ExprBinOp, f: &mut PyFormatter) -> FormatResult<()> { diff --git a/crates/ruff_python_formatter/src/expression/expr_call.rs b/crates/ruff_python_formatter/src/expression/expr_call.rs index 2372b4a5776b9..5483aabdcaee8 100644 --- a/crates/ruff_python_formatter/src/expression/expr_call.rs +++ b/crates/ruff_python_formatter/src/expression/expr_call.rs @@ -1,13 +1,25 @@ -use ruff_formatter::write; +use crate::expression::CallChainLayout; +use ruff_formatter::FormatRuleWithOptions; use ruff_python_ast::node::AnyNodeRef; -use ruff_python_ast::ExprCall; +use ruff_python_ast::{Expr, ExprCall}; use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses}; use crate::prelude::*; use crate::FormatNodeRule; #[derive(Default)] -pub struct FormatExprCall; +pub struct FormatExprCall { + call_chain_layout: CallChainLayout, +} + +impl FormatRuleWithOptions> for FormatExprCall { + type Options = CallChainLayout; + + fn with_options(mut self, options: Self::Options) -> Self { + self.call_chain_layout = options; + self + } +} impl FormatNodeRule for FormatExprCall { fn fmt_fields(&self, item: &ExprCall, f: &mut PyFormatter) -> FormatResult<()> { @@ -17,7 +29,32 @@ impl FormatNodeRule for FormatExprCall { arguments, } = item; - write!(f, [func.format(), arguments.format()]) + let call_chain_layout = self.call_chain_layout.apply_in_node(item, f); + + let fmt_inner = format_with(|f| { + match func.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)?, + Expr::Subscript(expr) => expr.format().with_options(call_chain_layout).fmt(f)?, + _ => func.format().fmt(f)?, + } + + arguments.format().fmt(f) + }); + + // Allow to indent the parentheses while + // ```python + // g1 = ( + // queryset.distinct().order_by(field.name).values_list(field_name_flat_long_long=True) + // ) + // ``` + if call_chain_layout == CallChainLayout::Fluent + && self.call_chain_layout == CallChainLayout::Default + { + group(&fmt_inner).fmt(f) + } else { + fmt_inner.fmt(f) + } } } @@ -27,6 +64,12 @@ impl NeedsParentheses for ExprCall { _parent: AnyNodeRef, context: &PyFormatContext, ) -> OptionalParentheses { - self.func.needs_parentheses(self.into(), context) + if CallChainLayout::from_expression(self.into(), context.source()) + == CallChainLayout::Fluent + { + OptionalParentheses::Multiline + } else { + self.func.needs_parentheses(self.into(), context) + } } } diff --git a/crates/ruff_python_formatter/src/expression/expr_subscript.rs b/crates/ruff_python_formatter/src/expression/expr_subscript.rs index 464028c2e1a60..1a2304801d990 100644 --- a/crates/ruff_python_formatter/src/expression/expr_subscript.rs +++ b/crates/ruff_python_formatter/src/expression/expr_subscript.rs @@ -1,18 +1,29 @@ -use ruff_python_ast::{Expr, ExprSubscript}; - -use ruff_formatter::{format_args, write}; +use ruff_formatter::{format_args, write, FormatRuleWithOptions}; use ruff_python_ast::node::{AnyNodeRef, AstNode}; +use ruff_python_ast::{Expr, ExprSubscript}; use crate::comments::trailing_comments; use crate::context::PyFormatContext; use crate::context::{NodeLevel, WithNodeLevel}; use crate::expression::expr_tuple::TupleParentheses; use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses}; +use crate::expression::CallChainLayout; use crate::prelude::*; use crate::FormatNodeRule; #[derive(Default)] -pub struct FormatExprSubscript; +pub struct FormatExprSubscript { + call_chain_layout: CallChainLayout, +} + +impl FormatRuleWithOptions> for FormatExprSubscript { + type Options = CallChainLayout; + + fn with_options(mut self, options: Self::Options) -> Self { + self.call_chain_layout = options; + self + } +} impl FormatNodeRule for FormatExprSubscript { fn fmt_fields(&self, item: &ExprSubscript, f: &mut PyFormatter) -> FormatResult<()> { @@ -23,6 +34,8 @@ impl FormatNodeRule for FormatExprSubscript { ctx: _, } = item; + let call_chain_layout = self.call_chain_layout.apply_in_node(item, f); + let comments = f.context().comments().clone(); let dangling_comments = comments.dangling_comments(item.as_any_node_ref()); debug_assert!( @@ -30,12 +43,19 @@ impl FormatNodeRule for FormatExprSubscript { "A subscript expression can only have a single dangling comment, the one after the bracket" ); + let format_value = format_with(|f| 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), + Expr::Subscript(expr) => expr.format().with_options(call_chain_layout).fmt(f), + _ => value.format().fmt(f), + }); + if let NodeLevel::Expression(Some(_)) = f.context().node_level() { // Enforce the optional parentheses for parenthesized values. let mut f = WithNodeLevel::new(NodeLevel::Expression(None), f); - write!(f, [value.format()])?; + write!(f, [format_value])?; } else { - value.format().fmt(f)?; + format_value.fmt(f)?; } let format_slice = format_with(|f: &mut PyFormatter| { @@ -73,10 +93,16 @@ impl NeedsParentheses for ExprSubscript { fn needs_parentheses( &self, _parent: AnyNodeRef, - _context: &PyFormatContext, + context: &PyFormatContext, ) -> OptionalParentheses { { - OptionalParentheses::Never + if CallChainLayout::from_expression(self.into(), context.source()) + == CallChainLayout::Fluent + { + OptionalParentheses::Multiline + } else { + self.value.needs_parentheses(self.into(), context) + } } } } diff --git a/crates/ruff_python_formatter/src/expression/mod.rs b/crates/ruff_python_formatter/src/expression/mod.rs index c96f9b59db085..48a946345909c 100644 --- a/crates/ruff_python_formatter/src/expression/mod.rs +++ b/crates/ruff_python_formatter/src/expression/mod.rs @@ -1,13 +1,12 @@ use std::cmp::Ordering; -use ruff_python_ast as ast; -use ruff_python_ast::{Expr, Operator}; - use ruff_formatter::{ write, FormatOwnedWithRule, FormatRefWithRule, FormatRule, FormatRuleWithOptions, }; +use ruff_python_ast as ast; use ruff_python_ast::node::AnyNodeRef; use ruff_python_ast::visitor::preorder::{walk_expr, PreorderVisitor}; +use ruff_python_ast::{Expr, Operator}; use crate::builders::parenthesize_if_expands; use crate::context::{NodeLevel, WithNodeLevel}; @@ -69,7 +68,7 @@ impl FormatRule> for FormatExpr { let format_expr = format_with(|f| match expression { Expr::BoolOp(expr) => expr.format().with_options(Some(parentheses)).fmt(f), Expr::NamedExpr(expr) => expr.format().fmt(f), - Expr::BinOp(expr) => expr.format().with_options(Some(parentheses)).fmt(f), + Expr::BinOp(expr) => expr.format().fmt(f), Expr::UnaryOp(expr) => expr.format().fmt(f), Expr::Lambda(expr) => expr.format().fmt(f), Expr::IfExp(expr) => expr.format().fmt(f), @@ -99,9 +98,10 @@ impl FormatRule> for FormatExpr { let parenthesize = match parentheses { Parentheses::Preserve => { - is_expression_parenthesized(AnyNodeRef::from(expression), f.context().source()) + is_expression_parenthesized(expression.into(), f.context().source()) } Parentheses::Always => true, + // Fluent style means we already have parentheses Parentheses::Never => false, }; @@ -156,7 +156,7 @@ impl Format> for MaybeParenthesizeExpression<'_> { let comments = f.context().comments(); let preserve_parentheses = parenthesize.is_optional() - && is_expression_parenthesized(AnyNodeRef::from(*expression), f.context().source()); + && is_expression_parenthesized((*expression).into(), f.context().source()); let has_comments = comments.has_leading_comments(*expression) || comments.has_trailing_own_line_comments(*expression); @@ -177,9 +177,10 @@ impl Format> for MaybeParenthesizeExpression<'_> { Parenthesize::Optional | Parenthesize::IfBreaks => needs_parentheses, }; + let can_omit_optional_parentheses = can_omit_optional_parentheses(expression, f.context()); match needs_parentheses { OptionalParentheses::Multiline if *parenthesize != Parenthesize::IfRequired => { - if can_omit_optional_parentheses(expression, f.context()) { + if can_omit_optional_parentheses { optional_parentheses(&expression.format().with_options(Parentheses::Never)) .fmt(f) } else { @@ -264,7 +265,24 @@ impl<'ast> IntoFormat> for Expr { fn can_omit_optional_parentheses(expr: &Expr, context: &PyFormatContext) -> bool { let mut visitor = CanOmitOptionalParenthesesVisitor::new(context.source()); visitor.visit_subexpression(expr); - visitor.can_omit() + + if visitor.max_priority_count > 1 { + false + } else if visitor.max_priority == OperatorPriority::Attribute { + true + } else if !visitor.any_parenthesized_expressions { + // Only use the more complex IR when there is any expression that we can possibly split by + false + } else { + // Only use the layout if the first or last expression has parentheses of some sort. + let first_parenthesized = visitor + .first + .is_some_and(|first| has_parentheses(first, visitor.source)); + let last_parenthesized = visitor + .last + .is_some_and(|last| has_parentheses(last, visitor.source)); + first_parenthesized || last_parenthesized + } } #[derive(Clone, Debug)] @@ -364,8 +382,10 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> { self.last = Some(expr); return; } - Expr::Subscript(_) => { - // Don't walk the value. Splitting before the value looks weird. + Expr::Subscript(ast::ExprSubscript { value, .. }) => { + self.any_parenthesized_expressions = true; + // Only walk the function, the subscript is always parenthesized + self.visit_expr(value); // Don't walk the slice, because the slice is always parenthesized. return; } @@ -408,26 +428,6 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> { walk_expr(self, expr); } - - fn can_omit(self) -> bool { - if self.max_priority_count > 1 { - false - } else if self.max_priority == OperatorPriority::Attribute { - true - } else if !self.any_parenthesized_expressions { - // Only use the more complex IR when there is any expression that we can possibly split by - false - } else { - // Only use the layout if the first or last expression has parentheses of some sort. - let first_parenthesized = self - .first - .is_some_and(|first| has_parentheses(first, self.source)); - let last_parenthesized = self - .last - .is_some_and(|last| has_parentheses(last, self.source)); - first_parenthesized || last_parenthesized - } - } } impl<'input> PreorderVisitor<'input> for CanOmitOptionalParenthesesVisitor<'input> { @@ -447,6 +447,124 @@ impl<'input> PreorderVisitor<'input> for CanOmitOptionalParenthesesVisitor<'inpu } } +/// A call chain consists only of attribute access (`.` operator), function/method calls and +/// subscripts. We use fluent style for the call chain if there are at least two attribute dots +/// after call parentheses or subscript brackets. In case of fluent style the parentheses/bracket +/// will close on the previous line and the dot gets its own line, otherwise the line will start +/// with the closing parentheses/bracket and the dot follows immediately after. +/// +/// Below, the left hand side of the addition has only a single attribute access after a call, the +/// second `.filter`. The first `.filter` is a call, but it doesn't follow a call. The right hand +/// side has two, the `.limit_results` after the call and the `.filter` after the subscript, so it +/// gets formatted in fluent style. The outer expression we assign to `blogs` has zero since the +/// `.all` follows attribute parentheses and not call parentheses. +/// +/// ```python +/// blogs = ( +/// 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() +/// ``` +#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] +pub enum CallChainLayout { + /// The root of a call chain + #[default] + Default, + + /// A nested call chain element that uses fluent style. + Fluent, + + /// A nested call chain element not using fluent style. + NonFluent, +} + +impl CallChainLayout { + pub(crate) fn from_expression(mut expr: AnyNodeRef, source: &str) -> Self { + let mut attributes_after_parentheses = 0; + loop { + match expr { + AnyNodeRef::ExprAttribute(ast::ExprAttribute { value, .. }) => { + expr = AnyNodeRef::from(value.as_ref()); + // ``` + // f().g + // ^^^ value + // data[:100].T` + // ^^^^^^^^^^ value + // ``` + if matches!(value.as_ref(), Expr::Call(_) | Expr::Subscript(_)) { + attributes_after_parentheses += 1; + } else if is_expression_parenthesized(expr, source) { + // `(a).b`. We preserve these parentheses so don't recurse + attributes_after_parentheses += 1; + break; + } + } + // ``` + // f() + // ^^^ expr + // ^ func + // data[:100] + // ^^^^^^^^^^ expr + // ^^^^ value + // ``` + AnyNodeRef::ExprCall(ast::ExprCall { func: inner, .. }) + | AnyNodeRef::ExprSubscript(ast::ExprSubscript { value: inner, .. }) => { + expr = AnyNodeRef::from(inner.as_ref()); + } + _ => { + // We to format the following in fluent style: + // ``` + // f2 = (a).w().t(1,) + // ^ expr + // ``` + if is_expression_parenthesized(expr, source) { + attributes_after_parentheses += 1; + } + break; + } + } + // We preserve these parentheses so don't recurse + if is_expression_parenthesized(expr, source) { + break; + } + } + if attributes_after_parentheses < 2 { + CallChainLayout::NonFluent + } else { + CallChainLayout::Fluent + } + } + + /// Determine whether to actually apply fluent layout in attribute, call and subscript + /// formatting + pub(crate) fn apply_in_node<'a>( + self, + item: impl Into>, + f: &mut PyFormatter, + ) -> CallChainLayout { + match self { + CallChainLayout::Default => { + if f.context().node_level().is_parenthesized() { + CallChainLayout::from_expression(item.into(), f.context().source()) + } else { + CallChainLayout::NonFluent + } + } + layout @ (CallChainLayout::Fluent | CallChainLayout::NonFluent) => layout, + } + } +} + fn has_parentheses(expr: &Expr, source: &str) -> bool { has_own_parentheses(expr) || is_expression_parenthesized(AnyNodeRef::from(expr), source) } diff --git a/crates/ruff_python_formatter/src/expression/parentheses.rs b/crates/ruff_python_formatter/src/expression/parentheses.rs index abe7cc9d623c2..5e49aa1a7c090 100644 --- a/crates/ruff_python_formatter/src/expression/parentheses.rs +++ b/crates/ruff_python_formatter/src/expression/parentheses.rs @@ -285,3 +285,20 @@ impl<'ast> Format> for FormatInParenthesesOnlyGroup<'_, 'a } } } + +#[cfg(test)] +mod tests { + use crate::expression::parentheses::is_expression_parenthesized; + use ruff_python_ast::node::AnyNodeRef; + use ruff_python_parser::parse_expression; + + #[test] + fn test_has_parentheses() { + let expression = r#"(b().c("")).d()"#; + let expr = parse_expression(expression, "").unwrap(); + assert!(!is_expression_parenthesized( + AnyNodeRef::from(&expr), + expression + )); + } +} diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__comments4.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__comments4.py.snap deleted file mode 100644 index b7752423a8e47..0000000000000 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__comments4.py.snap +++ /dev/null @@ -1,348 +0,0 @@ ---- -source: crates/ruff_python_formatter/tests/fixtures.rs -input_file: crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/comments4.py ---- -## Input - -```py -from com.my_lovely_company.my_lovely_team.my_lovely_project.my_lovely_component import ( - MyLovelyCompanyTeamProjectComponent, # NOT DRY -) -from com.my_lovely_company.my_lovely_team.my_lovely_project.my_lovely_component import ( - MyLovelyCompanyTeamProjectComponent as component, # DRY -) - - -class C: - @pytest.mark.parametrize( - ("post_data", "message"), - [ - # metadata_version errors. - ( - {}, - "None is an invalid value for Metadata-Version. Error: This field is" - " required. see" - " https://packaging.python.org/specifications/core-metadata", - ), - ( - {"metadata_version": "-1"}, - "'-1' is an invalid value for Metadata-Version. Error: Unknown Metadata" - " Version see" - " https://packaging.python.org/specifications/core-metadata", - ), - # name errors. - ( - {"metadata_version": "1.2"}, - "'' is an invalid value for Name. Error: This field is required. see" - " https://packaging.python.org/specifications/core-metadata", - ), - ( - {"metadata_version": "1.2", "name": "foo-"}, - "'foo-' is an invalid value for Name. Error: Must start and end with a" - " letter or numeral and contain only ascii numeric and '.', '_' and" - " '-'. see https://packaging.python.org/specifications/core-metadata", - ), - # version errors. - ( - {"metadata_version": "1.2", "name": "example"}, - "'' is an invalid value for Version. Error: This field is required. see" - " https://packaging.python.org/specifications/core-metadata", - ), - ( - {"metadata_version": "1.2", "name": "example", "version": "dog"}, - "'dog' is an invalid value for Version. Error: Must start and end with" - " a letter or numeral and contain only ascii numeric and '.', '_' and" - " '-'. see https://packaging.python.org/specifications/core-metadata", - ), - ], - ) - def test_fails_invalid_post_data( - self, pyramid_config, db_request, post_data, message - ): - pyramid_config.testing_securitypolicy(userid=1) - db_request.POST = MultiDict(post_data) - - -def foo(list_a, list_b): - results = ( - User.query.filter(User.foo == "bar") - .filter( # Because foo. - db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b)) - ) - .filter(User.xyz.is_(None)) - # Another comment about the filtering on is_quux goes here. - .filter(db.not_(User.is_pending.astext.cast(db.Boolean).is_(True))) - .order_by(User.created_at.desc()) - .with_for_update(key_share=True) - .all() - ) - return results - - -def foo2(list_a, list_b): - # Standalone comment reasonably placed. - return ( - User.query.filter(User.foo == "bar") - .filter( - db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b)) - ) - .filter(User.xyz.is_(None)) - ) - - -def foo3(list_a, list_b): - return ( - # Standalone comment but weirdly placed. - User.query.filter(User.foo == "bar") - .filter( - db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b)) - ) - .filter(User.xyz.is_(None)) - ) -``` - -## Black Differences - -```diff ---- Black -+++ Ruff -@@ -58,37 +58,28 @@ - - def foo(list_a, list_b): - results = ( -- User.query.filter(User.foo == "bar") -- .filter( # Because foo. -+ User.query.filter(User.foo == "bar").filter( # Because foo. - db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b)) -- ) -- .filter(User.xyz.is_(None)) -+ ).filter(User.xyz.is_(None)). - # Another comment about the filtering on is_quux goes here. -- .filter(db.not_(User.is_pending.astext.cast(db.Boolean).is_(True))) -- .order_by(User.created_at.desc()) -- .with_for_update(key_share=True) -- .all() -+ filter(db.not_(User.is_pending.astext.cast(db.Boolean).is_(True))).order_by( -+ User.created_at.desc() -+ ).with_for_update(key_share=True).all() - ) - return results - - - def foo2(list_a, list_b): - # Standalone comment reasonably placed. -- return ( -- User.query.filter(User.foo == "bar") -- .filter( -- db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b)) -- ) -- .filter(User.xyz.is_(None)) -- ) -+ return User.query.filter(User.foo == "bar").filter( -+ db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b)) -+ ).filter(User.xyz.is_(None)) - - - def foo3(list_a, list_b): - return ( - # Standalone comment but weirdly placed. -- User.query.filter(User.foo == "bar") -- .filter( -+ User.query.filter(User.foo == "bar").filter( - db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b)) -- ) -- .filter(User.xyz.is_(None)) -+ ).filter(User.xyz.is_(None)) - ) -``` - -## Ruff Output - -```py -from com.my_lovely_company.my_lovely_team.my_lovely_project.my_lovely_component import ( - MyLovelyCompanyTeamProjectComponent, # NOT DRY -) -from com.my_lovely_company.my_lovely_team.my_lovely_project.my_lovely_component import ( - MyLovelyCompanyTeamProjectComponent as component, # DRY -) - - -class C: - @pytest.mark.parametrize( - ("post_data", "message"), - [ - # metadata_version errors. - ( - {}, - "None is an invalid value for Metadata-Version. Error: This field is" - " required. see" - " https://packaging.python.org/specifications/core-metadata", - ), - ( - {"metadata_version": "-1"}, - "'-1' is an invalid value for Metadata-Version. Error: Unknown Metadata" - " Version see" - " https://packaging.python.org/specifications/core-metadata", - ), - # name errors. - ( - {"metadata_version": "1.2"}, - "'' is an invalid value for Name. Error: This field is required. see" - " https://packaging.python.org/specifications/core-metadata", - ), - ( - {"metadata_version": "1.2", "name": "foo-"}, - "'foo-' is an invalid value for Name. Error: Must start and end with a" - " letter or numeral and contain only ascii numeric and '.', '_' and" - " '-'. see https://packaging.python.org/specifications/core-metadata", - ), - # version errors. - ( - {"metadata_version": "1.2", "name": "example"}, - "'' is an invalid value for Version. Error: This field is required. see" - " https://packaging.python.org/specifications/core-metadata", - ), - ( - {"metadata_version": "1.2", "name": "example", "version": "dog"}, - "'dog' is an invalid value for Version. Error: Must start and end with" - " a letter or numeral and contain only ascii numeric and '.', '_' and" - " '-'. see https://packaging.python.org/specifications/core-metadata", - ), - ], - ) - def test_fails_invalid_post_data( - self, pyramid_config, db_request, post_data, message - ): - pyramid_config.testing_securitypolicy(userid=1) - db_request.POST = MultiDict(post_data) - - -def foo(list_a, list_b): - results = ( - User.query.filter(User.foo == "bar").filter( # Because foo. - db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b)) - ).filter(User.xyz.is_(None)). - # Another comment about the filtering on is_quux goes here. - filter(db.not_(User.is_pending.astext.cast(db.Boolean).is_(True))).order_by( - User.created_at.desc() - ).with_for_update(key_share=True).all() - ) - return results - - -def foo2(list_a, list_b): - # Standalone comment reasonably placed. - return User.query.filter(User.foo == "bar").filter( - db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b)) - ).filter(User.xyz.is_(None)) - - -def foo3(list_a, list_b): - return ( - # Standalone comment but weirdly placed. - User.query.filter(User.foo == "bar").filter( - db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b)) - ).filter(User.xyz.is_(None)) - ) -``` - -## Black Output - -```py -from com.my_lovely_company.my_lovely_team.my_lovely_project.my_lovely_component import ( - MyLovelyCompanyTeamProjectComponent, # NOT DRY -) -from com.my_lovely_company.my_lovely_team.my_lovely_project.my_lovely_component import ( - MyLovelyCompanyTeamProjectComponent as component, # DRY -) - - -class C: - @pytest.mark.parametrize( - ("post_data", "message"), - [ - # metadata_version errors. - ( - {}, - "None is an invalid value for Metadata-Version. Error: This field is" - " required. see" - " https://packaging.python.org/specifications/core-metadata", - ), - ( - {"metadata_version": "-1"}, - "'-1' is an invalid value for Metadata-Version. Error: Unknown Metadata" - " Version see" - " https://packaging.python.org/specifications/core-metadata", - ), - # name errors. - ( - {"metadata_version": "1.2"}, - "'' is an invalid value for Name. Error: This field is required. see" - " https://packaging.python.org/specifications/core-metadata", - ), - ( - {"metadata_version": "1.2", "name": "foo-"}, - "'foo-' is an invalid value for Name. Error: Must start and end with a" - " letter or numeral and contain only ascii numeric and '.', '_' and" - " '-'. see https://packaging.python.org/specifications/core-metadata", - ), - # version errors. - ( - {"metadata_version": "1.2", "name": "example"}, - "'' is an invalid value for Version. Error: This field is required. see" - " https://packaging.python.org/specifications/core-metadata", - ), - ( - {"metadata_version": "1.2", "name": "example", "version": "dog"}, - "'dog' is an invalid value for Version. Error: Must start and end with" - " a letter or numeral and contain only ascii numeric and '.', '_' and" - " '-'. see https://packaging.python.org/specifications/core-metadata", - ), - ], - ) - def test_fails_invalid_post_data( - self, pyramid_config, db_request, post_data, message - ): - pyramid_config.testing_securitypolicy(userid=1) - db_request.POST = MultiDict(post_data) - - -def foo(list_a, list_b): - results = ( - User.query.filter(User.foo == "bar") - .filter( # Because foo. - db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b)) - ) - .filter(User.xyz.is_(None)) - # Another comment about the filtering on is_quux goes here. - .filter(db.not_(User.is_pending.astext.cast(db.Boolean).is_(True))) - .order_by(User.created_at.desc()) - .with_for_update(key_share=True) - .all() - ) - return results - - -def foo2(list_a, list_b): - # Standalone comment reasonably placed. - return ( - User.query.filter(User.foo == "bar") - .filter( - db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b)) - ) - .filter(User.xyz.is_(None)) - ) - - -def foo3(list_a, list_b): - return ( - # Standalone comment but weirdly placed. - User.query.filter(User.foo == "bar") - .filter( - db.or_(User.field_a.astext.in_(list_a), User.field_b.astext.in_(list_b)) - ) - .filter(User.xyz.is_(None)) - ) -``` - - diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__expression.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__expression.py.snap index ad9938cb83183..86a5b37df9883 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__expression.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__expression.py.snap @@ -300,40 +300,7 @@ last_call() ) # note: no trailing comma pre-3.6 call(*gidgets[:2]) call(a, *gidgets[:2]) -@@ -208,24 +209,14 @@ - what_is_up_with_those_new_coord_names = (coord_names | set(vars_to_create)) - set( - vars_to_remove - ) --result = ( -- session.query(models.Customer.id) -- .filter( -- models.Customer.account_id == account_id, models.Customer.email == email_address -- ) -- .order_by(models.Customer.id.asc()) -- .all() --) --result = ( -- session.query(models.Customer.id) -- .filter( -- models.Customer.account_id == account_id, models.Customer.email == email_address -- ) -- .order_by( -- models.Customer.id.asc(), -- ) -- .all() --) -+result = session.query(models.Customer.id).filter( -+ models.Customer.account_id == account_id, models.Customer.email == email_address -+).order_by(models.Customer.id.asc()).all() -+result = session.query(models.Customer.id).filter( -+ models.Customer.account_id == account_id, models.Customer.email == email_address -+).order_by( -+ models.Customer.id.asc(), -+).all() - Ø = set() - authors.łukasz.say_thanks() - mapping = { -@@ -328,13 +319,18 @@ +@@ -328,13 +329,18 @@ ): return True if ( @@ -355,7 +322,7 @@ last_call() ^ aaaaaaaa.i << aaaaaaaa.k >> aaaaaaaa.l**aaaaaaaa.m // aaaaaaaa.n ): return True -@@ -342,7 +338,8 @@ +@@ -342,7 +348,8 @@ ~aaaaaaaaaaaaaaaa.a + aaaaaaaaaaaaaaaa.b - aaaaaaaaaaaaaaaa.c * aaaaaaaaaaaaaaaa.d @ aaaaaaaaaaaaaaaa.e @@ -581,14 +548,24 @@ what_is_up_with_those_new_coord_names = (coord_names + set(vars_to_create)) + se what_is_up_with_those_new_coord_names = (coord_names | set(vars_to_create)) - set( vars_to_remove ) -result = session.query(models.Customer.id).filter( - models.Customer.account_id == account_id, models.Customer.email == email_address -).order_by(models.Customer.id.asc()).all() -result = session.query(models.Customer.id).filter( - models.Customer.account_id == account_id, models.Customer.email == email_address -).order_by( - models.Customer.id.asc(), -).all() +result = ( + session.query(models.Customer.id) + .filter( + models.Customer.account_id == account_id, models.Customer.email == email_address + ) + .order_by(models.Customer.id.asc()) + .all() +) +result = ( + session.query(models.Customer.id) + .filter( + models.Customer.account_id == account_id, models.Customer.email == email_address + ) + .order_by( + models.Customer.id.asc(), + ) + .all() +) Ø = set() authors.łukasz.say_thanks() mapping = { diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__fmtonoff.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__fmtonoff.py.snap index 88e2db7a36ea8..16f03bc4250a2 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__fmtonoff.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__fmtonoff.py.snap @@ -287,7 +287,7 @@ d={'a':1, # fmt: on goes + here, andhere, -@@ -80,38 +94,36 @@ +@@ -80,38 +94,42 @@ def import_as_names(): # fmt: off @@ -330,14 +330,19 @@ d={'a':1, - .filter(models.Customer.account_id == account_id, - models.Customer.email == email_address)\ - .order_by(models.Customer.id.asc())\ -- .all() -+ result = session.query(models.Customer.id).filter( -+ models.Customer.account_id == account_id, models.Customer.email == email_address -+ ).order_by(models.Customer.id.asc()).all() ++ result = ( ++ session.query(models.Customer.id) ++ .filter( ++ models.Customer.account_id == account_id, ++ models.Customer.email == email_address, ++ ) ++ .order_by(models.Customer.id.asc()) + .all() ++ ) # fmt: on -@@ -132,10 +144,10 @@ +@@ -132,10 +150,10 @@ """Another known limitation.""" # fmt: on # fmt: off @@ -352,7 +357,7 @@ d={'a':1, # fmt: on # fmt: off # ...but comments still get reformatted even though they should not be -@@ -153,9 +165,7 @@ +@@ -153,9 +171,7 @@ ) ) # fmt: off @@ -363,7 +368,7 @@ d={'a':1, # fmt: on _type_comment_re = re.compile( r""" -@@ -178,7 +188,7 @@ +@@ -178,7 +194,7 @@ $ """, # fmt: off @@ -372,7 +377,7 @@ d={'a':1, # fmt: on ) -@@ -216,8 +226,7 @@ +@@ -216,8 +232,7 @@ xxxxxxxxxx_xxxxxxxxxxx_xxxxxxx_xxxxxxxxx=5, ) # fmt: off @@ -512,9 +517,15 @@ def yield_expr(): def example(session): # fmt: off - result = session.query(models.Customer.id).filter( - models.Customer.account_id == account_id, models.Customer.email == email_address - ).order_by(models.Customer.id.asc()).all() + result = ( + session.query(models.Customer.id) + .filter( + models.Customer.account_id == account_id, + models.Customer.email == email_address, + ) + .order_by(models.Customer.id.asc()) + .all() + ) # fmt: on diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__function.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__function.py.snap index 0e3bfa106cfe5..dacb0b6cac47d 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__function.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__function.py.snap @@ -107,7 +107,7 @@ def __await__(): return (yield) ```diff --- Black +++ Ruff -@@ -65,18 +65,14 @@ +@@ -65,6 +65,7 @@ def spaces2(result=_core.Value(None)): assert fut is self._read_fut, (fut, self._read_fut) @@ -115,22 +115,6 @@ def __await__(): return (yield) def example(session): -- result = ( -- session.query(models.Customer.id) -- .filter( -- models.Customer.account_id == account_id, -- models.Customer.email == email_address, -- ) -- .order_by(models.Customer.id.asc()) -- .all() -- ) -+ result = session.query(models.Customer.id).filter( -+ models.Customer.account_id == account_id, -+ models.Customer.email == email_address, -+ ).order_by(models.Customer.id.asc()).all() - - - def long_lines(): ``` ## Ruff Output @@ -207,10 +191,15 @@ def spaces2(result=_core.Value(None)): def example(session): - result = session.query(models.Customer.id).filter( - models.Customer.account_id == account_id, - models.Customer.email == email_address, - ).order_by(models.Customer.id.asc()).all() + result = ( + session.query(models.Customer.id) + .filter( + models.Customer.account_id == account_id, + models.Customer.email == email_address, + ) + .order_by(models.Customer.id.asc()) + .all() + ) def long_lines(): diff --git a/crates/ruff_python_formatter/tests/snapshots/format@call_chains.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@call_chains.py.snap new file mode 100644 index 0000000000000..cca9b84a2f2df --- /dev/null +++ b/crates/ruff_python_formatter/tests/snapshots/format@call_chains.py.snap @@ -0,0 +1,340 @@ +--- +source: crates/ruff_python_formatter/tests/fixtures.rs +input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/call_chains.py +--- +## Input +```py +# 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, +) + + +``` + +## Output +```py +# 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/tests/snapshots/format@expression__attribute.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__attribute.py.snap index f1a5dbb035c50..a51328e6e7a71 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__attribute.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__attribute.py.snap @@ -141,10 +141,10 @@ a = Namespace() ( - a. + a. # trailing dot comment # comment # in between - b # trailing dot comment # trailing identifier comment + b # trailing identifier comment ) @@ -155,10 +155,10 @@ a.aaaaaaaaaaaaaaaaaaaaa.lllllllllllllllllllllllllllloooooooooong.chaaaaaaaaaaaaa # chain if and only if we need them, that is if there are own line comments inside # the chain. x1 = ( - a.b. + a.b. # comment 2 # comment 1 # comment 3 - c.d # comment 2 + c.d ) x20 = a.b diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap index 0960faa65260f..8120fdd4d8bb9 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap @@ -58,7 +58,7 @@ f( 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( @@ -178,11 +178,16 @@ f( really=True, ) -# TODO(konstin): 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( - models.Customer.account_id == 10000, - models.Customer.email == "user@example.org", -).order_by(models.Customer.id.asc()).all() +# 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( + models.Customer.account_id == 10000, + models.Customer.email == "user@example.org", + ) + .order_by(models.Customer.id.asc()) + .all() +) # TODO(konstin): Black has this special case for comment placement where everything stays in one line f("aaaaaaaa", "aaaaaaaa", "aaaaaaaa", "aaaaaaaa", "aaaaaaaa", "aaaaaaaa", "aaaaaaaa")