From 9c2a75284b9c9b7ee5da5ec2d9d5d87b90da0c1d Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 30 Jun 2023 09:52:14 +0200 Subject: [PATCH] Preserve parentheses around left side of binary expression ## Summary This PR fixes an issue where the binary expression formatting removed parentheses around the left hand side of an expression. ## Test Plan I added a new regression test and re-ran the ecosystem check. It brings down the `check-formatter-stability` output from a 3.4MB file down to 900KB. --- .../test/fixtures/ruff/expression/binary.py | 6 ++++++ .../src/expression/expr_bin_op.rs | 19 +++++++++++++++---- ...atibility@simple_cases__expression.py.snap | 17 ++++------------- .../format@expression__binary.py.snap | 12 ++++++++++++ .../format@expression__unary.py.snap | 6 ++++-- 5 files changed, 41 insertions(+), 19 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary.py index cbd93631ae..30cf4c4465 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/binary.py @@ -205,3 +205,9 @@ if ( # Unstable formatting in https://github.com/realtyem/synapse-unraid/blob/unraid_develop/synapse/handlers/presence.py for user_id in set(target_user_ids) - {u.user_id for u in updates}: updates.append(UserPresenceState.default(user_id)) + +# Keeps parenthesized left hand sides +( + log(self.price / self.strike) + + (self.risk_free - self.div_cont + 0.5 * (self.sigma**2)) * self.exp_time +) / self.sigmaT diff --git a/crates/ruff_python_formatter/src/expression/expr_bin_op.rs b/crates/ruff_python_formatter/src/expression/expr_bin_op.rs index 6e2dc5226f..859e03ce4f 100644 --- a/crates/ruff_python_formatter/src/expression/expr_bin_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_bin_op.rs @@ -1,12 +1,14 @@ use crate::comments::{trailing_comments, trailing_node_comments, Comments}; use crate::expression::binary_like::{BinaryLayout, FormatBinaryLike}; use crate::expression::parentheses::{ - default_expression_needs_parentheses, NeedsParentheses, Parenthesize, + default_expression_needs_parentheses, is_expression_parenthesized, NeedsParentheses, + Parenthesize, }; use crate::expression::Parentheses; use crate::prelude::*; use crate::FormatNodeRule; use ruff_formatter::{write, FormatOwnedWithRule, FormatRefWithRule, FormatRuleWithOptions}; +use ruff_python_ast::node::AstNode; use rustpython_parser::ast::{ Constant, Expr, ExprAttribute, ExprBinOp, ExprConstant, ExprUnaryOp, Operator, UnaryOp, }; @@ -44,9 +46,18 @@ impl<'ast> FormatBinaryLike<'ast> for ExprBinOp { fn fmt_default(&self, f: &mut PyFormatter<'ast, '_>) -> FormatResult<()> { let comments = f.context().comments().clone(); - let format_inner = format_with(|f| { - let binary_chain: SmallVec<[&ExprBinOp; 4]> = - iter::successors(Some(self), |parent| parent.left.as_bin_op_expr()).collect(); + let format_inner = format_with(|f: &mut PyFormatter| { + let source = f.context().contents(); + let binary_chain: SmallVec<[&ExprBinOp; 4]> = iter::successors(Some(self), |parent| { + parent.left.as_bin_op_expr().and_then(|bin_expression| { + if is_expression_parenthesized(bin_expression.as_any_node_ref(), source) { + None + } else { + Some(bin_expression) + } + }) + }) + .collect(); // SAFETY: `binary_chain` is guaranteed not to be empty because it always contains the current expression. let left_most = binary_chain.last().unwrap(); diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__expression.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__expression.py.snap index 9d7a94ba9a..86ac68c37e 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__expression.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__expression.py.snap @@ -274,20 +274,11 @@ last_call() Name None True -@@ -24,40 +24,46 @@ - 1 >> v2 - 1 % finished - 1 + v2 - v3 * 4 ^ 5**v6 / 7 // 8 --((1 + v2) - (v3 * 4)) ^ (((5**v6) / 7) // 8) -+(1 + v2 - (v3 * 4)) ^ (5**v6 / 7 // 8) - not great - ~great - +value +@@ -31,33 +31,39 @@ -1 ~int and not v1 ^ 123 + v2 | True --(~int) and (not ((v1 ^ (123 + v2)) | True)) + (~int) and (not ((v1 ^ (123 + v2)) | True)) -+(really ** -(confusing ** ~(operator**-precedence))) -+(~int) and (not (v1 ^ (123 + v2) | True)) ++really ** -confusing ** ~operator**-precedence flags & ~select.EPOLLIN and waiters.write_task is not None -lambda arg: None @@ -650,13 +641,13 @@ v1 << 2 1 >> v2 1 % finished 1 + v2 - v3 * 4 ^ 5**v6 / 7 // 8 -(1 + v2 - (v3 * 4)) ^ (5**v6 / 7 // 8) +((1 + v2) - (v3 * 4)) ^ (((5**v6) / 7) // 8) not great ~great +value -1 ~int and not v1 ^ 123 + v2 | True -(~int) and (not (v1 ^ (123 + v2) | True)) +(~int) and (not ((v1 ^ (123 + v2)) | True)) +really ** -confusing ** ~operator**-precedence flags & ~select.EPOLLIN and waiters.write_task is not None lambda x: True diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__binary.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__binary.py.snap index 670762dfe9..8b6f78bd10 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__binary.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__binary.py.snap @@ -211,6 +211,12 @@ if ( # Unstable formatting in https://github.com/realtyem/synapse-unraid/blob/unraid_develop/synapse/handlers/presence.py for user_id in set(target_user_ids) - {u.user_id for u in updates}: updates.append(UserPresenceState.default(user_id)) + +# Keeps parenthesized left hand sides +( + log(self.price / self.strike) + + (self.risk_free - self.div_cont + 0.5 * (self.sigma**2)) * self.exp_time +) / self.sigmaT ``` ## Output @@ -468,6 +474,12 @@ for user_id in set( target_user_ids ) - {NOT_IMPLEMENTED_set_value for value in NOT_IMPLEMENTED_set}: updates.append(UserPresenceState.default(user_id)) + +# Keeps parenthesized left hand sides +( + log(self.price / self.strike) + + (self.risk_free - self.div_cont + 0.5 * (self.sigma**2)) * self.exp_time +) / self.sigmaT ``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__unary.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__unary.py.snap index b31b763052..c6f1534b31 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__unary.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__unary.py.snap @@ -257,8 +257,10 @@ if aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa & ( pass if ( - not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + not ( + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + ) & aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ): pass