From 1c02fcd7ce60d75de8c6fbdb43734c323c2fecb4 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 28 Sep 2023 13:42:12 -0400 Subject: [PATCH] Avoid unnecessary comments check in `maybe_parenthesize_expression` (#7686) ## Summary No-op refactor, but we can evaluate early if the first part of `preserve_parentheses || has_comments` is `true`, and thus avoid looking up the node comments. ## Test Plan `cargo test` --- .../src/expression/mod.rs | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/crates/ruff_python_formatter/src/expression/mod.rs b/crates/ruff_python_formatter/src/expression/mod.rs index 6cae0c2973..f079159054 100644 --- a/crates/ruff_python_formatter/src/expression/mod.rs +++ b/crates/ruff_python_formatter/src/expression/mod.rs @@ -187,7 +187,6 @@ impl Format> for MaybeParenthesizeExpression<'_> { parenthesize, } = self; - let comments = f.context().comments(); let preserve_parentheses = parenthesize.is_optional() && is_expression_parenthesized( (*expression).into(), @@ -195,14 +194,20 @@ impl Format> for MaybeParenthesizeExpression<'_> { f.context().source(), ); - let node_comments = comments.leading_dangling_trailing(*expression); + // If we want to preserve parentheses, short-circuit. + if preserve_parentheses { + return expression.format().with_options(Parentheses::Always).fmt(f); + } - let has_comments = node_comments.has_leading() || node_comments.has_trailing_own_line(); + let node_comments = f + .context() + .comments() + .leading_dangling_trailing(*expression); // If the expression has comments, we always want to preserve the parentheses. This also // ensures that we correctly handle parenthesized comments, and don't need to worry about // them in the implementation below. - if preserve_parentheses || has_comments { + if node_comments.has_leading() || node_comments.has_trailing_own_line() { return expression.format().with_options(Parentheses::Always).fmt(f); } @@ -252,11 +257,9 @@ impl Format> for MaybeParenthesizeExpression<'_> { // The group id is necessary because the nested expressions may reference it. let group_id = f.group_id("optional_parentheses"); let f = &mut WithNodeLevel::new(NodeLevel::Expression(Some(group_id)), f); - ruff_formatter::prelude::best_fit_parenthesize( - &expression.format().with_options(Parentheses::Never), - ) - .with_group_id(Some(group_id)) - .fmt(f) + best_fit_parenthesize(&expression.format().with_options(Parentheses::Never)) + .with_group_id(Some(group_id)) + .fmt(f) } } },