Skip to content

Commit

Permalink
Fix parentheses around return type annotations
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Sep 17, 2024
1 parent d86e5ad commit d32126c
Show file tree
Hide file tree
Showing 9 changed files with 140 additions and 266 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -82,30 +82,6 @@

func([(x, y,) for (x, y) in z], bar)

# Ensure that return type annotations (which use `parenthesize_if_expands`) are also hugged.
def func() -> [1, 2, 3,]:
pass

def func() -> ([1, 2, 3,]):
pass

def func() -> ([1, 2, 3,]):
pass

def func() -> ( # comment
[1, 2, 3,]):
pass

def func() -> (
[1, 2, 3,] # comment
):
pass

def func() -> (
[1, 2, 3,]
# comment
):
pass

# Ensure that nested lists are hugged.
func([
Expand Down
28 changes: 20 additions & 8 deletions crates/ruff_python_formatter/src/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ use crate::expression::parentheses::{
OptionalParentheses, Parentheses, Parenthesize,
};
use crate::prelude::*;
use crate::preview::is_hug_parens_with_braces_and_square_brackets_enabled;
use crate::preview::{
is_empty_parameters_return_value, is_hug_parens_with_braces_and_square_brackets_enabled,
};

mod binary_like;
pub(crate) mod expr_attribute;
Expand Down Expand Up @@ -324,7 +326,7 @@ fn format_with_parentheses_comments(
)
}

/// Wraps an expression in an optional parentheses except if its [`NeedsParentheses::needs_parentheses`] implementation
/// Wraps an expression in optional parentheses except if its [`NeedsParentheses::needs_parentheses`] implementation
/// indicates that it is okay to omit the parentheses. For example, parentheses can always be omitted for lists,
/// because they already bring their own parentheses.
pub(crate) fn maybe_parenthesize_expression<'a, T>(
Expand Down Expand Up @@ -389,16 +391,19 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {

match needs_parentheses {
OptionalParentheses::Multiline => match parenthesize {
Parenthesize::IfBreaksOrIfRequired => {
Parenthesize::IfBreaksOptionalParentheses
if !is_empty_parameters_return_value(f.context()) =>
{
parenthesize_if_expands(&expression.format().with_options(Parentheses::Never))
.fmt(f)
}

Parenthesize::IfRequired => {
expression.format().with_options(Parentheses::Never).fmt(f)
}

Parenthesize::Optional | Parenthesize::IfBreaks => {
Parenthesize::Optional
| Parenthesize::IfBreaks
| Parenthesize::IfBreaksOptionalParentheses => {
if can_omit_optional_parentheses(expression, f.context()) {
optional_parentheses(&expression.format().with_options(Parentheses::Never))
.fmt(f)
Expand All @@ -411,7 +416,7 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
}
},
OptionalParentheses::BestFit => match parenthesize {
Parenthesize::IfBreaksOrIfRequired => {
Parenthesize::IfBreaksOptionalParentheses => {
parenthesize_if_expands(&expression.format().with_options(Parentheses::Never))
.fmt(f)
}
Expand All @@ -435,10 +440,17 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
}
},
OptionalParentheses::Never => match parenthesize {
Parenthesize::IfBreaksOrIfRequired => {
parenthesize_if_expands(&expression.format().with_options(Parentheses::Never))
Parenthesize::IfBreaksOptionalParentheses => {
if is_empty_parameters_return_value(f.context()) {
optional_parentheses(&expression.format().with_options(Parentheses::Never))
.fmt(f)
} else {
parenthesize_if_expands(
&expression.format().with_options(Parentheses::Never),
)
.with_indent(!is_expression_huggable(expression, f.context()))
.fmt(f)
}
}

Parenthesize::Optional | Parenthesize::IfBreaks | Parenthesize::IfRequired => {
Expand Down
49 changes: 24 additions & 25 deletions crates/ruff_python_formatter/src/expression/parentheses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,11 @@ pub(crate) enum Parenthesize {
/// Adding parentheses is desired to prevent the comments from wandering.
IfRequired,

/// Parenthesizes the expression if the group doesn't fit on a line (e.g., even name expressions are parenthesized), or if
/// the expression doesn't break, but _does_ reports that it always requires parentheses in this position (e.g., walrus
/// operators in function return annotations).
IfBreaksOrIfRequired,
/// Same as [`Self::IfBreaks`] except:
/// * it adds optional parentheses around expressions that normally don't require parentheses
/// ([`NeedsParentheses::Never`]) but exceed the configured line width.
/// * it doesn't use the best fit layout, instead it adds parentheses when the value exceeds the line width.
IfBreaksOptionalParentheses,
}

impl Parenthesize {
Expand Down Expand Up @@ -416,27 +417,25 @@ impl Format<PyFormatContext<'_>> for FormatEmptyParenthesized<'_> {
debug_assert!(self.comments[end_of_line_split..]
.iter()
.all(|comment| comment.line_position().is_own_line()));
write!(
f,
[group(&format_args![
token(self.left),
// end-of-line comments
trailing_comments(&self.comments[..end_of_line_split]),
// Avoid unstable formatting with
// ```python
// x = () - (#
// )
// ```
// Without this the comment would go after the empty tuple first, but still expand
// the bin op. In the second formatting pass they are trailing bin op comments
// so the bin op collapse. Suboptimally we keep parentheses around the bin op in
// either case.
(!self.comments[..end_of_line_split].is_empty()).then_some(hard_line_break()),
// own line comments, which need to be indented
soft_block_indent(&dangling_comments(&self.comments[end_of_line_split..])),
token(self.right)
])]
)
group(&format_args![
token(self.left),
// end-of-line comments
trailing_comments(&self.comments[..end_of_line_split]),
// Avoid unstable formatting with
// ```python
// x = () - (#
// )
// ```
// Without this the comment would go after the empty tuple first, but still expand
// the bin op. In the second formatting pass they are trailing bin op comments
// so the bin op collapse. Suboptimally we keep parentheses around the bin op in
// either case.
(!self.comments[..end_of_line_split].is_empty()).then_some(hard_line_break()),
// own line comments, which need to be indented
soft_block_indent(&dangling_comments(&self.comments[end_of_line_split..])),
token(self.right)
])
.fmt(f)
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_python_formatter/src/other/with_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ impl FormatNodeRule<WithItem> for FormatWithItem {
maybe_parenthesize_expression(
context_expr,
item,
Parenthesize::IfBreaksOrIfRequired,
Parenthesize::IfBreaksOptionalParentheses,
)
.fmt(f)?;
} else {
Expand Down
5 changes: 5 additions & 0 deletions crates/ruff_python_formatter/src/preview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,8 @@ pub(crate) fn is_comprehension_leading_expression_comments_same_line_enabled(
) -> bool {
context.is_preview()
}

/// See [#9447](https://github.com/astral-sh/ruff/issues/9447)
pub(crate) fn is_empty_parameters_return_value(context: &PyFormatContext) -> bool {
context.is_preview()
}
29 changes: 12 additions & 17 deletions crates/ruff_python_formatter/src/statement/stmt_function_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,11 @@ fn format_function_header(f: &mut PyFormatter, item: &StmtFunctionDef) -> Format
write!(f, [token("def"), space(), name.format()])?;

if let Some(type_params) = type_params.as_ref() {
write!(f, [type_params.format()])?;
type_params.format().fmt(f)?;
}

let format_inner = format_with(|f: &mut PyFormatter| {
write!(f, [parameters.format()])?;
parameters.format().fmt(f)?;

if let Some(return_annotation) = returns.as_ref() {
write!(f, [space(), token("->"), space()])?;
Expand All @@ -127,7 +127,7 @@ fn format_function_header(f: &mut PyFormatter, item: &StmtFunctionDef) -> Format
} else {
Parentheses::Never
};
write!(f, [return_annotation.format().with_options(parentheses)])?;
return_annotation.format().with_options(parentheses).fmt(f)
} else if comments.has_trailing(return_annotation.as_ref()) {
// Intentionally parenthesize any return annotations with trailing comments.
// This avoids an instability in cases like:
Expand Down Expand Up @@ -156,15 +156,16 @@ fn format_function_header(f: &mut PyFormatter, item: &StmtFunctionDef) -> Format
// requires that the parent be aware of how the child is formatted, which
// is challenging. As a compromise, we break those expressions to avoid an
// instability.
write!(
f,
[return_annotation.format().with_options(Parentheses::Always)]
)?;

return_annotation
.format()
.with_options(Parentheses::Always)
.fmt(f)
} else {
let parenthesize = if parameters.is_empty() && !comments.has(parameters.as_ref()) {
// If the parameters are empty, add parentheses if the return annotation
// breaks at all.
Parenthesize::IfBreaksOrIfRequired
Parenthesize::IfBreaksOptionalParentheses
} else {
// Otherwise, use our normal rules for parentheses, which allows us to break
// like:
Expand All @@ -179,17 +180,11 @@ fn format_function_header(f: &mut PyFormatter, item: &StmtFunctionDef) -> Format
// ```
Parenthesize::IfBreaks
};
write!(
f,
[maybe_parenthesize_expression(
return_annotation,
item,
parenthesize
)]
)?;
maybe_parenthesize_expression(return_annotation, item, parenthesize).fmt(f)
}
} else {
Ok(())
}
Ok(())
});

group(&format_inner).fmt(f)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,20 +155,7 @@ def SimplePyFn(
```diff
--- Black
+++ Ruff
@@ -29,14 +29,18 @@
# magic trailing comma in return type, no params
-def a() -> tuple[
- a,
- b,
-]: ...
+def a() -> (
+ tuple[
+ a,
+ b,
+ ]
+): ...
@@ -36,7 +36,9 @@
# magic trailing comma in return type, params
Expand All @@ -179,26 +166,7 @@ def SimplePyFn(
p,
q,
]:
@@ -68,11 +72,13 @@
# long return type, no param list
-def foo() -> list[
- Loooooooooooooooooooooooooooooooooooong,
- Loooooooooooooooooooong,
- Looooooooooooong,
-]: ...
+def foo() -> (
+ list[
+ Loooooooooooooooooooooooooooooooooooong,
+ Loooooooooooooooooooong,
+ Looooooooooooong,
+ ]
+): ...
# long function name, no param list, no return value
@@ -93,7 +99,11 @@
@@ -93,7 +95,11 @@
# unskippable type hint (??)
Expand All @@ -211,7 +179,7 @@ def SimplePyFn(
pass
@@ -112,7 +122,13 @@
@@ -112,7 +118,13 @@
# don't lose any comments (no magic)
Expand All @@ -226,7 +194,7 @@ def SimplePyFn(
... # 6
@@ -120,12 +136,18 @@
@@ -120,12 +132,18 @@
def foo( # 1
a, # 2
b,
Expand Down Expand Up @@ -283,12 +251,10 @@ def foo(
# magic trailing comma in return type, no params
def a() -> (
tuple[
a,
b,
]
): ...
def a() -> tuple[
a,
b,
]: ...
# magic trailing comma in return type, params
Expand Down Expand Up @@ -326,13 +292,11 @@ def aaaaaaaaaaaaaaaaa(
# long return type, no param list
def foo() -> (
list[
Loooooooooooooooooooooooooooooooooooong,
Loooooooooooooooooooong,
Looooooooooooong,
]
): ...
def foo() -> list[
Loooooooooooooooooooooooooooooooooooong,
Loooooooooooooooooooong,
Looooooooooooong,
]: ...
# long function name, no param list, no return value
Expand Down Expand Up @@ -592,5 +556,3 @@ def SimplePyFn(
Buffer[UInt8, 2],
]: ...
```


Loading

0 comments on commit d32126c

Please sign in to comment.