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 00000000000000..9bb0d88db59ccd --- /dev/null +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/call_chains.py @@ -0,0 +1,74 @@ +# Test cases for call chains and optional parentheses +raise OsError("") from a.aaaaa( + aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa +).a(aaaa) + +raise OsError( + "sökdjffffsldkfjlhsakfjhalsökafhsöfdahsödfjösaaksjdllllllllllllll" +) from a.aaaaa( + aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa +).a( + aaaa +) + +blogs1 = Blog.objects.filter(entry__headline__contains="Lennon").filter( + entry__pub_date__year=2008 +) + +blogs2 = 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 +result = ( + session.query(models.Customer.id) + .filter( + models.Customer.account_id == account_id, models.Customer.email == email_address + ) + .count() +) + +# Currently formatted wrongly (no nested call chains) +raise ( + Blog.objects.filter( + entry__headline__contains="Lennon", + ).filter( + entry__pub_date__year=2008, + ) + + Blog.objects.filter( + entry__headline__contains="Lennon", + ) + .filter( + entry__pub_date__year=2008, + ) + .filter( + entry__pub_date__year=2008, + ) +).a() + 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 8c372180ce7f51..e72ad7e34dfce9 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/expression/expr_attribute.rs b/crates/ruff_python_formatter/src/expression/expr_attribute.rs index a577ae88d06106..28beda679bb92b 100644 --- a/crates/ruff_python_formatter/src/expression/expr_attribute.rs +++ b/crates/ruff_python_formatter/src/expression/expr_attribute.rs @@ -1,7 +1,6 @@ -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}; @@ -9,7 +8,18 @@ use crate::prelude::*; use crate::FormatNodeRule; #[derive(Default)] -pub struct FormatExprAttribute; +pub struct FormatExprAttribute { + parentheses: Option, +} + +impl FormatRuleWithOptions> for FormatExprAttribute { + type Options = Option; + + fn with_options(mut self, options: Self::Options) -> Self { + self.parentheses = options; + self + } +} impl FormatNodeRule for FormatExprAttribute { fn fmt_fields(&self, item: &ExprAttribute, f: &mut PyFormatter) -> FormatResult<()> { @@ -37,11 +47,22 @@ 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 self.parentheses == Some(Parentheses::FluentStyle) { + // Fluent style: We need to pass the parenthese on to inner attributes or call chains + match value.as_ref() { + Expr::Attribute(_) => value + .format() + .with_options(Parentheses::FluentStyle) + .fmt(f)?, + Expr::Call(_) | Expr::Subscript(_) => { + value + .format() + .with_options(Parentheses::FluentStyle) + .fmt(f)?; + soft_line_break().fmt(f)? + } + _ => value.format().fmt(f)?, + } } else { value.format().fmt(f)?; } @@ -50,16 +71,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 self.parentheses == Some(Parentheses::FluentStyle) { + // 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( diff --git a/crates/ruff_python_formatter/src/expression/expr_call.rs b/crates/ruff_python_formatter/src/expression/expr_call.rs index 85ecfff06bc0fa..207871dc4192b2 100644 --- a/crates/ruff_python_formatter/src/expression/expr_call.rs +++ b/crates/ruff_python_formatter/src/expression/expr_call.rs @@ -1,11 +1,10 @@ +use ruff_formatter::{write, FormatRuleWithOptions}; +use ruff_python_ast::node::AnyNodeRef; use ruff_python_ast::{Expr, ExprCall, Ranged}; +use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer}; use ruff_text_size::{TextRange, TextSize}; use crate::builders::empty_parenthesized_with_dangling_comments; -use ruff_formatter::write; -use ruff_python_ast::node::AnyNodeRef; -use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer}; - use crate::expression::expr_generator_exp::GeneratorExpParentheses; use crate::expression::parentheses::{ parenthesized, NeedsParentheses, OptionalParentheses, Parentheses, @@ -14,7 +13,18 @@ use crate::prelude::*; use crate::FormatNodeRule; #[derive(Default)] -pub struct FormatExprCall; +pub struct FormatExprCall { + parentheses: Option, +} + +impl FormatRuleWithOptions> for FormatExprCall { + type Options = Option; + + fn with_options(mut self, options: Self::Options) -> Self { + self.parentheses = options; + self + } +} impl FormatNodeRule for FormatExprCall { fn fmt_fields(&self, item: &ExprCall, f: &mut PyFormatter) -> FormatResult<()> { @@ -25,6 +35,19 @@ impl FormatNodeRule for FormatExprCall { keywords, } = item; + if self.parentheses == Some(Parentheses::FluentStyle) { + // Fluent style: We need to pass the parenthese on to inner attributes or call chains + match func.as_ref() { + Expr::Attribute(_) | Expr::Call(_) | Expr::Subscript(_) => func + .format() + .with_options(Parentheses::FluentStyle) + .fmt(f)?, + _ => func.format().fmt(f)?, + } + } else { + func.format().fmt(f)?; + } + // We have a case with `f()` without any argument, which is a special case because we can // have a comment with no node attachment inside: // ```python @@ -36,14 +59,11 @@ impl FormatNodeRule for FormatExprCall { let comments = f.context().comments().clone(); return write!( f, - [ - func.format(), - empty_parenthesized_with_dangling_comments( - text("("), - comments.dangling_comments(item), - text(")"), - ) - ] + [empty_parenthesized_with_dangling_comments( + text("("), + comments.dangling_comments(item), + text(")"), + )] ); } @@ -88,7 +108,6 @@ impl FormatNodeRule for FormatExprCall { write!( f, [ - func.format(), // The outer group is for things like // ```python // get_collection( @@ -104,7 +123,6 @@ impl FormatNodeRule for FormatExprCall { // hey_this_is_a_very_long_call, it_has_funny_attributes_asdf_asdf, really=True // ) // ``` - // TODO(konstin): Doesn't work see wrongly formatted test parenthesized("(", &group(&all_args), ")") ] ) diff --git a/crates/ruff_python_formatter/src/expression/mod.rs b/crates/ruff_python_formatter/src/expression/mod.rs index 690509c1cdc075..91d04d09a2c48a 100644 --- a/crates/ruff_python_formatter/src/expression/mod.rs +++ b/crates/ruff_python_formatter/src/expression/mod.rs @@ -81,11 +81,11 @@ impl FormatRule> for FormatExpr { Expr::Yield(expr) => expr.format().fmt(f), Expr::YieldFrom(expr) => expr.format().fmt(f), Expr::Compare(expr) => expr.format().with_options(Some(parentheses)).fmt(f), - Expr::Call(expr) => expr.format().fmt(f), + Expr::Call(expr) => expr.format().with_options(Some(parentheses)).fmt(f), Expr::FormattedValue(expr) => expr.format().fmt(f), Expr::JoinedStr(expr) => expr.format().fmt(f), Expr::Constant(expr) => expr.format().fmt(f), - Expr::Attribute(expr) => expr.format().fmt(f), + Expr::Attribute(expr) => expr.format().with_options(Some(parentheses)).fmt(f), Expr::Subscript(expr) => expr.format().fmt(f), Expr::Starred(expr) => expr.format().fmt(f), Expr::Name(expr) => expr.format().fmt(f), @@ -100,7 +100,8 @@ impl FormatRule> for FormatExpr { is_expression_parenthesized(AnyNodeRef::from(expression), f.context().source()) } Parentheses::Always => true, - Parentheses::Never => false, + // Fluent style means we already have parentheses + Parentheses::Never | Parentheses::FluentStyle => false, }; if parenthesize { @@ -114,7 +115,7 @@ impl FormatRule> for FormatExpr { NodeLevel::Expression(_) | NodeLevel::ParenthesizedExpression => None, }; - let result = Format::fmt(&format_expr, f); + let result = format_expr.fmt(f); if let Some(saved_level) = saved_level { f.context_mut().set_node_level(saved_level); @@ -165,7 +166,23 @@ impl Format> for MaybeParenthesizeExpression<'_> { || comments.has_trailing_own_line_comments(*expression); if preserve_parentheses || has_comments { - return expression.format().with_options(Parentheses::Always).fmt(f); + if can_omit_optional_parentheses(expression, f.context()) + == CanOmitOptionalParenthesesResult::FluentStyle + { + return parenthesized( + "(", + &format_with(|f| { + expression + .format() + .with_options(Parentheses::FluentStyle) + .fmt(f) + }), + ")", + ) + .fmt(f); + } else { + return expression.format().with_options(Parentheses::Always).fmt(f); + } } let needs_parentheses = expression.needs_parentheses(*parent, f.context()); @@ -180,14 +197,27 @@ impl Format> for MaybeParenthesizeExpression<'_> { Parenthesize::Optional | Parenthesize::IfBreaks => needs_parentheses, }; + if can_omit_optional_parentheses(expression, f.context()) + == CanOmitOptionalParenthesesResult::FluentStyle + { + return parenthesize_if_expands( + &expression.format().with_options(Parentheses::FluentStyle), + ) + .fmt(f); + } + match needs_parentheses { OptionalParentheses::Multiline if *parenthesize != Parenthesize::IfRequired => { - if can_omit_optional_parentheses(expression, f.context()) { - optional_parentheses(&expression.format().with_options(Parentheses::Never)) - .fmt(f) - } else { - parenthesize_if_expands(&expression.format().with_options(Parentheses::Never)) - .fmt(f) + match can_omit_optional_parentheses(expression, f.context()) { + CanOmitOptionalParenthesesResult::Yes + | CanOmitOptionalParenthesesResult::FluentStyle => { + optional_parentheses(&expression.format().with_options(Parentheses::Never)) + .fmt(f) + } + CanOmitOptionalParenthesesResult::No => parenthesize_if_expands( + &expression.format().with_options(Parentheses::Never), + ) + .fmt(f), } } OptionalParentheses::Always => { @@ -264,7 +294,10 @@ impl<'ast> IntoFormat> for Expr { /// * The expression contains at least one parenthesized sub expression (optimization to avoid unnecessary work) /// /// This mimics Black's [`_maybe_split_omitting_optional_parens`](https://github.com/psf/black/blob/d1248ca9beaf0ba526d265f4108836d89cf551b7/src/black/linegen.py#L746-L820) -fn can_omit_optional_parentheses(expr: &Expr, context: &PyFormatContext) -> bool { +fn can_omit_optional_parentheses( + expr: &Expr, + context: &PyFormatContext, +) -> CanOmitOptionalParenthesesResult { let mut visitor = CanOmitOptionalParenthesesVisitor::new(context.source()); visitor.visit_subexpression(expr); visitor.can_omit() @@ -280,6 +313,13 @@ struct CanOmitOptionalParenthesesVisitor<'input> { source: &'input str, } +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) enum CanOmitOptionalParenthesesResult { + Yes, + No, + FluentStyle, +} + impl<'input> CanOmitOptionalParenthesesVisitor<'input> { fn new(source: &'input str) -> Self { Self { @@ -413,14 +453,18 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> { walk_expr(self, expr); } - fn can_omit(self) -> bool { + fn can_omit(self) -> CanOmitOptionalParenthesesResult { if self.max_priority_count > 1 { - false + if self.max_priority == OperatorPriority::Attribute { + CanOmitOptionalParenthesesResult::FluentStyle + } else { + CanOmitOptionalParenthesesResult::No + } } else if self.max_priority == OperatorPriority::Attribute { - true + CanOmitOptionalParenthesesResult::Yes } else if !self.any_parenthesized_expressions { // Only use the more complex IR when there is any expression that we can possibly split by - false + CanOmitOptionalParenthesesResult::No } else { // Only use the layout if the first or last expression has parentheses of some sort. let first_parenthesized = self @@ -429,7 +473,11 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> { let last_parenthesized = self .last .map_or(false, |last| has_parentheses(last, self.source)); - first_parenthesized || last_parenthesized + if first_parenthesized || last_parenthesized { + CanOmitOptionalParenthesesResult::Yes + } else { + CanOmitOptionalParenthesesResult::No + } } } } diff --git a/crates/ruff_python_formatter/src/expression/parentheses.rs b/crates/ruff_python_formatter/src/expression/parentheses.rs index bdef4796bf0bcb..29ec1182b4f260 100644 --- a/crates/ruff_python_formatter/src/expression/parentheses.rs +++ b/crates/ruff_python_formatter/src/expression/parentheses.rs @@ -71,6 +71,10 @@ pub enum Parentheses { /// Never add parentheses Never, + + /// Switch call chain and attribute formatting to fluent style. This is otherwise identical to + /// `Never`, fluent style implies a set of outer parentheses + FluentStyle, } pub(crate) fn is_expression_parenthesized(expr: AnyNodeRef, contents: &str) -> bool { 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 b7752423a8e470..00000000000000 --- 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 af4a17bd1522e8..e3c280cefba363 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 @@ -323,41 +323,7 @@ last_call() ) # note: no trailing comma pre-3.6 call(*gidgets[:2]) call(a, *gidgets[:2]) -@@ -207,25 +214,15 @@ - ) - 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 +325,18 @@ +@@ -328,13 +335,18 @@ ): return True if ( @@ -379,7 +345,7 @@ last_call() ^ aaaaaaaa.i << aaaaaaaa.k >> aaaaaaaa.l**aaaaaaaa.m // aaaaaaaa.n ): return True -@@ -342,7 +344,8 @@ +@@ -342,7 +354,8 @@ ~aaaaaaaaaaaaaaaa.a + aaaaaaaaaaaaaaaa.b - aaaaaaaaaaaaaaaa.c * aaaaaaaaaaaaaaaa.d @ aaaaaaaaaaaaaaaa.e @@ -611,14 +577,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 1f83b05cc29621..ec79ee54467626 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 @@ -289,7 +289,7 @@ d={'a':1, # fmt: on goes + here, andhere, -@@ -80,38 +93,35 @@ +@@ -80,38 +93,41 @@ def import_as_names(): # fmt: off @@ -331,14 +331,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 +142,10 @@ +@@ -132,10 +148,10 @@ """Another known limitation.""" # fmt: on # fmt: off @@ -353,7 +358,7 @@ d={'a':1, # fmt: on # fmt: off # ...but comments still get reformatted even though they should not be -@@ -153,9 +163,7 @@ +@@ -153,9 +169,7 @@ ) ) # fmt: off @@ -364,7 +369,7 @@ d={'a':1, # fmt: on _type_comment_re = re.compile( r""" -@@ -178,7 +186,7 @@ +@@ -178,7 +192,7 @@ $ """, # fmt: off @@ -373,7 +378,7 @@ d={'a':1, # fmt: on ) -@@ -216,8 +224,7 @@ +@@ -216,8 +230,7 @@ xxxxxxxxxx_xxxxxxxxxxx_xxxxxxx_xxxxxxxxx=5, ) # fmt: off @@ -511,9 +516,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 d300b646960559..e739a522f5ba6f 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 @@ -117,7 +117,7 @@ def __await__(): return (yield) def func_no_args(): -@@ -65,18 +64,14 @@ +@@ -65,6 +64,7 @@ def spaces2(result=_core.Value(None)): assert fut is self._read_fut, (fut, self._read_fut) @@ -125,22 +125,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 @@ -216,10 +200,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 00000000000000..6c344b4f7b3be5 --- /dev/null +++ b/crates/ruff_python_formatter/tests/snapshots/format@call_chains.py.snap @@ -0,0 +1,157 @@ +--- +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 +raise OsError("") from a.aaaaa( + aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa +).a(aaaa) + +raise OsError( + "sökdjffffsldkfjlhsakfjhalsökafhsöfdahsödfjösaaksjdllllllllllllll" +) from a.aaaaa( + aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa +).a( + aaaa +) + +blogs1 = Blog.objects.filter(entry__headline__contains="Lennon").filter( + entry__pub_date__year=2008 +) + +blogs2 = 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 +result = ( + session.query(models.Customer.id) + .filter( + models.Customer.account_id == account_id, models.Customer.email == email_address + ) + .count() +) + +# Currently formatted wrongly (no nested call chains) +raise ( + Blog.objects.filter( + entry__headline__contains="Lennon", + ).filter( + entry__pub_date__year=2008, + ) + + Blog.objects.filter( + entry__headline__contains="Lennon", + ) + .filter( + entry__pub_date__year=2008, + ) + .filter( + entry__pub_date__year=2008, + ) +).a() + +``` + +## Output +```py +# Test cases for call chains and optional parentheses +raise OsError("") from a.aaaaa( + aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa +).a(aaaa) + +raise OsError( + "sökdjffffsldkfjlhsakfjhalsökafhsöfdahsödfjösaaksjdllllllllllllll" +) from a.aaaaa( + aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfksajlhfajfjfsaahflakjslhdfkjalhdskjfa +).a(aaaa) + +blogs1 = Blog.objects.filter(entry__headline__contains="Lennon").filter( + entry__pub_date__year=2008 +) + +blogs2 = 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 +result = ( + session.query(models.Customer.id) + .filter( + models.Customer.account_id == account_id, models.Customer.email == email_address + ) + .count() +) + +# Currently formatted wrongly (no nested call chains) +raise ( + Blog.objects.filter( + entry__headline__contains="Lennon", + ).filter( + entry__pub_date__year=2008, + ) + + Blog.objects.filter( + entry__headline__contains="Lennon", + ).filter( + entry__pub_date__year=2008, + ).filter( + entry__pub_date__year=2008, + ) +).a() +``` + + + 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 bac556ce5f2485..701d25dfb7df8e 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( @@ -152,11 +152,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")