Add formatter support for call and class definition `Arguments` #6274

charliermarsh · 2023-08-02T13:58:40Z

Summary

This PR leverages the Arguments AST node introduced in #6259 in the formatter, which ensures that we correctly handle trailing comments in calls, like:

f(
  1,
  # comment
)

pass

(Previously, this was treated as a leading comment on pass.)

This also allows us to unify the argument handling across calls and class definitions.

Test Plan

A bunch of new fixture tests, plus improved Black compatibility.

charliermarsh · 2023-08-02T13:59:05Z

crates/ruff_python_formatter/src/builders.rs

@@ -252,7 +252,7 @@ impl<'ast> Format<PyFormatContext<'ast>> for EmptyWithDanglingComments<'_> {
            [group(&format_args![
                self.opening,
                // end-of-line comments
-                dangling_comments(&self.comments[..end_of_line_split]),
+                trailing_comments(&self.comments[..end_of_line_split]),


We don't want these to break, IIUC.

charliermarsh · 2023-08-02T13:59:22Z

crates/ruff_python_formatter/src/statement/stmt_class_def.rs

-    class_definition: &'a StmtClassDef,
-}
-
-impl Format<PyFormatContext<'_>> for FormatInheritanceClause<'_> {


We get all of this for free now, I think.

charliermarsh · 2023-08-02T13:59:39Z

crates/ruff_python_formatter/src/other/arguments.rs

+#[derive(Default)]
+pub struct FormatArguments;
+
+impl FormatNodeRule<Arguments> for FormatArguments {


This is largely moved from expr_call.

charliermarsh · 2023-08-02T14:21:18Z

Looking into ecosystem failure.

charliermarsh · 2023-08-02T14:30:56Z

(Need to fix a few things.)

github-actions · 2023-08-02T14:41:48Z

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      8.5±0.05ms     4.8 MB/sec    1.01      8.6±0.07ms     4.7 MB/sec
formatter/numpy/ctypeslib.py               1.01  1661.0±36.23µs    10.0 MB/sec    1.00  1646.3±14.92µs    10.1 MB/sec
formatter/numpy/globals.py                 1.00    172.1±4.74µs    17.1 MB/sec    1.02    174.7±5.03µs    16.9 MB/sec
formatter/pydantic/types.py                1.00      3.6±0.09ms     7.0 MB/sec    1.00      3.6±0.06ms     7.0 MB/sec
linter/all-rules/large/dataset.py          1.00     11.6±0.09ms     3.5 MB/sec    1.00     11.6±0.10ms     3.5 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.0±0.02ms     5.6 MB/sec    1.00      3.0±0.02ms     5.6 MB/sec
linter/all-rules/numpy/globals.py          1.00    319.1±2.21µs     9.2 MB/sec    1.00    318.5±1.63µs     9.3 MB/sec
linter/all-rules/pydantic/types.py         1.00      5.2±0.06ms     4.9 MB/sec    1.00      5.2±0.04ms     4.9 MB/sec
linter/default-rules/large/dataset.py      1.01      6.2±0.05ms     6.6 MB/sec    1.00      6.1±0.03ms     6.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1215.2±6.93µs    13.7 MB/sec    1.00   1215.8±4.55µs    13.7 MB/sec
linter/default-rules/numpy/globals.py      1.00    120.7±0.24µs    24.5 MB/sec    1.00    121.0±0.31µs    24.4 MB/sec
linter/default-rules/pydantic/types.py     1.00      2.7±0.01ms     9.6 MB/sec    1.00      2.7±0.02ms     9.6 MB/sec

Windows

group                                      main                                    pr
-----                                      ----                                    --
formatter/large/dataset.py                 1.00      9.9±0.07ms     4.1 MB/sec     1.00      9.9±0.08ms     4.1 MB/sec
formatter/numpy/ctypeslib.py               1.00  1929.6±100.51µs     8.6 MB/sec    1.00  1925.1±26.59µs     8.6 MB/sec
formatter/numpy/globals.py                 1.02    215.8±5.35µs    13.7 MB/sec     1.00    212.2±4.75µs    13.9 MB/sec
formatter/pydantic/types.py                1.00      4.2±0.04ms     6.1 MB/sec     1.00      4.2±0.07ms     6.1 MB/sec
linter/all-rules/large/dataset.py          1.00     13.3±0.07ms     3.1 MB/sec     1.00     13.3±0.14ms     3.1 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.5±0.03ms     4.7 MB/sec     1.00      3.5±0.02ms     4.7 MB/sec
linter/all-rules/numpy/globals.py          1.00    435.1±5.88µs     6.8 MB/sec     1.00    435.7±7.83µs     6.8 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.0±0.07ms     4.2 MB/sec     1.00      6.0±0.07ms     4.2 MB/sec
linter/default-rules/large/dataset.py      1.00      7.2±0.04ms     5.6 MB/sec     1.01      7.3±0.05ms     5.6 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1494.5±18.35µs    11.1 MB/sec     1.00  1493.1±20.33µs    11.2 MB/sec
linter/default-rules/numpy/globals.py      1.00    169.6±2.49µs    17.4 MB/sec     1.01    171.0±2.51µs    17.3 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.2±0.03ms     8.0 MB/sec     1.00      3.2±0.02ms     8.0 MB/sec

crates/ruff_python_formatter/src/comments/placement.rs

crates/ruff_python_formatter/src/other/arguments.rs

crates/ruff_python_formatter/src/statement/stmt_class_def.rs

crates/ruff_python_formatter/src/comments/placement.rs

## Summary Based on the confusion here: #6274 (comment). I looked into moving this logic into `placement.rs`, but I think it's trickier than it may appear.

Micha suggested this in #6274 (comment), and it allows us to unify the implementations for arguments and type params.

Based on feedback here: #6274 (comment).

charliermarsh added the formatter Related to the formatter label Aug 2, 2023

charliermarsh requested a review from konstin August 2, 2023 13:58

charliermarsh commented Aug 2, 2023

View reviewed changes

Base automatically changed from charlie/class-base-comments to main August 2, 2023 14:01

charliermarsh force-pushed the charlie/class-base-format branch from b5af1c1 to 4d34062 Compare August 2, 2023 14:02

zanieb approved these changes Aug 2, 2023

View reviewed changes

charliermarsh marked this pull request as draft August 2, 2023 14:28

charliermarsh force-pushed the charlie/class-base-format branch from 4d34062 to 1315f7c Compare August 2, 2023 14:45

Format

e277615

charliermarsh force-pushed the charlie/class-base-format branch from 1315f7c to e277615 Compare August 2, 2023 14:46

charliermarsh marked this pull request as ready for review August 2, 2023 14:58

charliermarsh merged commit 4c53bfe into main Aug 2, 2023

charliermarsh deleted the charlie/class-base-format branch August 2, 2023 15:54

zanieb mentioned this pull request Aug 2, 2023

Add formatting of type parameters in class and function definitions #6161

Merged