Enforce empty lines before classes/functions with sticky leading comments. (#3302)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

CHANGES.md

@@ -14,6 +14,8 @@
 
 <!-- Changes that affect Black's preview style -->
 
+- Enforce empty lines before classes and functions with sticky leading comments (#3302)
+
 ### Configuration
 
 <!-- Changes to how Black can be configured -->

docs/contributing/reference/reference_classes.rst

@@ -11,23 +11,29 @@
 .. autoclass:: black.brackets.BracketTracker
     :members:
 
-:class:`EmptyLineTracker`
--------------------------
-
-.. autoclass:: black.EmptyLineTracker
-    :members:
-
 :class:`Line`
 -------------
 
-.. autoclass:: black.Line
+.. autoclass:: black.lines.Line
     :members:
     :special-members: __str__, __bool__
 
+:class:`LinesBlock`
+-------------------------
+
+.. autoclass:: black.lines.LinesBlock
+    :members:
+
+:class:`EmptyLineTracker`
+-------------------------
+
+.. autoclass:: black.lines.EmptyLineTracker
+    :members:
+
 :class:`LineGenerator`
 ----------------------
 
-.. autoclass:: black.LineGenerator
+.. autoclass:: black.linegen.LineGenerator
     :show-inheritance:
     :members:
 
@@ -40,7 +46,7 @@
 :class:`Report`
 ---------------
 
-.. autoclass:: black.Report
+.. autoclass:: black.report.Report
     :members:
     :special-members: __str__
 

docs/the_black_code_style/future_style.md

@@ -63,26 +63,47 @@ limit. Line continuation backslashes are converted into parenthesized strings.
 Unnecessary parentheses are stripped. The stability and status of this feature is
 tracked in [this issue](https://github.com/psf/black/issues/2188).
 
-### Removing newlines in the beginning of code blocks
+### Improved empty line management
 
-_Black_ will remove newlines in the beginning of new code blocks, i.e. when the
+1.  _Black_ will remove newlines in the beginning of new code blocks, i.e. when the
-indentation level is increased. For example:
+    indentation level is increased. For example:
 
-```python
+    ```python
-def my_func():
+    def my_func():
 
         print("The line above me will be deleted!")
-```
+    ```
 
-will be changed to:
+    will be changed to:
 
-```python
+    ```python
-def my_func():
+    def my_func():
         print("The line above me will be deleted!")
-```
+    ```
 
-This new feature will be applied to **all code blocks**: `def`, `class`, `if`, `for`,
+    This new feature will be applied to **all code blocks**: `def`, `class`, `if`,
-`while`, `with`, `case` and `match`.
+    `for`, `while`, `with`, `case` and `match`.
+
+2.  _Black_ will enforce empty lines before classes and functions with leading comments.
+    For example:
+
+    ```python
+    some_var = 1
+    # Leading sticky comment
+    def my_func():
+        ...
+    ```
+
+    will be changed to:
+
+    ```python
+    some_var = 1
+
+
+    # Leading sticky comment
+    def my_func():
+        ...
+    ```
 
 ### Improved parentheses management
 

src/black/__init__.py

@@ -61,7 +61,7 @@
     unmask_cell,
 )
 from black.linegen import LN, LineGenerator, transform_line
-from black.lines import EmptyLineTracker, Line
+from black.lines import EmptyLineTracker, LinesBlock
 from black.mode import (
     FUTURE_FLAG_TO_FEATURE,
     VERSION_TO_FEATURES,
@@ -1075,7 +1075,7 @@ def f(
 
 def _format_str_once(src_contents: str, *, mode: Mode) -> str:
     src_node = lib2to3_parse(src_contents.lstrip(), mode.target_versions)
-    dst_contents = []
+    dst_blocks: List[LinesBlock] = []
     if mode.target_versions:
         versions = mode.target_versions
     else:
@@ -1084,22 +1084,25 @@ def _format_str_once(src_contents: str, *, mode: Mode) -> str:
 
     normalize_fmt_off(src_node, preview=mode.preview)
     lines = LineGenerator(mode=mode)
-    elt = EmptyLineTracker(is_pyi=mode.is_pyi)
+    elt = EmptyLineTracker(mode=mode)
-    empty_line = Line(mode=mode)
-    after = 0
     split_line_features = {
         feature
         for feature in {Feature.TRAILING_COMMA_IN_CALL, Feature.TRAILING_COMMA_IN_DEF}
         if supports_feature(versions, feature)
     }
+    block: Optional[LinesBlock] = None
     for current_line in lines.visit(src_node):
-        dst_contents.append(str(empty_line) * after)
+        block = elt.maybe_empty_lines(current_line)
-        before, after = elt.maybe_empty_lines(current_line)
+        dst_blocks.append(block)
-        dst_contents.append(str(empty_line) * before)
         for line in transform_line(
             current_line, mode=mode, features=split_line_features
         ):
-            dst_contents.append(str(line))
+            block.content_lines.append(str(line))
+    if dst_blocks:
+        dst_blocks[-1].after = 0
+    dst_contents = []
+    for block in dst_blocks:
+        dst_contents.extend(block.all_lines())
     return "".join(dst_contents)
 
 

src/black/lines.py

@@ -448,6 +448,28 @@ def __bool__(self) -> bool:
         return bool(self.leaves or self.comments)
 
 
+@dataclass
+class LinesBlock:
+    """Class that holds information about a block of formatted lines.
+
+    This is introduced so that the EmptyLineTracker can look behind the standalone
+    comments and adjust their empty lines for class or def lines.
+    """
+
+    mode: Mode
+    previous_block: Optional["LinesBlock"]
+    original_line: Line
+    before: int = 0
+    content_lines: List[str] = field(default_factory=list)
+    after: int = 0
+
+    def all_lines(self) -> List[str]:
+        empty_line = str(Line(mode=self.mode))
+        return (
+            [empty_line * self.before] + self.content_lines + [empty_line * self.after]
+        )
+
+
 @dataclass
 class EmptyLineTracker:
     """Provides a stateful method that returns the number of potential extra
@@ -458,33 +480,55 @@ class EmptyLineTracker:
     are consumed by `maybe_empty_lines()` and included in the computation.
     """
 
-    is_pyi: bool = False
+    mode: Mode
     previous_line: Optional[Line] = None
-    previous_after: int = 0
+    previous_block: Optional[LinesBlock] = None
     previous_defs: List[int] = field(default_factory=list)
+    semantic_leading_comment: Optional[LinesBlock] = None
 
-    def maybe_empty_lines(self, current_line: Line) -> Tuple[int, int]:
+    def maybe_empty_lines(self, current_line: Line) -> LinesBlock:
         """Return the number of extra empty lines before and after the `current_line`.
 
         This is for separating `def`, `async def` and `class` with extra empty
         lines (two on module-level).
         """
         before, after = self._maybe_empty_lines(current_line)
+        previous_after = self.previous_block.after if self.previous_block else 0
         before = (
             # Black should not insert empty lines at the beginning
             # of the file
             0
             if self.previous_line is None
-            else before - self.previous_after
+            else before - previous_after
         )
-        self.previous_after = after
+        block = LinesBlock(
+            mode=self.mode,
+            previous_block=self.previous_block,
+            original_line=current_line,
+            before=before,
+            after=after,
+        )
+
+        # Maintain the semantic_leading_comment state.
+        if current_line.is_comment:
+            if self.previous_line is None or (
+                not self.previous_line.is_decorator
+                # `or before` means this comment already has an empty line before
+                and (not self.previous_line.is_comment or before)
+                and (self.semantic_leading_comment is None or before)
+            ):
+                self.semantic_leading_comment = block
+        elif not current_line.is_decorator:
+            self.semantic_leading_comment = None
+
         self.previous_line = current_line
-        return before, after
+        self.previous_block = block
+        return block
 
     def _maybe_empty_lines(self, current_line: Line) -> Tuple[int, int]:
         max_allowed = 1
         if current_line.depth == 0:
-            max_allowed = 1 if self.is_pyi else 2
+            max_allowed = 1 if self.mode.is_pyi else 2
         if current_line.leaves:
             # Consume the first leaf's extra newlines.
             first_leaf = current_line.leaves[0]
@@ -495,7 +539,7 @@ 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:
-            if self.is_pyi:
+            if self.mode.is_pyi:
                 assert self.previous_line is not None
                 if depth and not current_line.is_def and self.previous_line.is_def:
                     # Empty lines between attributes and methods should be preserved.
@@ -563,7 +607,7 @@ def _maybe_empty_lines_for_class_or_def(
             return 0, 0
 
         if self.previous_line.is_decorator:
-            if self.is_pyi and current_line.is_stub_class:
+            if self.mode.is_pyi and current_line.is_stub_class:
                 # Insert an empty line after a decorated stub class
                 return 0, 1
 
@@ -574,14 +618,27 @@ def _maybe_empty_lines_for_class_or_def(
         ):
             return 0, 0
 
+        comment_to_add_newlines: Optional[LinesBlock] = None
         if (
             self.previous_line.is_comment
             and self.previous_line.depth == current_line.depth
             and before == 0
         ):
+            slc = self.semantic_leading_comment
+            if (
+                Preview.empty_lines_before_class_or_def_with_leading_comments
+                in current_line.mode
+                and slc is not None
+                and slc.previous_block is not None
+                and not slc.previous_block.original_line.is_class
+                and not slc.previous_block.original_line.opens_block
+                and slc.before <= 1
+            ):
+                comment_to_add_newlines = slc
+            else:
                 return 0, 0
 
-        if self.is_pyi:
+        if self.mode.is_pyi:
             if current_line.is_class or self.previous_line.is_class:
                 if self.previous_line.depth < current_line.depth:
                     newlines = 0
@@ -609,6 +666,13 @@ def _maybe_empty_lines_for_class_or_def(
                 newlines = 0
         else:
             newlines = 1 if current_line.depth else 2
+        if comment_to_add_newlines is not None:
+            previous_block = comment_to_add_newlines.previous_block
+            if previous_block is not None:
+                comment_to_add_newlines.before = (
+                    max(comment_to_add_newlines.before, newlines) - previous_block.after
+                )
+                newlines = 0
         return newlines, 0
 
 

src/black/mode.py

@@ -150,6 +150,7 @@ class Preview(Enum):
     """Individual preview style features."""
 
     annotation_parens = auto()
+    empty_lines_before_class_or_def_with_leading_comments = auto()
     long_docstring_quotes_on_newline = auto()
     normalize_docstring_quotes_and_prefixes_properly = auto()
     one_element_subscript = auto()

tests/data/preview/comments9.py

@@ -0,0 +1,254 @@
+# Test for https://github.com/psf/black/issues/246.
+
+some = statement
+# This comment should be split from the statement above by two lines.
+def function():
+    pass
+
+
+some = statement
+# This multiline comments section
+# should be split from the statement
+# above by two lines.
+def function():
+    pass
+
+
+some = statement
+# This comment should be split from the statement above by two lines.
+async def async_function():
+    pass
+
+
+some = statement
+# This comment should be split from the statement above by two lines.
+class MyClass:
+    pass
+
+
+some = statement
+# This should be stick to the statement above
+
+# This should be split from the above by two lines
+class MyClassWithComplexLeadingComments:
+    pass
+
+
+class ClassWithDocstring:
+    """A docstring."""
+# Leading comment after a class with just a docstring
+class MyClassAfterAnotherClassWithDocstring:
+    pass
+
+
+some = statement
+# leading 1
+@deco1
+# leading 2
+# leading 2 extra
+@deco2(with_args=True)
+# leading 3
+@deco3
+# leading 4
+def decorated():
+    pass
+
+
+some = statement
+# leading 1
+@deco1
+# leading 2
+@deco2(with_args=True)
+
+# leading 3 that already has an empty line
+@deco3
+# leading 4
+def decorated_with_split_leading_comments():
+    pass
+
+
+some = statement
+# leading 1
+@deco1
+# leading 2
+@deco2(with_args=True)
+# leading 3
+@deco3
+
+# leading 4 that already has an empty line
+def decorated_with_split_leading_comments():
+    pass
+
+
+def main():
+    if a:
+        # Leading comment before inline function
+        def inline():
+            pass
+        # Another leading comment
+        def another_inline():
+            pass
+    else:
+        # More leading comments
+        def inline_after_else():
+            pass
+
+
+if a:
+    # Leading comment before "top-level inline" function
+    def top_level_quote_inline():
+        pass
+    # Another leading comment
+    def another_top_level_quote_inline_inline():
+        pass
+else:
+    # More leading comments
+    def top_level_quote_inline_after_else():
+        pass
+
+
+class MyClass:
+    # First method has no empty lines between bare class def.
+    # More comments.
+    def first_method(self):
+        pass
+
+
+# output
+
+
+# Test for https://github.com/psf/black/issues/246.
+
+some = statement
+
+
+# This comment should be split from the statement above by two lines.
+def function():
+    pass
+
+
+some = statement
+
+
+# This multiline comments section
+# should be split from the statement
+# above by two lines.
+def function():
+    pass
+
+
+some = statement
+
+
+# This comment should be split from the statement above by two lines.
+async def async_function():
+    pass
+
+
+some = statement
+
+
+# This comment should be split from the statement above by two lines.
+class MyClass:
+    pass
+
+
+some = statement
+# This should be stick to the statement above
+
+
+# This should be split from the above by two lines
+class MyClassWithComplexLeadingComments:
+    pass
+
+
+class ClassWithDocstring:
+    """A docstring."""
+
+
+# Leading comment after a class with just a docstring
+class MyClassAfterAnotherClassWithDocstring:
+    pass
+
+
+some = statement
+
+
+# leading 1
+@deco1
+# leading 2
+# leading 2 extra
+@deco2(with_args=True)
+# leading 3
+@deco3
+# leading 4
+def decorated():
+    pass
+
+
+some = statement
+
+
+# leading 1
+@deco1
+# leading 2
+@deco2(with_args=True)
+
+# leading 3 that already has an empty line
+@deco3
+# leading 4
+def decorated_with_split_leading_comments():
+    pass
+
+
+some = statement
+
+
+# leading 1
+@deco1
+# leading 2
+@deco2(with_args=True)
+# leading 3
+@deco3
+
+# leading 4 that already has an empty line
+def decorated_with_split_leading_comments():
+    pass
+
+
+def main():
+    if a:
+        # Leading comment before inline function
+        def inline():
+            pass
+
+        # Another leading comment
+        def another_inline():
+            pass
+
+    else:
+        # More leading comments
+        def inline_after_else():
+            pass
+
+
+if a:
+    # Leading comment before "top-level inline" function
+    def top_level_quote_inline():
+        pass
+
+    # Another leading comment
+    def another_top_level_quote_inline_inline():
+        pass
+
+else:
+    # More leading comments
+    def top_level_quote_inline_after_else():
+        pass
+
+
+class MyClass:
+    # First method has no empty lines between bare class def.
+    # More comments.
+    def first_method(self):
+        pass

tests/data/preview/remove_await_parens.py

@@ -80,6 +80,7 @@ async def main():
 # output
 import asyncio
 
+
 # Control example
 async def main():
     await asyncio.sleep(1)

tests/data/simple_cases/comments5.py

@@ -58,9 +58,9 @@ def decorated1():
     ...
 
 
-# Note: crappy but inevitable.  The current design of EmptyLineTracker doesn't
+# Note: this is fixed in
-# allow this to work correctly.  The user will have to split those lines by
+# Preview.empty_lines_before_class_or_def_with_leading_comments.
-# hand.
+# In the current style, the user will have to split those lines by hand.
 some_instruction
 # This comment should be split from `some_instruction` by two lines but isn't.
 def g():

tests/data/simple_cases/docstring_no_extra_empty_line_before_eof.py

@@ -0,0 +1,4 @@
+# Make sure when the file ends with class's docstring,
+# It doesn't add extra blank lines.
+class ClassWithDocstring:
+    """A docstring."""