Add formatting for `StmtMatch` #6286

dhruvmanila · 2023-08-02T20:04:40Z

Summary

This PR adds support for StmtMatch with subs for MatchCase.

Test Plan

Add a few additional test cases around match statement, comments, line breaks.

resolves: #6298

dhruvmanila · 2023-08-02T20:04:51Z

Current dependencies on/for this PR:

main
- PR Add formatting for StmtMatch #6286 👈
  - PR Add formatting for MatchCase #6360

This comment was auto-generated by Graphite.

github-actions · 2023-08-02T20:30:53Z

PR Check Results

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      8.2±0.06ms     5.0 MB/sec    1.00      8.2±0.06ms     5.0 MB/sec
formatter/numpy/ctypeslib.py               1.00  1625.9±46.67µs    10.2 MB/sec    1.00   1620.8±7.58µs    10.3 MB/sec
formatter/numpy/globals.py                 1.00    182.0±3.88µs    16.2 MB/sec    1.00    182.9±0.27µs    16.1 MB/sec
formatter/pydantic/types.py                1.00      3.4±0.06ms     7.4 MB/sec    1.00      3.5±0.04ms     7.4 MB/sec
linter/all-rules/large/dataset.py          1.00     10.1±0.05ms     4.0 MB/sec    1.03     10.3±0.12ms     3.9 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      2.7±0.01ms     6.2 MB/sec    1.01      2.7±0.02ms     6.1 MB/sec
linter/all-rules/numpy/globals.py          1.00    377.9±0.90µs     7.8 MB/sec    1.00    379.4±0.77µs     7.8 MB/sec
linter/all-rules/pydantic/types.py         1.00      4.6±0.02ms     5.5 MB/sec    1.02      4.7±0.04ms     5.4 MB/sec
linter/default-rules/large/dataset.py      1.00      5.3±0.01ms     7.7 MB/sec    1.00      5.3±0.04ms     7.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1133.9±1.12µs    14.7 MB/sec    1.00   1132.8±1.35µs    14.7 MB/sec
linter/default-rules/numpy/globals.py      1.00    127.4±2.46µs    23.2 MB/sec    1.00    127.0±1.33µs    23.2 MB/sec
linter/default-rules/pydantic/types.py     1.00      2.3±0.02ms    10.9 MB/sec    1.00      2.3±0.01ms    10.9 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     10.0±0.13ms     4.1 MB/sec    1.04     10.4±0.16ms     3.9 MB/sec
formatter/numpy/ctypeslib.py               1.00  1896.9±30.41µs     8.8 MB/sec    1.03  1953.7±30.67µs     8.5 MB/sec
formatter/numpy/globals.py                 1.00    208.9±6.80µs    14.1 MB/sec    1.02    213.9±7.44µs    13.8 MB/sec
formatter/pydantic/types.py                1.00      4.2±0.06ms     6.1 MB/sec    1.04      4.3±0.05ms     5.9 MB/sec
linter/all-rules/large/dataset.py          1.00     12.8±0.12ms     3.2 MB/sec    1.01     13.0±0.14ms     3.1 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.5±0.04ms     4.8 MB/sec    1.01      3.5±0.04ms     4.8 MB/sec
linter/all-rules/numpy/globals.py          1.00    432.2±8.97µs     6.8 MB/sec    1.00    432.4±6.83µs     6.8 MB/sec
linter/all-rules/pydantic/types.py         1.00      5.8±0.05ms     4.4 MB/sec    1.03      6.0±0.12ms     4.3 MB/sec
linter/default-rules/large/dataset.py      1.00      6.9±0.17ms     5.9 MB/sec    1.00      6.9±0.10ms     5.9 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.02  1444.7±19.62µs    11.5 MB/sec    1.00  1415.7±15.93µs    11.8 MB/sec
linter/default-rules/numpy/globals.py      1.00    164.4±2.92µs    17.9 MB/sec    1.00    164.8±2.84µs    17.9 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.0±0.03ms     8.4 MB/sec    1.01      3.1±0.05ms     8.3 MB/sec

dhruvmanila · 2023-08-05T05:57:38Z

crates/ruff_python_formatter/src/statement/stmt_match.rs

+                [block_indent(&format_args![
+                    text("case"),
+                    space(),
+                    not_yet_implemented_custom_text("NOT_YET_IMPLEMENTED_MatchCase"),
+                    text(":"),
+                    block_indent(&case.body.format())
+                ])]


Temporary until the MatchCase formatting is implemented.

dhruvmanila · 2023-08-05T06:02:08Z

crates/ruff_python_formatter/tests/snapshots/format@statement__match.py.snap

+match (
+    # leading expr comment


This seems to be a known issue where the comment right next to an opening parentheses is moved down instead.

- match ( # leading expr comment + match ( + # leading expr comment

See also #6380 and #6381, but we don't have the handling for this for general expression parentheses yet, leading end-of-line comments inside of optional expression parentheses are rather rare

crates/ruff_python_formatter/src/comments/placement.rs

crates/ruff_python_formatter/src/statement/stmt_match.rs

MichaReiser · 2023-08-07T08:48:22Z

crates/ruff_python_formatter/src/statement/stmt_match.rs

+        )?;
+
+        for case in cases {
+            write!(


Do we need to retain empty lines between match cases?

Micha trying to write valid python code 😅

match test: case "a": pass case "b": pass

I think the newline handling will be done in the MatchCase implementation. Any newline between the match statement and the first case will be removed as is done in black as well:

match test: case "a": pass # vvvvv match test: case "a": pass

MichaReiser · 2023-08-08T08:52:28Z

crates/ruff_python_formatter/src/comments/visitor.rs

@@ -73,7 +73,6 @@ impl<'a> CommentsVisitor<'a> {
                enclosing: enclosing_node,
                preceding: self.preceding_node,
                following: Some(node),
-                parent: self.parents.iter().rev().nth(1).copied(),


MichaReiser · 2023-08-08T08:55:32Z

crates/ruff_python_formatter/src/other/match_case.rs

+            [
+                text("case"),
+                space(),
+                not_yet_implemented_custom_text("NOT_YET_IMPLEMENTED_Pattern"),


We can also do this as a separate PR: Ideally, we call into pattern.format() here. This requires implementing AsFormat and IntoFormat for Pattern, similar to how it is done for Mod

ruff/crates/ruff_python_formatter/src/module/mod.rs

Lines 1 to 35 in f45e864

use crate::context::PyFormatContext;

use crate::{AsFormat, IntoFormat, PyFormatter};

use ruff_formatter::{Format, FormatOwnedWithRule, FormatRefWithRule, FormatResult, FormatRule};

use ruff_python_ast::Mod;

pub(crate) mod mod_expression;

pub(crate) mod mod_module;

#[derive(Default)]

pub struct FormatMod;

impl FormatRule<Mod, PyFormatContext<'_>> for FormatMod {

fn fmt(&self, item: &Mod, f: &mut PyFormatter) -> FormatResult<()> {

match item {

Mod::Module(x) => x.format().fmt(f),

Mod::Expression(x) => x.format().fmt(f),

}

}

}

impl<'ast> AsFormat<PyFormatContext<'ast>> for Mod {

type Format<'a> = FormatRefWithRule<'a, Mod, FormatMod, PyFormatContext<'ast>>;

fn format(&self) -> Self::Format<'_> {

FormatRefWithRule::new(self, FormatMod)

}

}

impl<'ast> IntoFormat<PyFormatContext<'ast>> for Mod {

type Format = FormatOwnedWithRule<Mod, FormatMod, PyFormatContext<'ast>>;

fn into_format(self) -> Self::Format {

FormatOwnedWithRule::new(self, FormatMod)

}

}

Yes, it's already a work in progress in the next PR (https://github.com/astral-sh/ruff/pull/6360/files#diff-05760c9829e7694c86aaf97014143c0785c12da546f06dcfbc87caf67b15d773) ;)

## Summary This PR adds support for `StmtMatch` with subs for `MatchCase`. ## Test Plan Add a few additional test cases around `match` statement, comments, line breaks. resolves: astral-sh#6298

konstin added the formatter Related to the formatter label Aug 3, 2023

dhruvmanila force-pushed the dhruv/format-match-stmt branch 2 times, most recently from b1591ba to 73a6595 Compare August 5, 2023 04:22

dhruvmanila mentioned this pull request Aug 5, 2023

Add formatting for MatchCase #6360

Merged

dhruvmanila changed the title ~~Add formatting for StmtMatch and MatchCase~~ Add formatting for StmtMatch Aug 5, 2023

dhruvmanila commented Aug 5, 2023

View reviewed changes

dhruvmanila marked this pull request as ready for review August 5, 2023 06:33

dhruvmanila requested review from MichaReiser and konstin August 5, 2023 06:33

konstin approved these changes Aug 7, 2023

View reviewed changes

crates/ruff_python_formatter/src/comments/placement.rs Outdated Show resolved Hide resolved

crates/ruff_python_formatter/src/statement/stmt_match.rs Outdated Show resolved Hide resolved

MichaReiser reviewed Aug 7, 2023

View reviewed changes

dhruvmanila requested a review from MichaReiser August 7, 2023 17:55

dhruvmanila added 6 commits August 7, 2023 23:34

Add formatting for StmtMatch

1b14772

Add test cases for match statements

e70b45a

Remove MatchCase comment handling

4c514dc

Move mock implementation of MatchCase

52f670b

Fix CI, use dangling_comments

b39537e

Use trailing comments (fixed on main)

16fc898

dhruvmanila force-pushed the dhruv/format-match-stmt branch from e34d757 to 16fc898 Compare August 7, 2023 18:06

Fix snapshots

6691fdf

MichaReiser approved these changes Aug 8, 2023

View reviewed changes

dhruvmanila merged commit 001aa48 into main Aug 8, 2023
17 checks passed

dhruvmanila deleted the dhruv/format-match-stmt branch August 8, 2023 13:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add formatting for `StmtMatch` #6286

Add formatting for `StmtMatch` #6286

dhruvmanila commented Aug 2, 2023 •

edited

Loading

dhruvmanila commented Aug 2, 2023 •

edited

Loading

github-actions bot commented Aug 2, 2023 •

edited

Loading

dhruvmanila Aug 5, 2023

dhruvmanila Aug 5, 2023

konstin Aug 7, 2023

MichaReiser Aug 7, 2023

dhruvmanila Aug 7, 2023

MichaReiser Aug 8, 2023

MichaReiser Aug 8, 2023

dhruvmanila Aug 8, 2023

	use crate::context::PyFormatContext;
	use crate::{AsFormat, IntoFormat, PyFormatter};
	use ruff_formatter::{Format, FormatOwnedWithRule, FormatRefWithRule, FormatResult, FormatRule};
	use ruff_python_ast::Mod;

	pub(crate) mod mod_expression;
	pub(crate) mod mod_module;

	#[derive(Default)]
	pub struct FormatMod;

	impl FormatRule<Mod, PyFormatContext<'_>> for FormatMod {
	fn fmt(&self, item: &Mod, f: &mut PyFormatter) -> FormatResult<()> {
	match item {
	Mod::Module(x) => x.format().fmt(f),
	Mod::Expression(x) => x.format().fmt(f),
	}
	}
	}

	impl<'ast> AsFormat<PyFormatContext<'ast>> for Mod {
	type Format<'a> = FormatRefWithRule<'a, Mod, FormatMod, PyFormatContext<'ast>>;

	fn format(&self) -> Self::Format<'_> {
	FormatRefWithRule::new(self, FormatMod)
	}
	}

	impl<'ast> IntoFormat<PyFormatContext<'ast>> for Mod {
	type Format = FormatOwnedWithRule<Mod, FormatMod, PyFormatContext<'ast>>;

	fn into_format(self) -> Self::Format {
	FormatOwnedWithRule::new(self, FormatMod)
	}
	}

Add formatting for StmtMatch #6286

Add formatting for StmtMatch #6286

Conversation

dhruvmanila commented Aug 2, 2023 • edited Loading

Summary

Test Plan

dhruvmanila commented Aug 2, 2023 • edited Loading

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

PR Check Results

Benchmark

Linux

Windows

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add formatting for `StmtMatch` #6286

Add formatting for `StmtMatch` #6286

dhruvmanila commented Aug 2, 2023 •

edited

Loading

dhruvmanila commented Aug 2, 2023 •

edited

Loading

github-actions bot commented Aug 2, 2023 •

edited

Loading