Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix formatting lambda with empty arguments #5944

Merged
merged 1 commit into from
Jul 21, 2023

Conversation

konstin
Copy link
Member

@konstin konstin commented Jul 21, 2023

Summary Fix implemented in astral-sh/RustPython-Parser#35: Previously, empty lambda arguments (e.g. lambda: 1) would get the range of the entire expression, which leads to incorrect comment placement. Now empty lambda arguments get an empty range between the lambda and the : tokens.

Test Plan Added a regression test.

149 instances of unstable formatting remaining.

$ cargo run --bin ruff_dev --release -- format-dev --stability-check --error-file formatter-ecosystem-errors.txt --multi-project target/checkouts > formatter-ecosystem-progress.txt
$ rg "Unstable formatting" target/formatter-ecosystem-errors.txt | wc -l
149

**Summary** Fix implemented in astral-sh/RustPython-Parser#35: Previously, empty lambda arguments (e.g. `lambda: 1`) would get the range of the entire expression, which leads to incorrect comment placement. Now empty lambda arguments get an empty range between the `lambda` and the `:` tokens.

**Test Plan** Added a regression test.

149 instances of unstable formatting remaining.

```
$ cargo run --bin ruff_dev --release -- format-dev --stability-check --error-file formatter-ecosystem-errors.txt --multi-project target/checkouts > formatter-ecosystem-progress.txt
$ rg "Unstable formatting" target/formatter-ecosystem-errors.txt | wc -l
149
```
@konstin
Copy link
Member Author

konstin commented Jul 21, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@konstin konstin force-pushed the lambda_ranges_and_formatting_bugs branch from 721efed to 62deacb Compare July 21, 2023 10:46
@github-actions
Copy link
Contributor

github-actions bot commented Jul 21, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     10.0±0.25ms     4.1 MB/sec    1.10     11.0±0.30ms     3.7 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.0±0.16ms     8.2 MB/sec    1.05      2.1±0.07ms     7.7 MB/sec
formatter/numpy/globals.py                 1.00   237.7±16.72µs    12.4 MB/sec    1.06   251.2±18.97µs    11.7 MB/sec
formatter/pydantic/types.py                1.00      4.4±0.20ms     5.8 MB/sec    1.09      4.8±0.18ms     5.3 MB/sec
linter/all-rules/large/dataset.py          1.15     16.7±0.45ms     2.4 MB/sec    1.00     14.6±0.31ms     2.8 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.05      4.1±0.16ms     4.0 MB/sec    1.00      3.9±0.20ms     4.2 MB/sec
linter/all-rules/numpy/globals.py          1.01   543.0±19.35µs     5.4 MB/sec    1.00   539.0±32.34µs     5.5 MB/sec
linter/all-rules/pydantic/types.py         1.06      7.3±0.22ms     3.5 MB/sec    1.00      6.8±0.25ms     3.7 MB/sec
linter/default-rules/large/dataset.py      1.17      9.0±0.22ms     4.5 MB/sec    1.00      7.7±0.17ms     5.3 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.12  1914.5±83.27µs     8.7 MB/sec    1.00  1702.7±44.73µs     9.8 MB/sec
linter/default-rules/numpy/globals.py      1.00    201.1±9.55µs    14.7 MB/sec    1.01    202.5±5.68µs    14.6 MB/sec
linter/default-rules/pydantic/types.py     1.06      3.8±0.16ms     6.6 MB/sec    1.00      3.6±0.07ms     7.0 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     11.3±0.16ms     3.6 MB/sec    1.00     11.3±0.15ms     3.6 MB/sec
formatter/numpy/ctypeslib.py               1.01      2.2±0.02ms     7.7 MB/sec    1.00      2.2±0.03ms     7.7 MB/sec
formatter/numpy/globals.py                 1.00    232.7±4.04µs    12.7 MB/sec    1.01   234.8±12.98µs    12.6 MB/sec
formatter/pydantic/types.py                1.00      4.8±0.04ms     5.3 MB/sec    1.00      4.8±0.09ms     5.3 MB/sec
linter/all-rules/large/dataset.py          1.01     15.9±0.13ms     2.6 MB/sec    1.00     15.8±0.13ms     2.6 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.1±0.04ms     4.0 MB/sec    1.00      4.1±0.03ms     4.1 MB/sec
linter/all-rules/numpy/globals.py          1.01    424.6±7.20µs     7.0 MB/sec    1.00    422.2±7.62µs     7.0 MB/sec
linter/all-rules/pydantic/types.py         1.01      7.2±0.19ms     3.6 MB/sec    1.00      7.1±0.11ms     3.6 MB/sec
linter/default-rules/large/dataset.py      1.00      8.2±0.06ms     4.9 MB/sec    1.01      8.3±0.05ms     4.9 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1652.0±13.61µs    10.1 MB/sec    1.01  1674.9±19.85µs     9.9 MB/sec
linter/default-rules/numpy/globals.py      1.00    173.8±2.63µs    17.0 MB/sec    1.01    175.2±2.40µs    16.8 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.6±0.04ms     7.0 MB/sec    1.01      3.7±0.02ms     6.9 MB/sec

@konstin konstin force-pushed the lambda_ranges_and_formatting_bugs branch from 1dd8126 to 62deacb Compare July 21, 2023 11:12
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.

149 instances of unstable formatting remaining.

From how many initial regressions?

@konstin konstin added the formatter Related to the formatter label Jul 21, 2023
@konstin
Copy link
Member Author

konstin commented Jul 21, 2023

From how many initial regressions?

197 was the last measurement before, but i haven't keeping track of which are mine and which are unrelated from main

@konstin konstin merged commit 972f9a9 into main Jul 21, 2023
@konstin konstin deleted the lambda_ranges_and_formatting_bugs branch July 21, 2023 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants