Repository: psf/black
Pull request: Treat functions/classes in blocks as if they're nested
### Description

One curveball is that we still want two preceding newlines before blocks
that are probably logically disconnected. In other words:

    if condition:

        def foo():
            return "hi"
                             # <- aside: this is the goal of this commit
    else:

        def foo():
            return "cya"
                             # <- the two newlines spacing here should stay
                             #    since this probably isn't related
    with open("db.json", encoding="utf-8") as f:
        data = f.read()

Unfortunately that means we have to special case specific clause types
instead of just being able to just for a colon leaf. The hack used here
is to check whether we're adding preceding newlines for a standalone or
dependent clause. "Standalone" being a clause that doesn't need another
clause to be valid (eg. if) and vice versa.

### Checklist - did you ...

<!-- If any of the following items aren't relevant for your contribution
     please still tick them so we know you've gone through the checklist.

    All user-facing changes should get an entry. Otherwise, signal to us
    this should get the magical label to silence the CHANGELOG entry check.
    Tests are required for bugfixes and new features. Documentation changes
    are necessary for formatting and most enhancement changes. -->

- [x] Add a CHANGELOG entry if necessary?
- [x] Add / update tests if necessary?
- [x] Add new / update outdated documentation? -> probably n/a?

---

Fixes GH-647

I understand if this is way too hacky to be acceptable but I spent too much time to not at least give it a shot :)



Problem statement:
Inconsistent spacing around top level functions
Operating system: macOS 10.14.2
Python version: 3.7.1
*Black* version: 18.9b0
Does also happen on master: yes

---

I have a bit of OS-specific code that looks like this (when formatted by black):

```py
import os

if os.name == "posix":
    import termios

    def foo():
        pass


elif os.name == "nt":
    try:
        import msvcrt

        def foo():
            pass

    except ImportError:

        def foo():
            pass


else:

    def foo():
        pass


other_code()
```

Note now `foo` has two empty lines below it most of the time (including in `if..elif..else`), except in the `try` block of a `try..except`.

Is this intentional? Seems to me there's no need to waste an extra line when the function definition is in any block; one empty line should be enough.

Hints from issue discussion:
This is indeed the case: https://black.now.sh/?version=stable&state=_Td6WFoAAATm1rRGAgAhARYAAAB0L-Wj4AFlAKtdAD2IimZxl1N_WlwxRhR7MSgE-ANVlfGy-Pf-urovcUul06DGIKtNJ_PI3I6sH4UsuvCCJK3cSn3O4qbqqEoVdC4RVidtOakKqE1C2PRTK-dEU4CIhv140rXetsz6FlHHNcogH0t3yNeHxqr6iAwuCmSu430fNaT3oxW598BtIbDYmvw9WdVHpVPmNBsa5a2N64u1JlxqaEp3vqccXdLeWyZdVl1aJp0nIQ-QAAAA7ohML_SstGsAAccB5gIAAOsZSAKxxGf7AgAAAAAEWVo=

This is caused by Black discovering that those functions are in fact top level and applying two blank lines before the next *top-level* statement (which is why it doesn't happen before the `except ImportError` line in the example).

We'd accept a fix for this but probably not something we'll be working on soon.

Commit message: Treat functions/classes in blocks as if they're nested

One curveball is that we still want two preceding newlines before blocks
that are probably logically disconnected. In other words:

    if condition:

        def foo():
            return "hi"
                             # <- aside: this is the goal of this commit
    else:

        def foo():
            return "cya"
                             # <- the two newlines spacing here should stay
                             #    since this probably isn't related
    with open("db.json", encoding="utf-8") as f:
        data = f.read()

Unfortunately that means we have to special case specific clause types
instead of just being able to just for a colon leaf. The hack used here
is to check whether we're adding preceding newlines for a standalone or
dependent clause. "Standalone" being a clause that doesn't need another
clause to be valid (eg. if) and vice versa.

Patch to review:
diff --git a/CHANGES.md b/CHANGES.md
index ef0a57d6142..0d9ffb20729 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### _Black_
+
+- Functions and classes in blocks now have more consistent surrounding spacing (#2472)
+
 ### Packaging
 
 - Fix missing modules in self-contained binaries (#2466)
diff --git a/src/black/lines.py b/src/black/lines.py
index 63225c0e6d3..67ac0c02e68 100644
--- a/src/black/lines.py
+++ b/src/black/lines.py
@@ -447,11 +447,30 @@ def _maybe_empty_lines(self, current_line: Line) -> Tuple[int, int]:
             before = 0
         depth = current_line.depth
         while self.previous_defs and self.previous_defs[-1] >= depth:
-            self.previous_defs.pop()
             if self.is_pyi:
                 before = 0 if depth else 1
             else:
-                before = 1 if depth else 2
+                if depth:
+                    before = 1
+                elif (
+                    not depth
+                    and self.previous_defs[-1]
+                    and current_line.leaves[-1].type == token.COLON
+                    and (
+                        current_line.leaves[0].value
+                        not in ("with", "try", "for", "while", "if")
+                    )
+                ):
+                    # We shouldn't add two newlines between an indented function and
+                    # and a dependent non-indented clause. This is to avoid issues with
+                    # conditional function definitions that are technically top-level
+                    # and therefore get two trailing newlines, but look weird and
+                    # inconsistent when they're followed by elif, else, etc. This is
+                    # worsen by that these functions only get *one* preceding already.
+                    before = 1
+                else:
+                    before = 2
+            self.previous_defs.pop()
         if current_line.is_decorator or current_line.is_def or current_line.is_class:
             return self._maybe_empty_lines_for_class_or_def(current_line, before)
 
diff --git a/tests/data/function2.py b/tests/data/function2.py
index cfc259ea7bd..5bb36c26318 100644
--- a/tests/data/function2.py
+++ b/tests/data/function2.py
@@ -23,6 +23,35 @@ def inner():
         pass
     print("Inner defs should breathe a little.")
 
+
+if os.name == "posix":
+    import termios
+    def i_should_be_followed_by_only_one_newline():
+        pass
+elif os.name == "nt":
+    try:
+        import msvcrt
+        def i_should_be_followed_by_only_one_newline():
+            pass
+
+    except ImportError:
+
+        def i_should_be_followed_by_only_one_newline():
+            pass
+
+elif False:
+
+    class IHopeYouAreHavingALovelyDay:
+        def __call__(self):
+            print("i_should_be_followed_by_only_one_newline")
+else:
+
+    def foo():
+        pass
+
+with hmm_but_this_should_get_two_preceding_newlines():
+    pass
+
 # output
 
 def f(
@@ -56,3 +85,37 @@ def inner():
         pass
 
     print("Inner defs should breathe a little.")
+
+
+if os.name == "posix":
+    import termios
+
+    def i_should_be_followed_by_only_one_newline():
+        pass
+
+elif os.name == "nt":
+    try:
+        import msvcrt
+
+        def i_should_be_followed_by_only_one_newline():
+            pass
+
+    except ImportError:
+
+        def i_should_be_followed_by_only_one_newline():
+            pass
+
+elif False:
+
+    class IHopeYouAreHavingALovelyDay:
+        def __call__(self):
+            print("i_should_be_followed_by_only_one_newline")
+
+else:
+
+    def foo():
+        pass
+
+
+with hmm_but_this_should_get_two_preceding_newlines():
+    pass


Code review comments:
diff_hunk: @@ -447,11 +447,30 @@ def _maybe_empty_lines(self, current_line: Line) -> Tuple[int, int]:
             before = 0
         depth = current_line.depth
         while self.previous_defs and self.previous_defs[-1] >= depth:
-            self.previous_defs.pop()
             if self.is_pyi:
                 before = 0 if depth else 1
             else:
-                before = 1 if depth else 2
+                if depth:
+                    before = 1
+                elif (
+                    not depth
+                    and self.previous_defs[-1]
+                    and current_line.leaves[-1].type == token.COLON
+                    and (
+                        current_line.leaves[0].value
+                        not in ("with", "try", "for", "while", "if")
+                    )
+                ):
+                    # We shouldn't add two newlines between an indented function and
+                    # and a dependent non-indented clause. This is to avoid issues with
+                    # conditional function definitions that are technically top-level
+                    # and therefore get two trailing newlines, but look weird and
+                    # inconsistent when they're followed by elif, else, etc. This is
+                    # worsen by that these functions only get *one* preceding already.
path: src/black/lines.py
text: @user1:
```suggestion
                    # worse because these functions only get *one* preceding newline already.
```

diff_hunk: @@ -447,11 +447,30 @@ def _maybe_empty_lines(self, current_line: Line) -> Tuple[int, int]:
             before = 0
         depth = current_line.depth
         while self.previous_defs and self.previous_defs[-1] >= depth:
-            self.previous_defs.pop()
             if self.is_pyi:
                 before = 0 if depth else 1
             else:
-                before = 1 if depth else 2
+                if depth:
+                    before = 1
+                elif (
+                    not depth
+                    and self.previous_defs[-1]
+                    and current_line.leaves[-1].type == token.COLON
+                    and (
+                        current_line.leaves[0].value
+                        not in ("with", "try", "for", "while", "if")
+                    )
+                ):
+                    # We shouldn't add two newlines between an indented function and
+                    # and a dependent non-indented clause. This is to avoid issues with
path: src/black/lines.py
text: @user1:
```suggestion
                    # a dependent non-indented clause. This is to avoid issues with
```