mirror of https://github.com/astral-sh/ruff
Fix comment placement in lambda parameters (#21868)
Summary
--
This PR makes two changes to comment placement in lambda parameters.
First, we
now insert a line break if the first parameter has a leading comment:
```py
# input
(
lambda
* # comment 2
x:
x
)
# main
(
lambda # comment 2
*x: x
)
# this PR
(
lambda
# comment 2
*x: x
)
```
Note the missing space in the output from main. This case is currently
unstable
on main. Also note that the new formatting is more consistent with our
stable
formatting in cases where the lambda has its own dangling comment:
```py
# input
(
lambda # comment 1
* # comment 2
x:
x
)
# output
(
lambda # comment 1
# comment 2
*x: x
)
```
and when a parameter without a comment precedes the split `*x`:
```py
# input
(
lambda y,
* # comment 2
x:
x
)
# output
(
lambda y,
# comment 2
*x: x
)
```
This does change the stable formatting, but I think such cases are rare
(expecting zero hits in the ecosystem report), this fixes an existing
instability, and it should not change any code we've previously
formatted.
Second, this PR modifies the comment placement such that `# comment 2`
in these
outputs is still a leading comment on the parameter. This is also not
the case
on main, where it becomes a [dangling lambda
comment](https://play.ruff.rs/3b29bb7e-70e4-4365-88e0-e60fe1857a35?secondary=Comments).
This doesn't cause any
instability that I'm aware of on main, but it does cause problems when
trying to
adjust the placement of dangling lambda comments in #21385. Changing the
placement in this way should not affect any formatting here.
Test Plan
--
New lambda tests, plus existing tests covering the cases above with
multiple
comments around the parameters (see lambda.py 122-143, and 122-205 or so
more
broadly)
I also checked manually that the comments are now leading on the
parameter:
```shell
❯ cargo run --bin ruff_python_formatter -- --emit stdout --target-version 3.10 --print-comments <<EOF
(
lambda
# comment 2
*x: x
)
EOF
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.15s
Running `target/debug/ruff_python_formatter --emit stdout --target-version 3.10 --print-comments`
# Comment decoration: Range, Preceding, Following, Enclosing, Comment
21..32, None, Some((Parameters, 37..39)), (ExprLambda, 6..42), "# comment 2"
{
Node {
kind: Parameter,
range: 37..39,
source: `*x`,
}: {
"leading": [
SourceComment {
text: "# comment 2",
position: OwnLine,
formatted: true,
},
],
"dangling": [],
"trailing": [],
},
}
(
lambda
# comment 2
*x: x
)
```
But I didn't see a great place to put a test like this. Is there
somewhere I can assert this comment placement since it doesn't affect
any formatting yet? Or is it okay to wait until we use this in #21385?
This commit is contained in:
parent
9490fbf1e1
commit
0bec5c0362
|
|
@ -228,3 +228,24 @@ def a():
|
|||
g = 10
|
||||
)
|
||||
|
||||
(
|
||||
lambda
|
||||
* # comment 2
|
||||
x:
|
||||
x
|
||||
)
|
||||
|
||||
(
|
||||
lambda # comment 1
|
||||
* # comment 2
|
||||
x:
|
||||
x
|
||||
)
|
||||
|
||||
(
|
||||
lambda # comment 1
|
||||
y,
|
||||
* # comment 2
|
||||
x:
|
||||
x
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1816,7 +1816,7 @@ fn handle_lambda_comment<'a>(
|
|||
source: &str,
|
||||
) -> CommentPlacement<'a> {
|
||||
if let Some(parameters) = lambda.parameters.as_deref() {
|
||||
// Comments between the `lambda` and the parameters are dangling on the lambda:
|
||||
// End-of-line comments between the `lambda` and the parameters are dangling on the lambda:
|
||||
// ```python
|
||||
// (
|
||||
// lambda # comment
|
||||
|
|
@ -1824,8 +1824,24 @@ fn handle_lambda_comment<'a>(
|
|||
// y
|
||||
// )
|
||||
// ```
|
||||
//
|
||||
// But own-line comments are leading on the first parameter, if it exists:
|
||||
// ```python
|
||||
// (
|
||||
// lambda
|
||||
// # comment
|
||||
// x:
|
||||
// y
|
||||
// )
|
||||
// ```
|
||||
if comment.start() < parameters.start() {
|
||||
return CommentPlacement::dangling(comment.enclosing_node(), comment);
|
||||
return if let Some(first) = parameters.iter().next()
|
||||
&& comment.line_position().is_own_line()
|
||||
{
|
||||
CommentPlacement::leading(first.as_parameter(), comment)
|
||||
} else {
|
||||
CommentPlacement::dangling(comment.enclosing_node(), comment)
|
||||
};
|
||||
}
|
||||
|
||||
// Comments between the parameters and the body are dangling on the lambda:
|
||||
|
|
|
|||
|
|
@ -32,7 +32,69 @@ impl FormatNodeRule<ExprLambda> for FormatExprLambda {
|
|||
.split_at(dangling.partition_point(|comment| comment.end() < parameters.start()));
|
||||
|
||||
if dangling_before_parameters.is_empty() {
|
||||
write!(f, [space()])?;
|
||||
// If the first parameter has a leading comment, insert a hard line break. This
|
||||
// comment is associated as a leading comment on the first parameter:
|
||||
//
|
||||
// ```py
|
||||
// (
|
||||
// lambda
|
||||
// * # comment
|
||||
// x:
|
||||
// x
|
||||
// )
|
||||
// ```
|
||||
//
|
||||
// so a hard line break is needed to avoid formatting it like:
|
||||
//
|
||||
// ```py
|
||||
// (
|
||||
// lambda # comment
|
||||
// *x: x
|
||||
// )
|
||||
// ```
|
||||
//
|
||||
// which is unstable because it's missing the second space before the comment.
|
||||
//
|
||||
// Inserting the line break causes it to format like:
|
||||
//
|
||||
// ```py
|
||||
// (
|
||||
// lambda
|
||||
// # comment
|
||||
// *x :x
|
||||
// )
|
||||
// ```
|
||||
//
|
||||
// which is also consistent with the formatting in the presence of an actual
|
||||
// dangling comment on the lambda:
|
||||
//
|
||||
// ```py
|
||||
// (
|
||||
// lambda # comment 1
|
||||
// * # comment 2
|
||||
// x:
|
||||
// x
|
||||
// )
|
||||
// ```
|
||||
//
|
||||
// formats to:
|
||||
//
|
||||
// ```py
|
||||
// (
|
||||
// lambda # comment 1
|
||||
// # comment 2
|
||||
// *x: x
|
||||
// )
|
||||
// ```
|
||||
if parameters
|
||||
.iter()
|
||||
.next()
|
||||
.is_some_and(|parameter| comments.has_leading(parameter.as_parameter()))
|
||||
{
|
||||
hard_line_break().fmt(f)?;
|
||||
} else {
|
||||
write!(f, [space()])?;
|
||||
}
|
||||
} else {
|
||||
write!(f, [dangling_comments(dangling_before_parameters)])?;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,7 +1,6 @@
|
|||
---
|
||||
source: crates/ruff_python_formatter/tests/fixtures.rs
|
||||
input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/lambda.py
|
||||
snapshot_kind: text
|
||||
---
|
||||
## Input
|
||||
```python
|
||||
|
|
@ -235,6 +234,27 @@ def a():
|
|||
g = 10
|
||||
)
|
||||
|
||||
(
|
||||
lambda
|
||||
* # comment 2
|
||||
x:
|
||||
x
|
||||
)
|
||||
|
||||
(
|
||||
lambda # comment 1
|
||||
* # comment 2
|
||||
x:
|
||||
x
|
||||
)
|
||||
|
||||
(
|
||||
lambda # comment 1
|
||||
y,
|
||||
* # comment 2
|
||||
x:
|
||||
x
|
||||
)
|
||||
```
|
||||
|
||||
## Output
|
||||
|
|
@ -473,4 +493,24 @@ def a():
|
|||
g=2: d,
|
||||
g=10,
|
||||
)
|
||||
|
||||
|
||||
(
|
||||
lambda
|
||||
# comment 2
|
||||
*x: x
|
||||
)
|
||||
|
||||
(
|
||||
lambda # comment 1
|
||||
# comment 2
|
||||
*x: x
|
||||
)
|
||||
|
||||
(
|
||||
lambda # comment 1
|
||||
y,
|
||||
# comment 2
|
||||
*x: x
|
||||
)
|
||||
```
|
||||
|
|
|
|||
Loading…
Reference in New Issue