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

Group binary operators with same precedence only (#7010)

Browse source
This commit is contained in:
Micha Reiser 2023-08-31 09:19:45 +02:00 • committed by GitHub
parent eb552da8a9
commit 92143afeee
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 55 additions and 80 deletions
Download patch file Download diff file Expand all files Collapse all files

6
crates/ruff_python_formatter/src/expression/expr_bin_op.rs
View file

@ -18,6 +18,7 @@ use crate::expression::parentheses::{
NeedsParentheses, OptionalParentheses, NeedsParentheses, OptionalParentheses,
}; };
use crate::expression::string::StringLayout; use crate::expression::string::StringLayout;
use crate::expression::OperatorPrecedence;
use crate::prelude::*; use crate::prelude::*;
#[derive(Default)] #[derive(Default)]
@ -67,10 +68,13 @@ impl FormatNodeRule<ExprBinOp> for FormatExprBinOp {
BinOpLayout::Default => { BinOpLayout::Default => {
let format_inner = format_with(|f: &mut PyFormatter| { let format_inner = format_with(|f: &mut PyFormatter| {
let source = f.context().source(); let source = f.context().source();
let precedence = OperatorPrecedence::from(item.op);
let binary_chain: SmallVec<[&ExprBinOp; 4]> = let binary_chain: SmallVec<[&ExprBinOp; 4]> =
iter::successors(Some(item), |parent| { iter::successors(Some(item), |parent| {
parent.left.as_bin_op_expr().and_then(|bin_expression| { parent.left.as_bin_op_expr().and_then(|bin_expression| {
if is_expression_parenthesized(bin_expression.into(), source) { if OperatorPrecedence::from(bin_expression.op) != precedence
|| is_expression_parenthesized(bin_expression.into(), source)
{
None None
} else { } else {
Some(bin_expression) Some(bin_expression)

80
crates/ruff_python_formatter/src/expression/mod.rs
View file

@ -363,9 +363,9 @@ impl<'ast> IntoFormat<PyFormatContext<'ast>> for Expr {
/// Tests if it is safe to omit the optional parentheses. /// Tests if it is safe to omit the optional parentheses.
/// ///
/// We prefer parentheses at least in the following cases: /// We prefer parentheses at least in the following cases:
/// * The expression contains more than one unparenthesized expression with the same priority. For example, /// * The expression contains more than one unparenthesized expression with the same precedence. For example,
/// the expression `a * b * c` contains two multiply operations. We prefer parentheses in that case. /// the expression `a * b * c` contains two multiply operations. We prefer parentheses in that case.
/// `(a * b) * c` or `a * b + c` are okay, because the subexpression is parenthesized, or the expression uses operands with a lower priority /// `(a * b) * c` or `a * b + c` are okay, because the subexpression is parenthesized, or the expression uses operands with a lower precedence
/// * The expression contains at least one parenthesized sub expression (optimization to avoid unnecessary work) /// * The expression contains at least one parenthesized sub expression (optimization to avoid unnecessary work)
/// ///
/// This mimics Black's [`_maybe_split_omitting_optional_parens`](https://github.com/psf/black/blob/d1248ca9beaf0ba526d265f4108836d89cf551b7/src/black/linegen.py#L746-L820) /// This mimics Black's [`_maybe_split_omitting_optional_parens`](https://github.com/psf/black/blob/d1248ca9beaf0ba526d265f4108836d89cf551b7/src/black/linegen.py#L746-L820)
@ -373,11 +373,11 @@ fn can_omit_optional_parentheses(expr: &Expr, context: &PyFormatContext) -> bool
let mut visitor = CanOmitOptionalParenthesesVisitor::new(context); let mut visitor = CanOmitOptionalParenthesesVisitor::new(context);
visitor.visit_subexpression(expr); visitor.visit_subexpression(expr);
if visitor.max_priority == OperatorPriority::None { if visitor.max_precedence == OperatorPrecedence::None {
true true
} else if visitor.max_priority_count > 1 { } else if visitor.pax_precedence_count > 1 {
false false
} else if visitor.max_priority == OperatorPriority::Attribute { } else if visitor.max_precedence == OperatorPrecedence::Attribute {
true true
} else if !visitor.any_parenthesized_expressions { } else if !visitor.any_parenthesized_expressions {
// Only use the more complex IR when there is any expression that we can possibly split by // Only use the more complex IR when there is any expression that we can possibly split by
@ -397,8 +397,8 @@ fn can_omit_optional_parentheses(expr: &Expr, context: &PyFormatContext) -> bool
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
struct CanOmitOptionalParenthesesVisitor<'input> { struct CanOmitOptionalParenthesesVisitor<'input> {
max_priority: OperatorPriority, max_precedence: OperatorPrecedence,
max_priority_count: u32, pax_precedence_count: u32,
any_parenthesized_expressions: bool, any_parenthesized_expressions: bool,
last: Option<&'input Expr>, last: Option<&'input Expr>,
first: Option<&'input Expr>, first: Option<&'input Expr>,
@ -409,26 +409,26 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> {
fn new(context: &'input PyFormatContext) -> Self { fn new(context: &'input PyFormatContext) -> Self {
Self { Self {
context, context,
max_priority: OperatorPriority::None, max_precedence: OperatorPrecedence::None,
max_priority_count: 0, pax_precedence_count: 0,
any_parenthesized_expressions: false, any_parenthesized_expressions: false,
last: None, last: None,
first: None, first: None,
} }
} }
fn update_max_priority(&mut self, current_priority: OperatorPriority) { fn update_max_precedence(&mut self, precedence: OperatorPrecedence) {
self.update_max_priority_with_count(current_priority, 1); self.update_max_precedence_with_count(precedence, 1);
} }
fn update_max_priority_with_count(&mut self, current_priority: OperatorPriority, count: u32) { fn update_max_precedence_with_count(&mut self, precedence: OperatorPrecedence, count: u32) {
match self.max_priority.cmp(&current_priority) { match self.max_precedence.cmp(&precedence) {
Ordering::Less => { Ordering::Less => {
self.max_priority_count = count; self.pax_precedence_count = count;
self.max_priority = current_priority; self.max_precedence = precedence;
} }
Ordering::Equal => { Ordering::Equal => {
self.max_priority_count += count; self.pax_precedence_count += count;
} }
Ordering::Greater => {} Ordering::Greater => {}
} }
@ -455,8 +455,8 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> {
range: _, range: _,
op: _, op: _,
values, values,
}) => self.update_max_priority_with_count( }) => self.update_max_precedence_with_count(
OperatorPriority::BooleanOperation, OperatorPrecedence::BooleanOperation,
values.len().saturating_sub(1) as u32, values.len().saturating_sub(1) as u32,
), ),
Expr::BinOp(ast::ExprBinOp { Expr::BinOp(ast::ExprBinOp {
@ -464,11 +464,11 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> {
left: _, left: _,
right: _, right: _,
range: _, range: _,
}) => self.update_max_priority(OperatorPriority::from(*op)), }) => self.update_max_precedence(OperatorPrecedence::from(*op)),
Expr::IfExp(_) => { Expr::IfExp(_) => {
// + 1 for the if and one for the else // + 1 for the if and one for the else
self.update_max_priority_with_count(OperatorPriority::Conditional, 2); self.update_max_precedence_with_count(OperatorPrecedence::Conditional, 2);
} }
// It's impossible for a file smaller or equal to 4GB to contain more than 2^32 comparisons // It's impossible for a file smaller or equal to 4GB to contain more than 2^32 comparisons
@ -480,7 +480,10 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> {
ops, ops,
comparators: _, comparators: _,
}) => { }) => {
self.update_max_priority_with_count(OperatorPriority::Comparator, ops.len() as u32); self.update_max_precedence_with_count(
OperatorPrecedence::Comparator,
ops.len() as u32,
);
} }
Expr::Call(ast::ExprCall { Expr::Call(ast::ExprCall {
range: _, range: _,
@ -506,7 +509,7 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> {
operand: _, operand: _,
}) => { }) => {
if op.is_invert() { if op.is_invert() {
self.update_max_priority(OperatorPriority::BitwiseInversion); self.update_max_precedence(OperatorPrecedence::BitwiseInversion);
} }
} }
@ -519,7 +522,7 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> {
}) => { }) => {
self.visit_expr(value); self.visit_expr(value);
if has_parentheses(value, self.context).is_some() { if has_parentheses(value, self.context).is_some() {
self.update_max_priority(OperatorPriority::Attribute); self.update_max_precedence(OperatorPrecedence::Attribute);
} }
self.last = Some(expr); self.last = Some(expr);
return; return;
@ -541,7 +544,7 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> {
implicit_concatenated: true, implicit_concatenated: true,
.. ..
}) => { }) => {
self.update_max_priority(OperatorPriority::String); self.update_max_precedence(OperatorPrecedence::String);
} }
Expr::NamedExpr(_) Expr::NamedExpr(_)
@ -777,38 +780,45 @@ pub(crate) fn has_own_parentheses(
} }
} }
/// The precedence of [python operators](https://docs.python.org/3/reference/expressions.html#operator-precedence) from
/// lowest to highest priority.
///
/// Ruff uses the operator precedence to decide in which order to split operators:
/// Operators with a lower precedence split before higher-precedence operators.
/// Splitting by precedence ensures that the visual grouping reflects the precedence.
#[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq)] #[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq)]
enum OperatorPriority { enum OperatorPrecedence {
None, None,
Attribute, Attribute,
Comparator,
Exponential, Exponential,
BitwiseInversion, BitwiseInversion,
Multiplicative, Multiplicative,
Additive, Additive,
Shift, Shift,
BitwiseAnd, BitwiseAnd,
BitwiseOr,
BitwiseXor, BitwiseXor,
BitwiseOr,
Comparator,
// Implicit string concatenation
String, String,
BooleanOperation, BooleanOperation,
Conditional, Conditional,
} }
impl From<ast::Operator> for OperatorPriority { impl From<ast::Operator> for OperatorPrecedence {
fn from(value: Operator) -> Self { fn from(value: Operator) -> Self {
match value { match value {
Operator::Add | Operator::Sub => OperatorPriority::Additive, Operator::Add | Operator::Sub => OperatorPrecedence::Additive,
Operator::Mult Operator::Mult
| Operator::MatMult | Operator::MatMult
| Operator::Div | Operator::Div
| Operator::Mod | Operator::Mod
| Operator::FloorDiv => OperatorPriority::Multiplicative, | Operator::FloorDiv => OperatorPrecedence::Multiplicative,
Operator::Pow => OperatorPriority::Exponential, Operator::Pow => OperatorPrecedence::Exponential,
Operator::LShift | Operator::RShift => OperatorPriority::Shift, Operator::LShift | Operator::RShift => OperatorPrecedence::Shift,
Operator::BitOr => OperatorPriority::BitwiseOr, Operator::BitOr => OperatorPrecedence::BitwiseOr,
Operator::BitXor => OperatorPriority::BitwiseXor, Operator::BitXor => OperatorPrecedence::BitwiseXor,
Operator::BitAnd => OperatorPriority::BitwiseAnd, Operator::BitAnd => OperatorPrecedence::BitwiseAnd,
} }
} }
} }

46
crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__expression.py.snap
View file

@ -311,38 +311,6 @@ last_call()
sync(async_xxxx_xxx_xxxx_xxxxx_xxxx_xxx.__func__) sync(async_xxxx_xxx_xxxx_xxxxx_xxxx_xxx.__func__)
) )
xxxx_xxx_xxxx_xxxxx_xxxx_xxx: Callable[..., List[SomeClass]] = classmethod( xxxx_xxx_xxxx_xxxxx_xxxx_xxx: Callable[..., List[SomeClass]] = classmethod(
@@ -328,13 +331,18 @@
):
return True
if (
- ~aaaa.a + aaaa.b - aaaa.c * aaaa.d / aaaa.e
+ ~aaaa.a
+ + aaaa.b
+ - aaaa.c * aaaa.d / aaaa.e
| aaaa.f & aaaa.g % aaaa.h ^ aaaa.i << aaaa.k >> aaaa.l**aaaa.m // aaaa.n
):
return True
if (
- ~aaaaaaaa.a + aaaaaaaa.b - aaaaaaaa.c @ aaaaaaaa.d / aaaaaaaa.e
- | aaaaaaaa.f & aaaaaaaa.g % aaaaaaaa.h
+ ~aaaaaaaa.a
+ + aaaaaaaa.b
+ - aaaaaaaa.c @ aaaaaaaa.d / aaaaaaaa.e
+ | aaaaaaaa.f
+ & aaaaaaaa.g % aaaaaaaa.h
^ aaaaaaaa.i << aaaaaaaa.k >> aaaaaaaa.l**aaaaaaaa.m // aaaaaaaa.n
):
return True
@@ -342,7 +350,8 @@
~aaaaaaaaaaaaaaaa.a
+ aaaaaaaaaaaaaaaa.b
- aaaaaaaaaaaaaaaa.c * aaaaaaaaaaaaaaaa.d @ aaaaaaaaaaaaaaaa.e
- | aaaaaaaaaaaaaaaa.f & aaaaaaaaaaaaaaaa.g % aaaaaaaaaaaaaaaa.h
+ | aaaaaaaaaaaaaaaa.f
+ & aaaaaaaaaaaaaaaa.g % aaaaaaaaaaaaaaaa.h
^ aaaaaaaaaaaaaaaa.i
<< aaaaaaaaaaaaaaaa.k
>> aaaaaaaaaaaaaaaa.l**aaaaaaaaaaaaaaaa.m // aaaaaaaaaaaaaaaa.n
``` ```
## Ruff Output ## Ruff Output
@ -681,18 +649,13 @@ if (
): ):
return True return True
if ( if (
~aaaa.a ~aaaa.a + aaaa.b - aaaa.c * aaaa.d / aaaa.e
+ aaaa.b
- aaaa.c * aaaa.d / aaaa.e
| aaaa.f & aaaa.g % aaaa.h ^ aaaa.i << aaaa.k >> aaaa.l**aaaa.m // aaaa.n | aaaa.f & aaaa.g % aaaa.h ^ aaaa.i << aaaa.k >> aaaa.l**aaaa.m // aaaa.n
): ):
return True return True
if ( if (
~aaaaaaaa.a ~aaaaaaaa.a + aaaaaaaa.b - aaaaaaaa.c @ aaaaaaaa.d / aaaaaaaa.e
+ aaaaaaaa.b | aaaaaaaa.f & aaaaaaaa.g % aaaaaaaa.h
- aaaaaaaa.c @ aaaaaaaa.d / aaaaaaaa.e
| aaaaaaaa.f
& aaaaaaaa.g % aaaaaaaa.h
^ aaaaaaaa.i << aaaaaaaa.k >> aaaaaaaa.l**aaaaaaaa.m // aaaaaaaa.n ^ aaaaaaaa.i << aaaaaaaa.k >> aaaaaaaa.l**aaaaaaaa.m // aaaaaaaa.n
): ):
return True return True
@ -700,8 +663,7 @@ if (
~aaaaaaaaaaaaaaaa.a ~aaaaaaaaaaaaaaaa.a
+ aaaaaaaaaaaaaaaa.b + aaaaaaaaaaaaaaaa.b
- aaaaaaaaaaaaaaaa.c * aaaaaaaaaaaaaaaa.d @ aaaaaaaaaaaaaaaa.e - aaaaaaaaaaaaaaaa.c * aaaaaaaaaaaaaaaa.d @ aaaaaaaaaaaaaaaa.e
| aaaaaaaaaaaaaaaa.f | aaaaaaaaaaaaaaaa.f & aaaaaaaaaaaaaaaa.g % aaaaaaaaaaaaaaaa.h
& aaaaaaaaaaaaaaaa.g % aaaaaaaaaaaaaaaa.h
^ aaaaaaaaaaaaaaaa.i ^ aaaaaaaaaaaaaaaa.i
<< aaaaaaaaaaaaaaaa.k << aaaaaaaaaaaaaaaa.k
>> aaaaaaaaaaaaaaaa.l**aaaaaaaaaaaaaaaa.m // aaaaaaaaaaaaaaaa.n >> aaaaaaaaaaaaaaaa.l**aaaaaaaaaaaaaaaa.m // aaaaaaaaaaaaaaaa.n

3
crates/ruff_python_formatter/tests/snapshots/format@expression__binary_implicit_string.py.snap
View file

@ -215,8 +215,7 @@ self._assert_skipping(
( (
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaa" "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
"bbbbbbbbbbbbbbbbbbbbbbbbbbbbb" "bbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
"cccccccccccccccccccccccccc" "cccccccccccccccccccccccccc" % aaaaaaaaaaaa
% aaaaaaaaaaaa
+ x + x
) )