mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-11-03 13:14:34 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Correctly handle left/right breaking of binary expression

Browse source 
<!--
Thank you for contributing to Ruff! To help us out with reviewing, please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary
Black supports for layouts when it comes to breaking binary expressions:

```rust
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
enum BinaryLayout {
    /// Put each operand on their own line if either side expands
    Default,

    /// Try to expand the left to make it fit. Add parentheses if the left or right don't fit.
    ///
    ///```python
    /// [
    ///     a,
    ///     b
    /// ] & c
    ///```
    ExpandLeft,

    /// Try to expand the right to make it fix. Add parentheses if the left or right don't fit.
    ///
    /// ```python
    /// a & [
    ///     b,
    ///     c
    /// ]
    /// ```
    ExpandRight,

    /// Both the left and right side can be expanded. Try in the following order:
    /// * expand the right side
    /// * expand the left side
    /// * expand both sides
    ///
    /// to make the expression fit
    ///
    /// ```python
    /// [
    ///     a,
    ///     b
    /// ] & [
    ///     c,
    ///     d
    /// ]
    /// ```
    ExpandRightThenLeft,
}
```

Our current implementation only handles `ExpandRight` and `Default` correctly. This PR adds support for `ExpandRightThenLeft` and `ExpandLeft`. 

## Test Plan

I added tests that play through all 4 binary expression layouts.
This commit is contained in:
Micha Reiser 2023-06-21 09:40:05 +02:00 • committed by GitHub
parent a332f078db
commit 3973836420
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 613 additions and 83 deletions
Download patch file Download diff file Expand all files Collapse all files

135
crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary.py vendored
View file

@ -52,3 +52,138 @@ if (
ccccccccccc ccccccccccc
): ):
pass pass
# Left only breaks
if [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
] & aaaaaaaaaaaaaaaaaaaaaaaaaa:
...
if [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
] & aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
...
# Right only can break
if aaaaaaaaaaaaaaaaaaaaaaaaaa & [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
]:
...
if aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa & [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
]:
...
# Left or right can break
if [2222, 333] & [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
]:
...
if [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
] & [2222, 333]:
...
if [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
] & [fffffffffffffffff, gggggggggggggggggggg, hhhhhhhhhhhhhhhhhhhhh, iiiiiiiiiiiiiiii, jjjjjjjjjjjjj]:
...
if (
# comment
[
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
]
) & [
fffffffffffffffff,
gggggggggggggggggggg,
hhhhhhhhhhhhhhhhhhhhh,
iiiiiiiiiiiiiiii,
jjjjjjjjjjjjj,
]:
pass
...
# Nesting
if (aaaa + b) & [
fffffffffffffffff,
gggggggggggggggggggg,
hhhhhhhhhhhhhhhhhhhhh,
iiiiiiiiiiiiiiii,
jjjjjjjjjjjjj,
]:
...
if [
fffffffffffffffff,
gggggggggggggggggggg,
hhhhhhhhhhhhhhhhhhhhh,
iiiiiiiiiiiiiiii,
jjjjjjjjjjjjj,
] & (a + b):
...
if [
fffffffffffffffff,
gggggggggggggggggggg,
hhhhhhhhhhhhhhhhhhhhh,
iiiiiiiiiiiiiiii,
jjjjjjjjjjjjj,
] & (
# comment
a
+ b
):
...
if (
[
fffffffffffffffff,
gggggggggggggggggggg,
hhhhhhhhhhhhhhhhhhhhh,
iiiiiiiiiiiiiiii,
jjjjjjjjjjjjj,
]
&
# comment
a + b
):
...

274
crates/ruff_python_formatter/src/expression/expr_bin_op.rs
View file

@ -36,68 +36,124 @@ impl FormatNodeRule<ExprBinOp> for FormatExprBinOp {
range: _, range: _,
} = item; } = item;
let should_break_right = self.parentheses == Some(Parentheses::Custom); let layout = if self.parentheses == Some(Parentheses::Custom) {
BinaryLayout::from(item)
if should_break_right {
let left_group = f.group_id("BinaryLeft");
write!(
f,
[
// Wrap the left in a group and gives it an id. The printer first breaks the
// right side if `right` contains any line break because the printer breaks
// sequences of groups from right to left.
// Indents the left side if the group breaks.
group(&format_args![
if_group_breaks(&text("(")),
indent_if_group_breaks(
&format_args![
soft_line_break(),
left.format(),
soft_line_break_or_space(),
op.format(),
space()
],
left_group
)
])
.with_group_id(Some(left_group)),
// Wrap the right in a group and indents its content but only if the left side breaks
group(&indent_if_group_breaks(&right.format(), left_group)),
// If the left side breaks, insert a hard line break to finish the indent and close the open paren.
if_group_breaks(&format_args![hard_line_break(), text(")")])
.with_group_id(Some(left_group))
]
)
} else { } else {
let comments = f.context().comments().clone(); BinaryLayout::Default
let operator_comments = comments.dangling_comments(item.as_any_node_ref()); };
let needs_space = !is_simple_power_expression(item);
let before_operator_space = if needs_space { match layout {
soft_line_break_or_space() BinaryLayout::Default => {
} else { let comments = f.context().comments().clone();
soft_line_break() let operator_comments = comments.dangling_comments(item.as_any_node_ref());
}; let needs_space = !is_simple_power_expression(item);
write!( let before_operator_space = if needs_space {
f, soft_line_break_or_space()
[ } else {
left.format(), soft_line_break()
before_operator_space, };
op.format(),
trailing_comments(operator_comments),
]
)?;
// Format the operator on its own line if the right side has any leading comments. write!(
if comments.has_leading_comments(right.as_ref()) { f,
write!(f, [hard_line_break()])?; [
} else if needs_space { left.format(),
write!(f, [space()])?; before_operator_space,
op.format(),
trailing_comments(operator_comments),
]
)?;
// Format the operator on its own line if the right side has any leading comments.
if comments.has_leading_comments(right.as_ref()) {
write!(f, [hard_line_break()])?;
} else if needs_space {
write!(f, [space()])?;
}
write!(f, [group(&right.format())])
} }
write!(f, [group(&right.format())]) BinaryLayout::ExpandLeft => {
let left = left.format().memoized();
let right = right.format().memoized();
write!(
f,
[best_fitting![
// Everything on a single line
format_args![left, space(), op.format(), space(), right],
// Break the left over multiple lines, keep the right flat
format_args![
group(&left).should_expand(true),
space(),
op.format(),
space(),
right
],
// The content doesn't fit, indent the content and break before the operator.
format_args![
text("("),
block_indent(&format_args![
left,
hard_line_break(),
op.format(),
space(),
right
]),
text(")")
]
]
.with_mode(BestFittingMode::AllLines)]
)
}
BinaryLayout::ExpandRight => {
let left_group = f.group_id("BinaryLeft");
write!(
f,
[
// Wrap the left in a group and gives it an id. The printer first breaks the
// right side if `right` contains any line break because the printer breaks
// sequences of groups from right to left.
// Indents the left side if the group breaks.
group(&format_args![
if_group_breaks(&text("(")),
indent_if_group_breaks(
&format_args![
soft_line_break(),
left.format(),
soft_line_break_or_space(),
op.format(),
space()
],
left_group
)
])
.with_group_id(Some(left_group)),
// Wrap the right in a group and indents its content but only if the left side breaks
group(&indent_if_group_breaks(&right.format(), left_group)),
// If the left side breaks, insert a hard line break to finish the indent and close the open paren.
if_group_breaks(&format_args![hard_line_break(), text(")")])
.with_group_id(Some(left_group))
]
)
}
BinaryLayout::ExpandRightThenLeft => {
// The formatter expands group-sequences from right to left, and expands both if
// there isn't enough space when expanding only one of them.
write!(
f,
[
group(&left.format()),
space(),
op.format(),
space(),
group(&right.format())
]
)
}
} }
} }
@ -179,10 +235,13 @@ impl NeedsParentheses for ExprBinOp {
) -> Parentheses { ) -> Parentheses {
match default_expression_needs_parentheses(self.into(), parenthesize, source, comments) { match default_expression_needs_parentheses(self.into(), parenthesize, source, comments) {
Parentheses::Optional => { Parentheses::Optional => {
if should_binary_break_right_side_first(self) { if BinaryLayout::from(self) == BinaryLayout::Default
Parentheses::Custom || comments.has_leading_comments(self.right.as_ref())
} else { || comments.has_dangling_comments(self)
{
Parentheses::Optional Parentheses::Optional
} else {
Parentheses::Custom
} }
} }
parentheses => parentheses, parentheses => parentheses,
@ -190,31 +249,80 @@ impl NeedsParentheses for ExprBinOp {
} }
} }
pub(super) fn should_binary_break_right_side_first(expr: &ExprBinOp) -> bool { #[derive(Copy, Clone, Debug, Eq, PartialEq)]
use ruff_python_ast::prelude::*; enum BinaryLayout {
/// Put each operand on their own line if either side expands
Default,
if expr.left.is_bin_op_expr() { /// Try to expand the left to make it fit. Add parentheses if the left or right don't fit.
false ///
} else { ///```python
match expr.right.as_ref() { /// [
Expr::Tuple(ExprTuple { /// a,
elts: expressions, .. /// b
}) /// ] & c
| Expr::List(ExprList { ///```
elts: expressions, .. ExpandLeft,
})
| Expr::Set(ExprSet { /// Try to expand the right to make it fix. Add parentheses if the left or right don't fit.
elts: expressions, .. ///
}) /// ```python
| Expr::Dict(ExprDict { /// a & [
values: expressions, /// b,
.. /// c
}) => !expressions.is_empty(), /// ]
Expr::Call(ExprCall { args, keywords, .. }) => !args.is_empty() && !keywords.is_empty(), /// ```
Expr::ListComp(_) | Expr::SetComp(_) | Expr::DictComp(_) | Expr::GeneratorExp(_) => { ExpandRight,
true
} /// Both the left and right side can be expanded. Try in the following order:
_ => false, /// * expand the right side
/// * expand the left side
/// * expand both sides
///
/// to make the expression fit
///
/// ```python
/// [
/// a,
/// b
/// ] & [
/// c,
/// d
/// ]
/// ```
ExpandRightThenLeft,
}
impl BinaryLayout {
fn from(expr: &ExprBinOp) -> Self {
match (can_break(&expr.left), can_break(&expr.right)) {
(false, false) => Self::Default,
(true, false) => Self::ExpandLeft,
(false, true) => Self::ExpandRight,
(true, true) => Self::ExpandRightThenLeft,
} }
} }
} }
fn can_break(expr: &Expr) -> bool {
use ruff_python_ast::prelude::*;
match expr {
Expr::Tuple(ExprTuple {
elts: expressions, ..
})
| Expr::List(ExprList {
elts: expressions, ..
})
| Expr::Set(ExprSet {
elts: expressions, ..
})
| Expr::Dict(ExprDict {
values: expressions,
..
}) => !expressions.is_empty(),
Expr::Call(ExprCall { args, keywords, .. }) => !(args.is_empty() && keywords.is_empty()),
Expr::ListComp(_) | Expr::SetComp(_) | Expr::DictComp(_) | Expr::GeneratorExp(_) => true,
_ => false,
}
}

287
crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__ruff_test__expression__binary_py.snap
View file

@ -58,6 +58,141 @@ if (
ccccccccccc ccccccccccc
): ):
pass pass
# Left only breaks
if [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
] & aaaaaaaaaaaaaaaaaaaaaaaaaa:
...
if [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
] & aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
...
# Right only can break
if aaaaaaaaaaaaaaaaaaaaaaaaaa & [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
]:
...
if aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa & [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
]:
...
# Left or right can break
if [2222, 333] & [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
]:
...
if [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
] & [2222, 333]:
...
if [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
] & [fffffffffffffffff, gggggggggggggggggggg, hhhhhhhhhhhhhhhhhhhhh, iiiiiiiiiiiiiiii, jjjjjjjjjjjjj]:
...
if (
# comment
[
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
]
) & [
fffffffffffffffff,
gggggggggggggggggggg,
hhhhhhhhhhhhhhhhhhhhh,
iiiiiiiiiiiiiiii,
jjjjjjjjjjjjj,
]:
pass
...
# Nesting
if (aaaa + b) & [
fffffffffffffffff,
gggggggggggggggggggg,
hhhhhhhhhhhhhhhhhhhhh,
iiiiiiiiiiiiiiii,
jjjjjjjjjjjjj,
]:
...
if [
fffffffffffffffff,
gggggggggggggggggggg,
hhhhhhhhhhhhhhhhhhhhh,
iiiiiiiiiiiiiiii,
jjjjjjjjjjjjj,
] & (a + b):
...
if [
fffffffffffffffff,
gggggggggggggggggggg,
hhhhhhhhhhhhhhhhhhhhh,
iiiiiiiiiiiiiiii,
jjjjjjjjjjjjj,
] & (
# comment
a
+ b
):
...
if (
[
fffffffffffffffff,
gggggggggggggggggggg,
hhhhhhhhhhhhhhhhhhhhh,
iiiiiiiiiiiiiiii,
jjjjjjjjjjjjj,
]
&
# comment
a + b
):
...
``` ```
@ -139,6 +274,158 @@ if (
ccccccccccc ccccccccccc
): ):
pass pass
# Left only breaks
if [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
] & aaaaaaaaaaaaaaaaaaaaaaaaaa:
...
if (
[
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
]
& aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
):
...
# Right only can break
if aaaaaaaaaaaaaaaaaaaaaaaaaa & [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
]:
...
if (
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
& [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
]
):
...
# Left or right can break
if [2222, 333] & [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
]:
...
if [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
] & [2222, 333]:
...
if [
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
] & [
fffffffffffffffff,
gggggggggggggggggggg,
hhhhhhhhhhhhhhhhhhhhh,
iiiiiiiiiiiiiiii,
jjjjjjjjjjjjj,
]:
...
if (
# comment
[
aaaaaaaaaaaaa,
bbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccc,
dddddddddddddddddddd,
eeeeeeeeee,
]
) & [
fffffffffffffffff,
gggggggggggggggggggg,
hhhhhhhhhhhhhhhhhhhhh,
iiiiiiiiiiiiiiii,
jjjjjjjjjjjjj,
]:
pass
...
# Nesting
if (aaaa + b) & [
fffffffffffffffff,
gggggggggggggggggggg,
hhhhhhhhhhhhhhhhhhhhh,
iiiiiiiiiiiiiiii,
jjjjjjjjjjjjj,
]:
...
if [
fffffffffffffffff,
gggggggggggggggggggg,
hhhhhhhhhhhhhhhhhhhhh,
iiiiiiiiiiiiiiii,
jjjjjjjjjjjjj,
] & (a + b):
...
if (
[
fffffffffffffffff,
gggggggggggggggggggg,
hhhhhhhhhhhhhhhhhhhhh,
iiiiiiiiiiiiiiii,
jjjjjjjjjjjjj,
]
&
(
# comment
a
+ b
)
):
...
if (
[
fffffffffffffffff,
gggggggggggggggggggg,
hhhhhhhhhhhhhhhhhhhhh,
iiiiiiiiiiiiiiii,
jjjjjjjjjjjjj,
]
&
# comment
a
+ b
):
...
``` ```