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

Use optional parentheses for tuples in return statements #6875

Merged
merged 3 commits into from
Aug 29, 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
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()),
)
```



Loading