Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use BestFits for non-fluent attribute chains #6817

Merged
merged 3 commits into from
Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,16 @@
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
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
26 changes: 10 additions & 16 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 @@ -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)
}
}
}
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 @@ -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
Expand Down Expand Up @@ -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
```


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
Loading