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..dea0603f863245 --- /dev/null +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/call_chains.py @@ -0,0 +1,82 @@ +# 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 +) + +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() +) + +raise ( + Blog.objects.filter( + entry__headline__contains="Lennon", + ) + .limit_results[:10] + .filter( + entry__pub_date__month=10, + ) +) + +# Nested call chains +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() 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..6dd2f6bb6346fc 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,19 @@ use crate::prelude::*; use crate::FormatNodeRule; #[derive(Default)] -pub struct FormatExprAttribute; +pub struct FormatExprAttribute { + /// Parentheses for fluent style + 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 +48,18 @@ 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 parentheses on to inner attributes or call chains + value + .format() + .with_options(Parentheses::FluentStyle) + .fmt(f)?; + match value.as_ref() { + Expr::Call(_) | Expr::Subscript(_) => { + soft_line_break().fmt(f)?; + } + _ => {} + } } else { value.format().fmt(f)?; } @@ -50,16 +68,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..a2364caa9c2bc3 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,19 @@ use crate::prelude::*; use crate::FormatNodeRule; #[derive(Default)] -pub struct FormatExprCall; +pub struct FormatExprCall { + /// Parentheses for fluent style + 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 +36,15 @@ impl FormatNodeRule for FormatExprCall { keywords, } = item; + if self.parentheses == Some(Parentheses::FluentStyle) { + // Fluent style: We need to pass the parentheses on to inner attributes or call chains + func.format() + .with_options(Parentheses::FluentStyle) + .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 @@ -32,21 +52,20 @@ impl FormatNodeRule for FormatExprCall { // # This function has a dangling comment // ) // ``` + let comments = f.context().comments().clone(); if args.is_empty() && keywords.is_empty() { - 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(")"), + )] ); } + debug_assert!(comments.dangling_comments(item).is_empty()); + let all_args = format_with(|f: &mut PyFormatter| { let source = f.context().source(); let mut joiner = f.join_comma_separated(item.end()); @@ -88,7 +107,6 @@ impl FormatNodeRule for FormatExprCall { write!( f, [ - func.format(), // The outer group is for things like // ```python // get_collection( @@ -104,7 +122,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/expr_subscript.rs b/crates/ruff_python_formatter/src/expression/expr_subscript.rs index 464028c2e1a605..0a6f3da4b0dd00 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::parentheses::{NeedsParentheses, OptionalParentheses, Parentheses}; use crate::prelude::*; use crate::FormatNodeRule; #[derive(Default)] -pub struct FormatExprSubscript; +pub struct FormatExprSubscript { + parentheses: Option, +} + +impl FormatRuleWithOptions> for FormatExprSubscript { + /// Parentheses for fluent style + type Options = Option; + + fn with_options(mut self, options: Self::Options) -> Self { + self.parentheses = options; + self + } +} impl FormatNodeRule for FormatExprSubscript { fn fmt_fields(&self, item: &ExprSubscript, f: &mut PyFormatter) -> FormatResult<()> { @@ -30,12 +41,21 @@ 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| { + if self.parentheses == Some(Parentheses::FluentStyle) { + // Fluent style: We need to pass the parentheses on to inner attributes or call chains + value.format().with_options(Parentheses::FluentStyle).fmt(f) + } else { + 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| { diff --git a/crates/ruff_python_formatter/src/expression/mod.rs b/crates/ruff_python_formatter/src/expression/mod.rs index a7c395eb041bdd..bf9e2b98214ed8 100644 --- a/crates/ruff_python_formatter/src/expression/mod.rs +++ b/crates/ruff_python_formatter/src/expression/mod.rs @@ -64,7 +64,16 @@ impl FormatRuleWithOptions> for FormatExpr { impl FormatRule> for FormatExpr { fn fmt(&self, expression: &Expr, f: &mut PyFormatter) -> FormatResult<()> { - let parentheses = self.parentheses; + let mut parentheses = self.parentheses; + + // Nested expressions that are in some outer expression's parentheses may be formatted in + // fluent style + if parentheses != Parentheses::FluentStyle + && f.context().node_level() == NodeLevel::ParenthesizedExpression + && is_fluent_style_call_chain(expression) + { + parentheses = Parentheses::FluentStyle; + } let format_expr = format_with(|f| match expression { Expr::BoolOp(expr) => expr.format().with_options(Some(parentheses)).fmt(f), @@ -83,12 +92,12 @@ 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::Subscript(expr) => expr.format().fmt(f), + Expr::Attribute(expr) => expr.format().with_options(Some(parentheses)).fmt(f), + Expr::Subscript(expr) => expr.format().with_options(Some(parentheses)).fmt(f), Expr::Starred(expr) => expr.format().fmt(f), Expr::Name(expr) => expr.format().fmt(f), Expr::List(expr) => expr.format().fmt(f), @@ -102,7 +111,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 { @@ -154,6 +164,17 @@ impl Format> for MaybeParenthesizeExpression<'_> { parenthesize, } = self; + // Fluent style means we always add parentheses, so we don't need to check for existing + // parentheses, comments or `needs_parentheses` + let omit_optional_parentheses_result = + can_omit_optional_parentheses(expression, f.context()); + if omit_optional_parentheses_result == CanOmitOptionalParentheses::FluentStyle { + return parenthesize_if_expands( + &expression.format().with_options(Parentheses::FluentStyle), + ) + .fmt(f); + } + let comments = f.context().comments(); let preserve_parentheses = parenthesize.is_optional() && is_expression_parenthesized(AnyNodeRef::from(*expression), f.context().source()); @@ -179,12 +200,15 @@ impl Format> for MaybeParenthesizeExpression<'_> { 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 omit_optional_parentheses_result { + CanOmitOptionalParentheses::Yes | CanOmitOptionalParentheses::FluentStyle => { + optional_parentheses(&expression.format().with_options(Parentheses::Never)) + .fmt(f) + } + CanOmitOptionalParentheses::No => parenthesize_if_expands( + &expression.format().with_options(Parentheses::Never), + ) + .fmt(f), } } OptionalParentheses::Always => { @@ -261,10 +285,38 @@ 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, +) -> CanOmitOptionalParentheses { let mut visitor = CanOmitOptionalParenthesesVisitor::new(context.source()); visitor.visit_subexpression(expr); - visitor.can_omit() + + if visitor.max_priority_count > 1 { + if visitor.max_priority == OperatorPriority::Attribute { + CanOmitOptionalParentheses::FluentStyle + } else { + CanOmitOptionalParentheses::No + } + } else if visitor.max_priority == OperatorPriority::Attribute { + CanOmitOptionalParentheses::Yes + } else if !visitor.any_parenthesized_expressions { + // Only use the more complex IR when there is any expression that we can possibly split by + CanOmitOptionalParentheses::No + } else { + // Only use the layout if the first or last expression has parentheses of some sort. + let first_parenthesized = visitor + .first + .map_or(false, |first| has_parentheses(first, visitor.source)); + let last_parenthesized = visitor + .last + .map_or(false, |last| has_parentheses(last, visitor.source)); + if first_parenthesized || last_parenthesized { + CanOmitOptionalParentheses::Yes + } else { + CanOmitOptionalParentheses::No + } + } } #[derive(Clone, Debug)] @@ -277,6 +329,13 @@ struct CanOmitOptionalParenthesesVisitor<'input> { source: &'input str, } +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub(crate) enum CanOmitOptionalParentheses { + Yes, + No, + FluentStyle, +} + impl<'input> CanOmitOptionalParenthesesVisitor<'input> { fn new(source: &'input str) -> Self { Self { @@ -365,8 +424,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; } @@ -409,26 +470,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 - .map_or(false, |first| has_parentheses(first, self.source)); - let last_parenthesized = self - .last - .map_or(false, |last| has_parentheses(last, self.source)); - first_parenthesized || last_parenthesized - } - } } impl<'input> PreorderVisitor<'input> for CanOmitOptionalParenthesesVisitor<'input> { @@ -448,6 +489,60 @@ impl<'input> PreorderVisitor<'input> for CanOmitOptionalParenthesesVisitor<'inpu } } +/// Returns `true` if this expression is a call chain that should be formatted in fluent style. +/// +/// 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. +/// +/// We only use this check if we're already in a parenthesized context, otherwise refer to +/// [`CanOmitOptionalParentheses`]. +/// +/// Below, the left hand side of 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() +/// ``` +fn is_fluent_style_call_chain(mut expr: &Expr) -> bool { + let mut attributes_after_parentheses = 0; + loop { + match expr { + Expr::Attribute(ast::ExprAttribute { value, .. }) => { + // `f().x` | `data[:100].T` + if matches!(value.as_ref(), Expr::Call(_) | Expr::Subscript(_)) { + attributes_after_parentheses += 1; + } + expr = value; + } + Expr::Call(ast::ExprCall { func: inner, .. }) + | Expr::Subscript(ast::ExprSubscript { value: inner, .. }) => { + expr = inner; + } + _ => break, + } + } + attributes_after_parentheses >= 2 +} + 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 abe7cc9d623c20..a3f87e3ae8551b 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 ad9938cb83183d..86a5b37df98839 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 88e2db7a36ea85..16f03bc4250a23 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 0e3bfa106cfe5e..dacb0b6cac47d3 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 00000000000000..db33b632cdc8fd --- /dev/null +++ b/crates/ruff_python_formatter/tests/snapshots/format@call_chains.py.snap @@ -0,0 +1,176 @@ +--- +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 +) + +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() +) + +raise ( + Blog.objects.filter( + entry__headline__contains="Lennon", + ) + .limit_results[:10] + .filter( + entry__pub_date__month=10, + ) +) + +# Nested call chains +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() +``` + +## 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) + +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() +) + +raise ( + Blog.objects.filter( + entry__headline__contains="Lennon", + ) + .limit_results[:10] + .filter( + entry__pub_date__month=10, + ) +) + +# Nested call chains +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() +``` + + + 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")