From 8eadacda33078b32fdf27e88d9f310b4ae930527 Mon Sep 17 00:00:00 2001 From: Chris Pryer <14341145+cnpryer@users.noreply.github.com> Date: Mon, 24 Jul 2023 10:44:36 -0400 Subject: [PATCH] Update `TupleParentheses` usage (#5810) --- .../fixtures/ruff/expression/list_comp.py | 4 + .../src/expression/expr_subscript.rs | 2 +- .../src/expression/expr_tuple.rs | 81 ++++++++++++++----- .../src/expression/mod.rs | 6 +- .../src/other/comprehension.rs | 2 +- .../src/statement/stmt_for.rs | 2 +- .../format@expression__list_comp.py.snap | 21 ++++- 7 files changed, 88 insertions(+), 30 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/list_comp.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/list_comp.py index 34a594a7f5..d0cf86ea2f 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/list_comp.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/list_comp.py @@ -53,3 +53,7 @@ selected_choices = [ str(v) for vvvvvvvvvvvvvvvvvvvvvvv in value if str(v) not in self.choices.field.empty_values ] + +# Tuples with BinOp +[i for i in (aaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, ccccccccccccccccccccc)] +[(aaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, ccccccccccccccccccccc) for i in b] \ No newline at end of file diff --git a/crates/ruff_python_formatter/src/expression/expr_subscript.rs b/crates/ruff_python_formatter/src/expression/expr_subscript.rs index 65cb0bc8c7..e5289594ca 100644 --- a/crates/ruff_python_formatter/src/expression/expr_subscript.rs +++ b/crates/ruff_python_formatter/src/expression/expr_subscript.rs @@ -49,7 +49,7 @@ impl FormatNodeRule for FormatExprSubscript { let result = if let Expr::Tuple(tuple) = slice.as_ref() { tuple .format() - .with_options(TupleParentheses::Subscript) + .with_options(TupleParentheses::Preserve) .fmt(f) } else { slice.format().fmt(f) diff --git a/crates/ruff_python_formatter/src/expression/expr_tuple.rs b/crates/ruff_python_formatter/src/expression/expr_tuple.rs index 5e89fd995c..098a7a572e 100644 --- a/crates/ruff_python_formatter/src/expression/expr_tuple.rs +++ b/crates/ruff_python_formatter/src/expression/expr_tuple.rs @@ -6,29 +6,53 @@ use ruff_formatter::{format_args, write, FormatRuleWithOptions}; use ruff_python_ast::node::AnyNodeRef; use crate::builders::{empty_parenthesized_with_dangling_comments, parenthesize_if_expands}; -use crate::expression::parentheses::{ - parenthesized, NeedsParentheses, OptionalParentheses, Parentheses, -}; +use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses}; use crate::prelude::*; #[derive(Eq, PartialEq, Debug, Default)] pub enum TupleParentheses { - /// Black omits parentheses for tuples inside of comprehensions. - Comprehension, - - /// Effectively `None` in `Option` + /// By default tuples with a single element will include parentheses. Tuples with multiple elements + /// will parenthesize if the expression expands. This means that tuples will often *preserve* + /// their parentheses, but this differs from `Preserve` in that we may also *introduce* + /// parentheses as well. #[default] Default, - /// Effectively `Some(Parentheses)` in `Option` - Expr(Parentheses), - /// Black omits parentheses for tuples inside of subscripts except if the tuple is parenthesized - /// in the source code. - Subscript, - - /// Handle the special case where we remove parentheses even if they were initially present + /// Handle special cases where parentheses are to be preserved. /// - /// Normally, black keeps parentheses, but in the case of loops it formats + /// Black omits parentheses for tuples inside subscripts except if the tuple is already + /// parenthesized in the source code. + /// ```python + /// x[a, :] + /// x[a, b:] + /// x[(a, b):] + /// ``` + Preserve, + + /// Handle the special cases where we don't include parentheses at all. + /// + /// + /// Black never formats tuple targets of for loops with parentheses if inside a comprehension. + /// For example, tuple targets will always be formatted on the same line, except when an element supports + /// line-breaking in an un-parenthesized context. + /// ```python + /// # Input + /// {k: v for x, (k, v) in this_is_a_very_long_variable_which_will_cause_a_trailing_comma_which_breaks_the_comprehension} + /// + /// # Black + /// { + /// k: v + /// for x, ( + /// k, + /// v, + /// ) in this_is_a_very_long_variable_which_will_cause_a_trailing_comma_which_breaks_the_comprehension + /// } + /// ``` + Never, + + /// Handle the special cases where we don't include parentheses if they are not required. + /// + /// Normally, black keeps parentheses, but in the case of for loops it formats /// ```python /// for (a, b) in x: /// pass @@ -38,9 +62,24 @@ pub enum TupleParentheses { /// for a, b in x: /// pass /// ``` - /// Black still does use parentheses in this position if the group breaks or magic trailing + /// Black still does use parentheses in these positions if the group breaks or magic trailing /// comma is used. - StripInsideForLoop, + /// + /// Additional examples: + /// ```python + /// for (a,) in []: + /// pass + /// for a, b in []: + /// pass + /// for a, b in []: # Strips parentheses + /// pass + /// for ( + /// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, + /// b, + /// ) in []: + /// pass + /// ``` + NeverPreserve, } #[derive(Default)] @@ -86,7 +125,7 @@ impl FormatNodeRule for FormatExprTuple { .fmt(f); } [single] => match self.parentheses { - TupleParentheses::Subscript + TupleParentheses::Preserve if !is_parenthesized(*range, elts, f.context().source()) => { write!(f, [single.format(), text(",")]) @@ -103,19 +142,19 @@ impl FormatNodeRule for FormatExprTuple { // Unlike other expression parentheses, tuple parentheses are part of the range of the // tuple itself. _ if is_parenthesized(*range, elts, f.context().source()) - && self.parentheses != TupleParentheses::StripInsideForLoop => + && self.parentheses != TupleParentheses::NeverPreserve => { parenthesized("(", &ExprSequence::new(item), ")").fmt(f) } _ => match self.parentheses { - TupleParentheses::Comprehension => { + TupleParentheses::Never => { let separator = format_with(|f| group(&format_args![text(","), space()]).fmt(f)); f.join_with(separator) .entries(elts.iter().formatted()) .finish() } - TupleParentheses::Subscript => group(&ExprSequence::new(item)).fmt(f), + TupleParentheses::Preserve => group(&ExprSequence::new(item)).fmt(f), _ => parenthesize_if_expands(&ExprSequence::new(item)).fmt(f), }, } diff --git a/crates/ruff_python_formatter/src/expression/mod.rs b/crates/ruff_python_formatter/src/expression/mod.rs index db922c4ae0..fae56271ac 100644 --- a/crates/ruff_python_formatter/src/expression/mod.rs +++ b/crates/ruff_python_formatter/src/expression/mod.rs @@ -9,7 +9,6 @@ use ruff_python_ast::visitor::preorder::{walk_expr, PreorderVisitor}; use crate::builders::parenthesize_if_expands; use crate::context::NodeLevel; -use crate::expression::expr_tuple::TupleParentheses; use crate::expression::parentheses::{ is_expression_parenthesized, optional_parentheses, parenthesized, NeedsParentheses, OptionalParentheses, Parentheses, Parenthesize, @@ -91,10 +90,7 @@ impl FormatRule> 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() - .with_options(TupleParentheses::Expr(parentheses)) - .fmt(f), + Expr::Tuple(expr) => expr.format().fmt(f), Expr::Slice(expr) => expr.format().fmt(f), }); diff --git a/crates/ruff_python_formatter/src/other/comprehension.rs b/crates/ruff_python_formatter/src/other/comprehension.rs index 85de94885e..d35c8d5cb6 100644 --- a/crates/ruff_python_formatter/src/other/comprehension.rs +++ b/crates/ruff_python_formatter/src/other/comprehension.rs @@ -113,7 +113,7 @@ impl Format> for ExprTupleWithoutParentheses<'_> { match self.0 { Expr::Tuple(expr_tuple) => expr_tuple .format() - .with_options(TupleParentheses::Comprehension) + .with_options(TupleParentheses::Never) .fmt(f), other => other.format().fmt(f), } diff --git a/crates/ruff_python_formatter/src/statement/stmt_for.rs b/crates/ruff_python_formatter/src/statement/stmt_for.rs index 4d009ca818..9b98269d54 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_for.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_for.rs @@ -17,7 +17,7 @@ impl Format> for ExprTupleWithoutParentheses<'_> { match self.0 { Expr::Tuple(expr_tuple) => expr_tuple .format() - .with_options(TupleParentheses::StripInsideForLoop) + .with_options(TupleParentheses::NeverPreserve) .fmt(f), other => maybe_parenthesize_expression(other, self.0, Parenthesize::IfBreaks).fmt(f), } diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__list_comp.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__list_comp.py.snap index aa1e9aaf32..f480119b8e 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__list_comp.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__list_comp.py.snap @@ -59,7 +59,10 @@ selected_choices = [ str(v) for vvvvvvvvvvvvvvvvvvvvvvv in value if str(v) not in self.choices.field.empty_values ] -``` + +# Tuples with BinOp +[i for i in (aaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, ccccccccccccccccccccc)] +[(aaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, ccccccccccccccccccccc) for i in b]``` ## Output ```py @@ -122,6 +125,22 @@ selected_choices = [ for vvvvvvvvvvvvvvvvvvvvvvv in value if str(v) not in self.choices.field.empty_values ] + +# Tuples with BinOp +[ + i + for i in ( + aaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, + ccccccccccccccccccccc, + ) +] +[ + ( + aaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, + ccccccccccccccccccccc, + ) + for i in b +] ```