Treat comments on open parentheses in return annotations as dangling (#6413)

## Summary

Given:

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

We currently treat `# Hello` as a trailing comment on the parameters
(`(a: int)`). This PR adds a placement method to instead treat it as a
dangling comment on the function definition itself, so that it gets
formatted at the end of the definition, like:

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

The formatting in this case is unchanged, but it's incorrect IMO for
that to be a trailing comment on the parameters, and that placement
leads to an instability after changing the grouping in #6410.

Fixing this led to a _different_ instability related to tuple return
type annotations, like:

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

(This is a real example.)

To fix, I had to special-case tuples in that spot, though I'm not
certain that's correct.
This commit is contained in:
Charlie Marsh 2023-08-08 16:48:38 -04:00 committed by GitHub
parent d33618062e
commit 55d6fd53cd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 211 additions and 12 deletions

View file

@ -371,3 +371,41 @@ def f( # first
# third # 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

View file

@ -67,7 +67,10 @@ pub(super) fn place_comment<'a>(
handle_module_level_own_line_comment_before_class_or_function_comment(comment, locator) handle_module_level_own_line_comment_before_class_or_function_comment(comment, locator)
} }
AnyNodeRef::WithItem(_) => handle_with_item_comment(comment, locator), AnyNodeRef::WithItem(_) => handle_with_item_comment(comment, locator),
AnyNodeRef::StmtFunctionDef(_) => handle_leading_function_with_decorators_comment(comment), AnyNodeRef::StmtFunctionDef(function_def) => {
handle_leading_function_with_decorators_comment(comment)
.or_else(|comment| handle_leading_returns_comment(comment, function_def))
}
AnyNodeRef::StmtClassDef(class_def) => { AnyNodeRef::StmtClassDef(class_def) => {
handle_leading_class_with_decorators_comment(comment, class_def) handle_leading_class_with_decorators_comment(comment, class_def)
} }
@ -783,6 +786,40 @@ fn handle_leading_function_with_decorators_comment(comment: DecoratedComment) ->
} }
} }
/// Handles end-of-line comments between function parameters and the return type annotation,
/// attaching them as dangling comments to the function instead of making them trailing
/// parameter comments.
///
/// ```python
/// def double(a: int) -> ( # Hello
/// int
/// ):
/// return 2*a
/// ```
fn handle_leading_returns_comment<'a>(
comment: DecoratedComment<'a>,
function_def: &'a ast::StmtFunctionDef,
) -> CommentPlacement<'a> {
let parameters = function_def.parameters.as_ref();
let Some(returns) = function_def.returns.as_deref() else {
return CommentPlacement::Default(comment);
};
let is_preceding_parameters = comment
.preceding_node()
.is_some_and(|node| node == parameters.into());
let is_following_returns = comment
.following_node()
.is_some_and(|node| node == returns.into());
if comment.line_position().is_end_of_line() && is_preceding_parameters && is_following_returns {
CommentPlacement::dangling(comment.enclosing_node(), comment)
} else {
CommentPlacement::Default(comment)
}
}
/// Handle comments between decorators and the decorated node. /// Handle comments between decorators and the decorated node.
/// ///
/// For example, given: /// For example, given:

View file

@ -61,17 +61,20 @@ impl FormatNodeRule<StmtFunctionDef> for FormatStmtFunctionDef {
write!(f, [item.parameters.format()])?; write!(f, [item.parameters.format()])?;
if let Some(return_annotation) = item.returns.as_ref() { if let Some(return_annotation) = item.returns.as_ref() {
write!( write!(f, [space(), text("->"), space()])?;
f, if return_annotation.is_tuple_expr() {
[ write!(
space(), f,
text("->"), [return_annotation.format().with_options(Parentheses::Never)]
space(), )?;
optional_parentheses( } else {
&return_annotation.format().with_options(Parentheses::Never) write!(
) f,
] [optional_parentheses(
)?; &return_annotation.format().with_options(Parentheses::Never),
)]
)?;
}
} }
write!( write!(

View file

@ -377,6 +377,44 @@ def f( # first
# third # 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
``` ```
## Output ## Output
@ -905,6 +943,89 @@ def f( # first
/, # second /, # second
): ):
... ...
# 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,
) -> 1: # type: ignore[override]
...
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,
) -> (1, 2): # type: ignore[override]
...
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) -> int: # Hello
return 2 * a
def double(a: int) -> ( # Hello
):
return 2 * a
``` ```