From eaa310429fc9b6d0b67d7b174ec3444921c80873 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 30 Nov 2023 21:49:28 -0500 Subject: [PATCH] Insert trailing comma when function breaks with single argument (#8921) ## Summary Given: ```python def _example_function_xxxxxxx( variable: Optional[List[str]] ) -> List[example.ExampleConfig]: pass ``` We should be inserting a trailing comma after the argument (as long as it's a single-argument function). This was an inconsistency with Black, but also led to some internal inconsistencies, whereby we added the comma if the argument contained a trailing end-of-line comment, but not otherwise. Closes https://github.com/astral-sh/ruff/issues/8912. ## Test Plan `cargo test` Before: | project | similarity index | total files | changed files | |----------------|------------------:|------------------:|------------------:| | cpython | 0.75804 | 1799 | 1648 | | django | 0.99984 | 2772 | 34 | | home-assistant | 0.99963 | 10596 | 146 | | poetry | 0.99925 | 317 | 12 | | transformers | 0.99967 | 2657 | 322 | | twine | 1.00000 | 33 | 0 | | typeshed | 0.99980 | 3669 | 18 | | warehouse | 0.99977 | 654 | 13 | | zulip | 0.99970 | 1459 | 21 | After: | project | similarity index | total files | changed files | |----------------|------------------:|------------------:|------------------:| | cpython | 0.75804 | 1799 | 1648 | | django | 0.99984 | 2772 | 34 | | home-assistant | 0.99955 | 10596 | 213 | | poetry | 0.99917 | 317 | 13 | | transformers | 0.99967 | 2657 | 324 | | twine | 1.00000 | 33 | 0 | | typeshed | 0.99980 | 3669 | 18 | | warehouse | 0.99976 | 654 | 14 | | zulip | 0.99957 | 1459 | 36 | --- .../src/other/parameters.rs | 13 ++++ ...funcdef_return_type_trailing_comma.py.snap | 24 ++----- ..._return_annotation_brackets_string.py.snap | 11 +--- ...@cases__return_annotation_brackets.py.snap | 8 +-- ..._opening_parentheses_comment_value.py.snap | 2 +- .../format@statement__function.py.snap | 2 +- ...ormat@statement__return_annotation.py.snap | 66 +++++++++---------- 7 files changed, 60 insertions(+), 66 deletions(-) diff --git a/crates/ruff_python_formatter/src/other/parameters.rs b/crates/ruff_python_formatter/src/other/parameters.rs index 86cad5869b2ce..d095d33ae1975 100644 --- a/crates/ruff_python_formatter/src/other/parameters.rs +++ b/crates/ruff_python_formatter/src/other/parameters.rs @@ -252,6 +252,19 @@ impl FormatNodeRule for FormatParameters { let mut f = WithNodeLevel::new(NodeLevel::ParenthesizedExpression, f); // No parameters, format any dangling comments between `()` write!(f, [empty_parenthesized("(", dangling, ")")]) + } else if num_parameters == 1 { + // If we have a single argument, avoid the inner group, to ensure that we insert a + // trailing comma if the outer group breaks. + let mut f = WithNodeLevel::new(NodeLevel::ParenthesizedExpression, f); + write!( + f, + [ + token("("), + dangling_open_parenthesis_comments(parenthesis_dangling), + soft_block_indent(&format_inner), + token(")") + ] + ) } else { // Intentionally avoid `parenthesized`, which groups the entire formatted contents. // We want parameters to be grouped alongside return types, one level up, so we diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__funcdef_return_type_trailing_comma.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__funcdef_return_type_trailing_comma.py.snap index a10744872cd4a..09c380567e951 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__funcdef_return_type_trailing_comma.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__funcdef_return_type_trailing_comma.py.snap @@ -179,13 +179,7 @@ def SimplePyFn( p, q, ]: -@@ -63,16 +67,18 @@ - # long function definition, return type is longer - # this should maybe split on rhs? - def aaaaaaaaaaaaaaaaa( -- bbbbbbbbbbbbbbbbbb, -+ bbbbbbbbbbbbbbbbbb - ) -> list[Ccccccccccccccccccccccccccccccccccccccccccccccccccc, Dddddd]: ... +@@ -68,11 +72,13 @@ # long return type, no param list @@ -204,25 +198,19 @@ def SimplePyFn( # long function name, no param list, no return value -@@ -93,12 +99,16 @@ +@@ -93,7 +99,11 @@ # unskippable type hint (??) -def foo(a) -> list[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]: # type: ignore +def foo( -+ a ++ a, +) -> list[ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +]: # type: ignore pass - def foo( -- a, -+ a - ) -> list[ - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - ]: # abpedeifnore @@ -112,7 +122,13 @@ @@ -333,7 +321,7 @@ def aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa( # long function definition, return type is longer # this should maybe split on rhs? def aaaaaaaaaaaaaaaaa( - bbbbbbbbbbbbbbbbbb + bbbbbbbbbbbbbbbbbb, ) -> list[Ccccccccccccccccccccccccccccccccccccccccccccccccccc, Dddddd]: ... @@ -366,7 +354,7 @@ def thiiiiiiiiiiiiiiiiiis_iiiiiiiiiiiiiiiiiiiiiiiiiiiiiis_veeeeeeeeeeeeeeeeeeeee # unskippable type hint (??) def foo( - a + a, ) -> list[ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ]: # type: ignore @@ -374,7 +362,7 @@ def foo( def foo( - a + a, ) -> list[ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ]: # abpedeifnore diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_return_annotation_brackets_string.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_return_annotation_brackets_string.py.snap index 268fcf0eb8487..5f43f1a7d20b5 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_return_annotation_brackets_string.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_return_annotation_brackets_string.py.snap @@ -19,7 +19,7 @@ def frobnicate(a) -> "ThisIsTrulyUnreasonablyExtremelyLongClassName | list[ThisI ```diff --- Black +++ Ruff -@@ -1,13 +1,12 @@ +@@ -1,7 +1,6 @@ # Long string example def frobnicate() -> ( - "ThisIsTrulyUnreasonablyExtremelyLongClassName |" @@ -28,13 +28,6 @@ def frobnicate(a) -> "ThisIsTrulyUnreasonablyExtremelyLongClassName | list[ThisI ): pass - - # splitting the string breaks if there's any parameters - def frobnicate( -- a, -+ a - ) -> "ThisIsTrulyUnreasonablyExtremelyLongClassName | list[ThisIsTrulyUnreasonablyExtremelyLongClassName]": - pass ``` ## Ruff Output @@ -49,7 +42,7 @@ def frobnicate() -> ( # splitting the string breaks if there's any parameters def frobnicate( - a + a, ) -> "ThisIsTrulyUnreasonablyExtremelyLongClassName | list[ThisIsTrulyUnreasonablyExtremelyLongClassName]": pass ``` diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__return_annotation_brackets.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__return_annotation_brackets.py.snap index 415daad90c786..9bf6a451152c9 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__return_annotation_brackets.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__return_annotation_brackets.py.snap @@ -111,7 +111,7 @@ def foo(a,b) -> tuple[int, int, int,]: # Don't lose the comments -def double(a: int) -> int: # Hello +def double( -+ a: int ++ a: int, +) -> ( # Hello + int +): @@ -120,7 +120,7 @@ def foo(a,b) -> tuple[int, int, int,]: -def double(a: int) -> int: # Hello +def double( -+ a: int ++ a: int, +) -> ( + int # Hello +): @@ -168,7 +168,7 @@ def double(a: int) -> int: # Don't lose the comments def double( - a: int + a: int, ) -> ( # Hello int ): @@ -176,7 +176,7 @@ def double( def double( - a: int + a: int, ) -> ( int # Hello ): diff --git a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__opening_parentheses_comment_value.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__opening_parentheses_comment_value.py.snap index 4c226dd8c3d9a..2dc81368793b1 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__opening_parentheses_comment_value.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__opening_parentheses_comment_value.py.snap @@ -253,7 +253,7 @@ except ( # d 9 def e1( # e 1 - x + x, ): pass diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap index 5b0684becdfa4..7a8e97566ecb3 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap @@ -928,7 +928,7 @@ def f( # first def f( # first - a + a, ): # second ... diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__return_annotation.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__return_annotation.py.snap index d32669cbbc5b6..d7e7f4074a620 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__return_annotation.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__return_annotation.py.snap @@ -281,7 +281,7 @@ def double( def double( - a: int + a: int, ) -> ( # Hello int ): @@ -289,7 +289,7 @@ def double( def double( - a: int + a: int, ) -> ( # Hello ): return 2 * a @@ -298,7 +298,7 @@ def double( # Breaking over parameters and return types. (Black adds a trailing comma when the # function arguments break here with a single argument; we do not.) def f( - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, ) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: ... @@ -310,13 +310,13 @@ def f( def f( - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, ) -> a: ... def f( - a + a, ) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: ... @@ -334,13 +334,13 @@ def f[ def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]( - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, ) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: ... def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]( - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, ) -> a: ... @@ -380,7 +380,7 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> Set[ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" ]: @@ -388,7 +388,7 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> Set[ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" ]: @@ -396,7 +396,7 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - *args + *args, ) -> Set[ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" ]: @@ -431,13 +431,13 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: ... def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx.xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: ... @@ -457,7 +457,7 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> X and Y and foooooooooooooooooooooooooooooooooooo(): ... @@ -477,7 +477,7 @@ def xxxxxxxxxxxxxxxxxxxxxxxxxxxx() -> ( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> ( X | Y | foooooooooooooooooooooooooooooooooooo() # comment ): @@ -495,7 +495,7 @@ def double() -> ( # Dangling comments on return annotations. def double( - a: int + a: int, ) -> ( int # Hello ): @@ -503,7 +503,7 @@ def double( def double( - a: int + a: int, ) -> ( foo.bar # Hello ): @@ -511,7 +511,7 @@ def double( def double( - a: int + a: int, ) -> ( [int] # Hello ): @@ -519,7 +519,7 @@ def double( def double( - a: int + a: int, ) -> ( int | list[int] # Hello ): @@ -527,7 +527,7 @@ def double( def double( - a: int + a: int, ) -> ( int | list[ @@ -608,7 +608,7 @@ def process_board_action( @@ -95,50 +89,44 @@ # function arguments break here with a single argument; we do not.) def f( - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, -) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: - ... +) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: ... @@ -622,14 +622,14 @@ def process_board_action( def f( - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, -) -> a: - ... +) -> a: ... def f( - a + a, -) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: - ... +) -> ( @@ -652,14 +652,14 @@ def process_board_action( def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]( - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, -) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: - ... +) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: ... def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]( - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, -) -> a: - ... +) -> a: ... @@ -703,7 +703,7 @@ def process_board_action( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> Set[ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" -]: @@ -712,7 +712,7 @@ def process_board_action( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> Set[ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" -]: @@ -721,7 +721,7 @@ def process_board_action( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - *args + *args, ) -> Set[ "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" -]: @@ -761,14 +761,14 @@ def process_board_action( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, -) -> xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: - ... +) -> xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: ... def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, -) -> xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx.xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: - ... +) -> ( @@ -786,7 +786,7 @@ def process_board_action( -def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(x) -> X + Y + foooooooooooooooooooooooooooooooooooo(): - ... +def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( -+ x ++ x, +) -> X + Y + foooooooooooooooooooooooooooooooooooo(): ... @@ -798,7 +798,7 @@ def process_board_action( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, -) -> X and Y and foooooooooooooooooooooooooooooooooooo(): - ... +) -> X and Y and foooooooooooooooooooooooooooooooooooo(): ... @@ -814,7 +814,7 @@ def process_board_action( -def xxxxxxxxxxxxxxxxxxxxxxxxxxxx(x) -> X | Y | foooooooooooooooooooooooooooooooooooo(): - ... +def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( -+ x ++ x, +) -> X | Y | foooooooooooooooooooooooooooooooooooo(): ... @@ -826,7 +826,7 @@ def process_board_action( def xxxxxxxxxxxxxxxxxxxxxxxxxxxx( - x + x, ) -> ( X | Y | foooooooooooooooooooooooooooooooooooo() # comment -):