Fix incorect placement of trailing stub function comments (#11632)

This commit is contained in:
Micha Reiser 2024-05-31 14:06:17 +02:00 committed by GitHub
parent 889667ad84
commit 9b6d2ce1f2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 142 additions and 44 deletions

View file

@ -0,0 +1,28 @@
# Regression tests for https://github.com/astral-sh/ruff/issues/11569
# comment 1
def foo(self) -> None: ...
def bar(self) -> None: ...
# comment 2
# comment 3
def baz(self) -> None:
return None
# comment 4
def foo(self) -> None: ...
# comment 5
def baz(self) -> None:
return None
def foo(self) -> None:
... # comment 5
def baz(self) -> None:
return None
def foo(self) -> None: ...
# comment 5

View file

@ -164,7 +164,7 @@ impl Format<PyFormatContext<'_>> for FormatTrailingComments<'_> {
line_suffix( line_suffix(
&format_args![ &format_args![
empty_lines(lines_before_comment), empty_lines(lines_before_comment),
format_comment(trailing) format_comment(trailing),
], ],
// Reserving width isn't necessary because we don't split // Reserving width isn't necessary because we don't split
// comments and the empty lines expand any enclosing group. // comments and the empty lines expand any enclosing group.
@ -535,22 +535,13 @@ fn strip_comment_prefix(comment_text: &str) -> FormatResult<&str> {
/// ``` /// ```
/// ///
/// This builder will insert a single empty line before the comment. /// This builder will insert a single empty line before the comment.
pub(crate) fn empty_lines_before_trailing_comments<'a>( pub(crate) fn empty_lines_before_trailing_comments(
f: &PyFormatter, comments: &[SourceComment],
comments: &'a [SourceComment],
node_kind: NodeKind, node_kind: NodeKind,
) -> FormatEmptyLinesBeforeTrailingComments<'a> { ) -> FormatEmptyLinesBeforeTrailingComments {
// Black has different rules for stub vs. non-stub and top level vs. indented
let empty_lines = match (f.options().source_type(), f.context().node_level()) {
(PySourceType::Stub, NodeLevel::TopLevel(_)) => 1,
(PySourceType::Stub, _) => u32::from(node_kind == NodeKind::StmtClassDef),
(_, NodeLevel::TopLevel(_)) => 2,
(_, _) => 1,
};
FormatEmptyLinesBeforeTrailingComments { FormatEmptyLinesBeforeTrailingComments {
comments, comments,
empty_lines, node_kind,
} }
} }
@ -558,8 +549,7 @@ pub(crate) fn empty_lines_before_trailing_comments<'a>(
pub(crate) struct FormatEmptyLinesBeforeTrailingComments<'a> { pub(crate) struct FormatEmptyLinesBeforeTrailingComments<'a> {
/// The trailing comments of the node. /// The trailing comments of the node.
comments: &'a [SourceComment], comments: &'a [SourceComment],
/// The expected number of empty lines before the trailing comments. node_kind: NodeKind,
empty_lines: u32,
} }
impl Format<PyFormatContext<'_>> for FormatEmptyLinesBeforeTrailingComments<'_> { impl Format<PyFormatContext<'_>> for FormatEmptyLinesBeforeTrailingComments<'_> {
@ -569,9 +559,17 @@ impl Format<PyFormatContext<'_>> for FormatEmptyLinesBeforeTrailingComments<'_>
.iter() .iter()
.find(|comment| comment.line_position().is_own_line()) .find(|comment| comment.line_position().is_own_line())
{ {
// Black has different rules for stub vs. non-stub and top level vs. indented
let empty_lines = match (f.options().source_type(), f.context().node_level()) {
(PySourceType::Stub, NodeLevel::TopLevel(_)) => 1,
(PySourceType::Stub, _) => u32::from(self.node_kind == NodeKind::StmtClassDef),
(_, NodeLevel::TopLevel(_)) => 2,
(_, _) => 1,
};
let actual = lines_before(comment.start(), f.context().source()).saturating_sub(1); let actual = lines_before(comment.start(), f.context().source()).saturating_sub(1);
for _ in actual..self.empty_lines { for _ in actual..empty_lines {
write!(f, [empty_line()])?; empty_line().fmt(f)?;
} }
} }
Ok(()) Ok(())
@ -590,30 +588,16 @@ impl Format<PyFormatContext<'_>> for FormatEmptyLinesBeforeTrailingComments<'_>
/// ///
/// While `leading_comments` will preserve the existing empty line, this builder will insert an /// While `leading_comments` will preserve the existing empty line, this builder will insert an
/// additional empty line before the comment. /// additional empty line before the comment.
pub(crate) fn empty_lines_after_leading_comments<'a>( pub(crate) fn empty_lines_after_leading_comments(
f: &PyFormatter, comments: &[SourceComment],
comments: &'a [SourceComment], ) -> FormatEmptyLinesAfterLeadingComments {
) -> FormatEmptyLinesAfterLeadingComments<'a> { FormatEmptyLinesAfterLeadingComments { comments }
// Black has different rules for stub vs. non-stub and top level vs. indented
let empty_lines = match (f.options().source_type(), f.context().node_level()) {
(PySourceType::Stub, NodeLevel::TopLevel(_)) => 1,
(PySourceType::Stub, _) => 0,
(_, NodeLevel::TopLevel(_)) => 2,
(_, _) => 1,
};
FormatEmptyLinesAfterLeadingComments {
comments,
empty_lines,
}
} }
#[derive(Copy, Clone, Debug)] #[derive(Copy, Clone, Debug)]
pub(crate) struct FormatEmptyLinesAfterLeadingComments<'a> { pub(crate) struct FormatEmptyLinesAfterLeadingComments<'a> {
/// The leading comments of the node. /// The leading comments of the node.
comments: &'a [SourceComment], comments: &'a [SourceComment],
/// The expected number of empty lines after the leading comments.
empty_lines: u32,
} }
impl Format<PyFormatContext<'_>> for FormatEmptyLinesAfterLeadingComments<'_> { impl Format<PyFormatContext<'_>> for FormatEmptyLinesAfterLeadingComments<'_> {
@ -624,6 +608,14 @@ impl Format<PyFormatContext<'_>> for FormatEmptyLinesAfterLeadingComments<'_> {
.rev() .rev()
.find(|comment| comment.line_position().is_own_line()) .find(|comment| comment.line_position().is_own_line())
{ {
// Black has different rules for stub vs. non-stub and top level vs. indented
let empty_lines = match (f.options().source_type(), f.context().node_level()) {
(PySourceType::Stub, NodeLevel::TopLevel(_)) => 1,
(PySourceType::Stub, _) => 0,
(_, NodeLevel::TopLevel(_)) => 2,
(_, _) => 1,
};
let actual = lines_after(comment.end(), f.context().source()).saturating_sub(1); let actual = lines_after(comment.end(), f.context().source()).saturating_sub(1);
// If there are no empty lines, keep the comment tight to the node. // If there are no empty lines, keep the comment tight to the node.
if actual == 0 { if actual == 0 {
@ -632,12 +624,12 @@ impl Format<PyFormatContext<'_>> for FormatEmptyLinesAfterLeadingComments<'_> {
// If there are more than enough empty lines already, `leading_comments` will // If there are more than enough empty lines already, `leading_comments` will
// trim them as necessary. // trim them as necessary.
if actual >= self.empty_lines { if actual >= empty_lines {
return Ok(()); return Ok(());
} }
for _ in actual..self.empty_lines { for _ in actual..empty_lines {
write!(f, [empty_line()])?; empty_line().fmt(f)?;
} }
} }
Ok(()) Ok(())

View file

@ -55,7 +55,7 @@ impl FormatNodeRule<StmtClassDef> for FormatStmtClassDef {
// newline between the comment and the node, but we _require_ two newlines. If there are // newline between the comment and the node, but we _require_ two newlines. If there are
// _no_ newlines between the comment and the node, we don't insert _any_ newlines; if there // _no_ newlines between the comment and the node, we don't insert _any_ newlines; if there
// are more than two, then `leading_comments` will preserve the correct number of newlines. // are more than two, then `leading_comments` will preserve the correct number of newlines.
empty_lines_after_leading_comments(f, comments.leading(item)).fmt(f)?; empty_lines_after_leading_comments(comments.leading(item)).fmt(f)?;
write!( write!(
f, f,
@ -152,7 +152,7 @@ impl FormatNodeRule<StmtClassDef> for FormatStmtClassDef {
// //
// # comment // # comment
// ``` // ```
empty_lines_before_trailing_comments(f, comments.trailing(item), NodeKind::StmtClassDef) empty_lines_before_trailing_comments(comments.trailing(item), NodeKind::StmtClassDef)
.fmt(f)?; .fmt(f)?;
Ok(()) Ok(())

View file

@ -52,7 +52,7 @@ impl FormatNodeRule<StmtFunctionDef> for FormatStmtFunctionDef {
// newline between the comment and the node, but we _require_ two newlines. If there are // newline between the comment and the node, but we _require_ two newlines. If there are
// _no_ newlines between the comment and the node, we don't insert _any_ newlines; if there // _no_ newlines between the comment and the node, we don't insert _any_ newlines; if there
// are more than two, then `leading_comments` will preserve the correct number of newlines. // are more than two, then `leading_comments` will preserve the correct number of newlines.
empty_lines_after_leading_comments(f, comments.leading(item)).fmt(f)?; empty_lines_after_leading_comments(comments.leading(item)).fmt(f)?;
write!( write!(
f, f,
@ -86,7 +86,7 @@ impl FormatNodeRule<StmtFunctionDef> for FormatStmtFunctionDef {
// //
// # comment // # comment
// ``` // ```
empty_lines_before_trailing_comments(f, comments.trailing(item), NodeKind::StmtFunctionDef) empty_lines_before_trailing_comments(comments.trailing(item), NodeKind::StmtFunctionDef)
.fmt(f) .fmt(f)
} }
} }

View file

@ -240,7 +240,8 @@ impl FormatRule<Suite, PyFormatContext<'_>> for FormatSuite {
preceding_stub.end(), preceding_stub.end(),
f.context().source(), f.context().source(),
) < 2 ) < 2
}); })
&& !preceding_comments.has_trailing_own_line();
if !is_preceding_stub_function_without_empty_line { if !is_preceding_stub_function_without_empty_line {
match self.kind { match self.kind {

View file

@ -0,0 +1,77 @@
---
source: crates/ruff_python_formatter/tests/fixtures.rs
input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/stub_functions_trailing_comments.py
---
## Input
```python
# Regression tests for https://github.com/astral-sh/ruff/issues/11569
# comment 1
def foo(self) -> None: ...
def bar(self) -> None: ...
# comment 2
# comment 3
def baz(self) -> None:
return None
# comment 4
def foo(self) -> None: ...
# comment 5
def baz(self) -> None:
return None
def foo(self) -> None:
... # comment 5
def baz(self) -> None:
return None
def foo(self) -> None: ...
# comment 5
```
## Output
```python
# Regression tests for https://github.com/astral-sh/ruff/issues/11569
# comment 1
def foo(self) -> None: ...
def bar(self) -> None: ...
# comment 2
# comment 3
def baz(self) -> None:
return None
# comment 4
def foo(self) -> None: ...
# comment 5
def baz(self) -> None:
return None
def foo(self) -> None: ... # comment 5
def baz(self) -> None:
return None
def foo(self) -> None: ...
# comment 5
```