From 1cd7790a8a2fca332be31949be2a06ed9b90f7ae Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 24 Aug 2023 14:09:25 +0200 Subject: [PATCH] Use BestFits for non-fluent attribute chains (#6817) --- .../fixtures/ruff/expression/unsplittable.py | 13 ++++++++ .../test/fixtures/ruff/expression/yield.py | 7 ++++ .../src/expression/expr_call.rs | 26 ++++++--------- ...__trailing_commas_in_leading_parts.py.snap | 20 +++++++++--- .../format@expression__unsplittable.py.snap | 32 +++++++++++++++++++ .../format@expression__yield.py.snap | 18 +++++++++++ 6 files changed, 95 insertions(+), 21 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unsplittable.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unsplittable.py index 83a1158ec3..721939bcbe 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unsplittable.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unsplittable.py @@ -52,3 +52,16 @@ for converter in connection.ops.get_db_converters( expression ) + expression.get_db_converters(connection): ... + + +aaa = ( + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb # awkward comment +) + +def test(): + m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyField(Person, blank=True) + + m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyFieldAttributeChainField + +m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyField(Person, blank=True) +m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyFieldAttributeChainFieeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeld diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/yield.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/yield.py index ff587bd812..0959855cf5 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/yield.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/yield.py @@ -66,6 +66,13 @@ def foo(): 1 ) + yield ( + "# * Make sure each ForeignKey and OneToOneField has `on_delete` set " + "to the desired behavior" + ) + + yield aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + ccccccccccccccccccccccccccccccccccccccccccccccccccccccc + yield ("Cache key will cause errors if used with memcached: %r " "(longer than %s)" % ( key, MEMCACHE_MAX_KEY_LENGTH, diff --git a/crates/ruff_python_formatter/src/expression/expr_call.rs b/crates/ruff_python_formatter/src/expression/expr_call.rs index fe2813626f..250dbe8d65 100644 --- a/crates/ruff_python_formatter/src/expression/expr_call.rs +++ b/crates/ruff_python_formatter/src/expression/expr_call.rs @@ -1,5 +1,6 @@ use crate::expression::CallChainLayout; -use ruff_formatter::FormatRuleWithOptions; + +use ruff_formatter::{format_args, write, FormatRuleWithOptions}; use ruff_python_ast::node::AnyNodeRef; use ruff_python_ast::{Expr, ExprCall}; @@ -31,15 +32,11 @@ impl FormatNodeRule for FormatExprCall { 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) + let fmt_func = 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), }); // Allow to indent the parentheses while @@ -51,9 +48,9 @@ impl FormatNodeRule for FormatExprCall { if call_chain_layout == CallChainLayout::Fluent && self.call_chain_layout == CallChainLayout::Default { - group(&fmt_inner).fmt(f) + group(&format_args![fmt_func, arguments.format()]).fmt(f) } else { - fmt_inner.fmt(f) + write!(f, [fmt_func, arguments.format()]) } } } @@ -69,10 +66,7 @@ impl NeedsParentheses for ExprCall { { OptionalParentheses::Multiline } else { - match self.func.needs_parentheses(self.into(), context) { - OptionalParentheses::BestFit => OptionalParentheses::Never, - parentheses => parentheses, - } + self.func.needs_parentheses(self.into(), context) } } } diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__trailing_commas_in_leading_parts.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__trailing_commas_in_leading_parts.py.snap index 3abf27d802..514ea6258a 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__trailing_commas_in_leading_parts.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__trailing_commas_in_leading_parts.py.snap @@ -56,6 +56,18 @@ assert xxxxxxxxx.xxxxxxxxx.xxxxxxxxx( # Example from https://github.com/psf/black/issues/3229 +@@ -43,8 +41,6 @@ + ) + + # Regression test for https://github.com/psf/black/issues/3414. +-assert xxxxxxxxx.xxxxxxxxx.xxxxxxxxx( +- xxxxxxxxx +-).xxxxxxxxxxxxxxxxxx(), ( +- "xxx {xxxxxxxxx} xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" +-) ++assert ( ++ xxxxxxxxx.xxxxxxxxx.xxxxxxxxx(xxxxxxxxx).xxxxxxxxxxxxxxxxxx() ++), "xxx {xxxxxxxxx} xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" ``` ## Ruff Output @@ -104,11 +116,9 @@ assert ( ) # Regression test for https://github.com/psf/black/issues/3414. -assert xxxxxxxxx.xxxxxxxxx.xxxxxxxxx( - xxxxxxxxx -).xxxxxxxxxxxxxxxxxx(), ( - "xxx {xxxxxxxxx} xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" -) +assert ( + xxxxxxxxx.xxxxxxxxx.xxxxxxxxx(xxxxxxxxx).xxxxxxxxxxxxxxxxxx() +), "xxx {xxxxxxxxx} xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" ``` ## Black Output diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__unsplittable.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__unsplittable.py.snap index 354d07487d..d10a307725 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__unsplittable.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__unsplittable.py.snap @@ -58,6 +58,19 @@ for converter in connection.ops.get_db_converters( expression ) + expression.get_db_converters(connection): ... + + +aaa = ( + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb # awkward comment +) + +def test(): + m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyField(Person, blank=True) + + m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyFieldAttributeChainField + +m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyField(Person, blank=True) +m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyFieldAttributeChainFieeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeld ``` ## Output @@ -120,6 +133,25 @@ for converter in connection.ops.get_db_converters( expression ) + expression.get_db_converters(connection): ... + + +aaa = bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb # awkward comment + + +def test(): + m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = ( + models.ManyToManyField(Person, blank=True) + ) + + m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = ( + models.ManyToManyFieldAttributeChainField + ) + + +m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = ( + models.ManyToManyField(Person, blank=True) +) +m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyFieldAttributeChainFieeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeld ``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__yield.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__yield.py.snap index f8090247fc..7c78a36180 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__yield.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__yield.py.snap @@ -72,6 +72,13 @@ def foo(): 1 ) + yield ( + "# * Make sure each ForeignKey and OneToOneField has `on_delete` set " + "to the desired behavior" + ) + + yield aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + ccccccccccccccccccccccccccccccccccccccccccccccccccccccc + yield ("Cache key will cause errors if used with memcached: %r " "(longer than %s)" % ( key, MEMCACHE_MAX_KEY_LENGTH, @@ -168,6 +175,17 @@ def foo(): ) ) + yield ( + "# * Make sure each ForeignKey and OneToOneField has `on_delete` set " + "to the desired behavior" + ) + + yield ( + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + + ccccccccccccccccccccccccccccccccccccccccccccccccccccccc + ) + yield ( "Cache key will cause errors if used with memcached: %r "