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

Suite formatting and JoinNodesBuilder #4805

Merged
merged 3 commits into from
Jun 2, 2023
Merged

Suite formatting and JoinNodesBuilder #4805

merged 3 commits into from
Jun 2, 2023

Conversation

MichaReiser
Copy link
Member

Summary

This PR adds a new JoinNodesBuilder that simplifies writing a sequence of nodes while respecting the empty lines between nodes.

let mut joiner = f.join_nodes(NodeLevel::Statement);

for node in nodes {
	joiner.entry(node, &node.format());
}


joiner.finish()

Or if you always want to use the node's format implementation, then use

f.join_nodes(NodeLevel::Statement).nodes(nodes).finish()

This PR further adds a Format implementation for Vec<Suite> that adds the right amount of empty lines around classes and functions, and keeps empty lines for all other statements.

Test Plan

I added a few test plans. I intentionally didn't use the new formatting yet.

@MichaReiser
Copy link
Member Author

MichaReiser commented Jun 2, 2023

@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.00     14.0±0.10ms     2.9 MB/sec    1.01     14.2±0.11ms     2.9 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.4±0.01ms     4.9 MB/sec    1.01      3.4±0.01ms     4.9 MB/sec
linter/all-rules/numpy/globals.py          1.00    420.0±0.66µs     7.0 MB/sec    1.00    420.0±0.74µs     7.0 MB/sec
linter/all-rules/pydantic/types.py         1.00      5.8±0.01ms     4.4 MB/sec    1.00      5.8±0.02ms     4.4 MB/sec
linter/default-rules/large/dataset.py      1.00      6.8±0.02ms     6.0 MB/sec    1.00      6.8±0.01ms     6.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1498.2±2.04µs    11.1 MB/sec    1.00   1498.9±9.04µs    11.1 MB/sec
linter/default-rules/numpy/globals.py      1.00    170.3±0.29µs    17.3 MB/sec    1.00    170.5±0.21µs    17.3 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.1±0.01ms     8.3 MB/sec    1.00      3.1±0.01ms     8.3 MB/sec
parser/large/dataset.py                    1.00      5.2±0.00ms     7.9 MB/sec    1.22      6.3±0.00ms     6.4 MB/sec
parser/numpy/ctypeslib.py                  1.00   1014.6±0.49µs    16.4 MB/sec    1.17   1191.8±0.44µs    14.0 MB/sec
parser/numpy/globals.py                    1.00    104.8±0.14µs    28.2 MB/sec    1.13    118.3±0.21µs    24.9 MB/sec
parser/pydantic/types.py                   1.00      2.2±0.01ms    11.4 MB/sec    1.18      2.6±0.00ms     9.7 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.02     18.5±0.84ms     2.2 MB/sec    1.00     18.1±1.05ms     2.2 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.07      4.7±0.31ms     3.5 MB/sec    1.00      4.5±0.31ms     3.7 MB/sec
linter/all-rules/numpy/globals.py          1.05   543.4±46.08µs     5.4 MB/sec    1.00   516.7±37.04µs     5.7 MB/sec
linter/all-rules/pydantic/types.py         1.04      7.8±0.38ms     3.3 MB/sec    1.00      7.6±0.46ms     3.4 MB/sec
linter/default-rules/large/dataset.py      1.05      9.1±0.48ms     4.5 MB/sec    1.00      8.7±0.45ms     4.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.03  1898.0±94.29µs     8.8 MB/sec    1.00  1834.7±77.33µs     9.1 MB/sec
linter/default-rules/numpy/globals.py      1.04   216.7±11.69µs    13.6 MB/sec    1.00   207.6±15.63µs    14.2 MB/sec
linter/default-rules/pydantic/types.py     1.02      4.0±0.16ms     6.4 MB/sec    1.00      3.9±0.27ms     6.5 MB/sec
parser/large/dataset.py                    1.10      7.3±0.23ms     5.6 MB/sec    1.00      6.7±0.32ms     6.1 MB/sec
parser/numpy/ctypeslib.py                  1.10  1363.1±72.45µs    12.2 MB/sec    1.00  1236.5±72.08µs    13.5 MB/sec
parser/numpy/globals.py                    1.09   137.8±10.63µs    21.4 MB/sec    1.00    126.2±8.24µs    23.4 MB/sec
parser/pydantic/types.py                   1.10      3.1±0.12ms     8.3 MB/sec    1.00      2.8±0.11ms     9.2 MB/sec

@MichaReiser MichaReiser linked an issue Jun 2, 2023 that may be closed by this pull request
@MichaReiser MichaReiser added internal An internal refactor or improvement formatter Related to the formatter labels Jun 2, 2023
@MichaReiser MichaReiser force-pushed the join-nodes-builder branch 2 times, most recently from 6717291 to cfaf878 Compare June 2, 2023 11:01
use rustpython_parser::ast::Ranged;

/// Provides Python specific extensions to [`Formatter`].
pub(crate) trait PyFormatterExtensions<'buf, 'context> {
Copy link
Member

Choose a reason for hiding this comment

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

Is this meant as a general purpose "attach your utils crate" or as specific for the node builder?

Copy link
Member Author

@MichaReiser MichaReiser Jun 2, 2023

Choose a reason for hiding this comment

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

At rome, we only really had builder extensions but it might be useful to add some more (and move the trait?). For example, we could add a contents() or comments methods so that we don't need to always go through the context.

Do you want me to move the trait to the lib.rs?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll merge this to reduce the stack size. I can follow up with another PR if you want me to move it.

crates/ruff_python_formatter/src/builders.rs Outdated Show resolved Hide resolved
crates/ruff_python_formatter/src/builders.rs Show resolved Hide resolved
@konstin konstin changed the base branch from format-expr-name to main June 2, 2023 12:23
@MichaReiser MichaReiser changed the base branch from main to format-expr-name June 2, 2023 12:45
@MichaReiser MichaReiser changed the base branch from format-expr-name to main June 2, 2023 14:03
@MichaReiser MichaReiser enabled auto-merge (squash) June 2, 2023 14:06
@MichaReiser MichaReiser merged commit ebdc4af into main Jun 2, 2023
@MichaReiser MichaReiser deleted the join-nodes-builder branch June 2, 2023 14:14
konstin pushed a commit that referenced this pull request Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Formatter: JoinNodesBuilder
2 participants