Support `fmt: skip` for simple-statements and decorators #6561

MichaReiser · 2023-08-14T13:51:04Z

Summary

This PR implements support for trailing fmt: skip and fmt: off comments on statement and decorator level.

This PR does not yet implement fmt: skip support at the end of a case header, e.g. at the end of an if statement

Test Plan

Matching black tests, added new tests

MichaReiser · 2023-08-14T13:51:19Z

Current dependencies on/for this PR:

main
- PR Use single lookup for leading, dangling, and trailing comments #6589
  - PR Support fmt: skip for simple-statements and decorators #6561 👈
    - PR Support fmt: skip on compound statements #6593

This comment was auto-generated by Graphite.

github-actions · 2023-08-14T14:19:03Z

PR Check Results

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      3.4±0.03ms    11.9 MB/sec    1.00      3.4±0.04ms    12.0 MB/sec
formatter/numpy/ctypeslib.py               1.00    710.6±2.65µs    23.4 MB/sec    1.01   714.3±10.55µs    23.3 MB/sec
formatter/numpy/globals.py                 1.00     75.7±0.31µs    39.0 MB/sec    1.00     75.8±0.34µs    38.9 MB/sec
formatter/pydantic/types.py                1.00  1393.2±23.61µs    18.3 MB/sec    1.00  1391.0±25.38µs    18.3 MB/sec
linter/all-rules/large/dataset.py          1.00     10.6±0.25ms     3.9 MB/sec    1.01     10.6±0.29ms     3.8 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      2.8±0.07ms     5.9 MB/sec    1.00      2.8±0.07ms     5.9 MB/sec
linter/all-rules/numpy/globals.py          1.01    397.9±1.60µs     7.4 MB/sec    1.00    394.9±1.71µs     7.5 MB/sec
linter/all-rules/pydantic/types.py         1.03      5.6±0.21ms     4.6 MB/sec    1.00      5.4±0.10ms     4.7 MB/sec
linter/default-rules/large/dataset.py      1.01      5.5±0.09ms     7.4 MB/sec    1.00      5.5±0.07ms     7.4 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1209.6±6.54µs    13.8 MB/sec    1.00   1212.7±6.77µs    13.7 MB/sec
linter/default-rules/numpy/globals.py      1.00    143.8±3.91µs    20.5 MB/sec    1.00    143.8±5.33µs    20.5 MB/sec
linter/default-rules/pydantic/types.py     1.00      2.5±0.02ms    10.3 MB/sec    1.00      2.5±0.04ms    10.3 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      3.7±0.04ms    10.9 MB/sec    1.01      3.8±0.24ms    10.8 MB/sec
formatter/numpy/ctypeslib.py               1.00   772.2±14.12µs    21.6 MB/sec    1.00   769.0±15.21µs    21.7 MB/sec
formatter/numpy/globals.py                 1.00     80.5±1.37µs    36.6 MB/sec    1.02     81.9±3.27µs    36.0 MB/sec
formatter/pydantic/types.py                1.00  1550.2±34.30µs    16.5 MB/sec    1.00  1555.0±37.94µs    16.4 MB/sec
linter/all-rules/large/dataset.py          1.00     12.8±0.19ms     3.2 MB/sec    1.00     12.8±0.21ms     3.2 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.5±0.04ms     4.7 MB/sec    1.00      3.5±0.05ms     4.7 MB/sec
linter/all-rules/numpy/globals.py          1.01    445.7±7.51µs     6.6 MB/sec    1.00    440.5±7.69µs     6.7 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.6±0.10ms     3.8 MB/sec    1.00      6.6±0.19ms     3.8 MB/sec
linter/default-rules/large/dataset.py      1.01      7.1±0.10ms     5.8 MB/sec    1.00      7.0±0.06ms     5.8 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.02  1505.6±29.53µs    11.1 MB/sec    1.00  1482.6±21.01µs    11.2 MB/sec
linter/default-rules/numpy/globals.py      1.00    175.6±3.39µs    16.8 MB/sec    1.00    175.9±4.73µs    16.8 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.2±0.04ms     8.1 MB/sec    1.00      3.1±0.03ms     8.1 MB/sec

crates/ruff_python_formatter/src/statement/stmt_class_def.rs

MichaReiser · 2023-08-15T06:34:18Z

crates/ruff_python_formatter/src/lib.rs

-        self.fmt_node(node, f)?;
-        self.fmt_dangling_comments(node_comments.dangling, f)?;
-        trailing_comments(node_comments.trailing).fmt(f)
+        if self.is_suppressed(node_comments.trailing, f.context()) {


I considered handling the suppression in the FormatStmt so that it isn't necessary to override is_suppressed for all statements. However, it requires that FormatStmt resolves the trailing comments to test for a suppression comment, just for the FormatNodeRule to lookup the very same comments again.

charliermarsh · 2023-08-16T13:48:36Z

crates/ruff_python_formatter/src/comments/placement.rs

@@ -902,7 +902,7 @@ fn handle_leading_class_with_decorators_comment<'a>(
    comment: DecoratedComment<'a>,
    class_def: &'a ast::StmtClassDef,
 ) -> CommentPlacement<'a> {
-    if comment.start() < class_def.name.start() {
+    if comment.line_position().is_own_line() && comment.start() < class_def.name.start() {


End-of-line comments are already correctly associated, I'm guessing, since they're associated with the preceding rather than the following node?

Yes. The default placement correctly associates them with the decorator (preceding) node.

This was referenced Aug 14, 2023

Implement DerefMut for WithNodeLevel #6443

Merged

Format docstrings #6452

Merged

fmt: off..on suppression comments #6477

Merged

MichaReiser mentioned this pull request Aug 14, 2023

Indent statements in suppressed ranges #6507

Merged

MichaReiser force-pushed the fix-suppressed-indentation branch from 12f9bbc to bdab1c2 Compare August 14, 2023 15:47

MichaReiser force-pushed the fmt-skip-statement-and-decorators branch from 05c1442 to 3eb2b08 Compare August 14, 2023 15:47

MichaReiser force-pushed the fix-suppressed-indentation branch from bdab1c2 to df98068 Compare August 14, 2023 16:09

MichaReiser force-pushed the fmt-skip-statement-and-decorators branch from 3eb2b08 to 8731120 Compare August 14, 2023 16:09

MichaReiser commented Aug 14, 2023

View reviewed changes

crates/ruff_python_formatter/src/statement/stmt_class_def.rs Show resolved Hide resolved

MichaReiser force-pushed the fmt-skip-statement-and-decorators branch from 8731120 to 5f5873c Compare August 14, 2023 16:35

MichaReiser mentioned this pull request Aug 15, 2023

Use single lookup for leading, dangling, and trailing comments #6589

Merged

Base automatically changed from fix-suppressed-indentation to main August 15, 2023 06:00

MichaReiser changed the base branch from main to comments-single-lookup August 15, 2023 06:06

MichaReiser force-pushed the fmt-skip-statement-and-decorators branch 2 times, most recently from f40f7c9 to 5bc64b5 Compare August 15, 2023 06:11

MichaReiser marked this pull request as ready for review August 15, 2023 06:12

MichaReiser added the formatter Related to the formatter label Aug 15, 2023

MichaReiser mentioned this pull request Aug 15, 2023

Formatter: Suppression comments #5336

Closed

6 tasks

MichaReiser commented Aug 15, 2023

View reviewed changes

MichaReiser force-pushed the fmt-skip-statement-and-decorators branch from 5bc64b5 to 02330c4 Compare August 15, 2023 11:53

MichaReiser changed the title ~~Support fmt: skip on statement and decorator level~~ Support fmt: skip for simple-statements and decorator level Aug 15, 2023

MichaReiser changed the title ~~Support fmt: skip for simple-statements and decorator level~~ Support fmt: skip for simple-statements and decorators Aug 15, 2023

MichaReiser force-pushed the fmt-skip-statement-and-decorators branch from 02330c4 to ef86a00 Compare August 15, 2023 11:54

MichaReiser requested a review from charliermarsh August 15, 2023 11:55

MichaReiser mentioned this pull request Aug 15, 2023

Support fmt: skip on compound statements #6593

Merged

MichaReiser force-pushed the comments-single-lookup branch from be450a6 to 072263c Compare August 15, 2023 14:20

MichaReiser force-pushed the fmt-skip-statement-and-decorators branch from ef86a00 to 8215d9d Compare August 15, 2023 14:20

Base automatically changed from comments-single-lookup to main August 15, 2023 15:39

MichaReiser force-pushed the fmt-skip-statement-and-decorators branch 2 times, most recently from 247979d to 142d47b Compare August 16, 2023 06:07

charliermarsh approved these changes Aug 16, 2023

View reviewed changes

Support fmt: skip on statement and decorator level

e2de436

MichaReiser force-pushed the fmt-skip-statement-and-decorators branch from 142d47b to e2de436 Compare August 17, 2023 05:49

MichaReiser enabled auto-merge (squash) August 17, 2023 05:51

MichaReiser merged commit 4dc32a0 into main Aug 17, 2023

MichaReiser deleted the fmt-skip-statement-and-decorators branch August 17, 2023 05:58

MichaReiser mentioned this pull request Aug 22, 2023

Fix: Re-add missing node start positions #6780

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support `fmt: skip` for simple-statements and decorators #6561

Support `fmt: skip` for simple-statements and decorators #6561

MichaReiser commented Aug 14, 2023

MichaReiser commented Aug 14, 2023 •

edited

Loading

github-actions bot commented Aug 14, 2023 •

edited

Loading

MichaReiser Aug 15, 2023

charliermarsh Aug 16, 2023

MichaReiser Aug 17, 2023

Support fmt: skip for simple-statements and decorators #6561

Support fmt: skip for simple-statements and decorators #6561

Conversation

MichaReiser commented Aug 14, 2023

Summary

Test Plan

MichaReiser commented Aug 14, 2023 • edited Loading

github-actions bot commented Aug 14, 2023 • edited Loading

PR Check Results

Benchmark

Linux

Windows

MichaReiser Aug 15, 2023

Choose a reason for hiding this comment

charliermarsh Aug 16, 2023

Choose a reason for hiding this comment

MichaReiser Aug 17, 2023

Choose a reason for hiding this comment

Support `fmt: skip` for simple-statements and decorators #6561

Support `fmt: skip` for simple-statements and decorators #6561

MichaReiser commented Aug 14, 2023 •

edited

Loading

github-actions bot commented Aug 14, 2023 •

edited

Loading