mirror of
https://github.com/astral-sh/ruff.git
synced 2025-07-24 05:25:17 +00:00
Don't mistake a following if for an elif (#5296)
In the following code, the comment used to get wrongly associated with
the `if False` since it looked like an elif. This fixes it by checking
the indentation and adding a regression test
```python
if True:
pass
else: # Comment
if False:
pass
pass
```
Originally found in
1570b94a02/gradio/external.py (L478)
This commit is contained in:
parent
c52aa8f065
commit
930f03de98
3 changed files with 73 additions and 25 deletions
|
@ -62,3 +62,12 @@ else:
|
||||||
pass
|
pass
|
||||||
# Don't drop this comment 3
|
# Don't drop this comment 3
|
||||||
x = 3
|
x = 3
|
||||||
|
|
||||||
|
# Regression test for a following if that could get confused for an elif
|
||||||
|
# Originally found in https://github.com/gradio-app/gradio/blob/1570b94a02d23d051ae137e0063974fd8a48b34e/gradio/external.py#L478
|
||||||
|
if True:
|
||||||
|
pass
|
||||||
|
else: # Comment
|
||||||
|
if False:
|
||||||
|
pass
|
||||||
|
pass
|
||||||
|
|
|
@ -335,7 +335,52 @@ fn handle_in_between_bodies_end_of_line_comment<'a>(
|
||||||
return CommentPlacement::Default(comment);
|
return CommentPlacement::Default(comment);
|
||||||
}
|
}
|
||||||
|
|
||||||
if !locator.contains_line_break(TextRange::new(preceding.end(), comment.slice().start())) {
|
if locator.contains_line_break(TextRange::new(preceding.end(), comment.slice().start())) {
|
||||||
|
// The `elif` or except handlers have their own body to which we can attach the trailing comment
|
||||||
|
// ```python
|
||||||
|
// if test:
|
||||||
|
// a
|
||||||
|
// elif c: # comment
|
||||||
|
// b
|
||||||
|
// ```
|
||||||
|
if following.is_except_handler() {
|
||||||
|
return CommentPlacement::trailing(following, comment);
|
||||||
|
} else if following.is_stmt_if() {
|
||||||
|
// We have to exclude for following if statements that are not elif by checking the
|
||||||
|
// indentation
|
||||||
|
// ```python
|
||||||
|
// if True:
|
||||||
|
// pass
|
||||||
|
// else: # Comment
|
||||||
|
// if False:
|
||||||
|
// pass
|
||||||
|
// pass
|
||||||
|
// ```
|
||||||
|
let base_if_indent =
|
||||||
|
whitespace::indentation_at_offset(locator, following.range().start());
|
||||||
|
let maybe_elif_indent = whitespace::indentation_at_offset(
|
||||||
|
locator,
|
||||||
|
comment.enclosing_node().range().start(),
|
||||||
|
);
|
||||||
|
if base_if_indent == maybe_elif_indent {
|
||||||
|
return CommentPlacement::trailing(following, comment);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// There are no bodies for the "else" branch and other bodies that are represented as a `Vec<Stmt>`.
|
||||||
|
// This means, there's no good place to attach the comments to.
|
||||||
|
// Make this a dangling comments and manually format the comment in
|
||||||
|
// in the enclosing node's formatting logic. For `try`, it's the formatters responsibility
|
||||||
|
// to correctly identify the comments for the `finally` and `orelse` block by looking
|
||||||
|
// at the comment's range.
|
||||||
|
//
|
||||||
|
// ```python
|
||||||
|
// while x == y:
|
||||||
|
// pass
|
||||||
|
// else: # trailing
|
||||||
|
// print("nooop")
|
||||||
|
// ```
|
||||||
|
CommentPlacement::dangling(comment.enclosing_node(), comment)
|
||||||
|
} else {
|
||||||
// Trailing comment of the preceding statement
|
// Trailing comment of the preceding statement
|
||||||
// ```python
|
// ```python
|
||||||
// while test:
|
// while test:
|
||||||
|
@ -357,30 +402,6 @@ fn handle_in_between_bodies_end_of_line_comment<'a>(
|
||||||
} else {
|
} else {
|
||||||
CommentPlacement::trailing(preceding, comment)
|
CommentPlacement::trailing(preceding, comment)
|
||||||
}
|
}
|
||||||
} else if following.is_stmt_if() || following.is_except_handler() {
|
|
||||||
// The `elif` or except handlers have their own body to which we can attach the trailing comment
|
|
||||||
// ```python
|
|
||||||
// if test:
|
|
||||||
// a
|
|
||||||
// elif c: # comment
|
|
||||||
// b
|
|
||||||
// ```
|
|
||||||
CommentPlacement::trailing(following, comment)
|
|
||||||
} else {
|
|
||||||
// There are no bodies for the "else" branch and other bodies that are represented as a `Vec<Stmt>`.
|
|
||||||
// This means, there's no good place to attach the comments to.
|
|
||||||
// Make this a dangling comments and manually format the comment in
|
|
||||||
// in the enclosing node's formatting logic. For `try`, it's the formatters responsibility
|
|
||||||
// to correctly identify the comments for the `finally` and `orelse` block by looking
|
|
||||||
// at the comment's range.
|
|
||||||
//
|
|
||||||
// ```python
|
|
||||||
// while x == y:
|
|
||||||
// pass
|
|
||||||
// else: # trailing
|
|
||||||
// print("nooop")
|
|
||||||
// ```
|
|
||||||
CommentPlacement::dangling(comment.enclosing_node(), comment)
|
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
CommentPlacement::Default(comment)
|
CommentPlacement::Default(comment)
|
||||||
|
|
|
@ -68,6 +68,15 @@ else:
|
||||||
pass
|
pass
|
||||||
# Don't drop this comment 3
|
# Don't drop this comment 3
|
||||||
x = 3
|
x = 3
|
||||||
|
|
||||||
|
# Regression test for a following if that could get confused for an elif
|
||||||
|
# Originally found in https://github.com/gradio-app/gradio/blob/1570b94a02d23d051ae137e0063974fd8a48b34e/gradio/external.py#L478
|
||||||
|
if True:
|
||||||
|
pass
|
||||||
|
else: # Comment
|
||||||
|
if False:
|
||||||
|
pass
|
||||||
|
pass
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
||||||
|
@ -137,6 +146,15 @@ else:
|
||||||
pass
|
pass
|
||||||
# Don't drop this comment 3
|
# Don't drop this comment 3
|
||||||
x = 3
|
x = 3
|
||||||
|
|
||||||
|
# Regression test for a following if that could get confused for an elif
|
||||||
|
# Originally found in https://github.com/gradio-app/gradio/blob/1570b94a02d23d051ae137e0063974fd8a48b34e/gradio/external.py#L478
|
||||||
|
if True:
|
||||||
|
pass
|
||||||
|
else: # Comment
|
||||||
|
if False:
|
||||||
|
pass
|
||||||
|
pass
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue