Allow return type annotations to use their own parentheses (#6436)
## Summary
This PR modifies our logic for wrapping return type annotations.
Previously, we _always_ wrapped the annotation in parentheses if it
expanded; however, Black only exhibits this behavior when the function
parameters is empty (i.e., it doesn't and can't break). In other cases,
it uses the normal parenthesization rules, allowing nodes to bring their
own parentheses.
For example, given:
```python
def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
]:
...
def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(x) -> Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
]:
...
```
Black will format as:
```python
def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> (
Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
]
):
...
def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(
x,
) -> Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
]:
...
```
Whereas, prior to this PR, Ruff would format as:
```python
def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> (
Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
]
):
...
def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(
x,
) -> (
Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
]
):
...
```
Closes https://github.com/astral-sh/ruff/issues/6431.
## Test Plan
Before:
- `zulip`: 0.99702
- `django`: 0.99784
- `warehouse`: 0.99585
- `build`: 0.75623
- `transformers`: 0.99470
- `cpython`: 0.75988
- `typeshed`: 0.74853
After:
- `zulip`: 0.99724
- `django`: 0.99791
- `warehouse`: 0.99586
- `build`: 0.75623
- `transformers`: 0.99474
- `cpython`: 0.75956
- `typeshed`: 0.74857
This commit is contained in:
@@ -183,25 +183,45 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
|
||||
needs_parentheses
|
||||
}
|
||||
}
|
||||
Parenthesize::Optional | Parenthesize::IfBreaks => needs_parentheses,
|
||||
Parenthesize::Optional
|
||||
| Parenthesize::IfBreaks
|
||||
| Parenthesize::IfBreaksOrIfRequired => needs_parentheses,
|
||||
};
|
||||
|
||||
match needs_parentheses {
|
||||
OptionalParentheses::Multiline if *parenthesize != Parenthesize::IfRequired => {
|
||||
if can_omit_optional_parentheses(expression, f.context()) {
|
||||
optional_parentheses(&expression.format().with_options(Parentheses::Never))
|
||||
.fmt(f)
|
||||
} else {
|
||||
OptionalParentheses::Multiline => match parenthesize {
|
||||
Parenthesize::IfBreaksOrIfRequired => {
|
||||
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 => {
|
||||
if can_omit_optional_parentheses(expression, f.context()) {
|
||||
optional_parentheses(&expression.format().with_options(Parentheses::Never))
|
||||
.fmt(f)
|
||||
} else {
|
||||
parenthesize_if_expands(
|
||||
&expression.format().with_options(Parentheses::Never),
|
||||
)
|
||||
.fmt(f)
|
||||
}
|
||||
}
|
||||
},
|
||||
OptionalParentheses::Never => match parenthesize {
|
||||
Parenthesize::IfBreaksOrIfRequired => {
|
||||
parenthesize_if_expands(&expression.format().with_options(Parentheses::Never))
|
||||
.fmt(f)
|
||||
}
|
||||
|
||||
Parenthesize::Optional | Parenthesize::IfBreaks | Parenthesize::IfRequired => {
|
||||
expression.format().with_options(Parentheses::Never).fmt(f)
|
||||
}
|
||||
},
|
||||
OptionalParentheses::Always => {
|
||||
expression.format().with_options(Parentheses::Always).fmt(f)
|
||||
}
|
||||
OptionalParentheses::Never | OptionalParentheses::Multiline => {
|
||||
expression.format().with_options(Parentheses::Never).fmt(f)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -51,6 +51,11 @@ pub(crate) enum Parenthesize {
|
||||
/// * The expression is not enclosed by another parenthesized expression and it expands over multiple lines
|
||||
/// * The expression has leading or trailing comments. 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,
|
||||
}
|
||||
|
||||
impl Parenthesize {
|
||||
@@ -193,8 +198,8 @@ pub(crate) struct FormatOptionalParentheses<'content, 'ast> {
|
||||
|
||||
impl<'ast> Format<PyFormatContext<'ast>> for FormatOptionalParentheses<'_, 'ast> {
|
||||
fn fmt(&self, f: &mut Formatter<PyFormatContext<'ast>>) -> FormatResult<()> {
|
||||
// The group id is used as a condition in [`in_parentheses_only`] to create a conditional group
|
||||
// that is only active if the optional parentheses group expands.
|
||||
// The group id is used as a condition in [`in_parentheses_only_group`] to create a
|
||||
// conditional group that is only active if the optional parentheses group expands.
|
||||
let parens_id = f.group_id("optional_parentheses");
|
||||
|
||||
let mut f = WithNodeLevel::new(NodeLevel::Expression(Some(parens_id)), f);
|
||||
|
||||
Reference in New Issue
Block a user