From a82eb9544ca0eb8aad6e06d3e0d37e9336c19c3a Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 1 Aug 2023 13:33:01 -0400 Subject: [PATCH] Implement Black's rules around newlines before and after class docstrings (#6209) ## Summary Black allows up to one blank line _before_ a class docstring, and enforces one blank line _after_ a class docstring. This PR implements that handling. The cases in `crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/class_definition.py` match Black identically. --- .../ruff/statement/class_definition.py | 49 ++ .../src/statement/suite.rs | 90 +++- ...mple_cases__class_methods_new_line.py.snap | 485 ------------------ ...format@statement__class_definition.py.snap | 103 ++++ 4 files changed, 224 insertions(+), 503 deletions(-) delete mode 100644 crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__class_methods_new_line.py.snap diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/class_definition.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/class_definition.py index 7481a071f5..2a66123e13 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/class_definition.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/class_definition.py @@ -43,3 +43,52 @@ class TestTrailingComment2: # trailing comment pass +class Test: + """Docstring""" + + +class Test: + # comment + """Docstring""" + + +class Test: + """Docstring""" + x = 1 + + +class Test: + """Docstring""" + # comment + x = 1 + + +class Test: + + """Docstring""" + + +class Test: + # comment + + """Docstring""" + + +class Test: + + # comment + + """Docstring""" + + +class Test: + + """Docstring""" + x = 1 + + +class Test: + + """Docstring""" + # comment + x = 1 diff --git a/crates/ruff_python_formatter/src/statement/suite.rs b/crates/ruff_python_formatter/src/statement/suite.rs index b021d8c690..eec2e957d6 100644 --- a/crates/ruff_python_formatter/src/statement/suite.rs +++ b/crates/ruff_python_formatter/src/statement/suite.rs @@ -1,6 +1,6 @@ use ruff_formatter::{write, FormatOwnedWithRule, FormatRefWithRule, FormatRuleWithOptions}; use ruff_python_ast::helpers::is_compound_statement; -use ruff_python_ast::{Ranged, Stmt, Suite}; +use ruff_python_ast::{self as ast, Constant, Expr, Ranged, Stmt, Suite}; use ruff_python_trivia::{lines_after, lines_before, skip_trailing_trivia}; use crate::context::{NodeLevel, WithNodeLevel}; @@ -54,24 +54,64 @@ impl FormatRule> for FormatSuite { let mut f = WithNodeLevel::new(node_level, f); - if matches!(self.kind, SuiteKind::Other) - && is_class_or_function_definition(first) - && !comments.has_leading_comments(first) - { - // 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 - // if True: - // - // def test(): - // ... - // ``` - write!(f, [empty_line()])?; - } - - write!(f, [first.format()])?; - + // Format the first statement in the body, which often has special formatting rules. let mut last = first; + match self.kind { + SuiteKind::Other => { + if is_class_or_function_definition(first) && !comments.has_leading_comments(first) { + // 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 + // if True: + // + // def test(): + // ... + // ``` + write!(f, [empty_line()])?; + } + write!(f, [first.format()])?; + } + SuiteKind::Class if is_docstring(first) => { + if !comments.has_leading_comments(first) && lines_before(first.start(), source) > 1 + { + // Allow up to one empty line before a class docstring, e.g., this is + // stable formatting: + // ```python + // class Test: + // + // """Docstring""" + // ``` + write!(f, [empty_line()])?; + } + write!(f, [first.format()])?; + + // Enforce an empty line after a class docstring, e.g., these are both stable + // formatting: + // ```python + // class Test: + // """Docstring""" + // + // ... + // + // + // class Test: + // + // """Docstring""" + // + // ... + // ``` + if let Some(second) = iter.next() { + // Format the subsequent statement immediately. This rule takes precedence + // over the rules in the loop below (and most of them won't apply anyway, + // e.g., we know the first statement isn't an import). + write!(f, [empty_line(), second.format()])?; + last = second; + } + } + _ => { + write!(f, [first.format()])?; + } + } for statement in iter { if is_class_or_function_definition(last) || is_class_or_function_definition(statement) { @@ -182,6 +222,20 @@ const fn is_import_definition(stmt: &Stmt) -> bool { matches!(stmt, Stmt::Import(_) | Stmt::ImportFrom(_)) } +fn is_docstring(stmt: &Stmt) -> bool { + let Stmt::Expr(ast::StmtExpr { value, .. }) = stmt else { + return false; + }; + + matches!( + value.as_ref(), + Expr::Constant(ast::ExprConstant { + value: Constant::Str(..), + .. + }) + ) +} + impl FormatRuleWithOptions> for FormatSuite { type Options = SuiteKind; diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__class_methods_new_line.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__class_methods_new_line.py.snap deleted file mode 100644 index 436b6923ff..0000000000 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__class_methods_new_line.py.snap +++ /dev/null @@ -1,485 +0,0 @@ ---- -source: crates/ruff_python_formatter/tests/fixtures.rs -input_file: crates/ruff_python_formatter/resources/test/fixtures/black/simple_cases/class_methods_new_line.py ---- -## Input - -```py -class ClassSimplest: - pass -class ClassWithSingleField: - a = 1 -class ClassWithJustTheDocstring: - """Just a docstring.""" -class ClassWithInit: - def __init__(self): - pass -class ClassWithTheDocstringAndInit: - """Just a docstring.""" - def __init__(self): - pass -class ClassWithInitAndVars: - cls_var = 100 - def __init__(self): - pass -class ClassWithInitAndVarsAndDocstring: - """Test class""" - cls_var = 100 - def __init__(self): - pass -class ClassWithDecoInit: - @deco - def __init__(self): - pass -class ClassWithDecoInitAndVars: - cls_var = 100 - @deco - def __init__(self): - pass -class ClassWithDecoInitAndVarsAndDocstring: - """Test class""" - cls_var = 100 - @deco - def __init__(self): - pass -class ClassSimplestWithInner: - class Inner: - pass -class ClassSimplestWithInnerWithDocstring: - class Inner: - """Just a docstring.""" - def __init__(self): - pass -class ClassWithSingleFieldWithInner: - a = 1 - class Inner: - pass -class ClassWithJustTheDocstringWithInner: - """Just a docstring.""" - class Inner: - pass -class ClassWithInitWithInner: - class Inner: - pass - def __init__(self): - pass -class ClassWithInitAndVarsWithInner: - cls_var = 100 - class Inner: - pass - def __init__(self): - pass -class ClassWithInitAndVarsAndDocstringWithInner: - """Test class""" - cls_var = 100 - class Inner: - pass - def __init__(self): - pass -class ClassWithDecoInitWithInner: - class Inner: - pass - @deco - def __init__(self): - pass -class ClassWithDecoInitAndVarsWithInner: - cls_var = 100 - class Inner: - pass - @deco - def __init__(self): - pass -class ClassWithDecoInitAndVarsAndDocstringWithInner: - """Test class""" - cls_var = 100 - class Inner: - pass - @deco - def __init__(self): - pass -class ClassWithDecoInitAndVarsAndDocstringWithInner2: - """Test class""" - class Inner: - pass - cls_var = 100 - @deco - def __init__(self): - pass -``` - -## Black Differences - -```diff ---- Black -+++ Ruff -@@ -31,7 +31,6 @@ - - class ClassWithInitAndVarsAndDocstring: - """Test class""" -- - cls_var = 100 - - def __init__(self): -@@ -54,7 +53,6 @@ - - class ClassWithDecoInitAndVarsAndDocstring: - """Test class""" -- - cls_var = 100 - - @deco -@@ -109,7 +107,6 @@ - - class ClassWithInitAndVarsAndDocstringWithInner: - """Test class""" -- - cls_var = 100 - - class Inner: -@@ -141,7 +138,6 @@ - - class ClassWithDecoInitAndVarsAndDocstringWithInner: - """Test class""" -- - cls_var = 100 - - class Inner: -``` - -## Ruff Output - -```py -class ClassSimplest: - pass - - -class ClassWithSingleField: - a = 1 - - -class ClassWithJustTheDocstring: - """Just a docstring.""" - - -class ClassWithInit: - def __init__(self): - pass - - -class ClassWithTheDocstringAndInit: - """Just a docstring.""" - - def __init__(self): - pass - - -class ClassWithInitAndVars: - cls_var = 100 - - def __init__(self): - pass - - -class ClassWithInitAndVarsAndDocstring: - """Test class""" - cls_var = 100 - - def __init__(self): - pass - - -class ClassWithDecoInit: - @deco - def __init__(self): - pass - - -class ClassWithDecoInitAndVars: - cls_var = 100 - - @deco - def __init__(self): - pass - - -class ClassWithDecoInitAndVarsAndDocstring: - """Test class""" - cls_var = 100 - - @deco - def __init__(self): - pass - - -class ClassSimplestWithInner: - class Inner: - pass - - -class ClassSimplestWithInnerWithDocstring: - class Inner: - """Just a docstring.""" - - def __init__(self): - pass - - -class ClassWithSingleFieldWithInner: - a = 1 - - class Inner: - pass - - -class ClassWithJustTheDocstringWithInner: - """Just a docstring.""" - - class Inner: - pass - - -class ClassWithInitWithInner: - class Inner: - pass - - def __init__(self): - pass - - -class ClassWithInitAndVarsWithInner: - cls_var = 100 - - class Inner: - pass - - def __init__(self): - pass - - -class ClassWithInitAndVarsAndDocstringWithInner: - """Test class""" - cls_var = 100 - - class Inner: - pass - - def __init__(self): - pass - - -class ClassWithDecoInitWithInner: - class Inner: - pass - - @deco - def __init__(self): - pass - - -class ClassWithDecoInitAndVarsWithInner: - cls_var = 100 - - class Inner: - pass - - @deco - def __init__(self): - pass - - -class ClassWithDecoInitAndVarsAndDocstringWithInner: - """Test class""" - cls_var = 100 - - class Inner: - pass - - @deco - def __init__(self): - pass - - -class ClassWithDecoInitAndVarsAndDocstringWithInner2: - """Test class""" - - class Inner: - pass - - cls_var = 100 - - @deco - def __init__(self): - pass -``` - -## Black Output - -```py -class ClassSimplest: - pass - - -class ClassWithSingleField: - a = 1 - - -class ClassWithJustTheDocstring: - """Just a docstring.""" - - -class ClassWithInit: - def __init__(self): - pass - - -class ClassWithTheDocstringAndInit: - """Just a docstring.""" - - def __init__(self): - pass - - -class ClassWithInitAndVars: - cls_var = 100 - - def __init__(self): - pass - - -class ClassWithInitAndVarsAndDocstring: - """Test class""" - - cls_var = 100 - - def __init__(self): - pass - - -class ClassWithDecoInit: - @deco - def __init__(self): - pass - - -class ClassWithDecoInitAndVars: - cls_var = 100 - - @deco - def __init__(self): - pass - - -class ClassWithDecoInitAndVarsAndDocstring: - """Test class""" - - cls_var = 100 - - @deco - def __init__(self): - pass - - -class ClassSimplestWithInner: - class Inner: - pass - - -class ClassSimplestWithInnerWithDocstring: - class Inner: - """Just a docstring.""" - - def __init__(self): - pass - - -class ClassWithSingleFieldWithInner: - a = 1 - - class Inner: - pass - - -class ClassWithJustTheDocstringWithInner: - """Just a docstring.""" - - class Inner: - pass - - -class ClassWithInitWithInner: - class Inner: - pass - - def __init__(self): - pass - - -class ClassWithInitAndVarsWithInner: - cls_var = 100 - - class Inner: - pass - - def __init__(self): - pass - - -class ClassWithInitAndVarsAndDocstringWithInner: - """Test class""" - - cls_var = 100 - - class Inner: - pass - - def __init__(self): - pass - - -class ClassWithDecoInitWithInner: - class Inner: - pass - - @deco - def __init__(self): - pass - - -class ClassWithDecoInitAndVarsWithInner: - cls_var = 100 - - class Inner: - pass - - @deco - def __init__(self): - pass - - -class ClassWithDecoInitAndVarsAndDocstringWithInner: - """Test class""" - - cls_var = 100 - - class Inner: - pass - - @deco - def __init__(self): - pass - - -class ClassWithDecoInitAndVarsAndDocstringWithInner2: - """Test class""" - - class Inner: - pass - - cls_var = 100 - - @deco - def __init__(self): - pass -``` - - diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__class_definition.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__class_definition.py.snap index 894f2b96e8..8dadf4f8c2 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__class_definition.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__class_definition.py.snap @@ -49,6 +49,55 @@ class TestTrailingComment2: # trailing comment pass +class Test: + """Docstring""" + + +class Test: + # comment + """Docstring""" + + +class Test: + """Docstring""" + x = 1 + + +class Test: + """Docstring""" + # comment + x = 1 + + +class Test: + + """Docstring""" + + +class Test: + # comment + + """Docstring""" + + +class Test: + + # comment + + """Docstring""" + + +class Test: + + """Docstring""" + x = 1 + + +class Test: + + """Docstring""" + # comment + x = 1 ``` ## Output @@ -114,6 +163,60 @@ class TestTrailingComment1(Aaaa): # trailing comment class TestTrailingComment2: # trailing comment pass + + +class Test: + """Docstring""" + + +class Test: + # comment + """Docstring""" + + +class Test: + """Docstring""" + + x = 1 + + +class Test: + """Docstring""" + + # comment + x = 1 + + +class Test: + + """Docstring""" + + +class Test: + # comment + + """Docstring""" + + +class Test: + # comment + + """Docstring""" + + +class Test: + + """Docstring""" + + x = 1 + + +class Test: + + """Docstring""" + + # comment + x = 1 ```