Skip to content

Commit

Permalink
Use optional parentheses for tuples in return statements (astral-sh#6875
Browse files Browse the repository at this point in the history
)
  • Loading branch information
MichaReiser authored Aug 29, 2023
1 parent 19ccf1d commit adb4869
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 33 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@

return len(self.nodeseeeeeeeee), sum(
len(node.parents) for node in self.node_map.values()
)


return len(self.nodeseeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee), sum(
len(node.parents) for node in self.node_map.values()
)


return (
len(self.nodeseeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee), sum(
len(node.parents) for node in self.node_map.values()
)
)
16 changes: 15 additions & 1 deletion crates/ruff_python_formatter/src/expression/expr_tuple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,20 @@ pub enum TupleParentheses {
/// ```
Preserve,

/// The same as [`Self::Default`] except that it uses [`optional_parentheses`] rather than
/// [`parenthesize_if_expands`]. This avoids adding parentheses if breaking any containing parenthesized
/// expression makes the tuple fit.
///
/// Avoids adding parentheses around the tuple because breaking the `sum` call expression is sufficient
/// to make it fit.
///
/// ```python
/// return len(self.nodeseeeeeeeee), sum(
// len(node.parents) for node in self.node_map.values()
// )
/// ```
OptionalParentheses,

/// 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.
Expand Down Expand Up @@ -158,7 +172,7 @@ impl FormatNodeRule<ExprTuple> for FormatExprTuple {
.finish()
}
TupleParentheses::Preserve => group(&ExprSequence::new(item)).fmt(f),
TupleParentheses::NeverPreserve => {
TupleParentheses::NeverPreserve | TupleParentheses::OptionalParentheses => {
optional_parentheses(&ExprSequence::new(item)).fmt(f)
}
TupleParentheses::Default => {
Expand Down
39 changes: 27 additions & 12 deletions crates/ruff_python_formatter/src/statement/stmt_return.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use ruff_formatter::write;
use ruff_python_ast::StmtReturn;
use ruff_python_ast::{Expr, StmtReturn};

use crate::comments::{SourceComment, SuppressionKind};
use crate::expression::expr_tuple::TupleParentheses;
use crate::expression::maybe_parenthesize_expression;
use crate::expression::parentheses::Parenthesize;
use crate::prelude::*;
Expand All @@ -12,17 +13,31 @@ pub struct FormatStmtReturn;
impl FormatNodeRule<StmtReturn> for FormatStmtReturn {
fn fmt_fields(&self, item: &StmtReturn, f: &mut PyFormatter) -> FormatResult<()> {
let StmtReturn { range: _, value } = item;
if let Some(value) = value {
write!(
f,
[
text("return"),
space(),
maybe_parenthesize_expression(value, item, Parenthesize::IfBreaks)
]
)
} else {
text("return").fmt(f)

text("return").fmt(f)?;

match value.as_deref() {
Some(Expr::Tuple(tuple)) if !f.context().comments().has_leading(tuple) => {
write!(
f,
[
space(),
tuple
.format()
.with_options(TupleParentheses::OptionalParentheses)
]
)
}
Some(value) => {
write!(
f,
[
space(),
maybe_parenthesize_expression(value, item, Parenthesize::IfBreaks)
]
)
}
None => Ok(()),
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,33 +252,23 @@ self.assertEqual(
def test():
return (
(
"((TIME_TO_SEC(%(lhs)s) * 1000000 + MICROSECOND(%(lhs)s)) -"
" (TIME_TO_SEC(%(rhs)s) * 1000000 + MICROSECOND(%(rhs)s)))"
) % {"lhs": lhs_sql, "rhs": rhs_sql},
tuple(lhs_params) * 2 + tuple(rhs_params) * 2,
)
"((TIME_TO_SEC(%(lhs)s) * 1000000 + MICROSECOND(%(lhs)s)) -"
" (TIME_TO_SEC(%(rhs)s) * 1000000 + MICROSECOND(%(rhs)s)))"
) % {"lhs": lhs_sql, "rhs": rhs_sql}, tuple(lhs_params) * 2 + tuple(rhs_params) * 2
def test2():
return (
"RETURNING %s INTO %s"
% (
", ".join(field_names),
", ".join(["%s"] * len(params)),
),
tuple(params),
)
return "RETURNING %s INTO %s" % (
", ".join(field_names),
", ".join(["%s"] * len(params)),
), tuple(params)
def test3():
return (
(
"(CASE WHEN JSON_TYPE(%s, %%s) IN (%s) "
"THEN JSON_TYPE(%s, %%s) ELSE JSON_EXTRACT(%s, %%s) END)"
) % (lhs, datatype_values, lhs, lhs),
(tuple(params) + (json_path,)) * 3,
)
"(CASE WHEN JSON_TYPE(%s, %%s) IN (%s) "
"THEN JSON_TYPE(%s, %%s) ELSE JSON_EXTRACT(%s, %%s) END)"
) % (lhs, datatype_values, lhs, lhs), (tuple(params) + (json_path,)) * 3
```
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
source: crates/ruff_python_formatter/tests/fixtures.rs
input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/return.py
---
## Input
```py
return len(self.nodeseeeeeeeee), sum(
len(node.parents) for node in self.node_map.values()
)
return len(self.nodeseeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee), sum(
len(node.parents) for node in self.node_map.values()
)
return (
len(self.nodeseeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee), sum(
len(node.parents) for node in self.node_map.values()
)
)
```

## Output
```py
return len(self.nodeseeeeeeeee), sum(
len(node.parents) for node in self.node_map.values()
)
return len(
self.nodeseeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
), sum(len(node.parents) for node in self.node_map.values())
return (
len(self.nodeseeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee),
sum(len(node.parents) for node in self.node_map.values()),
)
```



0 comments on commit adb4869

Please sign in to comment.