Skip to content

Commit

Permalink
Track magic comma
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Jul 10, 2023
1 parent 8d704c2 commit bba6fd0
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 28 deletions.
36 changes: 21 additions & 15 deletions crates/ruff_python_formatter/src/expression/mod.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
use std::cmp::Ordering;

use rustpython_parser::ast;
use rustpython_parser::ast::{Expr, Operator};
use std::cmp::Ordering;

use crate::builders::optional_parentheses;
use ruff_formatter::{
format_args, FormatOwnedWithRule, FormatRefWithRule, FormatRule, FormatRuleWithOptions,
};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::expression::ExpressionRef;
use ruff_python_ast::visitor::preorder::{walk_expr, PreorderVisitor};

use crate::builders::optional_parentheses;
use crate::context::NodeLevel;
use crate::expression::expr_tuple::TupleParentheses;
use crate::expression::parentheses::{
Expand Down Expand Up @@ -218,19 +219,24 @@ impl<'ast> IntoFormat<PyFormatContext<'ast>> for Expr {
/// `(a * b) * c` or `a * b + c` are okay, because the subexpression is parenthesized, or the expression uses operands with a lower priority
/// * The expression contains at least one parenthesized sub expression (optimization to avoid unnecessary work)
fn can_omit_optional_parentheses(expr: &Expr, context: &PyFormatContext) -> bool {
let mut visitor = MaxOperatorPriorityVisitor::new(context.source());
if context.comments().has_leading_comments(expr) {
false
} else {
let mut visitor = MaxOperatorPriorityVisitor::new(context.source());

visitor.visit_subexpression(expr);
visitor.visit_subexpression(expr);

let (max_operator_priority, operation_count, any_parenthesized_expression) = visitor.finish();
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
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
}
}
}

Expand Down Expand Up @@ -377,7 +383,7 @@ impl<'input> MaxOperatorPriorityVisitor<'input> {
impl<'input> PreorderVisitor<'input> for MaxOperatorPriorityVisitor<'input> {
fn visit_expr(&mut self, expr: &'input Expr) {
// Rule only applies for non-parenthesized expressions.
if is_expression_parenthesized(AnyNodeRef::from(expr), self.source) {
if is_expression_parenthesized(ExpressionRef::from(expr), self.source) {
self.any_parenthesized_expressions = true;
} else {
self.visit_subexpression(expr);
Expand All @@ -397,7 +403,7 @@ fn has_parentheses(expr: &Expr, source: &str) -> bool {
| Expr::DictComp(_)
| Expr::Call(_)
| Expr::Subscript(_)
) || is_expression_parenthesized(AnyNodeRef::from(expr), source)
) || is_expression_parenthesized(ExpressionRef::from(expr), source)
}

#[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq)]
Expand Down
16 changes: 3 additions & 13 deletions crates/ruff_python_formatter/src/expression/parentheses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,27 +32,17 @@ pub(super) fn default_expression_needs_parentheses(
// `Optional` or `IfBreaks`: Add parentheses if the expression doesn't fit on a line but enforce
// parentheses if the expression has leading comments
else if !parenthesize.is_preserve() {
if can_omit_optional_parentheses(node, context) {
Parentheses::Optional
} else {
if context.comments().has_leading_comments(node) {
Parentheses::Always
} else {
Parentheses::Optional
}
} else {
//`Preserve` and expression has no parentheses in the source code
Parentheses::Never
}
}

fn can_omit_optional_parentheses(expr: ExpressionRef, context: &PyFormatContext) -> bool {
if context.comments().has_leading_comments(expr) {
false
} else {
true
}
}

// fn has_magic_comma(expr: AnyNodeRef)

/// Configures if the expression should be parenthesized.
#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]
pub enum Parenthesize {
Expand Down

0 comments on commit bba6fd0

Please sign in to comment.