Skip to content

Commit 3bf1c66

Browse files
charliermarshcharliermarsh
authored
Group function definition parameters with return type annotations (#6410)
## Summary This PR removes the group around function definition parameters, instead grouping the parameters with the type parameters and return type annotation. This increases Zulip's similarity score from 0.99385 to 0.99699, so it's a meaningful improvement. However, there's at least one stability error that I'm working on, and I'm really just looking for high-level feedback at this point, because I'm not happy with the solution. Closes #6352. ## Test Plan Before: - `zulip`: 0.99396 - `django`: 0.99784 - `warehouse`: 0.99578 - `build`: 0.75436 - `transformers`: 0.99407 - `cpython`: 0.75987 - `typeshed`: 0.74432 After: - `zulip`: 0.99702 - `django`: 0.99784 - `warehouse`: 0.99585 - `build`: 0.75623 - `transformers`: 0.99470 - `cpython`: 0.75988 - `typeshed`: 0.74853
1 parent eaada03 commit 3bf1c66

File tree

5 files changed

+147
-27
lines changed

5 files changed

+147
-27
lines changed

‎crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/function.py

+26
Original file line numberDiff line numberDiff line change
@@ -409,3 +409,29 @@ def double(a: int) -> ( # Hello
409409
def double(a: int) -> ( # Hello
410410
):
411411
return 2*a
412+
413+
# Breaking over parameters and return types. (Black adds a trailing comma when the
414+
# function arguments break here with a single argument; we do not.)
415+
def f(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
416+
...
417+
418+
def f(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, a) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
419+
...
420+
421+
def f(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) -> a:
422+
...
423+
424+
def f(a) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
425+
...
426+
427+
def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]() -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
428+
...
429+
430+
def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]() -> a:
431+
...
432+
433+
def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa](aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
434+
...
435+
436+
def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa](aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) -> a:
437+
...

‎crates/ruff_python_formatter/src/other/parameters.rs

+13-5
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,15 @@ use ruff_python_trivia::{SimpleToken, SimpleTokenKind, SimpleTokenizer};
77
use ruff_text_size::{TextRange, TextSize};
88

99
use crate::comments::{
10-
leading_comments, leading_node_comments, trailing_comments, CommentLinePosition, SourceComment,
10+
dangling_open_parenthesis_comments, leading_comments, leading_node_comments, trailing_comments,
11+
CommentLinePosition, SourceComment,
1112
};
1213
use crate::context::{NodeLevel, WithNodeLevel};
13-
use crate::expression::parentheses::{empty_parenthesized, parenthesized};
14+
use crate::expression::parentheses::empty_parenthesized;
1415
use crate::prelude::*;
1516
use crate::FormatNodeRule;
1617

17-
#[derive(Eq, PartialEq, Debug, Default)]
18+
#[derive(Debug, Copy, Clone, Eq, PartialEq, Default)]
1819
pub enum ParametersParentheses {
1920
/// By default, parameters will always preserve their surrounding parentheses.
2021
#[default]
@@ -246,10 +247,17 @@ impl FormatNodeRule<Parameters> for FormatParameters {
246247
// No parameters, format any dangling comments between `()`
247248
write!(f, [empty_parenthesized("(", dangling, ")")])
248249
} else {
250+
// Intentionally avoid `parenthesized`, which groups the entire formatted contents.
251+
// We want parameters to be grouped alongside return types, one level up, so we
252+
// format them "inline" here.
249253
write!(
250254
f,
251-
[parenthesized("(", &group(&format_inner), ")")
252-
.with_dangling_comments(parenthesis_dangling)]
255+
[
256+
text("("),
257+
dangling_open_parenthesis_comments(parenthesis_dangling),
258+
soft_block_indent(&group(&format_inner)),
259+
text(")")
260+
]
253261
)
254262
}
255263
}

‎crates/ruff_python_formatter/src/statement/stmt_function_def.rs

+21-17
Original file line numberDiff line numberDiff line change
@@ -58,24 +58,28 @@ impl FormatNodeRule<StmtFunctionDef> for FormatStmtFunctionDef {
5858
write!(f, [type_params.format()])?;
5959
}
6060

61-
write!(f, [item.parameters.format()])?;
62-
63-
if let Some(return_annotation) = item.returns.as_ref() {
64-
write!(f, [space(), text("->"), space()])?;
65-
if return_annotation.is_tuple_expr() {
66-
write!(
67-
f,
68-
[return_annotation.format().with_options(Parentheses::Never)]
69-
)?;
70-
} else {
71-
write!(
72-
f,
73-
[optional_parentheses(
74-
&return_annotation.format().with_options(Parentheses::Never),
75-
)]
76-
)?;
61+
let format_inner = format_with(|f: &mut PyFormatter| {
62+
write!(f, [item.parameters.format()])?;
63+
if let Some(return_annotation) = item.returns.as_ref() {
64+
write!(f, [space(), text("->"), space()])?;
65+
if return_annotation.is_tuple_expr() {
66+
write!(
67+
f,
68+
[return_annotation.format().with_options(Parentheses::Never)]
69+
)?;
70+
} else {
71+
write!(
72+
f,
73+
[optional_parentheses(
74+
&return_annotation.format().with_options(Parentheses::Never),
75+
)]
76+
)?;
77+
}
7778
}
78-
}
79+
Ok(())
80+
});
81+
82+
write!(f, [group(&format_inner)])?;
7983

8084
write!(
8185
f,

‎crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__return_annotation_brackets.py.snap

+8-4
Original file line numberDiff line numberDiff line change
@@ -100,18 +100,20 @@ def foo() -> tuple[int, int, int,]:
100100
```diff
101101
--- Black
102102
+++ Ruff
103-
@@ -26,7 +26,9 @@
103+
@@ -26,7 +26,11 @@
104104
return 2 * a
105105
106106
107107
-def double(a: int) -> int: # Hello
108-
+def double(a: int) -> (
108+
+def double(
109+
+ a: int
110+
+) -> (
109111
+ int # Hello
110112
+):
111113
return 2 * a
112114
113115
114-
@@ -54,7 +56,9 @@
116+
@@ -54,7 +58,9 @@
115117
a: int,
116118
b: int,
117119
c: int,
@@ -155,7 +157,9 @@ def double(a: int) -> int: # Hello
155157
return 2 * a
156158
157159
158-
def double(a: int) -> (
160+
def double(
161+
a: int
162+
) -> (
159163
int # Hello
160164
):
161165
return 2 * a

‎crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap

+79-1
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,32 @@ def double(a: int) -> ( # Hello
415415
def double(a: int) -> ( # Hello
416416
):
417417
return 2*a
418+
419+
# Breaking over parameters and return types. (Black adds a trailing comma when the
420+
# function arguments break here with a single argument; we do not.)
421+
def f(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
422+
...
423+
424+
def f(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, a) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
425+
...
426+
427+
def f(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) -> a:
428+
...
429+
430+
def f(a) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
431+
...
432+
433+
def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]() -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
434+
...
435+
436+
def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]() -> a:
437+
...
438+
439+
def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa](aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
440+
...
441+
442+
def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa](aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) -> a:
443+
...
418444
```
419445

420446
## Output
@@ -1023,9 +1049,61 @@ def double(a: int) -> int: # Hello
10231049
return 2 * a
10241050

10251051

1026-
def double(a: int) -> ( # Hello
1052+
def double(
1053+
a: int
1054+
) -> ( # Hello
10271055
):
10281056
return 2 * a
1057+
1058+
1059+
# Breaking over parameters and return types. (Black adds a trailing comma when the
1060+
# function arguments break here with a single argument; we do not.)
1061+
def f(
1062+
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
1063+
) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
1064+
...
1065+
1066+
1067+
def f(
1068+
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, a
1069+
) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
1070+
...
1071+
1072+
1073+
def f(
1074+
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
1075+
) -> a:
1076+
...
1077+
1078+
1079+
def f(
1080+
a
1081+
) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
1082+
...
1083+
1084+
1085+
def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]() -> (
1086+
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
1087+
):
1088+
...
1089+
1090+
1091+
def f[
1092+
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
1093+
]() -> a:
1094+
...
1095+
1096+
1097+
def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa](
1098+
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
1099+
) -> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
1100+
...
1101+
1102+
1103+
def f[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa](
1104+
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
1105+
) -> a:
1106+
...
10291107
```
10301108

10311109

0 commit comments

Comments
 (0)