Skip to content

Commit

Permalink
Insert trailing comma when function breaks with single argument (#8921)
Browse files Browse the repository at this point in the history
## 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 #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 |
  • Loading branch information
charliermarsh authored Dec 1, 2023
1 parent 019d9ae commit eaa3104
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 66 deletions.
13 changes: 13 additions & 0 deletions crates/ruff_python_formatter/src/other/parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 @@
Expand Down Expand Up @@ -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]: ...
Expand Down Expand Up @@ -366,15 +354,15 @@ def thiiiiiiiiiiiiiiiiiis_iiiiiiiiiiiiiiiiiiiiiiiiiiiiiis_veeeeeeeeeeeeeeeeeeeee
# unskippable type hint (??)
def foo(
a
a,
) -> list[
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
]: # type: ignore
pass
def foo(
a
a,
) -> list[
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
]: # abpedeifnore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 |"
Expand All @@ -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
Expand All @@ -49,7 +42,7 @@ def frobnicate() -> (
# splitting the string breaks if there's any parameters
def frobnicate(
a
a,
) -> "ThisIsTrulyUnreasonablyExtremelyLongClassName | list[ThisIsTrulyUnreasonablyExtremelyLongClassName]":
pass
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
+):
Expand All @@ -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
+):
Expand Down Expand Up @@ -168,15 +168,15 @@ def double(a: int) -> int:
# Don't lose the comments
def double(
a: int
a: int,
) -> ( # Hello
int
):
return 2 * a
def double(
a: int
a: int,
) -> (
int # Hello
):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ except ( # d 9
def e1( # e 1
x
x,
):
pass
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,7 @@ def f( # first


def f( # first
a
a,
): # second
...

Expand Down
Loading

0 comments on commit eaa3104

Please sign in to comment.