Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefer breaking return-type over parameters #14148

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading