Skip to content

Commit

Permalink
Prefer breaking return-type over parameters
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Nov 7, 2024
1 parent 5b500b8 commit 0c27a95
Show file tree
Hide file tree
Showing 6 changed files with 387 additions and 55 deletions.
65 changes: 41 additions & 24 deletions crates/ruff_python_formatter/src/other/parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ use crate::comments::{
trailing_comments, SourceComment,
};
use crate::context::{NodeLevel, WithNodeLevel};
use crate::expression::parentheses::empty_parenthesized;
use crate::expression::parentheses::{empty_parenthesized, parenthesized};
use crate::prelude::*;
use crate::preview::is_empty_parameters_no_unnecessary_parentheses_around_return_value_enabled;

#[derive(Debug, Copy, Clone, Eq, PartialEq, Default)]
pub enum ParametersParentheses {
Expand Down Expand Up @@ -249,30 +250,46 @@ impl FormatNodeRule<Parameters> for FormatParameters {
} else if num_parameters == 1 && posonlyargs.is_empty() && kwonlyargs.is_empty() {
// 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(")")
]
)
if is_empty_parameters_no_unnecessary_parentheses_around_return_value_enabled(
f.context(),
) {
parenthesized("(", &format_inner, ")")
.with_dangling_comments(parenthesis_dangling)
.fmt(f)
} else {
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
// format them "inline" here.
let mut f = WithNodeLevel::new(NodeLevel::ParenthesizedExpression, f);
write!(
f,
[
token("("),
dangling_open_parenthesis_comments(parenthesis_dangling),
soft_block_indent(&group(&format_inner)),
token(")")
]
)
if is_empty_parameters_no_unnecessary_parentheses_around_return_value_enabled(
f.context(),
) {
parenthesized("(", &group(&format_inner), ")")
.with_dangling_comments(parenthesis_dangling)
.fmt(f)
} else {
// Intentionally avoid `parenthesized`, which groups the entire formatted contents.
// We want parameters to be grouped alongside return types, one level up, so we
// format them "inline" here.
let mut f = WithNodeLevel::new(NodeLevel::ParenthesizedExpression, f);
write!(
f,
[
token("("),
dangling_open_parenthesis_comments(parenthesis_dangling),
soft_block_indent(&group(&format_inner)),
token(")")
]
)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/ruff_python_formatter/tests/fixtures.rs
input_file: crates/ruff_python_formatter/resources/test/fixtures/black/cases/funcdef_return_type_trailing_comma.py
snapshot_kind: text
---
## Input

Expand Down Expand Up @@ -155,31 +156,38 @@ def SimplePyFn(
```diff
--- Black
+++ Ruff
@@ -36,7 +36,9 @@
@@ -62,9 +62,9 @@
# long function definition, return type is longer
# this should maybe split on rhs?
-def aaaaaaaaaaaaaaaaa(
- bbbbbbbbbbbbbbbbbb,
-) -> list[Ccccccccccccccccccccccccccccccccccccccccccccccccccc, Dddddd]: ...
+def aaaaaaaaaaaaaaaaa(bbbbbbbbbbbbbbbbbb) -> list[
+ Ccccccccccccccccccccccccccccccccccccccccccccccccccc, Dddddd
+]: ...
# magic trailing comma in return type, params
-def foo(a: A, b: B) -> list[
+def foo(
+ a: A, b: B
+) -> list[
p,
q,
]:
@@ -93,7 +95,11 @@
# long return type, no param list
@@ -93,13 +93,13 @@
# unskippable type hint (??)
-def foo(a) -> list[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]: # type: ignore
+def foo(
+ a,
+) -> list[
+def foo(a) -> list[
+ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+]: # type: ignore
pass
@@ -112,7 +118,13 @@
-def foo(
- a,
-) -> list[
+def foo(a) -> list[
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
]: # abpedeifnore
pass
@@ -112,7 +112,13 @@
# don't lose any comments (no magic)
Expand All @@ -194,7 +202,7 @@ def SimplePyFn(
... # 6
@@ -120,12 +132,18 @@
@@ -120,12 +126,18 @@
def foo( # 1
a, # 2
b,
Expand Down Expand Up @@ -258,9 +266,7 @@ def a() -> tuple[
# magic trailing comma in return type, params
def foo(
a: A, b: B
) -> list[
def foo(a: A, b: B) -> list[
p,
q,
]:
Expand All @@ -286,9 +292,9 @@ def aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(
# long function definition, return type is longer
# this should maybe split on rhs?
def aaaaaaaaaaaaaaaaa(
bbbbbbbbbbbbbbbbbb,
) -> list[Ccccccccccccccccccccccccccccccccccccccccccccccccccc, Dddddd]: ...
def aaaaaaaaaaaaaaaaa(bbbbbbbbbbbbbbbbbb) -> list[
Ccccccccccccccccccccccccccccccccccccccccccccccccccc, Dddddd
]: ...
# long return type, no param list
Expand Down Expand Up @@ -317,17 +323,13 @@ def thiiiiiiiiiiiiiiiiiis_iiiiiiiiiiiiiiiiiiiiiiiiiiiiiis_veeeeeeeeeeeeeeeeeeeee
# unskippable type hint (??)
def foo(
a,
) -> list[
def foo(a) -> list[
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
]: # type: ignore
pass
def foo(
a,
) -> list[
def foo(a) -> list[
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
]: # abpedeifnore
pass
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
---
source: crates/ruff_python_formatter/tests/fixtures.rs
input_file: crates/ruff_python_formatter/resources/test/fixtures/black/cases/typed_params_trailing_comma.py
snapshot_kind: text
---
## Input

```python
def long_function_name_goes_here(
x: Callable[List[int]]
) -> Union[List[int], float, str, bytes, Tuple[int]]:
pass
def long_function_name_goes_here(
x: Callable[[str, Any], int]
) -> Union[List[int], float, str, bytes, Tuple[int]]:
pass
```

## Black Differences

```diff
--- Black
+++ Ruff
@@ -1,10 +1,10 @@
-def long_function_name_goes_here(
- x: Callable[List[int]],
-) -> Union[List[int], float, str, bytes, Tuple[int]]:
+def long_function_name_goes_here(x: Callable[List[int]]) -> Union[
+ List[int], float, str, bytes, Tuple[int]
+]:
pass
-def long_function_name_goes_here(
- x: Callable[[str, Any], int],
-) -> Union[List[int], float, str, bytes, Tuple[int]]:
+def long_function_name_goes_here(x: Callable[[str, Any], int]) -> Union[
+ List[int], float, str, bytes, Tuple[int]
+]:
pass
```

## Ruff Output

```python
def long_function_name_goes_here(x: Callable[List[int]]) -> Union[
List[int], float, str, bytes, Tuple[int]
]:
pass
def long_function_name_goes_here(x: Callable[[str, Any], int]) -> Union[
List[int], float, str, bytes, Tuple[int]
]:
pass
```

## Black Output

```python
def long_function_name_goes_here(
x: Callable[List[int]],
) -> Union[List[int], float, str, bytes, Tuple[int]]:
pass
def long_function_name_goes_here(
x: Callable[[str, Any], int],
) -> Union[List[int], float, str, bytes, Tuple[int]]:
pass
```
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/ruff_python_formatter/tests/fixtures.rs
input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/docstring_code_examples_dynamic_line_width.py
snapshot_kind: text
---
## Input
```python
Expand Down Expand Up @@ -2831,6 +2832,19 @@ def length_rst_in_section():
```diff
--- Stable
+++ Preview
@@ -676,9 +676,9 @@
... def __init__(self, df: pl.DataFrame):
... self._df = df
...
- ... def by_first_letter_of_column_values(
- ... self, col: str
- ... ) -> list[pl.DataFrame]:
+ ... def by_first_letter_of_column_values(self, col: str) -> list[
+ ... pl.DataFrame
+ ... ]:
... return [
... self._df.filter(pl.col(col).str.starts_with(c))
... for c in sorted(
@@ -700,9 +700,11 @@

Examples
Expand Down
Loading

0 comments on commit 0c27a95

Please sign in to comment.