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

Special ExprTuple formatting option for for-loops #5175

Merged
merged 1 commit into from
Jun 21, 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
50 changes: 45 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
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