Skip to content

Suite formatting and JoinNodesBuilder#4805

Merged
MichaReiser merged 3 commits intomainfrom
join-nodes-builder
Jun 2, 2023
Merged

Suite formatting and JoinNodesBuilder#4805
MichaReiser merged 3 commits intomainfrom
join-nodes-builder

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Jun 2, 2023

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.

This was referenced Jun 2, 2023
Merged
Merged
Merged
Merged
Merged
@MichaReiser
Copy link
Member Author

MichaReiser commented Jun 2, 2023 •
edited by konstin
Loading

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2023 •
edited
Loading

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
Formatter: JoinNodesBuilder #4801
Closed
@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
konstin
konstin approved these changes Jun 2, 2023
View reviewed changes
crates/ruff_python_formatter/src/builders.rs Outdated
use rustpython_parser::ast::Ranged;

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

@konstin konstin Jun 2, 2023

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 •
edited
Loading

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

@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.

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

@konstin konstin changed the base branch from format-expr-name to main June 2, 2023 12:23
This was referenced Jun 2, 2023
Merged
Merged
@MichaReiser MichaReiser changed the base branch from main to format-expr-name June 2, 2023 12:45
@MichaReiser MichaReiser force-pushed the join-nodes-builder branch from f5521f0 to d113ea6 Compare June 2, 2023 12:45
@MichaReiser MichaReiser mentioned this pull request Jun 2, 2023
Merged
@MichaReiser MichaReiser changed the base branch from format-expr-name to main June 2, 2023 14:03
@MichaReiser MichaReiser force-pushed the join-nodes-builder branch from d113ea6 to 530cb36 Compare 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
@MichaReiser MichaReiser mentioned this pull request Jun 4, 2023
Merged
konstin pushed a commit that referenced this pull request Jun 13, 2023
@MichaReiser @konstin
Suite formatting and JoinNodesBuilder (#4805)
a1bf6fc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@konstin konstin konstin approved these changes

Assignees

No one assigned

Labels

formatter Related to the formatter internal An internal refactor or improvement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Formatter: JoinNodesBuilder

2 participants

@MichaReiser @konstin