Skip to content

Allow return type annotations to use their own parentheses #6436

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

Merged
merged 7 commits into from
Aug 11, 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
Expand Up @@ -371,67 +371,3 @@ def f( # first
# third
):
...

# Handle comments on empty tuple return types.
def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override]
): ...

def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override]
# comment
): ...

def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override]
1
): ...

def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override]
1, 2
): ...

def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override]
(1, 2)
): ...

def handleMatch( # type: ignore[override] # https://github.com/python/mypy/issues/10197
self, m: Match[str], data: str
) -> Union[Tuple[None, None, None], Tuple[Element, int, int]]:
...

def double(a: int # Hello
) -> (int):
return 2 * a

def double(a: int) -> ( # Hello
int
):
return 2*a

def double(a: int) -> ( # Hello
):
return 2*a

# Breaking over parameters and return types. (Black adds a trailing comma when the
# function arguments break here with a single argument; we do not.)
def f(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
...

def f(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, a) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
...

def f(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) -> a:
...

def f(a) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
...

def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]() -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
...

def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]() -> a:
...

def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa](aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
...

def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa](aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) -> a:
...
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
# Handle comments on empty tuple return types.
def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override]
): ...

def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override]
# comment
): ...

def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override]
1
): ...

def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override]
1, 2
): ...

def zrevrangebylex(self, name: _Key, max: _Value, min: _Value, start: int | None = None, num: int | None = None) -> ( # type: ignore[override]
(1, 2)
): ...

def handleMatch( # type: ignore[override] # https://github.com/python/mypy/issues/10197
self, m: Match[str], data: str
) -> Union[Tuple[None, None, None], Tuple[Element, int, int]]:
...

def double(a: int # Hello
) -> (int):
return 2 * a

def double(a: int) -> ( # Hello
int
):
return 2*a

def double(a: int) -> ( # Hello
):
return 2*a

# Breaking over parameters and return types. (Black adds a trailing comma when the
# function arguments break here with a single argument; we do not.)
def f(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
...

def f(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, a) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
...

def f(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) -> a:
...

def f(a) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
...

def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]() -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
...

def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]() -> a:
...

def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa](aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
...

def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa](aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) -> a:
...

# Breaking return type annotations. Black adds parentheses if the parameters are
# empty; otherwise, it leverages the expressions own parentheses if possible.
def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> (
Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"]
):
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
]:
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> (
Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"]
):
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(x) -> Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
]:
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(x) -> (
Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"]
):
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(*args) -> Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
]:
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( # foo
) -> Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"]:
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(
# bar
) -> Set[
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"]:
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx:
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx.xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx:
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(
x) -> xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx:
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(
x) -> xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx.xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx:
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> (X + Y + foooooooooooooooooooooooooooooooooooo()):
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(x) -> (X + Y + foooooooooooooooooooooooooooooooooooo()):
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> (X and Y and foooooooooooooooooooooooooooooooooooo()):
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(x) -> (X and Y and foooooooooooooooooooooooooooooooooooo()):
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> (X | Y | foooooooooooooooooooooooooooooooooooo()):
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(x) -> (X | Y | foooooooooooooooooooooooooooooooooooo()):
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> (
X | Y | foooooooooooooooooooooooooooooooooooo() # comment
):
...

def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(x) -> (
X | Y | foooooooooooooooooooooooooooooooooooo() # comment
):
...


def double() -> first_item and foo.bar.baz().bop(1,):
return 2 * a


# Dangling comments on return annotations.
def double(a: int) -> (
int # Hello
):
return 2*a

def double(a: int) -> (
foo.bar # Hello
):
return 2*a

def double(a: int) -> (
[int] # Hello
):
return 2*a

def double(a: int) -> (
int | list[int] # Hello
):
pass

def double(a: int) -> (
int | list[int, int, int, int, int, int, int, int, int, int, int, int, int, int, int, int] # Hello
):
pass
38 changes: 29 additions & 9 deletions crates/ruff_python_formatter/src/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit less DRY but I find it much easier to reason about the match when every OptionalParentheses has a single branch.

OptionalParentheses::Multiline if *parenthesize != Parenthesize::IfRequired => {
if can_omit_optional_parentheses(expression, f.context()) {
optional_parentheses(&expression.format().with_options(Parentheses::Never))
OptionalParentheses::Multiline => match parenthesize {
Parenthesize::IfBreaksOrIfRequired => {
parenthesize_if_expands(&expression.format().with_options(Parentheses::Never))
.fmt(f)
} else {
}
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)
}
}
}
}
Expand Down
9 changes: 7 additions & 2 deletions crates/ruff_python_formatter/src/expression/parentheses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
Loading