Skip to content

Commit

Permalink
Use BestFits for non-call chain call expressions
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Aug 23, 2023
1 parent aa7e59b commit a658f3b
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
23 changes: 10 additions & 13 deletions crates/ruff_python_formatter/src/expression/expr_call.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -31,15 +32,11 @@ impl FormatNodeRule<ExprCall> 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
Expand All @@ -51,9 +48,9 @@ impl FormatNodeRule<ExprCall> 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()])
}
}
}
Expand All @@ -70,7 +67,7 @@ impl NeedsParentheses for ExprCall {
OptionalParentheses::Multiline
} else {
match self.func.needs_parentheses(self.into(), context) {
OptionalParentheses::IfFits => OptionalParentheses::Never,
OptionalParentheses::IfFits => OptionalParentheses::IfFits,
parentheses => parentheses,
}
}
Expand Down
1 change: 0 additions & 1 deletion crates/ruff_python_formatter/src/expression/parentheses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ pub(crate) enum Parenthesize {
/// Parenthesizes the expression if the group doesn't fit on a line (e.g., even name expressions are parenthesized), or if
/// the expression doesn't break, but _does_ reports that it always requires parentheses in this position (e.g., walrus
/// operators in function return annotations).
// TODO can this be replaced by IfBreaks
IfBreaksOrIfRequired,
}

Expand Down
9 changes: 5 additions & 4 deletions crates/ruff_python_formatter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,11 @@ if True:
#[test]
fn quick_test() {
let src = r#"
for converter in connection.ops.get_db_converters(
expression
) + expression.get_db_converters(connection):
...
a1 = Blog.objects.filter(entry__headline__contains="Lennon").filter(
entry__pub_date__year=2008
)
"#;
// Tokenize once
let mut tokens = Vec::new();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 "
Expand Down

0 comments on commit a658f3b

Please sign in to comment.