From be2b28dd7dfcf04b7285b2a8777e0d89fd1614b6 Mon Sep 17 00:00:00 2001
From: Mikhail Golubev <mikhail.golubev@jetbrains.com>
Date: Wed, 14 Jun 2017 11:50:07 +0300
Subject: [PATCH] PY-19705 Add blanks lines around methods of a class as
 required by PEP 8

---
 .../jetbrains/python/formatter/PyBlock.java   | 30 +++++---
 .../PythonFormattingModelBuilder.java         | 11 +--
 .../create_tests/create_tst.expected.py       |  1 +
 ...eInDictLiteralWhenNoHangingIndent_after.py |  1 +
 .../formatter/beforeTopLevelClass_after.py    |  1 +
 .../formatter/blankLineAroundClasses_after.py |  1 +
 .../blankLineBetweenMethods_after.py          |  1 +
 .../formatter/blankLinesAroundFirstMethod.py  | 65 ++++++++++++++++
 .../blankLinesAroundFirstMethod_after.py      | 76 +++++++++++++++++++
 .../formatter/commentBetweenClasses_after.py  |  2 +
 ...ankLinesBetweenMethodsAndAtTheEnd_after.py |  1 +
 .../TransformClassicClass_after.py            |  1 +
 python/testData/override/indent_after.py      |  1 +
 .../addMethodReplacePass_after.py             |  1 +
 .../firstMethod_after.py                      |  1 +
 .../extractsuperclass/fieldsNpe.after.py      |  2 +
 .../importMultiFile/dest_module.after.py      |  1 +
 .../instanceNotDeclaredInInit.after.py        |  1 +
 .../dest_module.after.py                      |  1 +
 .../multifile/target.append.py                |  1 +
 .../extractsuperclass/multifile/target.new.py |  1 +
 .../extractsuperclass/properties.after.py     |  1 +
 .../py3ParentHasObject.after.py               |  1 +
 .../extractsuperclass/simple.after.py         |  1 +
 .../extractsuperclass/withSuper.after.py      |  1 +
 .../pullup/fieldMove/Class.after.py           |  1 +
 .../com/jetbrains/python/PyFormatterTest.java |  5 ++
 27 files changed, 194 insertions(+), 17 deletions(-)
 create mode 100644 python/testData/formatter/blankLinesAroundFirstMethod.py
 create mode 100644 python/testData/formatter/blankLinesAroundFirstMethod_after.py

diff --git a/python/src/com/jetbrains/python/formatter/PyBlock.java b/python/src/com/jetbrains/python/formatter/PyBlock.java
index 543e761973e9..8e70a14dfe21 100644
--- a/python/src/com/jetbrains/python/formatter/PyBlock.java
+++ b/python/src/com/jetbrains/python/formatter/PyBlock.java
@@ -719,6 +719,8 @@ public class PyBlock implements ASTBlock {
   @Override
   @Nullable
   public Spacing getSpacing(@Nullable Block child1, @NotNull Block child2) {
+    final CommonCodeStyleSettings settings = myContext.getSettings();
+    final PyCodeStyleSettings pySettings = myContext.getPySettings();
     if (child1 instanceof ASTBlock && child2 instanceof ASTBlock) {
       final ASTNode node1 = ((ASTBlock)child1).getNode();
       ASTNode node2 = ((ASTBlock)child2).getNode();
@@ -727,6 +729,22 @@ public class PyBlock implements ASTBlock {
 
       PsiElement psi2 = node2.getPsi();
 
+      if (psi2 instanceof PyStatementList) {
+        // Quite odd getSpacing() doesn't get called with child1=null for the first statement
+        // in the statement list of a class, yet it does get called for the preceding colon and
+        // the statement list itself. Hence we have to handle blank lines around methods here in
+        // addition to SpacingBuilder.
+        if (myNode.getElementType() == PyElementTypes.CLASS_DECLARATION) {
+          final PyStatement[] statements = ((PyStatementList)psi2).getStatements();
+          if (statements.length > 0 && statements[0] instanceof PyFunction) {
+            return getBlankLinesForOption(settings.BLANK_LINES_AROUND_METHOD);
+          }
+        }
+        if (childType1 == PyTokenTypes.COLON && needLineBreakInStatement()) {
+          return Spacing.createSpacing(0, 0, 1, true, settings.KEEP_BLANK_LINES_IN_CODE);
+        }
+      }
+
       // pycodestyle.py enforces at most 2 blank lines only between comments directly
       // at the top-level of a file, not inside if, try/except, etc.
       if (psi1 instanceof PsiComment && myNode.getPsi() instanceof PsiFile) {
@@ -744,8 +762,6 @@ public class PyBlock implements ASTBlock {
       final IElementType childType2 = psi2.getNode().getElementType();
       //noinspection ConstantConditions
       child2 = getSubBlockByNode(node2);
-      final CommonCodeStyleSettings settings = myContext.getSettings();
-      final PyCodeStyleSettings pySettings = myContext.getPySettings();
 
       if ((childType1 == PyTokenTypes.EQ || childType2 == PyTokenTypes.EQ)) {
         final PyNamedParameter namedParameter = as(myNode.getPsi(), PyNamedParameter.class);
@@ -754,19 +770,13 @@ public class PyBlock implements ASTBlock {
         }
       }
 
-      if (childType1 == PyTokenTypes.COLON && psi2 instanceof PyStatementList) {
-        if (needLineBreakInStatement()) {
-          return Spacing.createSpacing(0, 0, 1, true, settings.KEEP_BLANK_LINES_IN_CODE);
-        }
-      }
-
       if (psi1 instanceof PyImportStatementBase) {
         if (psi2 instanceof PyImportStatementBase) {
           final Boolean leftImportIsGroupStart = psi1.getCopyableUserData(IMPORT_GROUP_BEGIN);
           final Boolean rightImportIsGroupStart = psi2.getCopyableUserData(IMPORT_GROUP_BEGIN);
           // Cleanup user data, it's no longer needed
           psi1.putCopyableUserData(IMPORT_GROUP_BEGIN, null);
-          // Don't remove IMPORT_GROUP_BEGIN from the element psi2 yet, because spacing is constructed pairwise: 
+          // Don't remove IMPORT_GROUP_BEGIN from the element psi2 yet, because spacing is constructed pairwise:
           // it might be needed on the next iteration.
           //psi2.putCopyableUserData(IMPORT_GROUP_BEGIN, null);
           if (rightImportIsGroupStart != null) {
@@ -962,7 +972,7 @@ public class PyBlock implements ASTBlock {
       }
     }
     else if (lastChild != null && PyElementTypes.LIST_LIKE_EXPRESSIONS.contains(lastChild.getElementType())) {
-      // handle pressing enter at the end of a list literal when there's no closing paren or bracket 
+      // handle pressing enter at the end of a list literal when there's no closing paren or bracket
       final ASTNode lastLastChild = lastChild.getLastChildNode();
       if (lastLastChild != null && lastLastChild.getPsi() instanceof PsiErrorElement) {
         // we're at a place like this: [foo, ... bar, <caret>
diff --git a/python/src/com/jetbrains/python/formatter/PythonFormattingModelBuilder.java b/python/src/com/jetbrains/python/formatter/PythonFormattingModelBuilder.java
index 948d99ea42de..14851b412397 100644
--- a/python/src/com/jetbrains/python/formatter/PythonFormattingModelBuilder.java
+++ b/python/src/com/jetbrains/python/formatter/PythonFormattingModelBuilder.java
@@ -79,13 +79,10 @@ public class PythonFormattingModelBuilder implements FormattingModelBuilderEx, C
     return new SpacingBuilder(commonSettings)
       .before(END_OF_LINE_COMMENT).spacing(2, 0, 0, commonSettings.KEEP_LINE_BREAKS, commonSettings.KEEP_BLANK_LINES_IN_CODE)
       .after(END_OF_LINE_COMMENT).spacing(0, 0, 1, commonSettings.KEEP_LINE_BREAKS, commonSettings.KEEP_BLANK_LINES_IN_CODE)
-      .between(CLASS_DECLARATION, STATEMENT_OR_DECLARATION).blankLines(commonSettings.BLANK_LINES_AROUND_CLASS)
-      .between(STATEMENT_OR_DECLARATION, CLASS_DECLARATION).blankLines(commonSettings.BLANK_LINES_AROUND_CLASS)
-      .between(FUNCTION_DECLARATION, STATEMENT_OR_DECLARATION).blankLines(commonSettings.BLANK_LINES_AROUND_METHOD)
-      .between(STATEMENT_OR_DECLARATION, FUNCTION_DECLARATION).blankLines(commonSettings.BLANK_LINES_AROUND_METHOD)
-      .after(FUNCTION_DECLARATION).blankLines(commonSettings.BLANK_LINES_AROUND_METHOD)
-      .after(CLASS_DECLARATION).blankLines(commonSettings.BLANK_LINES_AROUND_CLASS)
-      // Remove excess blank lines between imports (at most one is allowed). 
+      // Top-level definitions are supposed to be handled in PyBlock#getSpacing
+      .around(CLASS_DECLARATION).blankLines(commonSettings.BLANK_LINES_AROUND_CLASS)
+      .around(FUNCTION_DECLARATION).blankLines(commonSettings.BLANK_LINES_AROUND_METHOD)
+      // Remove excess blank lines between imports (at most one is allowed).
       // Note that ImportOptimizer gets rid of them anyway.
       // Empty lines between import groups are handles in PyBlock#getSpacing
       .between(IMPORT_STATEMENTS, IMPORT_STATEMENTS).spacing(0, Integer.MAX_VALUE, 1, false, 1)
diff --git a/python/testData/create_tests/create_tst.expected.py b/python/testData/create_tests/create_tst.expected.py
index b09ba847eef6..172a9a3c6ffb 100644
--- a/python/testData/create_tests/create_tst.expected.py
+++ b/python/testData/create_tests/create_tst.expected.py
@@ -2,6 +2,7 @@ from unittest import TestCase
 
 
 class Spam(TestCase):
+
     def eggs(self):
         self.fail()
 
diff --git a/python/testData/formatter/alignmentOfClosingBraceInDictLiteralWhenNoHangingIndent_after.py b/python/testData/formatter/alignmentOfClosingBraceInDictLiteralWhenNoHangingIndent_after.py
index 044070106f87..5c7cd915fe1d 100644
--- a/python/testData/formatter/alignmentOfClosingBraceInDictLiteralWhenNoHangingIndent_after.py
+++ b/python/testData/formatter/alignmentOfClosingBraceInDictLiteralWhenNoHangingIndent_after.py
@@ -1,4 +1,5 @@
 class Checkpoints(webapp2.RequestHandler):
+
     def get(self):
         self.response.write(json.dumps({"meta": {"code": 400,
                                                  "errorType": "paramError",
diff --git a/python/testData/formatter/beforeTopLevelClass_after.py b/python/testData/formatter/beforeTopLevelClass_after.py
index 6b331a7cd1b5..299757e2e9ba 100644
--- a/python/testData/formatter/beforeTopLevelClass_after.py
+++ b/python/testData/formatter/beforeTopLevelClass_after.py
@@ -2,5 +2,6 @@ from unittest import TestCase
 
 
 class MyTest(TestCase):
+
     def test_pass(self):
         self.assertEqual(1 + 1, 2)
diff --git a/python/testData/formatter/blankLineAroundClasses_after.py b/python/testData/formatter/blankLineAroundClasses_after.py
index d7f3fc0ca04a..b00290ebd5d5 100644
--- a/python/testData/formatter/blankLineAroundClasses_after.py
+++ b/python/testData/formatter/blankLineAroundClasses_after.py
@@ -1,4 +1,5 @@
 class Adjunct:
+
     def apply(self, right, arg):
         pass
 
diff --git a/python/testData/formatter/blankLineBetweenMethods_after.py b/python/testData/formatter/blankLineBetweenMethods_after.py
index 54cae234d6c3..9cfa2e27b90d 100644
--- a/python/testData/formatter/blankLineBetweenMethods_after.py
+++ b/python/testData/formatter/blankLineBetweenMethods_after.py
@@ -1,4 +1,5 @@
 class C:
+
     def foo(self):
         pass
 
diff --git a/python/testData/formatter/blankLinesAroundFirstMethod.py b/python/testData/formatter/blankLinesAroundFirstMethod.py
new file mode 100644
index 000000000000..c3e73409aeb9
--- /dev/null
+++ b/python/testData/formatter/blankLinesAroundFirstMethod.py
@@ -0,0 +1,65 @@
+class C1:
+    # comment 1
+
+    # comment 2
+    def __init__(self):
+        pass
+
+
+class C2:
+    # comment 2
+    def __init__(self):
+        pass
+
+
+class C3:
+    def __init__(self):
+        pass
+
+
+class C4:
+    """Docstring."""
+    # comment 1
+
+    # comment 2
+    def __init__(self):
+        pass
+
+
+class C5:
+    """Docstring."""
+    # comment 2
+    def __init__(self):
+        pass
+
+
+class C6:
+    """Docstring."""
+    def __init__(self):
+        pass
+
+
+class C7:
+    attr = 42
+    # comment 1
+
+    # comment 2
+    def __init__(self):
+        pass
+
+
+class C8:
+    attr = 42
+    # comment 2
+    def __init__(self):
+        pass
+
+
+class C9:
+    attr = 42
+    def __init__(self):
+        pass
+
+class C10:  # comment before statement list
+    def __init__(self):
+        pass
diff --git a/python/testData/formatter/blankLinesAroundFirstMethod_after.py b/python/testData/formatter/blankLinesAroundFirstMethod_after.py
new file mode 100644
index 000000000000..a905f49887e4
--- /dev/null
+++ b/python/testData/formatter/blankLinesAroundFirstMethod_after.py
@@ -0,0 +1,76 @@
+class C1:
+
+    # comment 1
+
+    # comment 2
+    def __init__(self):
+        pass
+
+
+class C2:
+
+    # comment 2
+    def __init__(self):
+        pass
+
+
+class C3:
+
+    def __init__(self):
+        pass
+
+
+class C4:
+    """Docstring."""
+
+    # comment 1
+
+    # comment 2
+    def __init__(self):
+        pass
+
+
+class C5:
+    """Docstring."""
+
+    # comment 2
+    def __init__(self):
+        pass
+
+
+class C6:
+    """Docstring."""
+
+    def __init__(self):
+        pass
+
+
+class C7:
+    attr = 42
+
+    # comment 1
+
+    # comment 2
+    def __init__(self):
+        pass
+
+
+class C8:
+    attr = 42
+
+    # comment 2
+    def __init__(self):
+        pass
+
+
+class C9:
+    attr = 42
+
+    def __init__(self):
+        pass
+
+
+class C10:  # comment before statement list
+
+    def __init__(self):
+        pass
diff --git a/python/testData/formatter/commentBetweenClasses_after.py b/python/testData/formatter/commentBetweenClasses_after.py
index e97c76b04590..9a82642ffadc 100644
--- a/python/testData/formatter/commentBetweenClasses_after.py
+++ b/python/testData/formatter/commentBetweenClasses_after.py
@@ -1,4 +1,5 @@
 class T1(object):
+
     def m1(self):
         pass
 
@@ -6,5 +7,6 @@ class T1(object):
 # comment about T2
 
 class T2(object):
+
     def m2(self):
         pass
diff --git a/python/testData/formatter/extraBlankLinesBetweenMethodsAndAtTheEnd_after.py b/python/testData/formatter/extraBlankLinesBetweenMethodsAndAtTheEnd_after.py
index 959949462bea..f45624cc4a67 100644
--- a/python/testData/formatter/extraBlankLinesBetweenMethodsAndAtTheEnd_after.py
+++ b/python/testData/formatter/extraBlankLinesBetweenMethodsAndAtTheEnd_after.py
@@ -1,4 +1,5 @@
 class Foo(object):
+
     def wrong_blank_lines(self):
         pass
 
diff --git a/python/testData/inspections/TransformClassicClass_after.py b/python/testData/inspections/TransformClassicClass_after.py
index 70402ca7f12c..19a4bb306c8b 100644
--- a/python/testData/inspections/TransformClassicClass_after.py
+++ b/python/testData/inspections/TransformClassicClass_after.py
@@ -1,4 +1,5 @@
 class A(object):
+
     def foo(self):
         pass
 
diff --git a/python/testData/override/indent_after.py b/python/testData/override/indent_after.py
index 807c1d5498f1..f51ffeff5a0a 100644
--- a/python/testData/override/indent_after.py
+++ b/python/testData/override/indent_after.py
@@ -2,5 +2,6 @@ class Dialog:
     def validate(self): pass
 
 class B(Dialog):
+
     def validate(self):
         <selection>Dialog.validate(self)</selection>
\ No newline at end of file
diff --git a/python/testData/quickFixes/PyAddMethodQuickFixTest/addMethodReplacePass_after.py b/python/testData/quickFixes/PyAddMethodQuickFixTest/addMethodReplacePass_after.py
index 3082267d4d02..595eb86ded71 100644
--- a/python/testData/quickFixes/PyAddMethodQuickFixTest/addMethodReplacePass_after.py
+++ b/python/testData/quickFixes/PyAddMethodQuickFixTest/addMethodReplacePass_after.py
@@ -1,4 +1,5 @@
 class A:
+
     def y(self):
         pass
 
diff --git a/python/testData/quickFixes/PyMakeFunctionFromMethodQuickFixTest/firstMethod_after.py b/python/testData/quickFixes/PyMakeFunctionFromMethodQuickFixTest/firstMethod_after.py
index 01298185c05c..fcd2fa9f867e 100644
--- a/python/testData/quickFixes/PyMakeFunctionFromMethodQuickFixTest/firstMethod_after.py
+++ b/python/testData/quickFixes/PyMakeFunctionFromMethodQuickFixTest/firstMethod_after.py
@@ -6,6 +6,7 @@ def f(x):
 
 
 class Child(Base):
+
     def __init__(self):
         super(Child, self).__init__()
 
diff --git a/python/testData/refactoring/extractsuperclass/fieldsNpe.after.py b/python/testData/refactoring/extractsuperclass/fieldsNpe.after.py
index 94167c7fa6bb..9a08604d360b 100644
--- a/python/testData/refactoring/extractsuperclass/fieldsNpe.after.py
+++ b/python/testData/refactoring/extractsuperclass/fieldsNpe.after.py
@@ -1,4 +1,5 @@
 class Ancestor(object):
+
     def __init__(self, a, b):
         self.a = a
         self.b = b
@@ -8,5 +9,6 @@ class Ancestor(object):
 
 
 class Basic(Ancestor):
+
     def func2(self):
         return self.func1()
\ No newline at end of file
diff --git a/python/testData/refactoring/extractsuperclass/importMultiFile/dest_module.after.py b/python/testData/refactoring/extractsuperclass/importMultiFile/dest_module.after.py
index eba5f32d8456..b717c28ecdcf 100644
--- a/python/testData/refactoring/extractsuperclass/importMultiFile/dest_module.after.py
+++ b/python/testData/refactoring/extractsuperclass/importMultiFile/dest_module.after.py
@@ -3,6 +3,7 @@ from shared_module import module_function as my_function, ModuleClass
 
 
 class NewParent(object):
+
     def do_useful_stuff(self):
         i = shared_module.MODULE_CONTANT
         my_function()
diff --git a/python/testData/refactoring/extractsuperclass/instanceNotDeclaredInInit.after.py b/python/testData/refactoring/extractsuperclass/instanceNotDeclaredInInit.after.py
index ef3511dc8c66..25fd7a3d8c68 100644
--- a/python/testData/refactoring/extractsuperclass/instanceNotDeclaredInInit.after.py
+++ b/python/testData/refactoring/extractsuperclass/instanceNotDeclaredInInit.after.py
@@ -1,4 +1,5 @@
 class Parent(object):
+
     def __init__(self):
         self.eggs = 12
 
diff --git a/python/testData/refactoring/extractsuperclass/moveAndMakeAbstractImportExistsPy3/dest_module.after.py b/python/testData/refactoring/extractsuperclass/moveAndMakeAbstractImportExistsPy3/dest_module.after.py
index ddce4a86100f..946e299b39e6 100644
--- a/python/testData/refactoring/extractsuperclass/moveAndMakeAbstractImportExistsPy3/dest_module.after.py
+++ b/python/testData/refactoring/extractsuperclass/moveAndMakeAbstractImportExistsPy3/dest_module.after.py
@@ -7,6 +7,7 @@ abstractmethod()
 
 
 class NewParent(metaclass=ABCMeta):
+
     @classmethod
     @abstractmethod
     def foo_method(cls):
diff --git a/python/testData/refactoring/extractsuperclass/multifile/target.append.py b/python/testData/refactoring/extractsuperclass/multifile/target.append.py
index 81722c98acac..681607f25d50 100644
--- a/python/testData/refactoring/extractsuperclass/multifile/target.append.py
+++ b/python/testData/refactoring/extractsuperclass/multifile/target.append.py
@@ -3,5 +3,6 @@ A = 1
 
 
 class Suppa:
+
     def foo(self):
         print "bar"
diff --git a/python/testData/refactoring/extractsuperclass/multifile/target.new.py b/python/testData/refactoring/extractsuperclass/multifile/target.new.py
index db6d3b7efa91..20c10a93ab7b 100644
--- a/python/testData/refactoring/extractsuperclass/multifile/target.new.py
+++ b/python/testData/refactoring/extractsuperclass/multifile/target.new.py
@@ -1,3 +1,4 @@
 class Suppa:
+
     def foo(self):
         print "bar"
diff --git a/python/testData/refactoring/extractsuperclass/properties.after.py b/python/testData/refactoring/extractsuperclass/properties.after.py
index c414f00ca149..14a336c41f66 100644
--- a/python/testData/refactoring/extractsuperclass/properties.after.py
+++ b/python/testData/refactoring/extractsuperclass/properties.after.py
@@ -20,6 +20,7 @@ class ToClass(object):
 
 
 class FromClass(ToClass):
+
     def __init__(self):
         pass
 
diff --git a/python/testData/refactoring/extractsuperclass/py3ParentHasObject.after.py b/python/testData/refactoring/extractsuperclass/py3ParentHasObject.after.py
index dd03d9a179c9..23b744155397 100644
--- a/python/testData/refactoring/extractsuperclass/py3ParentHasObject.after.py
+++ b/python/testData/refactoring/extractsuperclass/py3ParentHasObject.after.py
@@ -1,4 +1,5 @@
 class Parent(object):
+
     def spam(self):
         pass
 
diff --git a/python/testData/refactoring/extractsuperclass/simple.after.py b/python/testData/refactoring/extractsuperclass/simple.after.py
index 38248501dc4a..86b15163bcae 100644
--- a/python/testData/refactoring/extractsuperclass/simple.after.py
+++ b/python/testData/refactoring/extractsuperclass/simple.after.py
@@ -1,4 +1,5 @@
 class Suppa:
+
     def foo(self):
         print "bar"
 
diff --git a/python/testData/refactoring/extractsuperclass/withSuper.after.py b/python/testData/refactoring/extractsuperclass/withSuper.after.py
index 5f346b590cad..5831a7ab776a 100644
--- a/python/testData/refactoring/extractsuperclass/withSuper.after.py
+++ b/python/testData/refactoring/extractsuperclass/withSuper.after.py
@@ -1,4 +1,5 @@
 class Suppa(object):
+
     def foo(self):
         print "bar"
 
diff --git a/python/testData/refactoring/pullup/fieldMove/Class.after.py b/python/testData/refactoring/pullup/fieldMove/Class.after.py
index da34ab3a2897..e717954de1e0 100644
--- a/python/testData/refactoring/pullup/fieldMove/Class.after.py
+++ b/python/testData/refactoring/pullup/fieldMove/Class.after.py
@@ -2,5 +2,6 @@ from SuperClass import SuperClass
 
 
 class AnyClass(SuperClass):
+
     def __init__(self):
         super().__init__()
diff --git a/python/testSrc/com/jetbrains/python/PyFormatterTest.java b/python/testSrc/com/jetbrains/python/PyFormatterTest.java
index 7c793e3d1f5a..8ad3b04247fe 100644
--- a/python/testSrc/com/jetbrains/python/PyFormatterTest.java
+++ b/python/testSrc/com/jetbrains/python/PyFormatterTest.java
@@ -835,6 +835,11 @@ public class PyFormatterTest extends PyTestCase {
     doTest();
   }
 
+  // PY-19705
+  public void testBlankLinesAroundFirstMethod() {
+    doTest();
+  }
+
   public void testVariableAnnotations() {
     runWithLanguageLevel(LanguageLevel.PYTHON36, this::doTest);
   }
-- 
GitLab