Commit Graph

448 Commits

Author SHA1 Message Date
Brent Westbrook 553b45e27f
move dangling comment handling back out of placement.rs
Revert "re-apply 'pass preview to handle_lambda_comment'"

This reverts commit 33fcca9c53.
2025-12-10 15:58:48 -05:00
Brent Westbrook b96cf96e8b
add a few more call tests with comments 2025-12-10 14:06:08 -05:00
Brent Westbrook 9f9b76b035
move preview comment handling mostly into placement.rs 2025-12-10 09:38:15 -05:00
Brent Westbrook bb053f8388
fix lambda formatting in assignments 2025-12-10 08:51:54 -05:00
Brent Westbrook b823866155
revert preview and comment placement changes 2025-12-09 16:34:01 -05:00
Brent Westbrook fd34cd6042
update snaps 2025-12-09 15:56:20 -05:00
Brent Westbrook 65c943568c
Merge branch 'brent/fix-kwargs' into brent/indent-lambda-params 2025-12-09 15:42:05 -05:00
Brent Westbrook b0a82983af
avoid breaking when the first parameter has leading comments 2025-12-09 15:32:24 -05:00
Brent Westbrook 90f43bde84
add broken test cases
the new leading comment is causing the whole Parameters list to break. these
cases should instead format like:

```py
(
    lambda
    # comment
    *x, **y: x
)

(
    lambda
    # comment 2
    *x, **y: x
)
```

without line breaks in the parameter list
2025-12-09 15:06:48 -05:00
Brent Westbrook 4ffbd496e3
Merge branch 'main' into brent/indent-lambda-params 2025-12-09 14:34:28 -05:00
Brent Westbrook 0bec5c0362
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?
2025-12-09 14:07:48 -05:00
Brent Westbrook 21b442a4fc
accept snapshots 2025-12-09 08:53:53 -05:00
Brent Westbrook e8540d9b08
format new dangling comments 2025-12-08 12:56:57 -05:00
Brent Westbrook 8ede14a083
move comments within lambda parameters to dangling lambda comments 2025-12-08 12:29:37 -05:00
Brent Westbrook f20f3e0d49
fix assignment instability without parameters too 2025-12-05 16:08:37 -05:00
Brent Westbrook 0710e0bc3e
fix assignment instability with dangling comments 2025-12-05 16:03:46 -05:00
Brent Westbrook 1531c94b4e
revert the last two commits, back to a stable formatting 2025-12-05 15:44:21 -05:00
Brent Westbrook 86406c0bb1
wip 2025-12-05 15:41:49 -05:00
Brent Westbrook 8cbe03b318
add more cases without parameters 2025-12-05 13:58:06 -05:00
Brent Westbrook 25d70b408b
more tests 2025-12-05 12:50:18 -05:00
Brent Westbrook 5605387a77
add another dangling case between lambda and parameters 2025-12-05 10:22:32 -05:00
Brent Westbrook 43b53edcab
improve dangling header comment placement 2025-12-05 09:21:48 -05:00
Brent Westbrook a4b4a82e61
add another dangling eol case 2025-12-05 08:34:55 -05:00
Brent Westbrook dfd3460c7a
add some more tests 2025-12-04 18:02:52 -05:00
Brent Westbrook 3a20c6f196
copy mapper test case from can_omit_optional_parentheses 2025-12-04 10:25:13 -05:00
Brent Westbrook 2e84402f69
add comments and some supporting tests 2025-12-04 10:08:46 -05:00
Brent Westbrook 04963a6b6b
expand parent if the lambda body breaks 2025-12-03 14:16:16 -05:00
Brent Westbrook 258b1fd7eb
add wrapping case from the ecosystem check
the lambda is hugging its enclosing parentheses when it shouldn't be. there
seems to be an issue with `best_fitting!` because moving any of the options
we're passing to it out of `best_fitting!` avoids this behavior.

IR:

```
[
  source_position(0),
  source_position(1),
  "[",
  group(expand: propagated, [
    indent([
      soft_line_break,
      "(",
      group([
        indent([
          soft_line_break,
          "lambda ",
          group(["eval_df, _"]),
          ": ",
          best_fitting([
            [
              [
                <interned 0> [
                  "MetricValue(",
                  group(expand: propagated, [
                    indent([
                      soft_line_break,
                      group(expand: propagated, [
                        "scores=eval_df[",
                        group([
                          indent([soft_line_break, "\"prediction\""]),
                          soft_line_break
                        ]),
                        "].tolist",
                        group(["()"]),
                        ",",
                        soft_line_break_or_space,
                        "aggregate_results={",
                        group([
                          indent([
                            soft_line_break,
                            group([
                              "\"prediction_sum\": sum(",
                              group([
                                indent([
                                  soft_line_break,
                                  group([
                                    "eval_df[",
                                    group([
                                      indent([soft_line_break, "\"prediction\""]),
                                      soft_line_break
                                    ]),
                                    "]"
                                  ])
                                ]),
                                soft_line_break
                              ]),
                              ")"
                            ])
                          ]),
                          soft_line_break
                        ]),
                        "}",
                        if_group_breaks([","]),
                        expand_parent
                      ])
                    ]),
                    soft_line_break
                  ]),
                  ")"
                ]
              ]
            ]
            [[group(expand: true, [<ref interned *0>])]]
            [
              [
                "(",
                indent([hard_line_break, <ref interned *0>]),
                hard_line_break,
                ")"
              ]
            ]
          ])
        ]),
        soft_line_break
      ]),
      ")",
      if_group_breaks([","]),
      expand_parent
    ]),
    soft_line_break
  ]),
  "]",
  source_position(196),
  hard_line_break,
  source_position(196)
]
```
2025-12-03 11:52:48 -05:00
Brent Westbrook a3400a017a
use parenthesize_if_expands for fluent call chains 2025-12-03 11:51:02 -05:00
Brent Westbrook 9ef9d0302d
fix another ecosystem call expansion 2025-12-03 10:06:48 -05:00
Brent Westbrook 6f6c09c72a
fix snapshot changes for cases with comments 2025-12-03 09:45:00 -05:00
Brent Westbrook efa372b379
apply Micha's patch, fixing everything?
Co-authored-by: Micha Reiser <micha@reiser.io>
2025-12-03 09:14:52 -05:00
Brent Westbrook 97850661fd
add too-eagerly parenthesized case from ecosystem
the initial code here is only 83 characters wide, so the call expression should
fit without wrapping the whole body in parens
2025-12-02 15:34:32 -05:00
Brent Westbrook 89dc1ada39
add a couple more test cases
I tried to get Claude to come up with tests, but most of them weren't very
interesting. I think these two additional types of assignments might be worth
having, though.
2025-12-02 11:47:41 -05:00
Brent Westbrook e9f9507dc8
add some assignment tests with parentheses and comments 2025-12-02 11:47:04 -05:00
Brent Westbrook 24e15bfd95
exclude call and subscript expressions from has_own_parentheses 2025-12-02 11:28:19 -05:00
Brent Westbrook 9db5d43e18
possibly bad test for triple-quoted f-strings
parenthesizing these seems redundant. I would prefer our old formatting more
like this:

```py
def ddb():
    sql = (
        lambda var, table, n=N: f"""
        CREATE TABLE {table} AS
        SELECT ROW_NUMBER() OVER () AS id, {var}
        FROM (
            SELECT {var}
            FROM RANGE({n}) _ ({var})
            ORDER BY RANDOM()
        )
        """
    )
```

where the `f"""` serves as the parentheses, instead of the current:

```py
def ddb():
    sql = lambda var, table, n=N: (
        f"""
        CREATE TABLE {table} AS
        SELECT ROW_NUMBER() OVER () AS id, {var}
        FROM (
            SELECT {var}
            FROM RANGE({n}) _ ({var})
            ORDER BY RANDOM()
        )
        """
    )
```
2025-12-02 11:22:55 -05:00
Brent Westbrook ad3147703d
another bad test for long bodies with their own parens
this case ends up too long at 108 columns:

```py
class C:
    def foo():
        if True:
            transaction_count = self._query_txs_for_range(
                get_count_fn=lambda from_ts, to_ts, _chain_id=chain_id: db_evmtx.count_transactions_in_range(
                    chain_id=_chain_id,
                    from_ts=from_ts,
                    to_ts=to_ts,
                ),
            )
```

instead, it should be formatted like this, fitting within 88 columns:

```py
class C:
    def foo():
        if True:
            transaction_count = self._query_txs_for_range(
                get_count_fn=lambda from_ts, to_ts, _chain_id=chain_id: (
				    db_evmtx.count_transactions_in_range(
                        chain_id=_chain_id,
                        from_ts=from_ts,
                        to_ts=to_ts,
					)
                ),
            )
```

we can fix this by removing the `has_own_parentheses` check in the new lambda
formatting, but this breaks other cases. we might want to preserve this? in this
specific ecosystem case, the project has a `noqa: E501` comment, so this seems
to be what they want anyway, although we don't know that when formatting
2025-12-02 11:22:55 -05:00
Brent Westbrook f634bb5247
propagate lambda layout for annotated assignments 2025-12-02 11:22:55 -05:00
Brent Westbrook 6b47664019
add another bad test case from the ecosystem report
I would expect this to format as:

```py
class C:
    _is_recognized_dtype: Callable[[DtypeObj], bool] = lambda x: (
        lib.is_np_dtype(x, "M") or isinstance(x, DatetimeTZDtype)
    )
```

instead of the current:

```py
class C:
    _is_recognized_dtype: Callable[[DtypeObj], bool] = (
        lambda x: lib.is_np_dtype(x, "M") or isinstance(x, DatetimeTZDtype)
    )
```
2025-12-02 11:22:55 -05:00
Brent Westbrook 07bcf41a34
fix binary expression in lambda in return 2025-12-02 11:22:55 -05:00
Brent Westbrook d68a03a519
add another bad case from the ecosystem check
this should format like:

```py
def foo():
    if True:
        if True:
            return lambda x: (
                np.exp(cs(np.log(x.to(u.MeV).value))) * u.MeV * u.cm**2 / u.g
            )
```

instead of the current snapshot:

```diff
 def foo():
     if True:
         if True:
-            return (
-                lambda x: np.exp(cs(np.log(x.to(u.MeV).value))) * u.MeV * u.cm**2 / u.g
-            )
+            return lambda x: np.exp(
+                cs(np.log(x.to(u.MeV).value))
+            ) * u.MeV * u.cm**2 / u.g
```
2025-12-02 11:22:55 -05:00
Brent Westbrook 62c968c826
rough draft of ExprLambdaLayout::Assignment 2025-12-02 11:22:55 -05:00
Brent Westbrook 74093bdb50
add a poorly formatted case from the ecosystem report
this formats as:

```py
class C:
    function_dict: Dict[Text, Callable[[CRFToken], Any]] = {
        CRFEntityExtractorOptions.POS2: lambda crf_token: crf_token.pos_tag[
            :2
        ] if crf_token.pos_tag is not None else None,
    }
```

when I think it should look like:

```py
class C:
    function_dict: Dict[Text, Callable[[CRFToken], Any]] = {
        CRFEntityExtractorOptions.POS2: lambda crf_token: (
		    crf_token.pos_tag[:2] if crf_token.pos_tag is not None else None,
		)
    }
```
2025-12-02 11:22:55 -05:00
Brent Westbrook 1b58643040
wip: parenthesize long lambda bodies 2025-12-02 11:22:55 -05:00
Brent Westbrook 8cc884428d
keep lambda parameters on a single line 2025-12-02 11:22:55 -05:00
Brent Westbrook 1e70c991a2
baseline test cases 2025-12-02 11:22:55 -05:00
Dylan 62343a101a
Respect `fmt: skip` for compound statements on single line (#20633)
Closes #11216

Essentially the approach is to implement `Format` for a new struct
`FormatClause` which is just a clause header _and_ its body. We then
have the information we need to see whether there is a skip suppression
comment on the last child in the body and it all fits on one line.
2025-11-18 12:02:09 -06:00
Brent Westbrook cbc6863b8c
Fix panic when formatting comments in unary expressions (#21501)
## Summary

This is another attempt at https://github.com/astral-sh/ruff/pull/21410
that fixes https://github.com/astral-sh/ruff/issues/19226.

@MichaReiser helped me get something working in a very helpful pairing
session. I pushed one additional commit moving the comments back from
leading comments to trailing comments, which I think retains more of the
input formatting.

I was inspired by Dylan's PR (#21185) to make one of these tables:

<table>
                <thead>
                    <tr>
                    <th scope="col">Input</th>
                    <th scope="col">Main</th>
                    <th scope="col">PR</th>
                    </tr>
                </thead>
                <tbody>
<tr>
<td><pre lang="python">
if (
    not
    # comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
    pass
</pre></td>
<td><pre lang="python">
if (
    # comment
    not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
    + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
    pass

</pre></td>
<td><pre lang="python">
if (
    not
    # comment
    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
    + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
    pass

</pre></td>
</tr>
<tr>
<td><pre lang="python">
if (
    # unary comment
    not
    # operand comment
    (
        # comment
        aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
        + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
    )
):
    pass
</pre></td>
<td><pre lang="python">
if (
    # unary comment
    # operand comment
    not (
        # comment
        aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
        + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
    )
):
    pass

</pre></td>
<td><pre lang="python">
if (
    # unary comment
    not
    # operand comment
    (
        # comment
        aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
        + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
    )
):
    pass

</pre></td>
</tr>
<tr>
<td><pre lang="python">
if (
    not # comment
    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
    + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
    pass
</pre></td>
<td><pre lang="python">
if (  # comment
    not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
    + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
    pass

</pre></td>
<td><pre lang="python">
if (
    not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa  # comment
    + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
    pass

</pre></td>
</tr>
</tbody>
            </table>

hopefully it helps even though the snippets are much wider here.

The two main differences are (1) that we now retain own-line comments
between the unary operator and its operand instead of moving these to
leading comments on the operator itself, and (2) that we move
end-of-line comments between the operator and operand to dangling
end-of-line comments on the operand (the last example in the table).

## Test Plan

Existing tests, plus new ones based on the issue. As I noted below, I
also ran the output from main on the unary.py file back through this
branch to check that we don't reformat code from main. This made me feel
a bit better about not preview-gating the changes in this PR.

```shell
> git show main:crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unary.py | ruff format - | ./target/debug/ruff format --diff -
> echo $?
0
```

---------

Co-authored-by: Micha Reiser <micha@reiser.io>
Co-authored-by: Takayuki Maeda <takoyaki0316@gmail.com>
2025-11-18 10:48:14 -05:00
Dylan 8156b45173
Avoid syntax error when formatting attribute expressions with outer parentheses, parenthesized value, and trailing comment on value (#20418)
Closes #19350 

This fixes a syntax error caused by formatting. However, the new tests reveal that there are some cases where formatting attributes with certain comments behaves strangely, both before and after this PR, so some more polish may be in order.

For example, without parentheses around the value, and both before and after this PR, we have:

```python
# unformatted
variable = (
    something # a comment
    .first_method("some string")
)

# formatted
variable = something.first_method("some string")  # a comment
```

which is probably not where the comment ought to go.
2025-11-17 09:11:36 -06:00