Skip to content

Commit

Permalink
Parenthesize match..case if guards (#13513)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Sep 26, 2024
1 parent 8012707 commit 9442cd8
Show file tree
Hide file tree
Showing 7 changed files with 155 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -588,3 +588,28 @@ def foo():
match x:
case Child(aaaaaaaaa, bbbbbbbbbbbbbbb, cccccc), Doc(aaaaa, bbbbbbbbbb, ddddddddddddd):
pass


match guard_comments:
case "abcd" if ( # trailing open parentheses comment
aaaaaaaaahhhhhhhh == 1 and bbbbbbaaaaaaaaaaa == 2
):
pass

case "bcdef" if (
aaaaaaaaahhhhhhhh == 1 and bbbbbbaaaaaaaaaaa == 2 # trailing end of line comment
): # comment
pass

case "efgh" if (
# leading own line comment
aaaaaahhhhhh == 1
):
pass

case "hijk" if (
aaaaaaaaa == 1
# trailing own line comment
):
pass

4 changes: 4 additions & 0 deletions crates/ruff_python_formatter/src/expression/parentheses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ pub(crate) enum Parenthesize {
/// Same as [`Self::IfBreaks`] except that it uses [`parenthesize_if_expands`] for expressions
/// with the layout [`NeedsParentheses::BestFit`] which is used by non-splittable
/// expressions like literals, name, and strings.
///
/// Use this layout over `IfBreaks` when there's a sequence of `maybe_parenthesize_expression`
/// in a single logical-line and you want to break from right-to-left. Use `IfBreaks` for the
/// first expression and `IfBreaksParenthesized` for the rest.
IfBreaksParenthesized,

/// Same as [`Self::IfBreaksParenthesized`] but uses [`parenthesize_if_expands`] for nested
Expand Down
26 changes: 18 additions & 8 deletions crates/ruff_python_formatter/src/other/match_case.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ use ruff_python_ast::AstNode;
use ruff_python_ast::MatchCase;

use crate::builders::parenthesize_if_expands;
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses, Parentheses};
use crate::expression::maybe_parenthesize_expression;
use crate::expression::parentheses::{
NeedsParentheses, OptionalParentheses, Parentheses, Parenthesize,
};
use crate::pattern::maybe_parenthesize_pattern;
use crate::prelude::*;
use crate::preview::is_match_case_parentheses_enabled;
Expand Down Expand Up @@ -62,20 +65,27 @@ impl FormatNodeRule<MatchCase> for FormatMatchCase {
}
});

let format_guard = guard.as_deref().map(|guard| {
format_with(|f| {
write!(f, [space(), token("if"), space()])?;

if is_match_case_parentheses_enabled(f.context()) {
maybe_parenthesize_expression(guard, item, Parenthesize::IfBreaksParenthesized)
.fmt(f)
} else {
guard.format().fmt(f)
}
})
});

write!(
f,
[
clause_header(
ClauseHeader::MatchCase(item),
dangling_item_comments,
&format_with(|f| {
write!(f, [token("case"), space(), format_pattern])?;

if let Some(guard) = guard {
write!(f, [space(), token("if"), space(), guard.format()])?;
}

Ok(())
write!(f, [token("case"), space(), format_pattern, format_guard])
}),
),
clause_body(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_python_formatter/src/preview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ pub(crate) fn is_empty_parameters_no_unnecessary_parentheses_around_return_value

/// See [#6933](https://github.com/astral-sh/ruff/issues/6933).
/// This style also covers the black preview styles `remove_redundant_guard_parens` and `parens_for_long_if_clauses_in_case_block `.
/// WARNING: This preview style depends on `is_empty_parameters_no_unnecessary_parentheses_around_return_value_enabled`
/// because it relies on the new semantic of `IfBreaksParenthesized`.
pub(crate) fn is_match_case_parentheses_enabled(context: &PyFormatContext) -> bool {
context.is_preview()
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,36 +44,9 @@ match x:
```diff
--- Black
+++ Ruff
@@ -3,34 +3,36 @@
@@ -21,11 +21,17 @@
pass
match smth:
- case "test" if (
- "any long condition" != "another long condition" and "this is a long condition"
- ):
+ case "test" if "any long condition" != "another long condition" and "this is a long condition":
pass
- case test if (
- "any long condition" != "another long condition"
- and "this is a looooong condition"
- ):
+ case (
+ test
+ ) if "any long condition" != "another long condition" and "this is a looooong condition":
pass
- case test if (
- "any long condition" != "another long condition"
- and "this is a looooong condition"
- ): # some additional comments
+ case (
+ test
+ ) if "any long condition" != "another long condition" and "this is a looooong condition": # some additional comments
pass
- case test if True: # some comment
+ case test if (True): # some comment
pass
- case test if False: # some comment
+ case test if (False): # some comment
case test if False: # some comment
pass
- case test if True: # some comment
+ case test if (
Expand All @@ -92,12 +65,6 @@ match x:
pass # some comment
# case black_test_patma_052 (originally in the pattern_matching_complex test case)
match x:
case [1, 0] if x := x[:0]:
y = 1
- case [1, 0] if x := x[:0]:
+ case [1, 0] if (x := x[:0]):
y = 1
```

## Ruff Output
Expand All @@ -108,19 +75,23 @@ match match:
pass
match smth:
case "test" if "any long condition" != "another long condition" and "this is a long condition":
case "test" if (
"any long condition" != "another long condition" and "this is a long condition"
):
pass
case (
test
) if "any long condition" != "another long condition" and "this is a looooong condition":
case test if (
"any long condition" != "another long condition"
and "this is a looooong condition"
):
pass
case (
test
) if "any long condition" != "another long condition" and "this is a looooong condition": # some additional comments
case test if (
"any long condition" != "another long condition"
and "this is a looooong condition"
): # some additional comments
pass
case test if (True): # some comment
case test if True: # some comment
pass
case test if (False): # some comment
case test if False: # some comment
pass
case test if (
True # some comment
Expand All @@ -139,7 +110,7 @@ match smth:
match x:
case [1, 0] if x := x[:0]:
y = 1
case [1, 0] if (x := x[:0]):
case [1, 0] if x := x[:0]:
y = 1
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,20 +69,7 @@ match 1:
```diff
--- Black
+++ Ruff
@@ -1,10 +1,10 @@
match 1:
- case _ if True:
+ case _ if (True):
pass
match 1:
- case _ if True:
+ case _ if (True):
pass
@@ -25,27 +25,33 @@
@@ -25,12 +25,16 @@
match 1:
Expand All @@ -101,18 +88,7 @@ match 1:
pass
match 1:
- case _ if True: # this is a comment
+ case _ if (True): # this is a comment
pass
match 1:
- case _ if True: # comment over the line limit unless parens are removed x
+ case _ if (
+ True
+ ): # comment over the line limit unless parens are removed x
pass
@@ -45,7 +49,7 @@
match 1:
Expand All @@ -129,12 +105,12 @@ match 1:

```python
match 1:
case _ if (True):
case _ if True:
pass
match 1:
case _ if (True):
case _ if True:
pass
Expand Down Expand Up @@ -169,14 +145,12 @@ match 1:
match 1:
case _ if (True): # this is a comment
case _ if True: # this is a comment
pass
match 1:
case _ if (
True
): # comment over the line limit unless parens are removed x
case _ if True: # comment over the line limit unless parens are removed x
pass
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,31 @@ match n % 3, n % 5:
match x:
case Child(aaaaaaaaa, bbbbbbbbbbbbbbb, cccccc), Doc(aaaaa, bbbbbbbbbb, ddddddddddddd):
pass
match guard_comments:
case "abcd" if ( # trailing open parentheses comment
aaaaaaaaahhhhhhhh == 1 and bbbbbbaaaaaaaaaaa == 2
):
pass
case "bcdef" if (
aaaaaaaaahhhhhhhh == 1 and bbbbbbaaaaaaaaaaa == 2 # trailing end of line comment
): # comment
pass
case "efgh" if (
# leading own line comment
aaaaaahhhhhh == 1
):
pass
case "hijk" if (
aaaaaaaaa == 1
# trailing own line comment
):
pass
```

## Output
Expand Down Expand Up @@ -1232,24 +1257,80 @@ match x:
aaaaa, bbbbbbbbbb, ddddddddddddd
):
pass
match guard_comments:
case "abcd" if ( # trailing open parentheses comment
aaaaaaaaahhhhhhhh == 1 and bbbbbbaaaaaaaaaaa == 2
):
pass
case "bcdef" if (
aaaaaaaaahhhhhhhh == 1
and bbbbbbaaaaaaaaaaa == 2 # trailing end of line comment
): # comment
pass
case "efgh" if (
# leading own line comment
aaaaaahhhhhh == 1
):
pass
case "hijk" if (
aaaaaaaaa == 1
# trailing own line comment
):
pass
```


## Preview changes
```diff
--- Stable
+++ Preview
@@ -69,7 +69,7 @@
case "case comment with newlines" if foo == 2: # second
pass
- case "one", "newline" if (foo := 1): # third
+ case "one", "newline" if foo := 1: # third
pass
case "two newlines":
@@ -82,7 +82,9 @@
match long_lines:
- case "this is a long line for if condition" if aaaaaaaaahhhhhhhh == 1 and bbbbbbaaaaaaaaaaa == 2: # comment
+ case (
+ "this is a long line for if condition"
+ ) if aaaaaaaaahhhhhhhh == 1 and bbbbbbaaaaaaaaaaa == 2: # comment
+ 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 (
@@ -93,7 +95,7 @@
case "named expressions aren't special" if foo := 1:
pass
- case "named expressions aren't that special" if (foo := 1):
+ case "named expressions aren't that special" if foo := 1:
pass
case "but with already broken long lines" if (
@@ -101,9 +103,9 @@
): # another comment
pass
- case {
- "long_long_long_key": str(long_long_long_key)
- } if value := "long long long long long long long long long long long value":
+ case {"long_long_long_key": str(long_long_long_key)} if (
+ value := "long long long long long long long long long long long value"
+ ):
pass
@@ -198,7 +200,9 @@
# trailing own 2
):
Expand Down

0 comments on commit 9442cd8

Please sign in to comment.