Format empty lines in stub files like black's preview style (#7206)

## Summary

Fix all but one empty line differences with the black preview style in
typeshed. The remaining differences are breaking with type comments and
trailing commas in function definitions.

I compared the empty line differences with the preview mode of black
since stable has some oddities that would have been hard to replicate
(https://github.com/psf/black/issues/3861). Additionally, it assumes the
style proposed in https://github.com/psf/black/issues/3862.

An edge case that also surfaced with typeshed are newline before
trailing module comments.

**main**

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

|--------------|------------------:|------------------:|------------------:|
| cpython | 0.76083 | 1789 | 1632 |
| django | 0.99966 | 2760 | 58 |
| transformers | 0.99930 | 2587 | 447 |
| twine | 1.00000 | 33 | 0 |
| **typeshed** | 0.99978 | 3496 | **2173** |
| warehouse | 0.99825 | 648 | 22 |
| zulip | 0.99950 | 1437 | 27 |

**PR**
| project | similarity index | total files | changed files |

|--------------|------------------:|------------------:|------------------:|
| cpython | 0.76083 | 1789 | 1632 |
| django | 0.99966 | 2760 | 58 |
| transformers | 0.99930 | 2587 | 447 |
| twine | 1.00000 | 33 | 0 |
| **typeshed** | 0.99983 | 3496 | **18** |
| warehouse | 0.99825 | 648 | 22 |
| zulip | 0.99950 | 1437 | 27 |


Closes #6723

## Test Plan

The main driver was the typeshed diff. I added new test cases for all
kinds of possible empty line combinations in stub files, test cases for
newlines before trailing module comments.

---------

Co-authored-by: Micha Reiser <micha@reiser.io>
This commit is contained in:
konsti 2023-09-11 10:03:59 +02:00 committed by GitHub
parent 7440e54ec6
commit 3a2c3a7398
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 858 additions and 77 deletions

View file

@ -5,7 +5,6 @@ use ruff_text_size::Ranged;
use crate::comments::format::empty_lines_before_trailing_comments;
use crate::comments::{leading_comments, trailing_comments, SourceComment};
use crate::context::NodeLevel;
use crate::prelude::*;
use crate::statement::clause::{clause_body, clause_header, ClauseHeader};
use crate::statement::suite::SuiteKind;
@ -120,7 +119,7 @@ impl FormatNodeRule<StmtClassDef> for FormatStmtClassDef {
// # comment
// ```
//
// At the top-level, reformat as:
// At the top-level in a non-stub file, reformat as:
// ```python
// class Class:
// ...
@ -128,15 +127,7 @@ impl FormatNodeRule<StmtClassDef> for FormatStmtClassDef {
//
// # comment
// ```
empty_lines_before_trailing_comments(
comments.trailing(item),
if f.context().node_level() == NodeLevel::TopLevel {
2
} else {
1
},
)
.fmt(f)
empty_lines_before_trailing_comments(f, comments.trailing(item)).fmt(f)
}
fn fmt_dangling_comments(

View file

@ -5,7 +5,6 @@ use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::Ranged;
use crate::comments::SourceComment;
use crate::context::NodeLevel;
use crate::expression::maybe_parenthesize_expression;
use crate::expression::parentheses::{Parentheses, Parenthesize};
use crate::prelude::*;
@ -156,7 +155,7 @@ impl FormatNodeRule<StmtFunctionDef> for FormatStmtFunctionDef {
// # comment
// ```
//
// At the top-level, reformat as:
// At the top-level in a non-stub file, reformat as:
// ```python
// def func():
// ...
@ -164,15 +163,7 @@ impl FormatNodeRule<StmtFunctionDef> for FormatStmtFunctionDef {
//
// # comment
// ```
empty_lines_before_trailing_comments(
comments.trailing(item),
if f.context().node_level() == NodeLevel::TopLevel {
2
} else {
1
},
)
.fmt(f)
empty_lines_before_trailing_comments(f, comments.trailing(item)).fmt(f)
}
fn fmt_dangling_comments(

View file

@ -5,7 +5,9 @@ use ruff_python_ast::{self as ast, Constant, Expr, ExprConstant, Stmt, Suite};
use ruff_python_trivia::{lines_after, lines_after_ignoring_trivia, lines_before};
use ruff_text_size::{Ranged, TextRange};
use crate::comments::{leading_comments, trailing_comments, Comments};
use crate::comments::{
leading_comments, trailing_comments, Comments, LeadingDanglingTrailingComments,
};
use crate::context::{NodeLevel, WithNodeLevel};
use crate::expression::expr_constant::ExprConstantLayout;
use crate::expression::string::StringLayout;
@ -69,7 +71,10 @@ impl FormatRule<Suite, PyFormatContext<'_>> for FormatSuite {
// Format the first statement in the body, which often has special formatting rules.
let first = match self.kind {
SuiteKind::Other => {
if is_class_or_function_definition(first) && !comments.has_leading(first) {
if is_class_or_function_definition(first)
&& !comments.has_leading(first)
&& !source_type.is_stub()
{
// Add an empty line for any nested functions or classes defined within
// non-function or class compound statements, e.g., this is stable formatting:
// ```python
@ -94,7 +99,10 @@ impl FormatRule<Suite, PyFormatContext<'_>> for FormatSuite {
SuiteKind::Class => {
if let Some(docstring) = DocstringStmt::try_from_statement(first) {
if !comments.has_leading(first) && lines_before(first.start(), source) > 1 {
if !comments.has_leading(first)
&& lines_before(first.start(), source) > 1
&& !source_type.is_stub()
{
// Allow up to one empty line before a class docstring, e.g., this is
// stable formatting:
// ```python
@ -154,49 +162,23 @@ impl FormatRule<Suite, PyFormatContext<'_>> for FormatSuite {
&& !preceding_comments.has_trailing_own_line())
|| is_class_or_function_definition(following)
{
match self.kind {
SuiteKind::TopLevel if source_type.is_stub() => {
// Preserve the empty line if the definitions are separated by a comment
if preceding_comments.has_trailing() || following_comments.has_leading() {
empty_line().fmt(f)?;
} else {
// Two subsequent classes that both have an ellipsis only body
// ```python
// class A: ...
// class B: ...
// ```
let class_sequences_with_ellipsis_only =
preceding.as_class_def_stmt().is_some_and(|class| {
contains_only_an_ellipsis(&class.body, f.context().comments())
}) && following.as_class_def_stmt().is_some_and(|class| {
contains_only_an_ellipsis(&class.body, f.context().comments())
});
// Two subsequent functions where the preceding has an ellipsis only body
// ```python
// def test(): ...
// def b(): a
// ```
let function_with_ellipsis =
preceding.as_function_def_stmt().is_some_and(|function| {
contains_only_an_ellipsis(
&function.body,
f.context().comments(),
)
}) && following.is_function_def_stmt();
// Don't add an empty line between two classes that have an `...` body only or after
// a function with an `...` body. Otherwise add an empty line.
if !class_sequences_with_ellipsis_only && !function_with_ellipsis {
empty_line().fmt(f)?;
}
if source_type.is_stub() {
stub_file_empty_lines(
self.kind,
preceding,
following,
&preceding_comments,
&following_comments,
f,
)?;
} else {
match self.kind {
SuiteKind::TopLevel => {
write!(f, [empty_line(), empty_line()])?;
}
SuiteKind::Function | SuiteKind::Class | SuiteKind::Other => {
empty_line().fmt(f)?;
}
}
SuiteKind::TopLevel => {
write!(f, [empty_line(), empty_line()])?;
}
SuiteKind::Function | SuiteKind::Class | SuiteKind::Other => {
empty_line().fmt(f)?;
}
}
} else if is_import_definition(preceding)
@ -345,6 +327,89 @@ impl FormatRule<Suite, PyFormatContext<'_>> for FormatSuite {
}
}
/// Stub files have bespoke rules for empty lines.
///
/// These rules are ported from black (preview mode at time of writing) using the stubs test case:
/// <https://github.com/psf/black/blob/c160e4b7ce30c661ac4f2dfa5038becf1b8c5c33/src/black/lines.py#L576-L744>
fn stub_file_empty_lines(
kind: SuiteKind,
preceding: &Stmt,
following: &Stmt,
preceding_comments: &LeadingDanglingTrailingComments,
following_comments: &LeadingDanglingTrailingComments,
f: &mut PyFormatter,
) -> FormatResult<()> {
let source = f.context().source();
// Preserve the empty line if the definitions are separated by a comment
let empty_line_condition = preceding_comments.has_trailing()
|| following_comments.has_leading()
|| !stub_suite_can_omit_empty_line(preceding, following, f);
match kind {
SuiteKind::TopLevel => {
if empty_line_condition {
empty_line().fmt(f)
} else {
hard_line_break().fmt(f)
}
}
SuiteKind::Class | SuiteKind::Other | SuiteKind::Function => {
if empty_line_condition && lines_after_ignoring_trivia(preceding.end(), source) > 1 {
empty_line().fmt(f)
} else {
hard_line_break().fmt(f)
}
}
}
}
/// Only a function to compute it lazily
fn stub_suite_can_omit_empty_line(preceding: &Stmt, following: &Stmt, f: &PyFormatter) -> bool {
// Two subsequent class definitions that both have an ellipsis only body
// ```python
// class A: ...
// class B: ...
//
// @decorator
// class C: ...
// ```
let class_sequences_with_ellipsis_only = preceding
.as_class_def_stmt()
.is_some_and(|class| contains_only_an_ellipsis(&class.body, f.context().comments()))
&& following.as_class_def_stmt().is_some_and(|class| {
contains_only_an_ellipsis(&class.body, f.context().comments())
&& class.decorator_list.is_empty()
});
// Black for some reasons accepts decorators in place of empty lines
// ```python
// def _count1(): ...
// @final
// class LockType1: ...
//
// def _count2(): ...
//
// class LockType2: ...
// ```
let class_decorator_instead_of_empty_line = preceding.is_function_def_stmt()
&& following
.as_class_def_stmt()
.is_some_and(|class| !class.decorator_list.is_empty());
// A function definition following a stub function definition
// ```python
// def test(): ...
// def b(): a
// ```
let function_with_ellipsis = preceding
.as_function_def_stmt()
.is_some_and(|function| contains_only_an_ellipsis(&function.body, f.context().comments()))
&& following.is_function_def_stmt();
class_sequences_with_ellipsis_only
|| class_decorator_instead_of_empty_line
|| function_with_ellipsis
}
/// Returns `true` if a function or class body contains only an ellipsis with no comments.
pub(crate) fn contains_only_an_ellipsis(body: &[Stmt], comments: &Comments) -> bool {
match body {