Skip to content

Remove parentheses around some walrus operators#6173

Merged
charliermarsh merged 1 commit intomainfrom
charlie/named-expr-parens
Jul 29, 2023
Merged

Remove parentheses around some walrus operators#6173
charliermarsh merged 1 commit intomainfrom
charlie/named-expr-parens

Conversation

@charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Jul 29, 2023

Summary

Closes #5781

Test Plan

Added cases to crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/named_expr.py one-by-one and adjusted the condition as needed.

charliermarsh
charliermarsh commented Jul 29, 2023
View reviewed changes
crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/named_expr.py
if x := 1:
pass

(x := 1)
Copy link
Member Author

@charliermarsh charliermarsh Jul 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to figure out why the parentheses aren't being removed here (which is correct, but parent.is_stmt_expr() isn't in the conditional).

Copy link
Member

@MichaReiser MichaReiser Jul 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parentheses are only removed in statements other than ExprStmt (and only for top-level expressions).

ruff/crates/ruff_python_formatter/src/statement/stmt_expr.rs

Lines 16 to 20 in 40f5437

if is_arithmetic_like(value) {
maybe_parenthesize_expression(value, item, Parenthesize::Optional).fmt(f)
} else {
value.format().fmt(f)
}

The formatting calls expr.format() which preserves parentheses.

(I wonder if we can remove the arithmetic handling now too).

MichaReiser
MichaReiser approved these changes Jul 29, 2023
View reviewed changes
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/named_expr.py
if x := 1:
pass

(x := 1)
Copy link
Member

@MichaReiser MichaReiser Jul 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parentheses are only removed in statements other than ExprStmt (and only for top-level expressions).

ruff/crates/ruff_python_formatter/src/statement/stmt_expr.rs

Lines 16 to 20 in 40f5437

if is_arithmetic_like(value) {
maybe_parenthesize_expression(value, item, Parenthesize::Optional).fmt(f)
} else {
value.format().fmt(f)
}

The formatting calls expr.format() which preserves parentheses.

(I wonder if we can remove the arithmetic handling now too).

@charliermarsh charliermarsh merged commit 5d9814d into main Jul 29, 2023
@charliermarsh charliermarsh deleted the charlie/named-expr-parens branch July 29, 2023 14:06
@github-actions
Copy link
Contributor

github-actions bot commented Jul 29, 2023

PR Check Results

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      8.5±0.13ms     4.8 MB/sec    1.01      8.5±0.16ms     4.8 MB/sec
formatter/numpy/ctypeslib.py               1.00  1666.0±26.59µs    10.0 MB/sec    1.00  1671.4±25.16µs    10.0 MB/sec
formatter/numpy/globals.py                 1.02    188.1±7.32µs    15.7 MB/sec    1.00    183.6±4.22µs    16.1 MB/sec
formatter/pydantic/types.py                1.00      3.6±0.06ms     7.1 MB/sec    1.00      3.6±0.05ms     7.1 MB/sec
linter/all-rules/large/dataset.py          1.00     11.1±0.04ms     3.7 MB/sec    1.02     11.3±0.16ms     3.6 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      2.8±0.01ms     5.9 MB/sec    1.00      2.8±0.02ms     5.9 MB/sec
linter/all-rules/numpy/globals.py          1.00    383.7±0.52µs     7.7 MB/sec    1.00    384.1±2.09µs     7.7 MB/sec
linter/all-rules/pydantic/types.py         1.01      5.0±0.08ms     5.1 MB/sec    1.00      5.0±0.01ms     5.1 MB/sec
linter/default-rules/large/dataset.py      1.00      6.0±0.04ms     6.8 MB/sec    1.02      6.1±0.08ms     6.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1248.7±11.51µs    13.3 MB/sec    1.01   1257.1±9.75µs    13.2 MB/sec
linter/default-rules/numpy/globals.py      1.00    137.0±0.60µs    21.5 MB/sec    1.00    137.0±0.39µs    21.5 MB/sec
linter/default-rules/pydantic/types.py     1.00      2.6±0.03ms     9.6 MB/sec    1.01      2.7±0.05ms     9.6 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.01     10.2±0.10ms     4.0 MB/sec    1.00     10.1±0.12ms     4.0 MB/sec
formatter/numpy/ctypeslib.py               1.00  1966.6±31.46µs     8.5 MB/sec    1.00  1957.4±45.88µs     8.5 MB/sec
formatter/numpy/globals.py                 1.00    215.9±4.22µs    13.7 MB/sec    1.01    218.3±8.68µs    13.5 MB/sec
formatter/pydantic/types.py                1.01      4.3±0.06ms     5.9 MB/sec    1.00      4.3±0.06ms     5.9 MB/sec
linter/all-rules/large/dataset.py          1.00     13.4±0.12ms     3.0 MB/sec    1.00     13.5±0.13ms     3.0 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.5±0.03ms     4.7 MB/sec    1.00      3.5±0.04ms     4.7 MB/sec
linter/all-rules/numpy/globals.py          1.01    434.0±7.11µs     6.8 MB/sec    1.00    431.8±5.06µs     6.8 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.0±0.10ms     4.2 MB/sec    1.00      6.0±0.06ms     4.2 MB/sec
linter/default-rules/large/dataset.py      1.00      7.4±0.06ms     5.5 MB/sec    1.00      7.4±0.07ms     5.5 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1499.7±26.47µs    11.1 MB/sec    1.00  1503.9±18.83µs    11.1 MB/sec
linter/default-rules/numpy/globals.py      1.00    168.3±4.74µs    17.5 MB/sec    1.00    167.8±2.60µs    17.6 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.2±0.04ms     7.9 MB/sec    1.00      3.2±0.04ms     7.9 MB/sec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove unnecessary walrus operator parentheses

2 participants

@charliermarsh @MichaReiser