Formatting adds unnecessary parentheses to long patterns with `as` captures

[brandtbucher]

Summary

See this playground link, where I've put the expected formatting going in, and you can see the unexpected formatting coming out:

https://play.ruff.rs/81f62f2d-6858-43eb-8a9b-67dcebdc56f6?secondary=Format

It appears that "grouping" characters in patterns like (), [], and {} aren't being used correctly when they are followed by a capture (as name). Instead of breaking on these, the entire pattern (including the capture) is wrapped in parentheses and indented.

First encountered in https://github.com/python/cpython/pull/133123/files#r2071981099.

Version

0.11.8

[brandtbucher]

My guess is that ruff tries to wrap in parens, then start exploding the patterns if that isn't enough. Maybe trying to explode first would help?

[brandtbucher]

Or, maybe it is exploding from the outside in, and this is just how it tries to explode as patterns. That could be trickier.

[brandtbucher]

Black does the expected thing here.

[MichaReiser]

Thanks for reporting this incorrect formatting.

We use some heuristics to decide if we can omit the parentheses and it seems that it's overly restrictive when using as. It should probably recurse here

ruff/crates/ruff_python_formatter/src/pattern/mod.rs

Lines 290 to 334 in 424b720

	Pattern::MatchValue(value) => match &*value.value {
	Expr::StringLiteral(_) |
	Expr::BytesLiteral(_) |
	// F-strings are allowed according to python's grammar but fail with a syntax error at runtime.
	// That's why we need to support them for formatting.
	Expr::FString(_) |
	Expr::NumberLiteral(_) | Expr::Attribute(_) | Expr::UnaryOp(_) => {
	// require no state update other than visit_pattern does.
	}
	// `case 4+3j:` or `case 4-3j:
	// Can not contain arbitrary expressions. Limited to complex numbers.
	Expr::BinOp(_) => {
	self.update_max_precedence(OperatorPrecedence::Additive, 1);
	}
	_ => {
	debug_assert!(
	false,
	"Unsupported expression in pattern mach value: {:?}",
	value.value
	);
	}
	},
	Pattern::MatchClass(_) => {
	self.any_parenthesized_expressions = true;
	// 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::MatchOr(or_pattern) => {
	self.update_max_precedence(
	OperatorPrecedence::Or,
	or_pattern.patterns.len().saturating_sub(1),
	);
	for pattern in &or_pattern.patterns {
	self.visit_sub_pattern(pattern, context);
	}
	}
	}
	}
	fn visit_sub_pattern(&mut self, pattern: &'a Pattern, context: &PyFormatContext) {

and here

ruff/crates/ruff_python_formatter/src/pattern/mod.rs

Line 240 in 424b720

fn has_parentheses_and_is_non_empty(pattern: &Pattern, context: &PyFormatContext) -> bool {

But I'd have to think about if we can do this always or only sometimes.