From 9cc257ee7dd742b0d3ad3a412e6560248c9d3cad Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 22 Dec 2023 12:44:14 +0900 Subject: [PATCH] Improve `dummy_implementations` preview style formatting (#9240) --- .../resources/test/fixtures/ruff/newlines.py | 21 +- .../src/statement/clause.rs | 10 +- .../src/statement/suite.rs | 34 ++- ...ses__preview_dummy_implementations.py.snap | 276 ------------------ .../tests/snapshots/format@newlines.py.snap | 98 ++++++- .../format@statement__top_level.py.snap | 14 +- 6 files changed, 159 insertions(+), 294 deletions(-) delete mode 100644 crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_dummy_implementations.py.snap diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/newlines.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/newlines.py index e684befc4bb3d..648c10a531f28 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/newlines.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/newlines.py @@ -253,4 +253,23 @@ def y(): # empty line(s) at the end of the file due to nested function if True: def nested_trailing_function(): - pass \ No newline at end of file + pass + + +def overload1(): ... # trailing comment +def overload1(a: int): ... + +def overload2(): ... # trailing comment + +def overload2(a: int): ... + +def overload3(): + ... + # trailing comment +def overload3(a: int): ... + +def overload4(): + ... + # trailing comment + +def overload4(a: int): ... diff --git a/crates/ruff_python_formatter/src/statement/clause.rs b/crates/ruff_python_formatter/src/statement/clause.rs index 29157b15f9bb2..3f9b3de62cb59 100644 --- a/crates/ruff_python_formatter/src/statement/clause.rs +++ b/crates/ruff_python_formatter/src/statement/clause.rs @@ -392,9 +392,13 @@ pub(crate) fn clause_body<'a>( impl Format> for FormatClauseBody<'_> { fn fmt(&self, f: &mut Formatter>) -> FormatResult<()> { - // In stable, stubs are only collapsed in stub files, in preview this is consistently - // applied everywhere - if (f.options().source_type().is_stub() || is_dummy_implementations_enabled(f.context())) + // In stable, stubs are only collapsed in stub files, in preview stubs in functions + // or classes are collapsed too + let should_collapse_stub = f.options().source_type().is_stub() + || (is_dummy_implementations_enabled(f.context()) + && matches!(self.kind, SuiteKind::Function | SuiteKind::Class)); + + if should_collapse_stub && contains_only_an_ellipsis(self.body, f.context().comments()) && self.trailing_comments.is_empty() { diff --git a/crates/ruff_python_formatter/src/statement/suite.rs b/crates/ruff_python_formatter/src/statement/suite.rs index aa3f297cfdd2d..21b0841950089 100644 --- a/crates/ruff_python_formatter/src/statement/suite.rs +++ b/crates/ruff_python_formatter/src/statement/suite.rs @@ -12,7 +12,8 @@ use crate::context::{NodeLevel, TopLevelStatementPosition, WithIndentLevel, With use crate::expression::expr_string_literal::ExprStringLiteralKind; use crate::prelude::*; use crate::preview::{ - is_module_docstring_newlines_enabled, is_no_blank_line_before_class_docstring_enabled, + is_dummy_implementations_enabled, is_module_docstring_newlines_enabled, + is_no_blank_line_before_class_docstring_enabled, }; use crate::statement::stmt_expr::FormatStmtExpr; use crate::verbatim::{ @@ -261,12 +262,31 @@ impl FormatRule> for FormatSuite { f, )?; } else { - match self.kind { - SuiteKind::TopLevel => { - write!(f, [empty_line(), empty_line()])?; - } - SuiteKind::Function | SuiteKind::Class | SuiteKind::Other => { - empty_line().fmt(f)?; + // Preserve empty lines after a stub implementation but don't insert a new one if there isn't any present in the source. + // This is useful when having multiple function overloads that should be grouped to getter by omitting new lines between them. + let is_preceding_stub_function_without_empty_line = + is_dummy_implementations_enabled(f.context()) + && following.is_function_def_stmt() + && preceding + .as_function_def_stmt() + .is_some_and(|preceding_stub| { + contains_only_an_ellipsis( + &preceding_stub.body, + f.context().comments(), + ) && lines_after_ignoring_end_of_line_trivia( + preceding_stub.end(), + f.context().source(), + ) < 2 + }); + + if !is_preceding_stub_function_without_empty_line { + match self.kind { + SuiteKind::TopLevel => { + write!(f, [empty_line(), empty_line()])?; + } + SuiteKind::Function | SuiteKind::Class | SuiteKind::Other => { + empty_line().fmt(f)?; + } } } } diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_dummy_implementations.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_dummy_implementations.py.snap deleted file mode 100644 index f075c7ac110e2..0000000000000 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_dummy_implementations.py.snap +++ /dev/null @@ -1,276 +0,0 @@ ---- -source: crates/ruff_python_formatter/tests/fixtures.rs -input_file: crates/ruff_python_formatter/resources/test/fixtures/black/cases/preview_dummy_implementations.py ---- -## Input - -```python -from typing import NoReturn, Protocol, Union, overload - -class Empty: - ... - -def dummy(a): ... -async def other(b): ... - - -@overload -def a(arg: int) -> int: ... -@overload -def a(arg: str) -> str: ... -@overload -def a(arg: object) -> NoReturn: ... -def a(arg: Union[int, str, object]) -> Union[int, str]: - if not isinstance(arg, (int, str)): - raise TypeError - return arg - -class Proto(Protocol): - def foo(self, a: int) -> int: - ... - - def bar(self, b: str) -> str: ... - def baz(self, c: bytes) -> str: - ... - - -def dummy_two(): - ... -@dummy -def dummy_three(): - ... - -def dummy_four(): - ... - -@overload -def b(arg: int) -> int: ... - -@overload -def b(arg: str) -> str: ... -@overload -def b(arg: object) -> NoReturn: ... - -def b(arg: Union[int, str, object]) -> Union[int, str]: - if not isinstance(arg, (int, str)): - raise TypeError - return arg - -def has_comment(): - ... # still a dummy - -if some_condition: - ... - -if already_dummy: ... -``` - -## Black Differences - -```diff ---- Black -+++ Ruff -@@ -5,15 +5,23 @@ - - - def dummy(a): ... -+ -+ - async def other(b): ... - - - @overload - def a(arg: int) -> int: ... -+ -+ - @overload - def a(arg: str) -> str: ... -+ -+ - @overload - def a(arg: object) -> NoReturn: ... -+ -+ - def a(arg: Union[int, str, object]) -> Union[int, str]: - if not isinstance(arg, (int, str)): - raise TypeError -@@ -24,10 +32,13 @@ - def foo(self, a: int) -> int: ... - - def bar(self, b: str) -> str: ... -+ - def baz(self, c: bytes) -> str: ... - - - def dummy_two(): ... -+ -+ - @dummy - def dummy_three(): ... - -@@ -41,6 +52,8 @@ - - @overload - def b(arg: str) -> str: ... -+ -+ - @overload - def b(arg: object) -> NoReturn: ... - -@@ -54,8 +67,6 @@ - def has_comment(): ... # still a dummy - - --if some_condition: -- ... -+if some_condition: ... - --if already_dummy: -- ... -+if already_dummy: ... -``` - -## Ruff Output - -```python -from typing import NoReturn, Protocol, Union, overload - - -class Empty: ... - - -def dummy(a): ... - - -async def other(b): ... - - -@overload -def a(arg: int) -> int: ... - - -@overload -def a(arg: str) -> str: ... - - -@overload -def a(arg: object) -> NoReturn: ... - - -def a(arg: Union[int, str, object]) -> Union[int, str]: - if not isinstance(arg, (int, str)): - raise TypeError - return arg - - -class Proto(Protocol): - def foo(self, a: int) -> int: ... - - def bar(self, b: str) -> str: ... - - def baz(self, c: bytes) -> str: ... - - -def dummy_two(): ... - - -@dummy -def dummy_three(): ... - - -def dummy_four(): ... - - -@overload -def b(arg: int) -> int: ... - - -@overload -def b(arg: str) -> str: ... - - -@overload -def b(arg: object) -> NoReturn: ... - - -def b(arg: Union[int, str, object]) -> Union[int, str]: - if not isinstance(arg, (int, str)): - raise TypeError - return arg - - -def has_comment(): ... # still a dummy - - -if some_condition: ... - -if already_dummy: ... -``` - -## Black Output - -```python -from typing import NoReturn, Protocol, Union, overload - - -class Empty: ... - - -def dummy(a): ... -async def other(b): ... - - -@overload -def a(arg: int) -> int: ... -@overload -def a(arg: str) -> str: ... -@overload -def a(arg: object) -> NoReturn: ... -def a(arg: Union[int, str, object]) -> Union[int, str]: - if not isinstance(arg, (int, str)): - raise TypeError - return arg - - -class Proto(Protocol): - def foo(self, a: int) -> int: ... - - def bar(self, b: str) -> str: ... - def baz(self, c: bytes) -> str: ... - - -def dummy_two(): ... -@dummy -def dummy_three(): ... - - -def dummy_four(): ... - - -@overload -def b(arg: int) -> int: ... - - -@overload -def b(arg: str) -> str: ... -@overload -def b(arg: object) -> NoReturn: ... - - -def b(arg: Union[int, str, object]) -> Union[int, str]: - if not isinstance(arg, (int, str)): - raise TypeError - return arg - - -def has_comment(): ... # still a dummy - - -if some_condition: - ... - -if already_dummy: - ... -``` - - diff --git a/crates/ruff_python_formatter/tests/snapshots/format@newlines.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@newlines.py.snap index e939aa2d280a6..c275dc4c16a10 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@newlines.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@newlines.py.snap @@ -259,7 +259,27 @@ if True: # empty line(s) at the end of the file due to nested function if True: def nested_trailing_function(): - pass``` + pass + + +def overload1(): ... # trailing comment +def overload1(a: int): ... + +def overload2(): ... # trailing comment + +def overload2(a: int): ... + +def overload3(): + ... + # trailing comment +def overload3(a: int): ... + +def overload4(): + ... + # trailing comment + +def overload4(a: int): ... +``` ## Output ```python @@ -546,6 +566,40 @@ if True: def nested_trailing_function(): pass + + +def overload1(): + ... # trailing comment + + +def overload1(a: int): + ... + + +def overload2(): + ... # trailing comment + + +def overload2(a: int): + ... + + +def overload3(): + ... + # trailing comment + + +def overload3(a: int): + ... + + +def overload4(): + ... + # trailing comment + + +def overload4(a: int): + ... ``` @@ -569,6 +623,48 @@ if True: def fakehttp(): +@@ -283,20 +281,14 @@ + pass + + +-def overload1(): +- ... # trailing comment ++def overload1(): ... # trailing comment ++def overload1(a: int): ... + + +-def overload1(a: int): +- ... ++def overload2(): ... # trailing comment + + +-def overload2(): +- ... # trailing comment +- +- +-def overload2(a: int): +- ... ++def overload2(a: int): ... + + + def overload3(): +@@ -304,8 +296,7 @@ + # trailing comment + + +-def overload3(a: int): +- ... ++def overload3(a: int): ... + + + def overload4(): +@@ -313,5 +304,4 @@ + # trailing comment + + +-def overload4(a: int): +- ... ++def overload4(a: int): ... ``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__top_level.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__top_level.py.snap index da86b86473607..d0ecb00410678 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__top_level.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__top_level.py.snap @@ -150,21 +150,23 @@ def quuz(): class Baz(Qux): -@@ -49,12 +44,10 @@ +@@ -49,14 +44,8 @@ pass -def bar(): - ... -+def bar(): ... - - +- +- -def baz(): - ... +- +- ++def bar(): ... +def baz(): ... - - def quux(): + """Some docstring.""" + ```