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.

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

Add formatting for MatchCase #6360

Merged
merged 6 commits into from
Aug 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,56 @@ def foo():
match inside_func: # comment
case "bar":
pass


match newlines:

# case 1 leading comment


case "top level case comment with newlines": # case dangling comment
# pass leading comment
pass
# pass trailing comment


# case 2 leading comment



case "case comment with newlines" if foo == 2: # second
pass

case "one", "newline" if (foo := 1): # third
pass


case "two newlines":
pass



case "three newlines":
pass
case _:
pass
dhruvmanila marked this conversation as resolved.
Show resolved Hide resolved


match long_lines:
case "this is a long line for if condition" if aaaaaaaaahhhhhhhh == 1 and bbbbbbaaaaaaaaaaa == 2: # comment
pass

case "this is a long line for if condition with parentheses" if (aaaaaaaaahhhhhhhh == 1 and bbbbbbaaaaaaaaaaa == 2): # comment
pass

case "named expressions aren't special" if foo := 1:
pass

case "named expressions aren't that special" if (foo := 1):
pass

case "but with already broken long lines" if (
aaaaaaahhhhhhhhhhh == 1 and
bbbbbbbbaaaaaahhhh == 2
): # another comment
pass
1 change: 1 addition & 0 deletions crates/ruff_python_formatter/src/comments/placement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ fn is_first_statement_in_body(statement: AnyNodeRef, has_body: AnyNodeRef) -> bo
| AnyNodeRef::ExceptHandlerExceptHandler(ast::ExceptHandlerExceptHandler {
body, ..
})
| AnyNodeRef::MatchCase(ast::MatchCase { body, .. })
| AnyNodeRef::StmtFunctionDef(ast::StmtFunctionDef { body, .. })
| AnyNodeRef::StmtClassDef(ast::StmtClassDef { body, .. }) => {
are_same_optional(statement, body.first())
Expand Down
30 changes: 18 additions & 12 deletions crates/ruff_python_formatter/src/other/match_case.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use ruff_formatter::{write, Buffer, FormatResult};
use ruff_python_ast::MatchCase;

use crate::expression::maybe_parenthesize_expression;
use crate::expression::parentheses::Parenthesize;
use crate::comments::trailing_comments;
use crate::not_yet_implemented_custom_text;
use crate::prelude::*;
use crate::{FormatNodeRule, PyFormatter};
Expand All @@ -19,6 +18,9 @@ impl FormatNodeRule<MatchCase> for FormatMatchCase {
body,
} = item;

let comments = f.context().comments().clone();
let dangling_item_comments = comments.dangling_comments(item);

write!(
f,
[
Expand All @@ -29,17 +31,21 @@ impl FormatNodeRule<MatchCase> for FormatMatchCase {
)?;

if let Some(guard) = guard {
write!(
f,
[
space(),
text("if"),
space(),
maybe_parenthesize_expression(guard, item, Parenthesize::IfBreaks)
]
)?;
write!(f, [space(), text("if"), space(), guard.format()])?;
}

write!(f, [text(":"), block_indent(&body.format())])
write!(
f,
[
text(":"),
trailing_comments(dangling_item_comments),
block_indent(&body.format())
]
)
}

fn fmt_dangling_comments(&self, _node: &MatchCase, _f: &mut PyFormatter) -> FormatResult<()> {
// Handled as part of `fmt_fields`
Ok(())
}
}
39 changes: 39 additions & 0 deletions crates/ruff_python_formatter/src/pattern/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
use ruff_formatter::{FormatOwnedWithRule, FormatRefWithRule};
use ruff_python_ast::Pattern;

use crate::prelude::*;

pub(crate) mod pattern_match_as;
pub(crate) mod pattern_match_class;
pub(crate) mod pattern_match_mapping;
Expand All @@ -6,3 +11,37 @@ pub(crate) mod pattern_match_sequence;
pub(crate) mod pattern_match_singleton;
pub(crate) mod pattern_match_star;
pub(crate) mod pattern_match_value;

#[derive(Default)]
pub struct FormatPattern;

impl FormatRule<Pattern, PyFormatContext<'_>> for FormatPattern {
fn fmt(&self, item: &Pattern, f: &mut PyFormatter) -> FormatResult<()> {
match item {
Pattern::MatchValue(p) => p.format().fmt(f),
Pattern::MatchSingleton(p) => p.format().fmt(f),
Pattern::MatchSequence(p) => p.format().fmt(f),
Pattern::MatchMapping(p) => p.format().fmt(f),
Pattern::MatchClass(p) => p.format().fmt(f),
Pattern::MatchStar(p) => p.format().fmt(f),
Pattern::MatchAs(p) => p.format().fmt(f),
Pattern::MatchOr(p) => p.format().fmt(f),
}
}
}

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

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

impl<'ast> IntoFormat<PyFormatContext<'ast>> for Pattern {
type Format = FormatOwnedWithRule<Pattern, FormatPattern, PyFormatContext<'ast>>;

fn into_format(self) -> Self::Format {
FormatOwnedWithRule::new(self, FormatPattern)
}
}
30 changes: 26 additions & 4 deletions crates/ruff_python_formatter/src/statement/stmt_match.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use ruff_formatter::{write, Buffer, FormatResult};
use ruff_formatter::{format_args, write, Buffer, FormatResult};
use ruff_python_ast::StmtMatch;

use crate::comments::trailing_comments;
use crate::comments::{leading_alternate_branch_comments, trailing_comments};
use crate::context::{NodeLevel, WithNodeLevel};
use crate::expression::maybe_parenthesize_expression;
use crate::expression::parentheses::Parenthesize;
use crate::prelude::*;
Expand Down Expand Up @@ -35,8 +36,29 @@ impl FormatNodeRule<StmtMatch> for FormatStmtMatch {
]
)?;

for case in cases {
write!(f, [block_indent(&case.format())])?;
let mut cases_iter = cases.iter();
let Some(first) = cases_iter.next() else {
return Ok(());
};

// The new level is for the `case` nodes.
let mut f = WithNodeLevel::new(NodeLevel::CompoundStatement, f);

Copy link
Member Author

Choose a reason for hiding this comment

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

Otherwise, the node level is NodeLevel::TopLevel for the case nodes which is coming from the match statement. This will be problematic as the empty lines between case nodes should be at max 1.

write!(f, [block_indent(&first.format())])?;
let mut last_case = first;

for case in cases_iter {
write!(
f,
[block_indent(&format_args!(
&leading_alternate_branch_comments(
comments.leading_comments(case),
last_case.body.last(),
),
&case.format()
))]
)?;
last_case = case;
}

Ok(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ match x:
+ case NOT_YET_IMPLEMENTED_Pattern:
y = 0
- case [1, 0] if (x := x[:0]):
+ case NOT_YET_IMPLEMENTED_Pattern if x := x[:0]:
+ case NOT_YET_IMPLEMENTED_Pattern if (x := x[:0]):
y = 1
- case [1, 0]:
+ case NOT_YET_IMPLEMENTED_Pattern:
Expand Down Expand Up @@ -431,7 +431,7 @@ match (0, 1, 2):
match x:
case NOT_YET_IMPLEMENTED_Pattern:
y = 0
case NOT_YET_IMPLEMENTED_Pattern if x := x[:0]:
case NOT_YET_IMPLEMENTED_Pattern if (x := x[:0]):
y = 1
case NOT_YET_IMPLEMENTED_Pattern:
y = 2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ match bar1:
assert "map" == b


@@ -59,61 +62,47 @@
@@ -59,61 +62,51 @@
),
case,
):
Expand All @@ -217,11 +217,11 @@ match bar1:
- ):
+ case NOT_YET_IMPLEMENTED_Pattern:
pass
-
- case [a as match]:
+ case NOT_YET_IMPLEMENTED_Pattern:
pass
-
- case case:
+ case NOT_YET_IMPLEMENTED_Pattern:
pass
Expand Down Expand Up @@ -255,11 +255,11 @@ match bar1:
- case 1 as a:
+ case NOT_YET_IMPLEMENTED_Pattern:
pass
-
- case 2 as b, 3 as c:
+ case NOT_YET_IMPLEMENTED_Pattern:
pass
-
- case 4 as d, (5 as e), (6 | 7 as g), *h:
+ case NOT_YET_IMPLEMENTED_Pattern:
pass
Expand Down Expand Up @@ -351,8 +351,10 @@ match match(
):
case NOT_YET_IMPLEMENTED_Pattern:
pass

case NOT_YET_IMPLEMENTED_Pattern:
pass

case NOT_YET_IMPLEMENTED_Pattern:
pass

Expand All @@ -377,8 +379,10 @@ match something:
match something:
case NOT_YET_IMPLEMENTED_Pattern:
pass

case NOT_YET_IMPLEMENTED_Pattern:
pass

case NOT_YET_IMPLEMENTED_Pattern:
pass

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ def where_is(point):

match event.get():
- case Click((x, y), button=Button.LEFT): # This is a left click
+ case NOT_YET_IMPLEMENTED_Pattern:
+ case NOT_YET_IMPLEMENTED_Pattern: # This is a left click
handle_click_at(x, y)
- case Click():
+ case NOT_YET_IMPLEMENTED_Pattern:
Expand Down Expand Up @@ -306,7 +306,7 @@ match event.get():
raise ValueError(f"Unrecognized event: {other_event}")

match event.get():
case NOT_YET_IMPLEMENTED_Pattern:
case NOT_YET_IMPLEMENTED_Pattern: # This is a left click
handle_click_at(x, y)
case NOT_YET_IMPLEMENTED_Pattern:
pass # ignore other clicks
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,21 @@ def http_status(status):
```diff
--- Black
+++ Ruff
@@ -1,13 +1,10 @@
@@ -1,13 +1,13 @@
def http_status(status):
match status:
- case 400:
+ case NOT_YET_IMPLEMENTED_Pattern:
return "Bad request"
-
- case 401:
+ case NOT_YET_IMPLEMENTED_Pattern:
return "Unauthorized"
-
- case 403:
+ case NOT_YET_IMPLEMENTED_Pattern:
return "Forbidden"
-
- case 404:
+ case NOT_YET_IMPLEMENTED_Pattern:
return "Not found"
Expand All @@ -58,10 +58,13 @@ def http_status(status):
match status:
case NOT_YET_IMPLEMENTED_Pattern:
return "Bad request"

case NOT_YET_IMPLEMENTED_Pattern:
return "Unauthorized"

case NOT_YET_IMPLEMENTED_Pattern:
return "Forbidden"

case NOT_YET_IMPLEMENTED_Pattern:
return "Not found"
```
Expand Down
Loading
Loading