From 4d7802b76a1eeb354b901f3057dbfe2a7a4ca6e0 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 25 Sep 2024 18:16:31 +0200 Subject: [PATCH] Parenthesize `match..case` `if` guards --- .../test/fixtures/ruff/statement/match.py | 25 ++++++ .../src/expression/parentheses.rs | 4 + .../src/other/match_case.rs | 26 ++++-- crates/ruff_python_formatter/src/preview.rs | 2 + ...ses__pattern_matching_with_if_stmt.py.snap | 61 ++++--------- ...ove_redundant_parens_in_case_guard.py.snap | 38 ++------ .../snapshots/format@statement__match.py.snap | 87 ++++++++++++++++++- 7 files changed, 155 insertions(+), 88 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/match.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/match.py index af1e25c96f785..4067d508c07bb 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/match.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/match.py @@ -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 + diff --git a/crates/ruff_python_formatter/src/expression/parentheses.rs b/crates/ruff_python_formatter/src/expression/parentheses.rs index 002099a136548..9b8096dbaece1 100644 --- a/crates/ruff_python_formatter/src/expression/parentheses.rs +++ b/crates/ruff_python_formatter/src/expression/parentheses.rs @@ -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 diff --git a/crates/ruff_python_formatter/src/other/match_case.rs b/crates/ruff_python_formatter/src/other/match_case.rs index 08a68dabb8fc6..3fd5fd1fea31e 100644 --- a/crates/ruff_python_formatter/src/other/match_case.rs +++ b/crates/ruff_python_formatter/src/other/match_case.rs @@ -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; @@ -62,6 +65,19 @@ impl FormatNodeRule 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, [ @@ -69,13 +85,7 @@ impl FormatNodeRule for FormatMatchCase { 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( diff --git a/crates/ruff_python_formatter/src/preview.rs b/crates/ruff_python_formatter/src/preview.rs index f8b1b7a63ddcf..d688a90fb3adf 100644 --- a/crates/ruff_python_formatter/src/preview.rs +++ b/crates/ruff_python_formatter/src/preview.rs @@ -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() } diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__pattern_matching_with_if_stmt.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__pattern_matching_with_if_stmt.py.snap index ff72c23d009b5..02f6b0636a1f1 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__pattern_matching_with_if_stmt.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__pattern_matching_with_if_stmt.py.snap @@ -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 ( @@ -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 @@ -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 @@ -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 ``` diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__remove_redundant_parens_in_case_guard.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__remove_redundant_parens_in_case_guard.py.snap index 6f0c6257e3a7a..d848db4ce8a89 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__remove_redundant_parens_in_case_guard.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__remove_redundant_parens_in_case_guard.py.snap @@ -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: @@ -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: @@ -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 @@ -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 diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__match.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__match.py.snap index 038af7c075669..e12a5cf5a646c 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__match.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__match.py.snap @@ -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 @@ -1232,6 +1257,31 @@ 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 ``` @@ -1239,17 +1289,48 @@ match x: ```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 ):