Skip to content

Commit

Permalink
Special ExprTuple formatting option for for-loops
Browse files Browse the repository at this point in the history
## Motivation

While black keeps parentheses nearly everywhere, with the notable exception of in the body of for loops:
```python
for (a, b) in x:
    pass
```
becomes
```python
for a, b in x:
    pass
```

This currently blocks #5163, which this PR should unblock.

## Solution

This changes the `ExprTuple` formatting option to include one additional option that removes the parentheses when not using magic trailing comma and not breaking. It is supposed to be used through
```
#[derive(Debug)]
struct ExprTupleWithoutParentheses<'a>(&'a Expr);

impl Format<PyFormatContext<'_>> for ExprTupleWithoutParentheses<'_> {
    fn fmt(&self, f: &mut Formatter<PyFormatContext<'_>>) -> FormatResult<()> {
        match self.0 {
            Expr::Tuple(expr_tuple) => expr_tuple
                .format()
                .with_options(TupleParentheses::StripInsideForLoop)
                .fmt(f),
            other => other.format().with_options(Parenthesize::IfBreaks).fmt(f),
        }
    }
}
```

## Testing

The for loop formatting isn't merged due to missing this (and i didn't want to create more git weirdness across two people), but I've confirmed that when applying this to while loops instead of for loops, then
```rust
        write!(
            f,
            [
                text("while"),
                space(),
                ExprTupleWithoutParentheses(test.as_ref()),
                text(":"),
                trailing_comments(trailing_condition_comments),
                block_indent(&body.format())
            ]
        )?;
```
makes
```python
while (a, b):
    pass

while (
    ajssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssa,
    b,
):
    pass

while (a,b,):
    pass
```
formatted as
```python
while a, b:
    pass

while (
    ajssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssa,
    b,
):
    pass

while (
    a,
    b,
):
    pass
```
  • Loading branch information
konstin committed Jun 21, 2023
1 parent bc63cc9 commit 61ef362
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 6 deletions.
54 changes: 49 additions & 5 deletions crates/ruff_python_formatter/src/expression/expr_tuple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,48 @@ use ruff_formatter::formatter::Formatter;
use ruff_formatter::prelude::{
block_indent, group, if_group_breaks, soft_block_indent, soft_line_break_or_space, text,
};
use ruff_formatter::{format_args, write, Buffer, Format, FormatResult};
use ruff_formatter::{format_args, write, Buffer, Format, FormatResult, FormatRuleWithOptions};
use ruff_python_ast::prelude::{Expr, Ranged};
use ruff_text_size::TextRange;
use rustpython_parser::ast::ExprTuple;

#[derive(Eq, PartialEq, Debug, Default)]
pub enum TupleParentheses {
/// Effectively `None` in `Option<Parentheses>`
#[default]
Default,
/// Effectively `Some(Parentheses)` in `Option<Parentheses>`
Expr(Parentheses),
/// Handle the special case where we remove parentheses even if they were initially present
///
/// Normally, black keeps parentheses, but in the case of loops it formats
/// ```python
/// for (a, b) in x:
/// pass
/// ```
/// to
/// ```python
/// for a, b in x:
/// pass
/// ```
/// Black still does use parentheses in this position if the group breaks or magic trailing
/// comma is used.
StripInsideForLoop,
}

#[derive(Default)]
pub struct FormatExprTuple;
pub struct FormatExprTuple {
parentheses: TupleParentheses,
}

impl FormatRuleWithOptions<ExprTuple, PyFormatContext<'_>> for FormatExprTuple {
type Options = TupleParentheses;

fn with_options(mut self, options: Self::Options) -> Self {
self.parentheses = options;
self
}
}

impl FormatNodeRule<ExprTuple> for FormatExprTuple {
fn fmt_fields(&self, item: &ExprTuple, f: &mut PyFormatter) -> FormatResult<()> {
Expand Down Expand Up @@ -74,9 +109,14 @@ impl FormatNodeRule<ExprTuple> for FormatExprTuple {
&text(")"),
]
)?;
} else if is_parenthesized(*range, elts, f) {
// If the tuple has parentheses, keep them. Note that unlike other expr parentheses,
// those are actually part of the range
} else if is_parenthesized(*range, elts, f)
&& self.parentheses != TupleParentheses::StripInsideForLoop
{
// If the tuple has parentheses, we generally want to keep them. The exception are for
// loops, see `TupleParentheses::StripInsideForLoop` doc comment.
//
// Unlike other expression parentheses, tuple parentheses are part of the range of the
// tuple itself.
write!(
f,
[group(&format_args![
Expand Down Expand Up @@ -135,6 +175,10 @@ impl NeedsParentheses for ExprTuple {
source: &str,
comments: &Comments,
) -> Parentheses {
// We use Custom as signal for RemoveUnlessBreaks
if parenthesize == Parenthesize::RemoveUnlessBreaks {
return Parentheses::Custom;
}
match default_expression_needs_parentheses(self.into(), parenthesize, source, comments) {
Parentheses::Optional => Parentheses::Never,
parentheses => parentheses,
Expand Down
6 changes: 5 additions & 1 deletion crates/ruff_python_formatter/src/expression/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::comments::Comments;
use crate::context::NodeLevel;
use crate::expression::expr_tuple::TupleParentheses;
use crate::expression::parentheses::{NeedsParentheses, Parentheses, Parenthesize};
use crate::prelude::*;
use ruff_formatter::{
Expand Down Expand Up @@ -85,7 +86,10 @@ impl FormatRule<Expr, PyFormatContext<'_>> for FormatExpr {
Expr::Starred(expr) => expr.format().fmt(f),
Expr::Name(expr) => expr.format().fmt(f),
Expr::List(expr) => expr.format().fmt(f),
Expr::Tuple(expr) => expr.format().fmt(f),
Expr::Tuple(expr) => expr
.format()
.with_options(TupleParentheses::Expr(parentheses))
.fmt(f),
Expr::Slice(expr) => expr.format().fmt(f),
});

Expand Down
24 changes: 24 additions & 0 deletions crates/ruff_python_formatter/src/expression/parentheses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,30 @@ pub enum Parenthesize {

/// Parenthesizes the expression only if it doesn't fit on a line.
IfBreaks,

/// This is special case for StmtFor where we remove parentheses.
///
/// In most cases, we keep the parentheses, e.g.
/// ```python
/// (a, b) = x
/// a, b = x
/// ```
/// both remain as-in. The exception are for loops:
/// ```python
/// for a, b in x:
/// pass
/// for (a, b) in x:
/// pass
/// ```
/// is formatted as
/// ```python
/// for a, b in x:
/// pass
/// for a, b in x:
/// pass
/// ```
/// . That is, we remove the parentheses unless the group breaks
RemoveUnlessBreaks,
}

impl Parenthesize {
Expand Down

0 comments on commit 61ef362

Please