mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-04 02:38:25 +00:00
Fix instability in trailing clause body comments (#7556)
## Summary When we format the trailing comments on a clause body, we check if there are any newlines after the last statement; if not, we insert one. This logic didn't take into account that the last statement could itself have trailing comments, as in: ```python if True: pass # comment else: pass ``` We were thus inserting a newline after the comment, like: ```python if True: pass # comment else: pass ``` In the context of function definitions, this led to an instability, since we insert a newline _after_ a function, which would in turn lead to the bug above appearing in the second formatting pass. Closes https://github.com/astral-sh/ruff/issues/7465. ## Test Plan `cargo test` Small improvement in `transformers`, but no regressions. Before: | project | similarity index | total files | changed files | |--------------|------------------:|------------------:|------------------:| | cpython | 0.76083 | 1789 | 1631 | | django | 0.99983 | 2760 | 36 | | transformers | 0.99956 | 2587 | 404 | | twine | 1.00000 | 33 | 0 | | typeshed | 0.99983 | 3496 | 18 | | warehouse | 0.99967 | 648 | 15 | | zulip | 0.99972 | 1437 | 21 | After: | project | similarity index | total files | changed files | |--------------|------------------:|------------------:|------------------:| | cpython | 0.76083 | 1789 | 1631 | | django | 0.99983 | 2760 | 36 | | **transformers** | **0.99957** | **2587** | **402** | | twine | 1.00000 | 33 | 0 | | typeshed | 0.99983 | 3496 | 18 | | warehouse | 0.99967 | 648 | 15 | | zulip | 0.99972 | 1437 | 21 |
This commit is contained in:
parent
ab643017f9
commit
124d95d246
5 changed files with 138 additions and 2 deletions
|
@ -376,3 +376,11 @@ def f( # first
|
|||
def this_is_unusual() -> (please := no): ...
|
||||
|
||||
def this_is_unusual(x) -> (please := no): ...
|
||||
|
||||
# Regression test for: https://github.com/astral-sh/ruff/issues/7465
|
||||
try:
|
||||
def test():
|
||||
pass
|
||||
#comment
|
||||
except ImportError:
|
||||
pass
|
||||
|
|
|
@ -104,6 +104,40 @@ if True: print("a") # 1
|
|||
elif True: print("b") # 2
|
||||
else: print("c") # 3
|
||||
|
||||
# Regression test for: https://github.com/astral-sh/ruff/issues/7465
|
||||
if True:
|
||||
pass
|
||||
|
||||
# comment
|
||||
else:
|
||||
pass
|
||||
|
||||
if True:
|
||||
pass
|
||||
|
||||
# comment
|
||||
|
||||
else:
|
||||
pass
|
||||
|
||||
if True:
|
||||
pass
|
||||
# comment
|
||||
|
||||
else:
|
||||
pass
|
||||
|
||||
if True:
|
||||
pass
|
||||
# comment
|
||||
else:
|
||||
pass
|
||||
|
||||
if True:
|
||||
pass # comment
|
||||
else:
|
||||
pass
|
||||
|
||||
# Regression test for https://github.com/astral-sh/ruff/issues/5337
|
||||
if parent_body:
|
||||
if current_body:
|
||||
|
|
|
@ -3,7 +3,7 @@ use std::borrow::Cow;
|
|||
use ruff_formatter::{format_args, write, FormatError, FormatOptions, SourceCode};
|
||||
use ruff_python_ast::node::{AnyNodeRef, AstNode};
|
||||
use ruff_python_ast::PySourceType;
|
||||
use ruff_python_trivia::{lines_after, lines_after_ignoring_trivia, lines_before};
|
||||
use ruff_python_trivia::{lines_after, lines_before};
|
||||
use ruff_text_size::{Ranged, TextLen, TextRange};
|
||||
|
||||
use crate::comments::{CommentLinePosition, SourceComment};
|
||||
|
@ -94,7 +94,14 @@ impl Format<PyFormatContext<'_>> for FormatLeadingAlternateBranchComments<'_> {
|
|||
} else if let Some(last_preceding) = self.last_node {
|
||||
// The leading comments formatting ensures that it preserves the right amount of lines after
|
||||
// We need to take care of this ourselves, if there's no leading `else` comment.
|
||||
if lines_after_ignoring_trivia(last_preceding.end(), f.context().source()) > 1 {
|
||||
let end = if let Some(last_trailing) =
|
||||
f.context().comments().trailing(last_preceding).last()
|
||||
{
|
||||
last_trailing.end()
|
||||
} else {
|
||||
last_preceding.end()
|
||||
};
|
||||
if lines_after(end, f.context().source()) > 1 {
|
||||
write!(f, [empty_line()])?;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -382,6 +382,14 @@ def f( # first
|
|||
def this_is_unusual() -> (please := no): ...
|
||||
|
||||
def this_is_unusual(x) -> (please := no): ...
|
||||
|
||||
# Regression test for: https://github.com/astral-sh/ruff/issues/7465
|
||||
try:
|
||||
def test():
|
||||
pass
|
||||
#comment
|
||||
except ImportError:
|
||||
pass
|
||||
```
|
||||
|
||||
## Output
|
||||
|
@ -921,6 +929,17 @@ def this_is_unusual() -> (please := no):
|
|||
|
||||
def this_is_unusual(x) -> (please := no):
|
||||
...
|
||||
|
||||
|
||||
# Regression test for: https://github.com/astral-sh/ruff/issues/7465
|
||||
try:
|
||||
|
||||
def test():
|
||||
pass
|
||||
|
||||
# comment
|
||||
except ImportError:
|
||||
pass
|
||||
```
|
||||
|
||||
|
||||
|
|
|
@ -110,6 +110,40 @@ if True: print("a") # 1
|
|||
elif True: print("b") # 2
|
||||
else: print("c") # 3
|
||||
|
||||
# Regression test for: https://github.com/astral-sh/ruff/issues/7465
|
||||
if True:
|
||||
pass
|
||||
|
||||
# comment
|
||||
else:
|
||||
pass
|
||||
|
||||
if True:
|
||||
pass
|
||||
|
||||
# comment
|
||||
|
||||
else:
|
||||
pass
|
||||
|
||||
if True:
|
||||
pass
|
||||
# comment
|
||||
|
||||
else:
|
||||
pass
|
||||
|
||||
if True:
|
||||
pass
|
||||
# comment
|
||||
else:
|
||||
pass
|
||||
|
||||
if True:
|
||||
pass # comment
|
||||
else:
|
||||
pass
|
||||
|
||||
# Regression test for https://github.com/astral-sh/ruff/issues/5337
|
||||
if parent_body:
|
||||
if current_body:
|
||||
|
@ -237,6 +271,40 @@ elif True:
|
|||
else:
|
||||
print("c") # 3
|
||||
|
||||
# Regression test for: https://github.com/astral-sh/ruff/issues/7465
|
||||
if True:
|
||||
pass
|
||||
|
||||
# comment
|
||||
else:
|
||||
pass
|
||||
|
||||
if True:
|
||||
pass
|
||||
|
||||
# comment
|
||||
|
||||
else:
|
||||
pass
|
||||
|
||||
if True:
|
||||
pass
|
||||
# comment
|
||||
|
||||
else:
|
||||
pass
|
||||
|
||||
if True:
|
||||
pass
|
||||
# comment
|
||||
else:
|
||||
pass
|
||||
|
||||
if True:
|
||||
pass # comment
|
||||
else:
|
||||
pass
|
||||
|
||||
# Regression test for https://github.com/astral-sh/ruff/issues/5337
|
||||
if parent_body:
|
||||
if current_body:
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue