mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-18 01:20:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Don't reorder parameters in function calls (#7268)

Browse source 
## Summary

In `f(*args, a=b, *args2, **kwargs)` the args (`*args`, `*args2`) and
keywords (`a=b`, `**kwargs`) are interleaved, which we previously didn't
handle.

Fixes #6498

**main**

| project | similarity index | total files | changed files |

|--------------|------------------:|------------------:|------------------:|
| cpython | 0.76083 | 1789 | 1632 |
| **django** | 0.99966 | 2760 | 58 |
| transformers | 0.99930 | 2587 | 447 |
| twine | 1.00000 | 33 | 0 |
| typeshed | 0.99983 | 3496 | 18 |
| warehouse | 0.99825 | 648 | 22 |
| zulip | 0.99950 | 1437 | 27 |

**PR**

| project | similarity index | total files | changed files |

|--------------|------------------:|------------------:|------------------:|
| cpython | 0.76083 | 1789 | 1632 |
| **django** | 0.99967 | 2760 | 53 |
| transformers | 0.99930 | 2587 | 447 |
| twine | 1.00000 | 33 | 0 |
| typeshed | 0.99983 | 3496 | 18 |
| warehouse | 0.99825 | 648 | 22 |
| zulip | 0.99950 | 1437 | 27 |


## Test Plan

New fixtures
This commit is contained in:
konsti 2023-09-13 11:01:49 +02:00 • committed by GitHub
parent 56440ad835
commit f4c7bff36b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 203 additions and 129 deletions
Download patch file Download diff file Expand all files Collapse all files

206
crates/ruff_python_formatter/src/statement/stmt_function_def.rs
View file

@ -18,14 +18,9 @@ pub struct FormatStmtFunctionDef;
impl FormatNodeRule<StmtFunctionDef> for FormatStmtFunctionDef {
fn fmt_fields(&self, item: &StmtFunctionDef, f: &mut PyFormatter) -> FormatResult<()> {
let StmtFunctionDef {
range: _,
is_async,
decorator_list,
name,
type_params,
parameters,
returns,
body,
..
} = item;
let comments = f.context().comments().clone();
@ -47,101 +42,7 @@ impl FormatNodeRule<StmtFunctionDef> for FormatStmtFunctionDef {
clause_header(
ClauseHeader::Function(item),
trailing_definition_comments,
&format_with(|f| {
if *is_async {
write!(f, [token("async"), space()])?;
}
write!(f, [token("def"), space(), name.format()])?;
if let Some(type_params) = type_params.as_ref() {
write!(f, [type_params.format()])?;
}
let format_inner = format_with(|f: &mut PyFormatter| {
write!(f, [parameters.format()])?;
if let Some(return_annotation) = returns.as_ref() {
write!(f, [space(), token("->"), space()])?;
if return_annotation.is_tuple_expr() {
let parentheses =
if comments.has_leading(return_annotation.as_ref()) {
Parentheses::Always
} else {
Parentheses::Never
};
write!(
f,
[return_annotation.format().with_options(parentheses)]
)?;
} else if comments.has_trailing(return_annotation.as_ref()) {
// Intentionally parenthesize any return annotations with trailing comments.
// This avoids an instability in cases like:
// ```python
// def double(
// a: int
// ) -> (
// int # Hello
// ):
// pass
// ```
// If we allow this to break, it will be formatted as follows:
// ```python
// def double(
// a: int
// ) -> int: # Hello
// pass
// ```
// On subsequent formats, the `# Hello` will be interpreted as a dangling
// comment on a function, yielding:
// ```python
// def double(a: int) -> int: # Hello
// pass
// ```
// Ideally, we'd reach that final formatting in a single pass, but doing so
// requires that the parent be aware of how the child is formatted, which
// is challenging. As a compromise, we break those expressions to avoid an
// instability.
write!(
f,
[return_annotation
.format()
.with_options(Parentheses::Always)]
)?;
} else {
write!(
f,
[maybe_parenthesize_expression(
return_annotation,
item,
if empty_parameters(parameters, f.context().source()) {
// If the parameters are empty, add parentheses if the return annotation
// breaks at all.
Parenthesize::IfBreaksOrIfRequired
} else {
// Otherwise, use our normal rules for parentheses, which allows us to break
// like:
// ```python
// def f(
// x,
// ) -> Tuple[
// int,
// int,
// ]:
// ...
// ```
Parenthesize::IfBreaks
},
)]
)?;
}
}
Ok(())
});
group(&format_inner).fmt(f)
}),
&format_with(|f| format_function_header(f, item)),
),
clause_body(body, trailing_definition_comments).with_kind(SuiteKind::Function),
]
@ -176,6 +77,109 @@ impl FormatNodeRule<StmtFunctionDef> for FormatStmtFunctionDef {
}
}
fn format_function_header(f: &mut PyFormatter, item: &StmtFunctionDef) -> FormatResult<()> {
let StmtFunctionDef {
range: _,
is_async,
decorator_list: _,
name,
type_params,
parameters,
returns,
body: _,
} = item;
let comments = f.context().comments().clone();
if *is_async {
write!(f, [token("async"), space()])?;
}
write!(f, [token("def"), space(), name.format()])?;
if let Some(type_params) = type_params.as_ref() {
write!(f, [type_params.format()])?;
}
let format_inner = format_with(|f: &mut PyFormatter| {
write!(f, [parameters.format()])?;
if let Some(return_annotation) = returns.as_ref() {
write!(f, [space(), token("->"), space()])?;
if return_annotation.is_tuple_expr() {
let parentheses = if comments.has_leading(return_annotation.as_ref()) {
Parentheses::Always
} else {
Parentheses::Never
};
write!(f, [return_annotation.format().with_options(parentheses)])?;
} else if comments.has_trailing(return_annotation.as_ref()) {
// Intentionally parenthesize any return annotations with trailing comments.
// This avoids an instability in cases like:
// ```python
// def double(
// a: int
// ) -> (
// int # Hello
// ):
// pass
// ```
// If we allow this to break, it will be formatted as follows:
// ```python
// def double(
// a: int
// ) -> int: # Hello
// pass
// ```
// On subsequent formats, the `# Hello` will be interpreted as a dangling
// comment on a function, yielding:
// ```python
// def double(a: int) -> int: # Hello
// pass
// ```
// Ideally, we'd reach that final formatting in a single pass, but doing so
// requires that the parent be aware of how the child is formatted, which
// is challenging. As a compromise, we break those expressions to avoid an
// instability.
write!(
f,
[return_annotation.format().with_options(Parentheses::Always)]
)?;
} else {
write!(
f,
[maybe_parenthesize_expression(
return_annotation,
item,
if empty_parameters(parameters, f.context().source()) {
// If the parameters are empty, add parentheses if the return annotation
// breaks at all.
Parenthesize::IfBreaksOrIfRequired
} else {
// Otherwise, use our normal rules for parentheses, which allows us to break
// like:
// ```python
// def f(
// x,
// ) -> Tuple[
// int,
// int,
// ]:
// ...
// ```
Parenthesize::IfBreaks
},
)]
)?;
}
}
Ok(())
});
group(&format_inner).fmt(f)
}
/// Returns `true` if [`Parameters`] is empty (no parameters, no comments, etc.).
fn empty_parameters(parameters: &Parameters, source: &str) -> bool {
let mut tokenizer = SimpleTokenizer::new(source, parameters.range())