Handle pattern parentheses in FormatPattern

crates/ruff_python_formatter/src/other/match_case.rs

@@ -4,7 +4,7 @@ use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
 use ruff_text_size::TextRange;
 
 use crate::comments::SourceComment;
-use crate::expression::parentheses::parenthesized;
+use crate::expression::parentheses::{parenthesized, Parentheses};
 use crate::prelude::*;
 use crate::statement::clause::{clause_body, clause_header, ClauseHeader};
 use crate::{FormatError, FormatNodeRule, PyFormatter};
@@ -38,12 +38,14 @@ impl FormatNodeRule<MatchCase> for FormatMatchCase {
                     &format_with(|f| {
                         write!(f, [text("case"), space()])?;
 
-                        if !open_parenthesis_comments.is_empty()
-                            || is_match_case_pattern_parenthesized(item, pattern, f.context())?
-                        {
-                            parenthesized("(", &pattern.format(), ")")
-                                .with_dangling_comments(open_parenthesis_comments)
-                                .fmt(f)?;
+                        if is_match_case_pattern_parenthesized(item, pattern, f.context())? {
+                            parenthesized(
+                                "(",
+                                &pattern.format().with_options(Parentheses::Never),
+                                ")",
+                            )
+                            .with_dangling_comments(open_parenthesis_comments)
+                            .fmt(f)?;
                         } else {
                             pattern.format().fmt(f)?;
                         }

crates/ruff_python_formatter/src/pattern/mod.rs

@@ -1,6 +1,10 @@
-use ruff_formatter::{FormatOwnedWithRule, FormatRefWithRule};
-use ruff_python_ast::Pattern;
+use ruff_formatter::{
+    write, FormatOwnedWithRule, FormatRefWithRule, FormatRule, FormatRuleWithOptions,
+};
+use ruff_python_ast::{Pattern, Ranged};
+use ruff_python_trivia::{first_non_trivia_token, SimpleToken, SimpleTokenKind, SimpleTokenizer};
 
+use crate::expression::parentheses::{parenthesized, Parentheses};
 use crate::prelude::*;
 
 pub(crate) mod pattern_match_as;
@@ -12,20 +16,66 @@ pub(crate) mod pattern_match_singleton;
 pub(crate) mod pattern_match_star;
 pub(crate) mod pattern_match_value;
 
-#[derive(Default)]
-pub struct FormatPattern;
+#[derive(Copy, Clone, PartialEq, Eq, Default)]
+pub struct FormatPattern {
+    parentheses: Parentheses,
+}
+
+impl FormatRuleWithOptions<Pattern, PyFormatContext<'_>> for FormatPattern {
+    type Options = Parentheses;
+
+    fn with_options(mut self, options: Self::Options) -> Self {
+        self.parentheses = options;
+        self
+    }
+}
 
 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),
+    fn fmt(&self, pattern: &Pattern, f: &mut PyFormatter) -> FormatResult<()> {
+        let parentheses = self.parentheses;
+
+        let format_pattern = format_with(|f| match pattern {
+            Pattern::MatchValue(pattern) => pattern.format().fmt(f),
+            Pattern::MatchSingleton(pattern) => pattern.format().fmt(f),
+            Pattern::MatchSequence(pattern) => pattern.format().fmt(f),
+            Pattern::MatchMapping(pattern) => pattern.format().fmt(f),
+            Pattern::MatchClass(pattern) => pattern.format().fmt(f),
+            Pattern::MatchStar(pattern) => pattern.format().fmt(f),
+            Pattern::MatchAs(pattern) => pattern.format().fmt(f),
+            Pattern::MatchOr(pattern) => pattern.format().fmt(f),
+        });
+
+        let parenthesize = match parentheses {
+            Parentheses::Preserve => is_pattern_parenthesized(pattern, f.context().source()),
+            Parentheses::Always => true,
+            Parentheses::Never => false,
+        };
+
+        if parenthesize {
+            let comments = f.context().comments().clone();
+
+            // Any comments on the open parenthesis.
+            //
+            // For example, `# comment` in:
+            // ```python
+            // (  # comment
+            //    1
+            // )
+            // ```
+            let open_parenthesis_comment = comments
+                .leading(pattern)
+                .first()
+                .filter(|comment| comment.line_position().is_end_of_line());
+
+            parenthesized("(", &format_pattern, ")")
+                .with_dangling_comments(
+                    open_parenthesis_comment
+                        .map(std::slice::from_ref)
+                        .unwrap_or_default(),
+                )
+                .fmt(f)
+        } else {
+            write!(f, [format_pattern])
         }
     }
 }
@@ -34,14 +84,38 @@ 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)
+        FormatRefWithRule::new(self, FormatPattern::default())
     }
 }
 
 impl<'ast> IntoFormat<PyFormatContext<'ast>> for Pattern {
     type Format = FormatOwnedWithRule<Pattern, FormatPattern, PyFormatContext<'ast>>;
 
     fn into_format(self) -> Self::Format {
-        FormatOwnedWithRule::new(self, FormatPattern)
+        FormatOwnedWithRule::new(self, FormatPattern::default())
+    }
+}
+
+fn is_pattern_parenthesized(pattern: &Pattern, contents: &str) -> bool {
+    // First test if there's a closing parentheses because it tends to be cheaper.
+    if matches!(
+        first_non_trivia_token(pattern.end(), contents),
+        Some(SimpleToken {
+            kind: SimpleTokenKind::RParen,
+            ..
+        })
+    ) {
+        let mut tokenizer =
+            SimpleTokenizer::up_to_without_back_comment(pattern.start(), contents).skip_trivia();
+
+        matches!(
+            tokenizer.next_back(),
+            Some(SimpleToken {
+                kind: SimpleTokenKind::LParen,
+                ..
+            })
+        )
+    } else {
+        false
     }
 }

crates/ruff_python_formatter/src/pattern/pattern_match_as.rs

@@ -1,7 +1,6 @@
 use ruff_formatter::{write, Buffer, FormatResult};
-use ruff_python_ast::{Pattern, PatternMatchAs};
+use ruff_python_ast::PatternMatchAs;
 
-use crate::expression::parentheses::parenthesized;
 use crate::prelude::*;
 use crate::{FormatNodeRule, PyFormatter};
 
@@ -18,19 +17,7 @@ impl FormatNodeRule<PatternMatchAs> for FormatPatternMatchAs {
 
         if let Some(name) = name {
             if let Some(pattern) = pattern {
-                // Parenthesize nested `PatternMatchAs` like `(a as b) as c`.
-                if matches!(
-                    pattern.as_ref(),
-                    Pattern::MatchAs(PatternMatchAs {
-                        pattern: Some(_),
-                        ..
-                    })
-                ) {
-                    parenthesized("(", &pattern.format(), ")").fmt(f)?;
-                } else {
-                    pattern.format().fmt(f)?;
-                }
-
+                pattern.format().fmt(f)?;
                 write!(f, [space(), text("as"), space()])?;
             }
             name.format().fmt(f)

crates/ruff_python_formatter/src/pattern/pattern_match_value.rs

@@ -1,16 +1,16 @@
 use ruff_formatter::{write, Buffer, FormatResult};
 use ruff_python_ast::PatternMatchValue;
 
+use crate::expression::parentheses::Parentheses;
 use crate::{AsFormat, FormatNodeRule, PyFormatter};
 
 #[derive(Default)]
 pub struct FormatPatternMatchValue;
 
 impl FormatNodeRule<PatternMatchValue> for FormatPatternMatchValue {
     fn fmt_fields(&self, item: &PatternMatchValue, f: &mut PyFormatter) -> FormatResult<()> {
-        // TODO(charlie): Avoid double parentheses for parenthesized top-level `PatternMatchValue`.
         let PatternMatchValue { value, range: _ } = item;
-        let formatted = value.format();
+        let formatted = value.format().with_options(Parentheses::Never);
         write!(f, [formatted])
     }
 }

crates/ruff_python_formatter/tests/snapshots/black_compatibility@py_310__pattern_matching_extras.py.snap

@@ -234,17 +234,14 @@ match bar1:
          pass
  
  
-@@ -101,19 +102,22 @@
+@@ -101,19 +102,19 @@
      case 2 as b, 3 as c:
          pass
  
 -    case 4 as d, (5 as e), (6 | 7 as g), *h:
-+    case (
-+        4 as d,
-+        5 as e,
-+        NOT_YET_IMPLEMENTED_PatternMatchOf | (y) as g,
-+        *NOT_YET_IMPLEMENTED_PatternMatchStar,
-+    ):
++    case 4 as d, (5 as e), (
++        NOT_YET_IMPLEMENTED_PatternMatchOf | (y) as g
++    ), *NOT_YET_IMPLEMENTED_PatternMatchStar:
          pass
  
  
@@ -371,12 +368,9 @@ match something:
     case 2 as b, 3 as c:
         pass
 
-    case (
-        4 as d,
-        5 as e,
-        NOT_YET_IMPLEMENTED_PatternMatchOf | (y) as g,
-        *NOT_YET_IMPLEMENTED_PatternMatchStar,
-    ):
+    case 4 as d, (5 as e), (
+        NOT_YET_IMPLEMENTED_PatternMatchOf | (y) as g
+    ), *NOT_YET_IMPLEMENTED_PatternMatchStar:
         pass
 
 

crates/ruff_python_formatter/tests/snapshots/black_compatibility@py_310__pattern_matching_simple.py.snap

@@ -133,13 +133,13 @@ def where_is(point):
  
  match command.split():
 -    case ["go", ("north" | "south" | "east" | "west")]:
-+    case ["go", NOT_YET_IMPLEMENTED_PatternMatchOf | (y)]:
++    case ["go", (NOT_YET_IMPLEMENTED_PatternMatchOf | (y))]:
          current_room = current_room.neighbor(...)
          # how do I know which direction to go?
  
  match command.split():
 -    case ["go", ("north" | "south" | "east" | "west") as direction]:
-+    case ["go", NOT_YET_IMPLEMENTED_PatternMatchOf | (y) as direction]:
++    case ["go", (NOT_YET_IMPLEMENTED_PatternMatchOf | (y)) as direction]:
          current_room = current_room.neighbor(direction)
  
  match command.split():
@@ -239,12 +239,12 @@ match command.split():
         ...  # Code for picking up the given object
 
 match command.split():
-    case ["go", NOT_YET_IMPLEMENTED_PatternMatchOf | (y)]:
+    case ["go", (NOT_YET_IMPLEMENTED_PatternMatchOf | (y))]:
         current_room = current_room.neighbor(...)
         # how do I know which direction to go?
 
 match command.split():
-    case ["go", NOT_YET_IMPLEMENTED_PatternMatchOf | (y) as direction]:
+    case ["go", (NOT_YET_IMPLEMENTED_PatternMatchOf | (y)) as direction]:
         current_room = current_room.neighbor(direction)
 
 match command.split():

crates/ruff_python_formatter/tests/snapshots/format@statement__match.py.snap

@@ -464,23 +464,23 @@ match foo:
 match foo:
     case 1:
         y = 0
-    case ((1)):
+    case (1):
         y = 1
     case (  # comment
-        (1)
+        1
     ):
         y = 1
     case (
         # comment
-        (1)
+        1
     ):
         y = 1
     case (
-        (1)  # comment
+        1  # comment
     ):
         y = 1
     case (
-        (1)
+        1
         # comment
     ):
         y = 1