mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-11-17 19:27:11 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Avoid extra parentheses for long match patterns with as captures (#21176)

Browse source 
Summary
--

This PR fixes #17796 by taking the approach mentioned in
https://github.com/astral-sh/ruff/issues/17796#issuecomment-2847943862
of simply recursing into the `MatchAs` patterns when checking if we need
parentheses. This allows us to reuse the parentheses in the inner
pattern before also breaking the `MatchAs` pattern itself:

```diff
 match class_pattern:
     case Class(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx) as capture:
         pass
-    case (
-        Class(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx) as capture
-    ):
+    case Class(
+        xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+    ) as capture:
         pass
-    case (
-        Class(
-            xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
-        ) as capture
-    ):
+    case Class(
+        xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+    ) as capture:
         pass
     case (
         Class(
@@ -685,13 +683,11 @@
 match sequence_pattern_brackets:
     case [xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx] as capture:
         pass
-    case (
-        [xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx] as capture
-    ):
+    case [
+        xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+    ] as capture:
         pass
-    case (
-        [
-            xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
-        ] as capture
-    ):
+    case [
+        xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+    ] as capture:
         pass
```

I haven't really resolved the question of whether or not it's okay
always to recurse, but I'm hoping the ecosystem check on this PR might
shed some light on that.

Test Plan
--

New tests based on the issue and then reviewing the ecosystem check here
This commit is contained in:
Brent Westbrook 2025-11-03 17:06:52 -05:00 • committed by GitHub
parent 3c5e4e1477
commit 63b1c1ea8b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 307 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files

55
crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/match.py vendored
View file

@ -613,3 +613,58 @@ match guard_comments:
):
pass
# regression tests from https://github.com/astral-sh/ruff/issues/17796
match class_pattern:
case Class(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx) as capture:
pass
case Class(
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
) as capture:
pass
case Class(
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
) as capture:
pass
case Class(
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
) as really_really_really_really_really_really_really_really_really_really_really_really_long_capture:
pass
match sequence_pattern_brackets:
case [xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx] as capture:
pass
case [
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
] as capture:
pass
case [
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
] as capture:
pass
match class_pattern:
# 1
case Class(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx) as capture: # 2
# 3
pass # 4
# 5
case Class( # 6
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 7
) as capture: # 8
pass
case Class( # 9
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 10
) as capture: # 11
pass
case Class( # 12
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 13
) as really_really_really_really_really_really_really_really_really_really_really_really_long_capture: # 14
pass
case Class( # 0
# 1
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 2
# 3
) as capture:
pass

20
crates/ruff_python_formatter/src/pattern/mod.rs
View file

@ -1,5 +1,5 @@
use ruff_formatter::{FormatOwnedWithRule, FormatRefWithRule, FormatRule, FormatRuleWithOptions};
use ruff_python_ast::{AnyNodeRef, Expr};
use ruff_python_ast::{AnyNodeRef, Expr, PatternMatchAs};
use ruff_python_ast::{MatchCase, Pattern};
use ruff_python_trivia::CommentRanges;
use ruff_python_trivia::{
@ -14,6 +14,7 @@ use crate::expression::parentheses::{
NeedsParentheses, OptionalParentheses, Parentheses, optional_parentheses, parenthesized,
};
use crate::prelude::*;
use crate::preview::is_avoid_parens_for_long_as_captures_enabled;
pub(crate) mod pattern_arguments;
pub(crate) mod pattern_keyword;
@ -242,8 +243,14 @@ pub(crate) fn can_pattern_omit_optional_parentheses(
Pattern::MatchValue(_)
| Pattern::MatchSingleton(_)
| Pattern::MatchStar(_)
| Pattern::MatchAs(_)
| Pattern::MatchOr(_) => false,
Pattern::MatchAs(PatternMatchAs { pattern, .. }) => match pattern {
Some(pattern) => {
is_avoid_parens_for_long_as_captures_enabled(context)
&& has_parentheses_and_is_non_empty(pattern, context)
}
None => false,
},
Pattern::MatchSequence(sequence) => {
!sequence.patterns.is_empty() || context.comments().has_dangling(pattern)
}
@ -318,7 +325,14 @@ impl<'a> CanOmitOptionalParenthesesVisitor<'a> {
// The pattern doesn't start with a parentheses pattern, but with the class's identifier.
self.first.set_if_none(First::Token);
}
Pattern::MatchStar(_) | Pattern::MatchSingleton(_) | Pattern::MatchAs(_) => {}
Pattern::MatchAs(PatternMatchAs { pattern, .. }) => {
if let Some(pattern) = pattern
&& is_avoid_parens_for_long_as_captures_enabled(context)
{
self.visit_sub_pattern(pattern, context);
}
}
Pattern::MatchStar(_) | Pattern::MatchSingleton(_) => {}
Pattern::MatchOr(or_pattern) => {
self.update_max_precedence(
OperatorPrecedence::Or,

9
crates/ruff_python_formatter/src/preview.rs
View file

@ -43,3 +43,12 @@ pub(crate) const fn is_remove_parens_around_except_types_enabled(
pub(crate) const fn is_allow_newline_after_block_open_enabled(context: &PyFormatContext) -> bool {
context.is_preview()
}
/// Returns `true` if the
/// [`avoid_parens_for_long_as_captures`](https://github.com/astral-sh/ruff/pull/21176) preview
/// style is enabled.
pub(crate) const fn is_avoid_parens_for_long_as_captures_enabled(
context: &PyFormatContext,
) -> bool {
context.is_preview()
}

227
crates/ruff_python_formatter/tests/snapshots/format@statement__match.py.snap
View file

@ -1,7 +1,6 @@
---
source: crates/ruff_python_formatter/tests/fixtures.rs
input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/match.py
snapshot_kind: text
---
## Input
```python
@ -620,6 +619,61 @@ match guard_comments:
):
pass
# regression tests from https://github.com/astral-sh/ruff/issues/17796
match class_pattern:
case Class(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx) as capture:
pass
case Class(
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
) as capture:
pass
case Class(
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
) as capture:
pass
case Class(
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
) as really_really_really_really_really_really_really_really_really_really_really_really_long_capture:
pass
match sequence_pattern_brackets:
case [xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx] as capture:
pass
case [
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
] as capture:
pass
case [
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
] as capture:
pass
match class_pattern:
# 1
case Class(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx) as capture: # 2
# 3
pass # 4
# 5
case Class( # 6
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 7
) as capture: # 8
pass
case Class( # 9
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 10
) as capture: # 11
pass
case Class( # 12
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 13
) as really_really_really_really_really_really_really_really_really_really_really_really_long_capture: # 14
pass
case Class( # 0
# 1
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 2
# 3
) as capture:
pass
```
## Output
@ -1285,4 +1339,175 @@ match guard_comments:
# trailing own line comment
):
pass
# regression tests from https://github.com/astral-sh/ruff/issues/17796
match class_pattern:
case Class(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx) as capture:
pass
case (
Class(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx) as capture
):
pass
case (
Class(
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
) as capture
):
pass
case (
Class(
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
) as really_really_really_really_really_really_really_really_really_really_really_really_long_capture
):
pass
match sequence_pattern_brackets:
case [xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx] as capture:
pass
case (
[xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx] as capture
):
pass
case (
[
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
] as capture
):
pass
match class_pattern:
# 1
case (
Class(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx) as capture
): # 2
# 3
pass # 4
# 5
case (
Class( # 6
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 7
) as capture
): # 8
pass
case (
Class( # 9
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 10
) as capture
): # 11
pass
case (
Class( # 12
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 13
) as really_really_really_really_really_really_really_really_really_really_really_really_long_capture
): # 14
pass
case (
Class( # 0
# 1
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 2
# 3
) as capture
):
pass
```
## Preview changes
```diff
--- Stable
+++ Preview
@@ -665,15 +665,13 @@
match class_pattern:
case Class(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx) as capture:
pass
- case (
- Class(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx) as capture
- ):
+ case Class(
+ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ ) as capture:
pass
- case (
- Class(
- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
- ) as capture
- ):
+ case Class(
+ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ ) as capture:
pass
case (
Class(
@@ -685,37 +683,31 @@
match sequence_pattern_brackets:
case [xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx] as capture:
pass
- case (
- [xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx] as capture
- ):
+ case [
+ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ ] as capture:
pass
- case (
- [
- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
- ] as capture
- ):
+ case [
+ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ ] as capture:
pass
match class_pattern:
# 1
- case (
- Class(xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx) as capture
- ): # 2
+ case Class(
+ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ ) as capture: # 2
# 3
pass # 4
# 5
- case (
- Class( # 6
- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 7
- ) as capture
- ): # 8
+ case Class( # 6
+ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 7
+ ) as capture: # 8
pass
- case (
- Class( # 9
- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 10
- ) as capture
- ): # 11
+ case Class( # 9
+ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 10
+ ) as capture: # 11
pass
case (
Class( # 12
@@ -723,11 +715,9 @@
) as really_really_really_really_really_really_really_really_really_really_really_really_long_capture
): # 14
pass
- case (
- Class( # 0
- # 1
- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 2
- # 3
- ) as capture
- ):
+ case Class( # 0
+ # 1
+ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx # 2
+ # 3
+ ) as capture:
pass
```