Skip to content

Commit

Permalink
Don't mistake a following if for an elif
Browse files Browse the repository at this point in the history
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 https://github.com/gradio-app/gradio/blob/1570b94a02d23d051ae137e0063974fd8a48b34e/gradio/external.py#L478
  • Loading branch information
konstin committed Jun 22, 2023
1 parent 8010d93 commit d6f0cd1
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,12 @@
pass
# Don't drop this comment 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
27 changes: 24 additions & 3 deletions crates/ruff_python_formatter/src/comments/placement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,16 +357,37 @@ fn handle_in_between_bodies_end_of_line_comment<'a>(
} else {
CommentPlacement::trailing(preceding, comment)
}
} else if following.is_stmt_if() || following.is_except_handler() {
} else {
// 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 {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ mapping = {
C: 0.1 * (10.0 / 12),
D: 0.1 * (10.0 / 12),
}
# Regression test for formatter panic with comment after parenthesized dict value
# Originally found in https://github.com/bolucat/Firefox/blob/636a717ef025c16434997dc89e42351ef740ee6b/testing/marionette/client/marionette_driver/geckoinstance.py#L109
a = {
1: (2),
# comment
3: True,
}
```


Expand Down Expand Up @@ -112,6 +120,14 @@ mapping = {
C: 0.1 * (10.0 / 12),
D: 0.1 * (10.0 / 12),
}
# Regression test for formatter panic with comment after parenthesized dict value
# Originally found in https://github.com/bolucat/Firefox/blob/636a717ef025c16434997dc89e42351ef740ee6b/testing/marionette/client/marionette_driver/geckoinstance.py#L109
a = {
1: (2),
# comment
3: True,
}
```


Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,15 @@ else:
pass
# Don't drop this comment 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
```


Expand Down Expand Up @@ -137,6 +146,15 @@ else:
pass
# Don't drop this comment 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
```


0 comments on commit d6f0cd1

Please sign in to comment.