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.

Sign up for GitHub

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

Jump to bottom

Format PatternMatchClass #6860

Merged
merged 3 commits into from
Aug 25, 2023
Merged

Format PatternMatchClass #6860

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0698de0
Format PatternMatchClass
LaBatata101 Aug 24, 2023
8527683
Merge branch 'main' into format-pattern-match-class
charliermarsh Aug 25, 2023
87cf862
Add more tests
charliermarsh Aug 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/match.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,3 +263,30 @@ def foo():
# comment
):
y = 1


match pattern_match_class:
case Foo(
# own line
):
...

case Point2D(0, 0):
...

case Point3D(x=0, y=0, z=000000000000000000000000000000000000000000000000000000000000000000000000000000000):
...

case Bar(0, a=None, b="hello"):
...

case FooBar(# leading
# leading
# leading
# leading
0 # trailing
# trailing
# trailing
# trailing
):
...
Copy link
Member

Choose a reason for hiding this comment

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

Could you add something like

match a:
    case A(
        b # b
        = # c
        2 # d
        # e
    ):
        pass

with the left hand side and the right hand side split of the assignment split by comment? I wanted to add an additional comment before the b but that's blocked by #6866

40 changes: 31 additions & 9 deletions crates/ruff_python_formatter/src/pattern/pattern_match_class.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,41 @@
use ruff_formatter::{write, Buffer, FormatResult};
use ruff_formatter::{write, FormatResult};
use ruff_python_ast::PatternMatchClass;

use crate::{not_yet_implemented_custom_text, FormatNodeRule, PyFormatter};
use crate::expression::parentheses::parenthesized;
use crate::prelude::*;
use crate::{FormatNodeRule, PyFormatter};

#[derive(Default)]
pub struct FormatPatternMatchClass;

impl FormatNodeRule<PatternMatchClass> for FormatPatternMatchClass {
fn fmt_fields(&self, item: &PatternMatchClass, f: &mut PyFormatter) -> FormatResult<()> {
write!(
f,
[not_yet_implemented_custom_text(
"NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0)",
item
)]
)
let PatternMatchClass {
range,
cls,
patterns,
kwd_attrs,
kwd_patterns,
} = item;

let items = format_with(|f| {
let mut join = f.join_comma_separated(range.end());

if !patterns.is_empty() {
join.nodes(patterns.iter());
}

if !kwd_attrs.is_empty() {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need the if !kwd_attrs.is_empty()?

for (key, value) in kwd_attrs.iter().zip(kwd_patterns.iter()) {
join.entry(
key,
&format_with(|f| write!(f, [key.format(), text("="), value.format()])),
);
}
}
join.finish()
});

write!(f, [cls.format(), parenthesized("(", &items, ")")])
Copy link
Member

Choose a reason for hiding this comment

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

Try adding an extra group around items

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would the function group work here?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, group(&items) should do the trick if I'm not mistaken

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing changed for this crates/ruff_python_formatter/tests/snapshots/black_compatibility@py_310__pattern_matching_extras.py.snap file when using group

Copy link
Member

Choose a reason for hiding this comment

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

See my comment below. This is fine, assuming we're talking about the same mismatch.

}
}
11 changes: 1 addition & 10 deletions ...on_formatter/tests/snapshots/black_compatibility@py_310__pattern_matching_complex.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,6 @@ match x:
```diff
--- Black
+++ Ruff
@@ -6,7 +6,7 @@
y = 0
# case black_test_patma_142
match x:
- case bytes(z):
+ case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0):
y = 0
# case black_test_patma_073
match x:
@@ -16,23 +16,23 @@
y = 1
# case black_test_patma_006
Expand Down Expand Up @@ -282,7 +273,7 @@ match x:
y = 0
# case black_test_patma_142
match x:
case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0):
case bytes(z):
y = 0
# case black_test_patma_073
match x:
Expand Down
89 changes: 26 additions & 63 deletions ...hon_formatter/tests/snapshots/black_compatibility@py_310__pattern_matching_extras.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,36 +131,6 @@ match bar1:
```diff
--- Black
+++ Ruff
@@ -5,9 +5,9 @@
print(b)
case [a as b, c, d, e as f]:
print(f)
- case Point(a as b):
+ case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0):
print(b)
- case Point(int() as x, int() as y):
+ case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0):
print(x, y)


@@ -15,7 +15,7 @@
case: int = re.match(something)

match re.match(case):
- case type("match", match):
+ case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0):
pass
case match:
pass
@@ -23,7 +23,7 @@

def func(match: case, case: match) -> case:
match Something():
- case func(match, case):
+ case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0):
...
case another:
...
@@ -32,23 +32,32 @@
match maybe, multiple:
case perhaps, 5:
Expand Down Expand Up @@ -199,21 +169,7 @@ match bar1:
assert "map" == b


@@ -59,12 +68,7 @@
),
case,
):
- case case(
- match=case,
- case=re.match(
- loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong
- ),
- ):
+ case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0):
pass

case [a as match]:
@@ -80,17 +84,14 @@
@@ -80,17 +89,14 @@


match a, *b(), c:
Expand All @@ -234,7 +190,7 @@ match bar1:
pass


@@ -101,19 +102,22 @@
@@ -101,7 +107,12 @@
case 2 as b, 3 as c:
pass

Expand All @@ -248,19 +204,16 @@ match bar1:
pass


match bar1:
- case Foo(aa=Callable() as aa, bb=int()):
+ case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0):
print(bar1.aa, bar1.bb)
case _:
print("no match", "\n")

@@ -114,6 +125,9 @@

match bar1:
- case Foo(
case Foo(
- normal=x, perhaps=[list, {"x": d, "y": 1.0}] as y, otherwise=something, q=t as u
- ):
+ case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0):
+ normal=x,
MichaReiser marked this conversation as resolved.
Show resolved Hide resolved
+ perhaps=[list, {"NOT_YET_IMPLEMENTED_PatternMatchMapping": _, 2: _}] as y,
Copy link
Member

Choose a reason for hiding this comment

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

Okay, the problem here is that PatternMatchMapping isn't yet implemented and the text emitted by the "fake" implementation is much longer than the expected string.

We can ignore this.

+ otherwise=something,
+ q=t as u,
):
pass
```

Expand All @@ -274,25 +227,25 @@ match something:
print(b)
case [a as b, c, d, e as f]:
print(f)
case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0):
case Point(a as b):
print(b)
case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0):
case Point(int() as x, int() as y):
print(x, y)


match = 1
case: int = re.match(something)

match re.match(case):
case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0):
case type("match", match):
pass
case match:
pass


def func(match: case, case: match) -> case:
match Something():
case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0):
case func(match, case):
...
case another:
...
Expand Down Expand Up @@ -337,7 +290,12 @@ match match(
),
case,
):
case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0):
case case(
match=case,
case=re.match(
loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong
),
):
pass

case [a as match]:
Expand Down Expand Up @@ -381,14 +339,19 @@ match something:


match bar1:
case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0):
case Foo(aa=Callable() as aa, bb=int()):
print(bar1.aa, bar1.bb)
case _:
print("no match", "\n")


match bar1:
case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0):
case Foo(
normal=x,
perhaps=[list, {"NOT_YET_IMPLEMENTED_PatternMatchMapping": _, 2: _}] as y,
otherwise=something,
q=t as u,
):
pass
```

Expand Down
Loading