diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unary.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unary.py index 1fb69736bd602..dd53db4884adc 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unary.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unary.py @@ -137,7 +137,7 @@ a: pass -# Regression: https://github.com/astral-sh/ruff/issues/5338 +# Regression test for: https://github.com/astral-sh/ruff/issues/5338 if a and not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa & aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: pass @@ -162,7 +162,7 @@ ): pass -# https://github.com/astral-sh/ruff/issues/7448 +# Regression test for: https://github.com/astral-sh/ruff/issues/7448 x = ( # a not # b @@ -172,3 +172,11 @@ True ) ) + +# Regression test for: https://github.com/astral-sh/ruff/issues/8090 +if "root" not in ( + long_tree_name_tree.split("/")[0] + for long_tree_name_tree in really_really_long_variable_name +): + msg = "Could not find root. Please try a different forest." + raise ValueError(msg) diff --git a/crates/ruff_python_formatter/src/expression/expr_tuple.rs b/crates/ruff_python_formatter/src/expression/expr_tuple.rs index 5d7e23a3dad0f..65184d7ccbc34 100644 --- a/crates/ruff_python_formatter/src/expression/expr_tuple.rs +++ b/crates/ruff_python_formatter/src/expression/expr_tuple.rs @@ -224,7 +224,7 @@ impl NeedsParentheses for ExprTuple { /// Return `true` if a tuple is parenthesized in the source code. pub(crate) fn is_tuple_parenthesized(tuple: &ExprTuple, source: &str) -> bool { let Some(elt) = tuple.elts.first() else { - return false; + return true; }; // Count the number of open parentheses between the start of the tuple and the first element. diff --git a/crates/ruff_python_formatter/src/expression/mod.rs b/crates/ruff_python_formatter/src/expression/mod.rs index 0f6550654db23..d33f05eadf6a8 100644 --- a/crates/ruff_python_formatter/src/expression/mod.rs +++ b/crates/ruff_python_formatter/src/expression/mod.rs @@ -14,6 +14,8 @@ use ruff_text_size::Ranged; use crate::builders::parenthesize_if_expands; use crate::comments::{leading_comments, trailing_comments, LeadingDanglingTrailingComments}; use crate::context::{NodeLevel, WithNodeLevel}; +use crate::expression::expr_generator_exp::is_generator_parenthesized; +use crate::expression::expr_tuple::is_tuple_parenthesized; use crate::expression::parentheses::{ is_expression_parenthesized, optional_parentheses, parenthesized, NeedsParentheses, OptionalParentheses, Parentheses, Parenthesize, @@ -510,7 +512,7 @@ fn can_omit_optional_parentheses(expr: &Expr, context: &PyFormatContext) -> bool if visitor.max_precedence == OperatorPrecedence::None { true - } else if visitor.pax_precedence_count > 1 { + } else if visitor.max_precedence_count > 1 { false } else if visitor.max_precedence == OperatorPrecedence::Attribute { true @@ -540,7 +542,7 @@ fn can_omit_optional_parentheses(expr: &Expr, context: &PyFormatContext) -> bool #[derive(Clone, Debug)] struct CanOmitOptionalParenthesesVisitor<'input> { max_precedence: OperatorPrecedence, - pax_precedence_count: u32, + max_precedence_count: u32, any_parenthesized_expressions: bool, last: Option<&'input Expr>, first: Option<&'input Expr>, @@ -552,7 +554,7 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> { Self { context, max_precedence: OperatorPrecedence::None, - pax_precedence_count: 0, + max_precedence_count: 0, any_parenthesized_expressions: false, last: None, first: None, @@ -566,11 +568,11 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> { fn update_max_precedence_with_count(&mut self, precedence: OperatorPrecedence, count: u32) { match self.max_precedence.cmp(&precedence) { Ordering::Less => { - self.pax_precedence_count = count; + self.max_precedence_count = count; self.max_precedence = precedence; } Ordering::Equal => { - self.pax_precedence_count += count; + self.max_precedence_count += count; } Ordering::Greater => {} } @@ -581,7 +583,6 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> { match expr { Expr::Dict(_) | Expr::List(_) - | Expr::Tuple(_) | Expr::Set(_) | Expr::ListComp(_) | Expr::SetComp(_) @@ -590,6 +591,21 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> { // The values are always parenthesized, don't visit. return; } + + Expr::Tuple(tuple) if is_tuple_parenthesized(tuple, self.context.source()) => { + self.any_parenthesized_expressions = true; + // The values are always parenthesized, don't visit. + return; + } + + Expr::GeneratorExp(generator) + if is_generator_parenthesized(generator, self.context.source()) => + { + self.any_parenthesized_expressions = true; + // The values are always parenthesized, don't visit. + return; + } + // It's impossible for a file smaller or equal to 4GB to contain more than 2^32 comparisons // because each comparison requires a left operand, and `n` `operands` and right sides. #[allow(clippy::cast_possible_truncation)] @@ -690,7 +706,8 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> { self.update_max_precedence(OperatorPrecedence::String); } - Expr::NamedExpr(_) + Expr::Tuple(_) + | Expr::NamedExpr(_) | Expr::GeneratorExp(_) | Expr::Lambda(_) | Expr::Await(_) @@ -914,10 +931,14 @@ pub(crate) fn has_own_parentheses( Some(OwnParentheses::NonEmpty) } + Expr::GeneratorExp(generator) + if is_generator_parenthesized(generator, context.source()) => + { + Some(OwnParentheses::NonEmpty) + } + // These expressions must contain _some_ child or trivia token in order to be non-empty. - Expr::List(ast::ExprList { elts, .. }) - | Expr::Set(ast::ExprSet { elts, .. }) - | Expr::Tuple(ast::ExprTuple { elts, .. }) => { + Expr::List(ast::ExprList { elts, .. }) | Expr::Set(ast::ExprSet { elts, .. }) => { if !elts.is_empty() || context.comments().has_dangling(AnyNodeRef::from(expr)) { Some(OwnParentheses::NonEmpty) } else { @@ -925,6 +946,14 @@ pub(crate) fn has_own_parentheses( } } + Expr::Tuple(tuple) if is_tuple_parenthesized(tuple, context.source()) => { + if !tuple.elts.is_empty() || context.comments().has_dangling(AnyNodeRef::from(expr)) { + Some(OwnParentheses::NonEmpty) + } else { + Some(OwnParentheses::Empty) + } + } + Expr::Dict(ast::ExprDict { keys, .. }) => { if !keys.is_empty() || context.comments().has_dangling(AnyNodeRef::from(expr)) { Some(OwnParentheses::NonEmpty) 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 7904dee4ca336..eb9f911a4ed93 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 @@ -475,9 +475,8 @@ aaaaaaaaaaaaaa + { aaaaaaaaaaaaaa + [ a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ] -( - aaaaaaaaaaaaaa - + (a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb) +aaaaaaaaaaaaaa + ( + a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ) aaaaaaaaaaaaaa + { a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb 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 9a291501337aa..d2f0bac0abf9e 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 @@ -143,7 +143,7 @@ if not \ a: pass -# Regression: https://github.com/astral-sh/ruff/issues/5338 +# Regression test for: https://github.com/astral-sh/ruff/issues/5338 if a and not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa & aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: pass @@ -168,7 +168,7 @@ if True: ): pass -# https://github.com/astral-sh/ruff/issues/7448 +# Regression test for: https://github.com/astral-sh/ruff/issues/7448 x = ( # a not # b @@ -178,6 +178,14 @@ x = ( True ) ) + +# Regression test for: https://github.com/astral-sh/ruff/issues/8090 +if "root" not in ( + long_tree_name_tree.split("/")[0] + for long_tree_name_tree in really_really_long_variable_name +): + msg = "Could not find root. Please try a different forest." + raise ValueError(msg) ``` ## Output @@ -328,7 +336,7 @@ if ( if not a: pass -# Regression: https://github.com/astral-sh/ruff/issues/5338 +# Regression test for: https://github.com/astral-sh/ruff/issues/5338 if ( a and not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa @@ -357,7 +365,7 @@ if True: ): pass -# https://github.com/astral-sh/ruff/issues/7448 +# Regression test for: https://github.com/astral-sh/ruff/issues/7448 x = ( # a # b @@ -367,6 +375,14 @@ x = ( True ) ) + +# Regression test for: https://github.com/astral-sh/ruff/issues/8090 +if "root" not in ( + long_tree_name_tree.split("/")[0] + for long_tree_name_tree in really_really_long_variable_name +): + msg = "Could not find root. Please try a different forest." + raise ValueError(msg) ```