Skip to content

Commit

Permalink
Only omit optinal parens if the expression ends or starts with a pare…
Browse files Browse the repository at this point in the history
…nthesized expression
  • Loading branch information
MichaReiser committed Jul 11, 2023
1 parent e83b302 commit ed3ad9e
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 159 deletions.
56 changes: 34 additions & 22 deletions crates/ruff_python_formatter/src/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,37 +198,30 @@ impl<'ast> IntoFormat<PyFormatContext<'ast>> for Expr {
///
/// This mimics Black's [`_maybe_split_omitting_optional_parens`](https://github.com/psf/black/blob/d1248ca9beaf0ba526d265f4108836d89cf551b7/src/black/linegen.py#L746-L820)
fn can_omit_optional_parentheses(expr: &Expr, context: &PyFormatContext) -> bool {
let mut visitor = MaxOperatorPriorityVisitor::new(context.source());

let mut visitor = CanOmitOptionalParenthesesVisitor::new(context.source());
visitor.visit_subexpression(expr);

let (max_operator_priority, operation_count, any_parenthesized_expression) = visitor.finish();

if operation_count > 1 {
false
} else if max_operator_priority == OperatorPriority::Attribute {
true
} else {
// Only use the more complex IR when there is any expression that we can possibly split by
any_parenthesized_expression
}
visitor.can_omit()
}

#[derive(Clone, Debug)]
struct MaxOperatorPriorityVisitor<'input> {
struct CanOmitOptionalParenthesesVisitor<'input> {
max_priority: OperatorPriority,
max_priority_count: u32,
any_parenthesized_expressions: bool,
last: Option<&'input Expr>,
first: Option<&'input Expr>,
source: &'input str,
}

impl<'input> MaxOperatorPriorityVisitor<'input> {
impl<'input> CanOmitOptionalParenthesesVisitor<'input> {
fn new(source: &'input str) -> Self {
Self {
source,
max_priority: OperatorPriority::None,
max_priority_count: 0,
any_parenthesized_expressions: false,
last: None,
first: None,
}
}

Expand Down Expand Up @@ -305,6 +298,7 @@ impl<'input> MaxOperatorPriorityVisitor<'input> {
self.any_parenthesized_expressions = true;
// Only walk the function, the arguments are always parenthesized
self.visit_expr(func);
self.last = Some(expr);
return;
}
Expr::Subscript(_) => {
Expand Down Expand Up @@ -351,23 +345,41 @@ impl<'input> MaxOperatorPriorityVisitor<'input> {
walk_expr(self, expr);
}

fn finish(self) -> (OperatorPriority, u32, bool) {
(
self.max_priority,
self.max_priority_count,
self.any_parenthesized_expressions,
)
fn can_omit(self) -> bool {
if self.max_priority_count > 1 {
false
} else if self.max_priority == OperatorPriority::Attribute {
true
} else if !self.any_parenthesized_expressions {
// Only use the more complex IR when there is any expression that we can possibly split by
false
} else {
// Only use the layout if the first or last expression has parentheses of some sort.
let first_parenthesized = self
.first
.map_or(false, |first| has_parentheses(first, self.source));
let last_parenthesized = self
.last
.map_or(false, |last| has_parentheses(last, self.source));
first_parenthesized || last_parenthesized
}
}
}

impl<'input> PreorderVisitor<'input> for MaxOperatorPriorityVisitor<'input> {
impl<'input> PreorderVisitor<'input> for CanOmitOptionalParenthesesVisitor<'input> {
fn visit_expr(&mut self, expr: &'input Expr) {
self.last = Some(expr);

// Rule only applies for non-parenthesized expressions.
if is_expression_parenthesized(AnyNodeRef::from(expr), self.source) {
self.any_parenthesized_expressions = true;
} else {
self.visit_subexpression(expr);
}

if self.first.is_none() {
self.first = Some(expr);
}
}
}

Expand Down
11 changes: 9 additions & 2 deletions crates/ruff_python_formatter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,15 @@ if True:
#[test]
fn quick_test() {
let src = r#"
def foo() -> tuple[int, int, int,]:
return 2
if a * [
bbbbbbbbbbbbbbbbbbbbbb,
cccccccccccccccccccccccccccccdddddddddddddddddddddddddd,
] + a * e * [
ffff,
gggg,
hhhhhhhhhhhhhh,
] * c:
pass
"#;
// Tokenize once
Expand Down

This file was deleted.

0 comments on commit ed3ad9e

Please sign in to comment.