Skip to content

Commit

Permalink
Insert necessary blank line between class and leading comments (#8224)
Browse files Browse the repository at this point in the history
## Summary

Given:

```python
# comment

class A:
    def foo(self):
        pass
```

We need to insert an additional newline between `# comment` and `class
A`. We were missing this handling for the case in which `# comment` is a
leading comment on `class A`, as opposed to a trailing comment of some
preceding statement.

In practice, I think this only applies to the specific case in which a
class or function is the first statement in a module, and there's a
single empty line between a leading comment and that class or function.
If there are no empty lines, then the comment "sticks" to the
definition; if there are two or more, then `leading_comments` will
truncate appropriately. If the class or function is nested, then we only
need one empty line anyway.

Closes #8215.

## Test Plan

No change in similarity.

Before:

| project | similarity index | total files | changed files |

|----------------|------------------:|------------------:|------------------:|
| cpython | 0.75803 | 1799 | 1647 |
| django | 0.99983 | 2772 | 34 |
| home-assistant | 0.99953 | 10596 | 186 |
| poetry | 0.99891 | 317 | 17 |
| transformers | 0.99966 | 2657 | 330 |
| twine | 1.00000 | 33 | 0 |
| typeshed | 0.99978 | 3669 | 20 |
| warehouse | 0.99977 | 654 | 13 |
| zulip | 0.99970 | 1459 | 22 |

After:

| project | similarity index | total files | changed files |

|----------------|------------------:|------------------:|------------------:|
| cpython | 0.75803 | 1799 | 1648 |
| django | 0.99983 | 2772 | 34 |
| home-assistant | 0.99953 | 10596 | 186 |
| poetry | 0.99891 | 317 | 17 |
| transformers | 0.99966 | 2657 | 330 |
| twine | 1.00000 | 33 | 0 |
| typeshed | 0.99978 | 3669 | 20 |
| warehouse | 0.99977 | 654 | 13 |
| zulip | 0.99970 | 1459 | 22 |
  • Loading branch information
charliermarsh authored Oct 26, 2023
1 parent ff9fb0d commit 3c3d9ab
Show file tree
Hide file tree
Showing 8 changed files with 176 additions and 137 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# comment

class Test(
Aaaaaaaaaaaaaaaaa,
Bbbbbbbbbbbbbbbb,
Expand Down
69 changes: 67 additions & 2 deletions crates/ruff_python_formatter/src/comments/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,13 +507,12 @@ fn strip_comment_prefix(comment_text: &str) -> FormatResult<&str> {
///
/// For example, given:
/// ```python
/// def func():
/// class Class:
/// ...
/// # comment
/// ```
///
/// This builder will insert two empty lines before the comment.
/// ```
pub(crate) fn empty_lines_before_trailing_comments<'a>(
f: &PyFormatter,
comments: &'a [SourceComment],
Expand Down Expand Up @@ -555,3 +554,69 @@ impl Format<PyFormatContext<'_>> for FormatEmptyLinesBeforeTrailingComments<'_>
Ok(())
}
}

/// Format the empty lines between a node and its leading comments.
///
/// For example, given:
/// ```python
/// # comment
///
/// class Class:
/// ...
/// ```
///
/// While `leading_comments` will preserve the existing empty line, this builder will insert an
/// additional empty line before the comment.
pub(crate) fn empty_lines_after_leading_comments<'a>(
f: &PyFormatter,
comments: &'a [SourceComment],
) -> FormatEmptyLinesAfterLeadingComments<'a> {
// 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)]
pub(crate) struct FormatEmptyLinesAfterLeadingComments<'a> {
/// The leading comments of the node.
comments: &'a [SourceComment],
/// The expected number of empty lines after the leading comments.
empty_lines: u32,
}

impl Format<PyFormatContext<'_>> for FormatEmptyLinesAfterLeadingComments<'_> {
fn fmt(&self, f: &mut Formatter<PyFormatContext>) -> FormatResult<()> {
if let Some(comment) = self
.comments
.iter()
.rev()
.find(|comment| comment.line_position().is_own_line())
{
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 actual == 0 {
return Ok(());
}

// If there are more than enough empty lines already, `leading_comments` will
// trim them as necessary.
if actual >= self.empty_lines {
return Ok(());
}

for _ in actual..self.empty_lines {
write!(f, [empty_line()])?;
}
}
Ok(())
}
}
27 changes: 26 additions & 1 deletion crates/ruff_python_formatter/src/statement/stmt_class_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use ruff_python_ast::{Decorator, StmtClassDef};
use ruff_python_trivia::lines_after_ignoring_end_of_line_trivia;
use ruff_text_size::Ranged;

use crate::comments::format::empty_lines_before_trailing_comments;
use crate::comments::format::{
empty_lines_after_leading_comments, empty_lines_before_trailing_comments,
};
use crate::comments::{leading_comments, trailing_comments, SourceComment};
use crate::prelude::*;
use crate::statement::clause::{clause_body, clause_header, ClauseHeader};
Expand Down Expand Up @@ -32,6 +34,29 @@ impl FormatNodeRule<StmtClassDef> for FormatStmtClassDef {
let (leading_definition_comments, trailing_definition_comments) =
dangling_comments.split_at(trailing_definition_comments_start);

// If the class contains leading comments, insert newlines before them.
// For example, given:
// ```python
// # comment
//
// class Class:
// ...
// ```
//
// At the top-level in a non-stub file, reformat as:
// ```python
// # comment
//
//
// class Class:
// ...
// ```
// Note that this is only really relevant for the specific case in which there's a single
// 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
// 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)?;

write!(
f,
[
Expand Down
27 changes: 26 additions & 1 deletion crates/ruff_python_formatter/src/statement/stmt_function_def.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use ruff_formatter::write;
use ruff_python_ast::StmtFunctionDef;

use crate::comments::format::empty_lines_before_trailing_comments;
use crate::comments::format::{
empty_lines_after_leading_comments, empty_lines_before_trailing_comments,
};
use crate::comments::SourceComment;
use crate::expression::maybe_parenthesize_expression;
use crate::expression::parentheses::{Parentheses, Parenthesize};
Expand Down Expand Up @@ -30,6 +32,29 @@ impl FormatNodeRule<StmtFunctionDef> for FormatStmtFunctionDef {
let (leading_definition_comments, trailing_definition_comments) =
dangling_comments.split_at(trailing_definition_comments_start);

// If the class contains leading comments, insert newlines before them.
// For example, given:
// ```python
// # comment
//
// def func():
// ...
// ```
//
// At the top-level in a non-stub file, reformat as:
// ```python
// # comment
//
//
// def func():
// ...
// ```
// Note that this is only really relevant for the specific case in which there's a single
// 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
// 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)?;

write!(
f,
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def f():
```diff
--- Black
+++ Ruff
@@ -1,29 +1,205 @@
@@ -1,29 +1,206 @@
+# This file doesn't use the standard decomposition.
+# Decorator syntax test cases are separated by double # comments.
+# Those before the 'output' comment are valid under the old syntax.
Expand All @@ -172,6 +172,7 @@ def f():
+
+##
+
+
+@decorator
+def f():
+ ...
Expand Down Expand Up @@ -209,99 +210,99 @@ def f():
+ ...
+
+
+##
+
##
-@decorator()()
+
+@decorator(**kwargs)
+def f():
+ ...
+
+
+##
def f():
...
+
##
-@(decorator)
+
+@decorator(*args, **kwargs)
+def f():
+ ...
+
+
+##
def f():
...
+
##
-@sequence["decorator"]
+
+@decorator(
+ *args,
+ **kwargs,
+)
def f():
...
+
##
-@decorator[List[str]]
+
+@dotted.decorator
def f():
...
+
##
-@var := decorator
+
+@dotted.decorator(arg)
+def f():
+ ...
+
+
+##
+
+
+@dotted.decorator
+@dotted.decorator(kwarg=0)
+def f():
+ ...
+
+
+##
+
+
+@dotted.decorator(arg)
+@dotted.decorator(*args)
+def f():
+ ...
+
+
+##
+
+
+@dotted.decorator(kwarg=0)
+@dotted.decorator(**kwargs)
+def f():
+ ...
+
+
##
-@decorator()()
+##
+
+@dotted.decorator(*args)
def f():
...
+
##
-@(decorator)
+@dotted.decorator(*args, **kwargs)
+def f():
+ ...
+
+@dotted.decorator(**kwargs)
def f():
...
+
##
-@sequence["decorator"]
+
+@dotted.decorator(*args, **kwargs)
def f():
...
+##
+
##
-@decorator[List[str]]
+
+@dotted.decorator(
+ *args,
+ **kwargs,
+)
def f():
...
+def f():
+ ...
+
+
+##
+
##
-@var := decorator
+
+@double.dotted.decorator
+def f():
Expand Down Expand Up @@ -387,6 +388,7 @@ def f():
##
@decorator
def f():
...
Expand Down
Loading

0 comments on commit 3c3d9ab

Please sign in to comment.