Parenthesize with statements by MichaReiser · Pull Request #5758 · astral-sh/ruff

MichaReiser · 2023-07-14T07:57:01Z

Summary

This PR improves the parentheses handling for with items to get closer to black's formatting.

Case 1:

# Black / Input
with (
    [
        "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
        "bbbbbbbbbb",
        "cccccccccccccccccccccccccccccccccccccccccc",
        dddddddddddddddddddddddddddddddd,
    ] as example1,
    aaaaaaaaaaaaaaaaaaaaaaaaaa
    + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
    + cccccccccccccccccccccccccccc
    + ddddddddddddddddd as example2,
    CtxManager2() as example2,
    CtxManager2() as example2,
    CtxManager2() as example2,
):
    ...

# Before
with (
    [
        "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
        "bbbbbbbbbb",
        "cccccccccccccccccccccccccccccccccccccccccc",
        dddddddddddddddddddddddddddddddd,
    ] as example1,
    (
        aaaaaaaaaaaaaaaaaaaaaaaaaa
        + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
        + cccccccccccccccccccccccccccc
        + ddddddddddddddddd
    ) as example2,
    CtxManager2() as example2,
    CtxManager2() as example2,
    CtxManager2() as example2,
):
    ...

Notice how Ruff wraps the binary expression in an extra set of parentheses

Case 2:

Black does not expand the with-items if the with has no parentheses:

# Black / Input
with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c:
    ...

# Before
with (
    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c
):
    ...

Or

# Black / Input
with [
    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
    "bbbbbbbbbb",
    "cccccccccccccccccccccccccccccccccccccccccc",
    dddddddddddddddddddddddddddddddd,
] as example1, aaaaaaaaaaaaaaaaaaaaaaaaaa * bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb * cccccccccccccccccccccccccccc + ddddddddddddddddd as example2, CtxManager222222222222222() as example2:
    ...

# Before (Same as Case 1)
with (
    [
        "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
        "bbbbbbbbbb",
        "cccccccccccccccccccccccccccccccccccccccccc",
        dddddddddddddddddddddddddddddddd,
    ] as example1,
    (
        aaaaaaaaaaaaaaaaaaaaaaaaaa
        * bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
        * cccccccccccccccccccccccccccc
        + ddddddddddddddddd
    ) as example2,
    CtxManager222222222222222() as example2,
):
    ...

Test Plan

I added new snapshot tests

Improves the django similarity index from 0.973 to 0.977

MichaReiser · 2023-07-14T07:57:15Z

Current dependencies on/for this PR:

main
- PR Properly group assignment targets #5728
  - PR Cover Black's is_aritmetic_like formatting #5738
    - PR Parenthesize with statements #5758 👈

This comment was auto-generated by Graphite.

github-actions · 2023-07-14T08:28:36Z

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.16     12.9±0.28ms     3.1 MB/sec    1.00     11.1±0.19ms     3.7 MB/sec
formatter/numpy/ctypeslib.py               1.13      2.5±0.08ms     6.7 MB/sec    1.00      2.2±0.05ms     7.5 MB/sec
formatter/numpy/globals.py                 1.06   274.4±10.55µs    10.8 MB/sec    1.00   258.8±17.41µs    11.4 MB/sec
formatter/pydantic/types.py                1.11      5.4±0.15ms     4.7 MB/sec    1.00      4.9±0.18ms     5.2 MB/sec
linter/all-rules/large/dataset.py          1.01     17.1±0.30ms     2.4 MB/sec    1.00     16.9±0.21ms     2.4 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      4.3±0.14ms     3.9 MB/sec    1.00      4.2±0.09ms     3.9 MB/sec
linter/all-rules/numpy/globals.py          1.01   569.1±20.43µs     5.2 MB/sec    1.00   563.9±18.80µs     5.2 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.6±0.15ms     3.3 MB/sec    1.01      7.7±0.19ms     3.3 MB/sec
linter/default-rules/large/dataset.py      1.00      8.1±0.11ms     5.0 MB/sec    1.01      8.2±0.09ms     5.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1770.7±36.87µs     9.4 MB/sec    1.02  1805.3±81.86µs     9.2 MB/sec
linter/default-rules/numpy/globals.py      1.00    213.3±9.34µs    13.8 MB/sec    1.01    215.0±8.35µs    13.7 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.7±0.08ms     6.9 MB/sec    1.01      3.7±0.08ms     6.8 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     11.3±0.09ms     3.6 MB/sec    1.00     11.3±0.23ms     3.6 MB/sec
formatter/numpy/ctypeslib.py               1.01      2.2±0.04ms     7.7 MB/sec    1.00      2.2±0.03ms     7.7 MB/sec
formatter/numpy/globals.py                 1.02    232.8±4.11µs    12.7 MB/sec    1.00    228.7±8.58µs    12.9 MB/sec
formatter/pydantic/types.py                1.00      4.8±0.04ms     5.3 MB/sec    1.00      4.8±0.15ms     5.3 MB/sec
linter/all-rules/large/dataset.py          1.01     16.3±0.12ms     2.5 MB/sec    1.00     16.1±0.15ms     2.5 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.02      4.3±0.06ms     3.9 MB/sec    1.00      4.2±0.04ms     4.0 MB/sec
linter/all-rules/numpy/globals.py          1.00    420.9±5.96µs     7.0 MB/sec    1.03   433.7±11.40µs     6.8 MB/sec
linter/all-rules/pydantic/types.py         1.01      7.4±0.08ms     3.4 MB/sec    1.00      7.3±0.09ms     3.5 MB/sec
linter/default-rules/large/dataset.py      1.01      8.4±0.06ms     4.9 MB/sec    1.00      8.2±0.04ms     4.9 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1691.0±11.86µs     9.8 MB/sec    1.01  1706.5±13.51µs     9.8 MB/sec
linter/default-rules/numpy/globals.py      1.00    177.2±1.91µs    16.6 MB/sec    1.02    181.4±2.14µs    16.3 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.7±0.02ms     6.9 MB/sec    1.00      3.7±0.02ms     6.9 MB/sec

MichaReiser · 2023-07-14T15:54:06Z

crates/ruff_python_formatter/src/trivia.rs

    In,

+    /// `as`
+    As,


@davidszotten thanks for implementing the keyword lexing in a generic way. It was a piece of cake to add the new keywords that I needed.

MichaReiser · 2023-07-14T16:29:47Z

crates/ruff_python_formatter/src/expression/expr_await.rs

+            [
+                text("await"),
+                space(),
+                maybe_parenthesize_expression(value, item, Parenthesize::IfRequired)


I introduced this for the with handling and it gives us the nice property that it goes through NeedsParentheses. Meaning, it respects whether the expressions wants to be parenthesized or not.

MichaReiser · 2023-07-14T16:31:19Z

...thon_formatter/tests/snapshots/black_compatibility@simple_cases__remove_await_parens.py.snap

 async def main():
 -    await (yield x)
-+    await (NOT_YET_IMPLEMENTED_ExprYield)
+    await NOT_YET_IMPLEMENTED_ExprYield


Fixing this would be easy. Change the ExprYield's NeedsParentheses implementation. However, doing this now would result in unstable formatting because the NOT_IMPLEMENTED_ExprYield is not a yield expression.

return (yield) is such a weird syntax

konstin · 2023-07-14T17:00:18Z

crates/ruff_python_formatter/src/comments/placement.rs

    CommentPlacement::leading(starred.as_any_node_ref(), comment)
 }

+/// Handles trailing own line comments before the `as` keyword of a with item and


i wonder if an abstraction with two nodes with a single token in between makes sense, which would also force us to handle them consistently. We have the same pattern in slices (x[a:b]), argument lists x(a,b) and binary expressions (a+b). (also this feature is inconsistent because with (a as b) works but except (a as b) doesn't).

And dicts and dict comprehensions and walruses

konstin · 2023-07-14T17:11:31Z

...thon_formatter/tests/snapshots/black_compatibility@simple_cases__remove_await_parens.py.snap

 async def main():
 -    await (yield x)
-+    await (NOT_YET_IMPLEMENTED_ExprYield)
+    await NOT_YET_IMPLEMENTED_ExprYield


return (yield) is such a weird syntax

## Summary This PR improves the parentheses handling for with items to get closer to black's formatting. ### Case 1: ```python # Black / Input with ( [ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "bbbbbbbbbb", "cccccccccccccccccccccccccccccccccccccccccc", dddddddddddddddddddddddddddddddd, ] as example1, aaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + cccccccccccccccccccccccccccc + ddddddddddddddddd as example2, CtxManager2() as example2, CtxManager2() as example2, CtxManager2() as example2, ): ... # Before with ( [ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "bbbbbbbbbb", "cccccccccccccccccccccccccccccccccccccccccc", dddddddddddddddddddddddddddddddd, ] as example1, ( aaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + cccccccccccccccccccccccccccc + ddddddddddddddddd ) as example2, CtxManager2() as example2, CtxManager2() as example2, CtxManager2() as example2, ): ... ``` Notice how Ruff wraps the binary expression in an extra set of parentheses ### Case 2: Black does not expand the with-items if the with has no parentheses: ```python # Black / Input with aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c: ... # Before with ( aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb as c ): ... ``` Or ```python # Black / Input with [ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "bbbbbbbbbb", "cccccccccccccccccccccccccccccccccccccccccc", dddddddddddddddddddddddddddddddd, ] as example1, aaaaaaaaaaaaaaaaaaaaaaaaaa * bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb * cccccccccccccccccccccccccccc + ddddddddddddddddd as example2, CtxManager222222222222222() as example2: ... # Before (Same as Case 1) with ( [ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "bbbbbbbbbb", "cccccccccccccccccccccccccccccccccccccccccc", dddddddddddddddddddddddddddddddd, ] as example1, ( aaaaaaaaaaaaaaaaaaaaaaaaaa * bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb * cccccccccccccccccccccccccccc + ddddddddddddddddd ) as example2, CtxManager222222222222222() as example2, ): ... ``` ## Test Plan I added new snapshot tests Improves the django similarity index from 0.973 to 0.977

This was referenced Jul 14, 2023

Properly group assignment targets #5728

Merged

Cover Black's is_aritmetic_like formatting #5738

Merged

MichaReiser force-pushed the with-stmt-parentheses branch 2 times, most recently from 9cf4409 to 8f98884 Compare July 14, 2023 15:43

MichaReiser requested a review from konstin July 14, 2023 15:49

MichaReiser linked an issue Jul 14, 2023 that may be closed by this pull request

Formatter: With / AsyncWith (includes WithItem) #5368

Closed

3 tasks

MichaReiser added the formatter Related to the formatter label Jul 14, 2023

MichaReiser commented Jul 14, 2023

View reviewed changes

MichaReiser marked this pull request as ready for review July 14, 2023 15:54

Base automatically changed from handle-arith-like-expr to main July 14, 2023 15:54

MichaReiser force-pushed the with-stmt-parentheses branch from 8f98884 to c9d2d03 Compare July 14, 2023 16:28

Parenthesize with statements

5e98a87

MichaReiser force-pushed the with-stmt-parentheses branch from c9d2d03 to 5e98a87 Compare July 14, 2023 16:28

MichaReiser commented Jul 14, 2023

View reviewed changes

konstin approved these changes Jul 14, 2023

View reviewed changes

MichaReiser merged commit 3cda89e into main Jul 15, 2023

MichaReiser deleted the with-stmt-parentheses branch July 15, 2023 15:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parenthesize with statements#5758

Parenthesize with statements#5758
MichaReiser merged 1 commit intomainfrom
with-stmt-parentheses

MichaReiser commented Jul 14, 2023 •

edited

Loading

Uh oh!

MichaReiser commented Jul 14, 2023

Uh oh!

github-actions bot commented Jul 14, 2023 •

edited

Loading

Uh oh!

MichaReiser Jul 14, 2023

Uh oh!

MichaReiser Jul 14, 2023

Uh oh!

MichaReiser Jul 14, 2023

Uh oh!

konstin Jul 14, 2023

Uh oh!

konstin Jul 14, 2023

Uh oh!

davidszotten Jul 14, 2023

Uh oh!

MichaReiser Jul 15, 2023

Uh oh!

konstin Jul 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

MichaReiser commented Jul 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Case 1:

Case 2:

Test Plan

Uh oh!

MichaReiser commented Jul 14, 2023

Uh oh!

github-actions bot commented Jul 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Check Results

Ecosystem

Benchmark

Linux

Windows

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MichaReiser commented Jul 14, 2023 •

edited

Loading

github-actions bot commented Jul 14, 2023 •

edited

Loading