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

Formatter: Add EmptyWithDanglingComments helper #5951

Merged
merged 2 commits into from
Jul 23, 2023

Conversation

konstin
Copy link
Member

@konstin konstin commented Jul 21, 2023

Summary Add a EmptyWithDanglingComments format helper that formats comments inside empty parentheses, brackets or curly braces. Previously, this was implemented separately, and partially incorrectly, for each use case.

Empty (), [] and {} are special because there can be dangling comments, and they can be in
two positions:

x =# end-of-line
    # own line
]

These comments are dangling because they can't be assigned to any element inside as they would
in all other cases.

Test Plan Added a regression test.

145 (from previously 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
145

@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 requested a review from MichaReiser July 21, 2023 15:52
@github-actions
Copy link
Contributor

github-actions bot commented Jul 21, 2023

PR Check Results

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     11.1±0.05ms     3.7 MB/sec    1.00     11.1±0.05ms     3.7 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.2±0.01ms     7.5 MB/sec    1.00      2.2±0.00ms     7.5 MB/sec
formatter/numpy/globals.py                 1.00    243.1±1.28µs    12.1 MB/sec    1.03    251.4±0.44µs    11.7 MB/sec
formatter/pydantic/types.py                1.00      4.8±0.02ms     5.3 MB/sec    1.01      4.8±0.02ms     5.3 MB/sec
linter/all-rules/large/dataset.py          1.00     15.8±0.13ms     2.6 MB/sec    1.01     16.0±0.08ms     2.6 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.9±0.04ms     4.2 MB/sec    1.01      4.0±0.02ms     4.2 MB/sec
linter/all-rules/numpy/globals.py          1.01   512.4±11.00µs     5.8 MB/sec    1.00    509.8±1.23µs     5.8 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.2±0.07ms     3.6 MB/sec    1.00      7.2±0.05ms     3.6 MB/sec
linter/default-rules/large/dataset.py      1.00      8.2±0.07ms     5.0 MB/sec    1.00      8.2±0.04ms     5.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1725.1±3.45µs     9.7 MB/sec    1.00   1717.7±5.98µs     9.7 MB/sec
linter/default-rules/numpy/globals.py      1.00    189.0±0.98µs    15.6 MB/sec    1.00    188.6±0.42µs    15.6 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.6±0.02ms     7.0 MB/sec    1.01      3.6±0.01ms     7.0 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     11.4±0.16ms     3.6 MB/sec    1.01     11.5±0.18ms     3.5 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.2±0.04ms     7.6 MB/sec    1.00      2.2±0.03ms     7.6 MB/sec
formatter/numpy/globals.py                 1.00    245.5±6.76µs    12.0 MB/sec    1.01   248.8±10.73µs    11.9 MB/sec
formatter/pydantic/types.py                1.00      4.9±0.08ms     5.3 MB/sec    1.01      4.9±0.07ms     5.2 MB/sec
linter/all-rules/large/dataset.py          1.00     16.0±0.18ms     2.5 MB/sec    1.05     16.9±0.27ms     2.4 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.2±0.07ms     4.0 MB/sec    1.03      4.3±0.07ms     3.9 MB/sec
linter/all-rules/numpy/globals.py          1.00    499.4±7.87µs     5.9 MB/sec    1.01    504.3±7.19µs     5.9 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.4±0.11ms     3.5 MB/sec    1.03      7.6±0.15ms     3.4 MB/sec
linter/default-rules/large/dataset.py      1.00      8.4±0.13ms     4.8 MB/sec    1.00      8.4±0.11ms     4.9 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01  1738.6±26.82µs     9.6 MB/sec    1.00  1725.0±23.00µs     9.7 MB/sec
linter/default-rules/numpy/globals.py      1.01    196.3±6.81µs    15.0 MB/sec    1.00    194.3±4.58µs    15.2 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.7±0.07ms     6.9 MB/sec    1.01      3.7±0.07ms     6.8 MB/sec

/// ```
/// These comments are dangling because they can't be assigned to any element inside as they would
/// in all other cases.
pub(crate) fn empty_with_dangling_comments(
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Maybe empty_parenthesized_with_dangling_comments

Copy link
Member Author

Choose a reason for hiding this comment

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

added this, we need to figure out a better nomenclature for ( vs. {(, [, {}

**Summary** Add a `EmptyWithDanglingComments` format helper that formats comments inside empty parentheses, brackets or curly braces. Previously, this was implemented separately, and partially incorrectly, for each use case.

Empty `()`, `[]` and `{}` are special because there can be dangling comments, and they can be in
two positions:
```python
x = [  # end-of-line
    # own line
]
```
These comments are dangling because they can't be assigned to any element inside as they would
in all other cases.

**Test Plan** Added a regression test.

85 (from previously 89) 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
85
```
@konstin konstin changed the base branch from no_magic_trailing_commas_for_lambdas to main July 23, 2023 11:44
@konstin konstin force-pushed the EmptyWithDanglingComments branch from 8d55004 to c1ead0b Compare July 23, 2023 11:44
@konstin konstin merged commit 46f8961 into main Jul 23, 2023
@konstin konstin deleted the EmptyWithDanglingComments branch July 23, 2023 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants