Mark trailing comments in parenthesized tests (#6287)

## Summary

This ensures that we treat `# comment` as parenthesized in contexts
like:

```python
while (
    True
    # comment
):
    pass
```

The same logic applies equally to `for`, `async for`, `if`, `with`, and
`async with`. The general pattern is that you have an expression which
precedes a colon-separated suite.
This commit is contained in:
Charlie Marsh 2023-08-03 16:45:03 -04:00 committed by GitHub
parent 51ff98f9e9
commit 1705fcef36
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 86 additions and 37 deletions

View file

@ -138,3 +138,11 @@ else: # 7 preceding: last in body, following: fist in alt body, enclosing: exc
print(3) # 8 preceding: last in body, following: fist in alt body, enclosing: try print(3) # 8 preceding: last in body, following: fist in alt body, enclosing: try
finally: # 9 preceding: last in body, following: fist in alt body, enclosing: try finally: # 9 preceding: last in body, following: fist in alt body, enclosing: try
print(3) # 10 preceding: last in body, following: any, enclosing: try print(3) # 10 preceding: last in body, following: any, enclosing: try
try:
pass
except (
ZeroDivisionError
# comment
):
pass

View file

@ -23,9 +23,9 @@ pub(super) fn place_comment<'a>(
comment: DecoratedComment<'a>, comment: DecoratedComment<'a>,
locator: &Locator, locator: &Locator,
) -> CommentPlacement<'a> { ) -> CommentPlacement<'a> {
// Handle comments before and after bodies such as the different branches of an if statement // Handle comments before and after bodies such as the different branches of an if statement.
let comment = if comment.line_position().is_own_line() { let comment = if comment.line_position().is_own_line() {
match handle_own_line_comment_after_body(comment, locator) { match handle_own_line_comment_around_body(comment, locator) {
CommentPlacement::Default(comment) => comment, CommentPlacement::Default(comment) => comment,
placement => { placement => {
return placement; return placement;
@ -222,7 +222,9 @@ fn is_first_statement_in_body(statement: AnyNodeRef, has_body: AnyNodeRef) -> bo
} }
} }
/// Handles own line comments after a body, either at the end or between bodies. /// Handles own-line comments around a body (at the end of the body, at the end of the header
/// preceding the body, or between bodies):
///
/// ```python /// ```python
/// for x in y: /// for x in y:
/// pass /// pass
@ -232,8 +234,14 @@ fn is_first_statement_in_body(statement: AnyNodeRef, has_body: AnyNodeRef) -> bo
/// print("I have no comments") /// print("I have no comments")
/// # This should be a trailing comment of the print /// # This should be a trailing comment of the print
/// # This is a trailing comment of the entire statement /// # This is a trailing comment of the entire statement
///
/// if (
/// True
/// # This should be a trailing comment of `True` and not a leading comment of `pass`
/// ):
/// pass
/// ``` /// ```
fn handle_own_line_comment_after_body<'a>( fn handle_own_line_comment_around_body<'a>(
comment: DecoratedComment<'a>, comment: DecoratedComment<'a>,
locator: &Locator, locator: &Locator,
) -> CommentPlacement<'a> { ) -> CommentPlacement<'a> {
@ -265,16 +273,23 @@ fn handle_own_line_comment_after_body<'a>(
return CommentPlacement::Default(comment); return CommentPlacement::Default(comment);
} }
// Check if we're between bodies and should attach to the following body. If that is not the // Check if we're between bodies and should attach to the following body.
// case, either because there is no following branch or because the indentation is too deep, let comment = match handle_own_line_comment_between_branches(comment, preceding, locator) {
// attach to the recursively last statement in the preceding body with the matching indentation. CommentPlacement::Default(comment) => comment,
match handle_own_line_comment_between_branches(comment, preceding, locator) { placement => return placement,
CommentPlacement::Default(comment) => { };
// Knowing the comment is not between branches, handle comments after the last branch
handle_own_line_comment_after_branch(comment, preceding, locator) // Otherwise, there's no following branch or the indentation is too deep, so attach to the
} // recursively last statement in the preceding body with the matching indentation.
placement => placement, let comment = match handle_own_line_comment_after_branch(comment, preceding, locator) {
} CommentPlacement::Default(comment) => comment,
placement => return placement,
};
// If the following node is the first in its body, and there's a non-trivia token between the
// comment and the following node (like a parenthesis), then it means the comment is trailing
// the preceding node, not leading the following one.
handle_own_line_comment_in_clause(comment, preceding, locator)
} }
/// Handles own line comments between two branches of a node. /// Handles own line comments between two branches of a node.
@ -374,6 +389,36 @@ fn handle_own_line_comment_between_branches<'a>(
} }
} }
/// Handles own-line comments at the end of a clause, immediately preceding a body:
/// ```python
/// if (
/// True
/// # This should be a trailing comment of `True` and not a leading comment of `pass`
/// ):
/// pass
/// ```
fn handle_own_line_comment_in_clause<'a>(
comment: DecoratedComment<'a>,
preceding: AnyNodeRef<'a>,
locator: &Locator,
) -> CommentPlacement<'a> {
if let Some(following) = comment.following_node() {
if is_first_statement_in_body(following, comment.enclosing_node())
&& SimpleTokenizer::new(
locator.contents(),
TextRange::new(comment.end(), following.start()),
)
.skip_trivia()
.next()
.is_some()
{
return CommentPlacement::trailing(preceding, comment);
}
}
CommentPlacement::Default(comment)
}
/// Handles leading comments in front of a match case or a trailing comment of the `match` statement. /// Handles leading comments in front of a match case or a trailing comment of the `match` statement.
/// ```python /// ```python
/// match pt: /// match pt:
@ -1427,7 +1472,7 @@ fn last_child_in_body(node: AnyNodeRef) -> Option<AnyNodeRef> {
| AnyNodeRef::StmtClassDef(ast::StmtClassDef { body, .. }) | AnyNodeRef::StmtClassDef(ast::StmtClassDef { body, .. })
| AnyNodeRef::StmtWith(ast::StmtWith { body, .. }) | AnyNodeRef::StmtWith(ast::StmtWith { body, .. })
| AnyNodeRef::StmtAsyncWith(ast::StmtAsyncWith { body, .. }) | AnyNodeRef::StmtAsyncWith(ast::StmtAsyncWith { body, .. })
| AnyNodeRef::MatchCase(ast::MatchCase { body, .. }) | AnyNodeRef::MatchCase(MatchCase { body, .. })
| AnyNodeRef::ExceptHandlerExceptHandler(ast::ExceptHandlerExceptHandler { | AnyNodeRef::ExceptHandlerExceptHandler(ast::ExceptHandlerExceptHandler {
body, .. body, ..
}) })

View file

@ -193,16 +193,6 @@ class C:
```diff ```diff
--- Black --- Black
+++ Ruff +++ Ruff
@@ -28,8 +28,8 @@
while (
# Just a comment
call()
+ ):
# Another
- ):
print(i)
xxxxxxxxxxxxxxxx = Yyyy2YyyyyYyyyyy(
push_manager=context.request.resource_manager,
@@ -110,19 +110,20 @@ @@ -110,19 +110,20 @@
value, is_going_to_be="too long to fit in a single line", srsly=True value, is_going_to_be="too long to fit in a single line", srsly=True
), "Not what we expected" ), "Not what we expected"
@ -293,8 +283,8 @@ class C:
while ( while (
# Just a comment # Just a comment
call() call()
):
# Another # Another
):
print(i) print(i)
xxxxxxxxxxxxxxxx = Yyyy2YyyyyYyyyyy( xxxxxxxxxxxxxxxx = Yyyy2YyyyyYyyyyy(
push_manager=context.request.resource_manager, push_manager=context.request.resource_manager,

View file

@ -193,16 +193,6 @@ class C:
```diff ```diff
--- Black --- Black
+++ Ruff +++ Ruff
@@ -28,8 +28,8 @@
while (
# Just a comment
call()
+ ):
# Another
- ):
print(i)
xxxxxxxxxxxxxxxx = Yyyy2YyyyyYyyyyy(
push_manager=context.request.resource_manager,
@@ -110,19 +110,20 @@ @@ -110,19 +110,20 @@
value, is_going_to_be="too long to fit in a single line", srsly=True value, is_going_to_be="too long to fit in a single line", srsly=True
), "Not what we expected" ), "Not what we expected"
@ -293,8 +283,8 @@ class C:
while ( while (
# Just a comment # Just a comment
call() call()
):
# Another # Another
):
print(i) print(i)
xxxxxxxxxxxxxxxx = Yyyy2YyyyyYyyyyy( xxxxxxxxxxxxxxxx = Yyyy2YyyyyYyyyyy(
push_manager=context.request.resource_manager, push_manager=context.request.resource_manager,

View file

@ -144,6 +144,14 @@ else: # 7 preceding: last in body, following: fist in alt body, enclosing: exc
print(3) # 8 preceding: last in body, following: fist in alt body, enclosing: try print(3) # 8 preceding: last in body, following: fist in alt body, enclosing: try
finally: # 9 preceding: last in body, following: fist in alt body, enclosing: try finally: # 9 preceding: last in body, following: fist in alt body, enclosing: try
print(3) # 10 preceding: last in body, following: any, enclosing: try print(3) # 10 preceding: last in body, following: any, enclosing: try
try:
pass
except (
ZeroDivisionError
# comment
):
pass
``` ```
## Output ## Output
@ -304,6 +312,14 @@ else: # 7 preceding: last in body, following: fist in alt body, enclosing: exc
print(3) # 8 preceding: last in body, following: fist in alt body, enclosing: try print(3) # 8 preceding: last in body, following: fist in alt body, enclosing: try
finally: # 9 preceding: last in body, following: fist in alt body, enclosing: try finally: # 9 preceding: last in body, following: fist in alt body, enclosing: try
print(3) # 10 preceding: last in body, following: any, enclosing: try print(3) # 10 preceding: last in body, following: any, enclosing: try
try:
pass
except (
ZeroDivisionError
# comment
):
pass
``` ```