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 |
This commit is contained in:
Charlie Marsh 2023-11-30 21:49:28 -05:00 committed by GitHub
parent 019d9aebe9
commit eaa310429f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 60 additions and 66 deletions

View file

@ -252,6 +252,19 @@ impl FormatNodeRule<Parameters> 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

View file

@ -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

View file

@ -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
```

View file

@ -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
):

View file

@ -253,7 +253,7 @@ except ( # d 9
def e1( # e 1
x
x,
):
pass

View file

@ -928,7 +928,7 @@ def f( # first
def f( # first
a
a,
): # second
...

View file

@ -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
-):