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

Parenthesize match..case if guards #13513

Merged
merged 1 commit into from
Sep 26, 2024
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 @@ -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
Loading