Skip to content

Commit

Permalink
Mark trailing comments in parenthesized tests (#6287)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
charliermarsh authored Aug 3, 2023
1 parent 51ff98f commit 1705fce
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,11 @@ def f():
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
print(3) # 10 preceding: last in body, following: any, enclosing: try

try:
pass
except (
ZeroDivisionError
# comment
):
pass
75 changes: 60 additions & 15 deletions crates/ruff_python_formatter/src/comments/placement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ pub(super) fn place_comment<'a>(
comment: DecoratedComment<'a>,
locator: &Locator,
) -> 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() {
match handle_own_line_comment_after_body(comment, locator) {
match handle_own_line_comment_around_body(comment, locator) {
CommentPlacement::Default(comment) => comment,
placement => {
return placement;
Expand Down Expand Up @@ -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
/// for x in y:
/// pass
Expand All @@ -232,8 +234,14 @@ fn is_first_statement_in_body(statement: AnyNodeRef, has_body: AnyNodeRef) -> bo
/// print("I have no comments")
/// # This should be a trailing comment of the print
/// # 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>,
locator: &Locator,
) -> CommentPlacement<'a> {
Expand Down Expand Up @@ -265,16 +273,23 @@ fn handle_own_line_comment_after_body<'a>(
return CommentPlacement::Default(comment);
}

// Check if we're between bodies and should attach to the following body. If that is not the
// case, either because there is no following branch or because the indentation is too deep,
// attach to the recursively last statement in the preceding body with the matching indentation.
match handle_own_line_comment_between_branches(comment, preceding, locator) {
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)
}
placement => placement,
}
// Check if we're between bodies and should attach to the following body.
let comment = match handle_own_line_comment_between_branches(comment, preceding, locator) {
CommentPlacement::Default(comment) => comment,
placement => return placement,
};

// 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.
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.
Expand Down Expand Up @@ -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.
/// ```python
/// match pt:
Expand Down Expand Up @@ -1427,7 +1472,7 @@ fn last_child_in_body(node: AnyNodeRef) -> Option<AnyNodeRef> {
| AnyNodeRef::StmtClassDef(ast::StmtClassDef { body, .. })
| AnyNodeRef::StmtWith(ast::StmtWith { body, .. })
| AnyNodeRef::StmtAsyncWith(ast::StmtAsyncWith { body, .. })
| AnyNodeRef::MatchCase(ast::MatchCase { body, .. })
| AnyNodeRef::MatchCase(MatchCase { body, .. })
| AnyNodeRef::ExceptHandlerExceptHandler(ast::ExceptHandlerExceptHandler {
body, ..
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,16 +193,6 @@ class C:
```diff
--- Black
+++ 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 @@
value, is_going_to_be="too long to fit in a single line", srsly=True
), "Not what we expected"
Expand Down Expand Up @@ -293,8 +283,8 @@ class C:
while (
# Just a comment
call()
):
# Another
):
print(i)
xxxxxxxxxxxxxxxx = Yyyy2YyyyyYyyyyy(
push_manager=context.request.resource_manager,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,16 +193,6 @@ class C:
```diff
--- Black
+++ 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 @@
value, is_going_to_be="too long to fit in a single line", srsly=True
), "Not what we expected"
Expand Down Expand Up @@ -293,8 +283,8 @@ class C:
while (
# Just a comment
call()
):
# Another
):
print(i)
xxxxxxxxxxxxxxxx = Yyyy2YyyyyYyyyyy(
push_manager=context.request.resource_manager,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
finally: # 9 preceding: last in body, following: fist in alt body, enclosing: try
print(3) # 10 preceding: last in body, following: any, enclosing: try
try:
pass
except (
ZeroDivisionError
# comment
):
pass
```

## Output
Expand Down Expand Up @@ -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
finally: # 9 preceding: last in body, following: fist in alt body, enclosing: try
print(3) # 10 preceding: last in body, following: any, enclosing: try
try:
pass
except (
ZeroDivisionError
# comment
):
pass
```


Expand Down

0 comments on commit 1705fce

Please sign in to comment.