From 4abc0399b527c50369c448e00d6e204af74026e5 Mon Sep 17 00:00:00 2001 From: "Yilei \"Dolee\" Yang" Date: Tue, 25 Oct 2022 18:03:24 -0700 Subject: [PATCH 1/1] Enforce empty lines before classes/functions with sticky leading comments. (#3302) Co-authored-by: Jelle Zijlstra --- CHANGES.md | 2 + .../reference/reference_classes.rst | 22 +- docs/the_black_code_style/future_style.md | 49 +++- src/black/__init__.py | 21 +- src/black/lines.py | 86 +++++- src/black/mode.py | 1 + tests/data/preview/comments9.py | 254 ++++++++++++++++++ tests/data/preview/remove_await_parens.py | 1 + tests/data/simple_cases/comments5.py | 6 +- ...ocstring_no_extra_empty_line_before_eof.py | 4 + 10 files changed, 401 insertions(+), 45 deletions(-) create mode 100644 tests/data/preview/comments9.py create mode 100644 tests/data/simple_cases/docstring_no_extra_empty_line_before_eof.py diff --git a/CHANGES.md b/CHANGES.md index ba9f4c0..67451f7 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,6 +14,8 @@ +- Enforce empty lines before classes and functions with sticky leading comments (#3302) + ### Configuration diff --git a/docs/contributing/reference/reference_classes.rst b/docs/contributing/reference/reference_classes.rst index fa76596..3931e0e 100644 --- a/docs/contributing/reference/reference_classes.rst +++ b/docs/contributing/reference/reference_classes.rst @@ -11,23 +11,29 @@ .. autoclass:: black.brackets.BracketTracker :members: -:class:`EmptyLineTracker` +:class:`Line` +------------- + +.. autoclass:: black.lines.Line + :members: + :special-members: __str__, __bool__ + +:class:`LinesBlock` ------------------------- -.. autoclass:: black.EmptyLineTracker +.. autoclass:: black.lines.LinesBlock :members: -:class:`Line` -------------- +:class:`EmptyLineTracker` +------------------------- -.. autoclass:: black.Line +.. autoclass:: black.lines.EmptyLineTracker :members: - :special-members: __str__, __bool__ :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__ diff --git a/docs/the_black_code_style/future_style.md b/docs/the_black_code_style/future_style.md index a028a28..17b7eef 100644 --- a/docs/the_black_code_style/future_style.md +++ b/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 -indentation level is increased. For example: +1. _Black_ will remove newlines in the beginning of new code blocks, i.e. when the + indentation level is increased. For example: -```python -def my_func(): + ```python + def my_func(): - print("The line above me will be deleted!") -``` + print("The line above me will be deleted!") + ``` -will be changed to: + will be changed to: + + ```python + 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`, `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 -```python -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`, -`while`, `with`, `case` and `match`. + # Leading sticky comment + def my_func(): + ... + ``` ### Improved parentheses management diff --git a/src/black/__init__.py b/src/black/__init__.py index 5293796..d9fba41 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -61,7 +61,7 @@ from black.handle_ipynb_magics import ( 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 format_str(src_contents: str, *, mode: Mode) -> str: 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) - empty_line = Line(mode=mode) - after = 0 + elt = EmptyLineTracker(mode=mode) 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) - before, after = elt.maybe_empty_lines(current_line) - dst_contents.append(str(empty_line) * before) + block = elt.maybe_empty_lines(current_line) + dst_blocks.append(block) 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) diff --git a/src/black/lines.py b/src/black/lines.py index 3062265..0d07453 100644 --- a/src/black/lines.py +++ b/src/black/lines.py @@ -448,6 +448,28 @@ class Line: 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 @@ class EmptyLineTracker: 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 @@ class EmptyLineTracker: 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 @@ class EmptyLineTracker: ): 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 ): - return 0, 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 @@ class EmptyLineTracker: 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 diff --git a/src/black/mode.py b/src/black/mode.py index e3c3645..1e83f2a 100644 --- a/src/black/mode.py +++ b/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() diff --git a/tests/data/preview/comments9.py b/tests/data/preview/comments9.py new file mode 100644 index 0000000..449612c --- /dev/null +++ b/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 diff --git a/tests/data/preview/remove_await_parens.py b/tests/data/preview/remove_await_parens.py index eb7dad3..571210a 100644 --- a/tests/data/preview/remove_await_parens.py +++ b/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) diff --git a/tests/data/simple_cases/comments5.py b/tests/data/simple_cases/comments5.py index d83b6b8..c8c3881 100644 --- a/tests/data/simple_cases/comments5.py +++ b/tests/data/simple_cases/comments5.py @@ -58,9 +58,9 @@ def decorated1(): ... -# Note: crappy but inevitable. The current design of EmptyLineTracker doesn't -# allow this to work correctly. The user will have to split those lines by -# hand. +# Note: this is fixed in +# Preview.empty_lines_before_class_or_def_with_leading_comments. +# 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(): diff --git a/tests/data/simple_cases/docstring_no_extra_empty_line_before_eof.py b/tests/data/simple_cases/docstring_no_extra_empty_line_before_eof.py new file mode 100644 index 0000000..6fea860 --- /dev/null +++ b/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.""" -- 2.39.5