From c7703e205d64d1c5be8e8a2daee30824bf80bf05 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 8 Aug 2023 15:17:17 -0400 Subject: [PATCH] Move `empty_parenthesized` into the `parentheses.rs` (#6403) ## Summary This PR moves `empty_parenthesized` such that it's peer to `parenthesized`, and changes the API to better match that of `parenthesized` (takes `&str` rather than `StaticText`, has a `with_dangling_comments` method, etc.). It may be intentionally _not_ part of `parentheses.rs`, but to me they're so similar that it makes more sense for them to be in the same module, with the same API, etc. --- crates/ruff_python_formatter/src/builders.rs | 62 ------------------ .../src/expression/expr_dict.rs | 8 +-- .../src/expression/expr_list.rs | 10 +-- .../src/expression/expr_tuple.rs | 9 +-- .../src/expression/parentheses.rs | 65 ++++++++++++++++++- .../src/other/arguments.rs | 13 +--- .../src/other/parameters.rs | 12 +--- 7 files changed, 83 insertions(+), 96 deletions(-) diff --git a/crates/ruff_python_formatter/src/builders.rs b/crates/ruff_python_formatter/src/builders.rs index 5d7e0c2c3f..95a2a76ace 100644 --- a/crates/ruff_python_formatter/src/builders.rs +++ b/crates/ruff_python_formatter/src/builders.rs @@ -3,7 +3,6 @@ use ruff_python_ast::Ranged; use ruff_python_trivia::{SimpleToken, SimpleTokenKind, SimpleTokenizer}; use ruff_text_size::{TextRange, TextSize}; -use crate::comments::{dangling_comments, trailing_comments, SourceComment}; use crate::context::{NodeLevel, WithNodeLevel}; use crate::prelude::*; use crate::MagicTrailingComma; @@ -209,64 +208,3 @@ impl<'fmt, 'ast, 'buf> JoinCommaSeparatedBuilder<'fmt, 'ast, 'buf> { }) } } - -/// Format comments inside empty parentheses, brackets or curly braces. -/// -/// Empty `()`, `[]` and `{}` are special because there can be dangling comments, and they can be in -/// two positions: -/// ```python -/// x = [ # end-of-line -/// # own line -/// ] -/// ``` -/// These comments are dangling because they can't be assigned to any element inside as they would -/// in all other cases. -pub(crate) fn empty_parenthesized_with_dangling_comments( - opening: StaticText, - comments: &[SourceComment], - closing: StaticText, -) -> EmptyWithDanglingComments { - EmptyWithDanglingComments { - opening, - comments, - closing, - } -} - -pub(crate) struct EmptyWithDanglingComments<'a> { - opening: StaticText, - comments: &'a [SourceComment], - closing: StaticText, -} - -impl<'ast> Format> for EmptyWithDanglingComments<'_> { - fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { - let end_of_line_split = self - .comments - .partition_point(|comment| comment.line_position().is_end_of_line()); - debug_assert!(self.comments[end_of_line_split..] - .iter() - .all(|comment| comment.line_position().is_own_line())); - write!( - f, - [group(&format_args![ - self.opening, - // end-of-line comments - trailing_comments(&self.comments[..end_of_line_split]), - // Avoid unstable formatting with - // ```python - // x = () - (# - // ) - // ``` - // Without this the comment would go after the empty tuple first, but still expand - // the bin op. In the second formatting pass they are trailing bin op comments - // so the bin op collapse. Suboptimally we keep parentheses around the bin op in - // either case. - (!self.comments[..end_of_line_split].is_empty()).then_some(hard_line_break()), - // own line comments, which need to be indented - soft_block_indent(&dangling_comments(&self.comments[end_of_line_split..])), - self.closing - ])] - ) - } -} diff --git a/crates/ruff_python_formatter/src/expression/expr_dict.rs b/crates/ruff_python_formatter/src/expression/expr_dict.rs index 2cd41626ed..39fae66527 100644 --- a/crates/ruff_python_formatter/src/expression/expr_dict.rs +++ b/crates/ruff_python_formatter/src/expression/expr_dict.rs @@ -4,9 +4,10 @@ use ruff_python_ast::Ranged; use ruff_python_ast::{Expr, ExprDict}; use ruff_text_size::TextRange; -use crate::builders::empty_parenthesized_with_dangling_comments; use crate::comments::leading_comments; -use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses}; +use crate::expression::parentheses::{ + empty_parenthesized, parenthesized, NeedsParentheses, OptionalParentheses, +}; use crate::prelude::*; use crate::FormatNodeRule; @@ -69,8 +70,7 @@ impl FormatNodeRule for FormatExprDict { let dangling = comments.dangling_comments(item); if values.is_empty() { - return empty_parenthesized_with_dangling_comments(text("{"), dangling, text("}")) - .fmt(f); + return empty_parenthesized("{", dangling, "}").fmt(f); } let format_pairs = format_with(|f| { diff --git a/crates/ruff_python_formatter/src/expression/expr_list.rs b/crates/ruff_python_formatter/src/expression/expr_list.rs index 89ef07bcd6..7c91b4ef36 100644 --- a/crates/ruff_python_formatter/src/expression/expr_list.rs +++ b/crates/ruff_python_formatter/src/expression/expr_list.rs @@ -1,9 +1,10 @@ -use ruff_formatter::prelude::{format_with, text}; +use ruff_formatter::prelude::format_with; use ruff_python_ast::node::AnyNodeRef; use ruff_python_ast::{ExprList, Ranged}; -use crate::builders::empty_parenthesized_with_dangling_comments; -use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses}; +use crate::expression::parentheses::{ + empty_parenthesized, parenthesized, NeedsParentheses, OptionalParentheses, +}; use crate::prelude::*; use crate::FormatNodeRule; @@ -22,8 +23,7 @@ impl FormatNodeRule for FormatExprList { let dangling = comments.dangling_comments(item); if elts.is_empty() { - return empty_parenthesized_with_dangling_comments(text("["), dangling, text("]")) - .fmt(f); + return empty_parenthesized("[", dangling, "]").fmt(f); } let items = format_with(|f| { diff --git a/crates/ruff_python_formatter/src/expression/expr_tuple.rs b/crates/ruff_python_formatter/src/expression/expr_tuple.rs index da21a5a21f..df1d2db7ce 100644 --- a/crates/ruff_python_formatter/src/expression/expr_tuple.rs +++ b/crates/ruff_python_formatter/src/expression/expr_tuple.rs @@ -4,8 +4,10 @@ use ruff_python_ast::ExprTuple; use ruff_python_ast::{Expr, Ranged}; use ruff_text_size::TextRange; -use crate::builders::{empty_parenthesized_with_dangling_comments, parenthesize_if_expands}; -use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses}; +use crate::builders::parenthesize_if_expands; +use crate::expression::parentheses::{ + empty_parenthesized, parenthesized, NeedsParentheses, OptionalParentheses, +}; use crate::prelude::*; #[derive(Eq, PartialEq, Debug, Default)] @@ -117,8 +119,7 @@ impl FormatNodeRule for FormatExprTuple { // In all other cases comments get assigned to a list element match elts.as_slice() { [] => { - return empty_parenthesized_with_dangling_comments(text("("), dangling, text(")")) - .fmt(f); + return empty_parenthesized("(", dangling, ")").fmt(f); } [single] => match self.parentheses { TupleParentheses::Preserve diff --git a/crates/ruff_python_formatter/src/expression/parentheses.rs b/crates/ruff_python_formatter/src/expression/parentheses.rs index 9c2a962bcc..6864c80458 100644 --- a/crates/ruff_python_formatter/src/expression/parentheses.rs +++ b/crates/ruff_python_formatter/src/expression/parentheses.rs @@ -4,7 +4,9 @@ use ruff_python_ast::node::AnyNodeRef; use ruff_python_ast::Ranged; use ruff_python_trivia::{first_non_trivia_token, SimpleToken, SimpleTokenKind, SimpleTokenizer}; -use crate::comments::{dangling_open_parenthesis_comments, SourceComment}; +use crate::comments::{ + dangling_comments, dangling_open_parenthesis_comments, trailing_comments, SourceComment, +}; use crate::context::{NodeLevel, WithNodeLevel}; use crate::prelude::*; @@ -307,6 +309,67 @@ impl<'ast> Format> for FormatInParenthesesOnlyGroup<'_, 'a } } +/// Format comments inside empty parentheses, brackets or curly braces. +/// +/// Empty `()`, `[]` and `{}` are special because there can be dangling comments, and they can be in +/// two positions: +/// ```python +/// x = [ # end-of-line +/// # own line +/// ] +/// ``` +/// These comments are dangling because they can't be assigned to any element inside as they would +/// in all other cases. +pub(crate) fn empty_parenthesized<'content>( + left: &'static str, + comments: &'content [SourceComment], + right: &'static str, +) -> FormatEmptyParenthesized<'content> { + FormatEmptyParenthesized { + left, + comments, + right, + } +} + +pub(crate) struct FormatEmptyParenthesized<'content> { + left: &'static str, + comments: &'content [SourceComment], + right: &'static str, +} + +impl Format> for FormatEmptyParenthesized<'_> { + fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { + let end_of_line_split = self + .comments + .partition_point(|comment| comment.line_position().is_end_of_line()); + debug_assert!(self.comments[end_of_line_split..] + .iter() + .all(|comment| comment.line_position().is_own_line())); + write!( + f, + [group(&format_args![ + text(self.left), + // end-of-line comments + trailing_comments(&self.comments[..end_of_line_split]), + // Avoid unstable formatting with + // ```python + // x = () - (# + // ) + // ``` + // Without this the comment would go after the empty tuple first, but still expand + // the bin op. In the second formatting pass they are trailing bin op comments + // so the bin op collapse. Suboptimally we keep parentheses around the bin op in + // either case. + (!self.comments[..end_of_line_split].is_empty()).then_some(hard_line_break()), + // own line comments, which need to be indented + soft_block_indent(&dangling_comments(&self.comments[end_of_line_split..])), + text(self.right) + ])] + ) + } +} + #[cfg(test)] mod tests { use ruff_python_ast::node::AnyNodeRef; diff --git a/crates/ruff_python_formatter/src/other/arguments.rs b/crates/ruff_python_formatter/src/other/arguments.rs index 291dd9159e..6810397b65 100644 --- a/crates/ruff_python_formatter/src/other/arguments.rs +++ b/crates/ruff_python_formatter/src/other/arguments.rs @@ -4,9 +4,8 @@ use ruff_python_ast::{Arguments, Expr, Ranged}; use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer}; use ruff_text_size::{TextRange, TextSize}; -use crate::builders::empty_parenthesized_with_dangling_comments; use crate::expression::expr_generator_exp::GeneratorExpParentheses; -use crate::expression::parentheses::{parenthesized, Parentheses}; +use crate::expression::parentheses::{empty_parenthesized, parenthesized, Parentheses}; use crate::prelude::*; use crate::FormatNodeRule; @@ -24,14 +23,8 @@ impl FormatNodeRule for FormatArguments { // ``` if item.args.is_empty() && item.keywords.is_empty() { let comments = f.context().comments().clone(); - return write!( - f, - [empty_parenthesized_with_dangling_comments( - text("("), - comments.dangling_comments(item), - text(")"), - )] - ); + let dangling = comments.dangling_comments(item); + return write!(f, [empty_parenthesized("(", dangling, ")")]); } let all_arguments = format_with(|f: &mut PyFormatter| { diff --git a/crates/ruff_python_formatter/src/other/parameters.rs b/crates/ruff_python_formatter/src/other/parameters.rs index fa15f83efe..cbd9b594b7 100644 --- a/crates/ruff_python_formatter/src/other/parameters.rs +++ b/crates/ruff_python_formatter/src/other/parameters.rs @@ -6,12 +6,11 @@ use ruff_python_ast::{Parameters, Ranged}; use ruff_python_trivia::{SimpleToken, SimpleTokenKind, SimpleTokenizer}; use ruff_text_size::{TextRange, TextSize}; -use crate::builders::empty_parenthesized_with_dangling_comments; use crate::comments::{ leading_comments, leading_node_comments, trailing_comments, CommentLinePosition, SourceComment, }; use crate::context::{NodeLevel, WithNodeLevel}; -use crate::expression::parentheses::parenthesized; +use crate::expression::parentheses::{empty_parenthesized, parenthesized}; use crate::prelude::*; use crate::FormatNodeRule; @@ -245,14 +244,7 @@ impl FormatNodeRule for FormatParameters { write!(f, [group(&format_inner)]) } else if num_parameters == 0 { // No parameters, format any dangling comments between `()` - write!( - f, - [empty_parenthesized_with_dangling_comments( - text("("), - dangling, - text(")"), - )] - ) + write!(f, [empty_parenthesized("(", dangling, ")")]) } else { write!( f,