From 46f8961292b3cc1275426c0e523b6f8fcccfdb4f Mon Sep 17 00:00:00 2001 From: konsti Date: Sun, 23 Jul 2023 14:32:16 +0200 Subject: [PATCH] Formatter: Add EmptyWithDanglingComments helper (#5951) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Summary** Add a `EmptyWithDanglingComments` format helper that formats comments inside empty parentheses, brackets or curly braces. Previously, this was implemented separately, and partially incorrectly, for each use case. 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. **Test Plan** Added a regression test. 145 (from previously 149) instances of unstable formatting remaining. ``` $ cargo run --bin ruff_dev --release -- format-dev --stability-check --error-file formatter-ecosystem-errors.txt --multi-project target/checkouts > formatter-ecosystem-progress.txt $ rg "Unstable formatting" target/formatter-ecosystem-errors.txt | wc -l 145 ``` --- .../test/fixtures/ruff/expression/dict.py | 3 ++ .../test/fixtures/ruff/statement/delete.py | 3 ++ crates/ruff_python_formatter/src/builders.rs | 52 +++++++++++++++++++ .../src/expression/expr_call.rs | 11 ++-- .../src/expression/expr_dict.rs | 18 +++---- .../src/expression/expr_list.rs | 28 ++-------- .../src/expression/expr_tuple.rs | 23 +++----- .../snapshots/format@expression__dict.py.snap | 6 +++ .../format@statement__delete.py.snap | 6 +++ 9 files changed, 94 insertions(+), 56 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/dict.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/dict.py index b53ff5c6f3..a9a527ec4e 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/dict.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/dict.py @@ -56,3 +56,6 @@ a = { # comment 3: True, } + +x={ # dangling end of line comment +} diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/delete.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/delete.py index fa5efd652a..c8ca7da9d7 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/delete.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/delete.py @@ -70,3 +70,6 @@ del ( # Deleted ) # Completed # Done + +del ( # dangling end of line comment +) diff --git a/crates/ruff_python_formatter/src/builders.rs b/crates/ruff_python_formatter/src/builders.rs index d60d92f2f8..c23938edd9 100644 --- a/crates/ruff_python_formatter/src/builders.rs +++ b/crates/ruff_python_formatter/src/builders.rs @@ -1,6 +1,7 @@ use ruff_text_size::{TextRange, TextSize}; use rustpython_parser::ast::Ranged; +use crate::comments::{dangling_comments, SourceComment}; use ruff_formatter::{format_args, write, Argument, Arguments}; use ruff_python_trivia::{ lines_after, skip_trailing_trivia, SimpleToken, SimpleTokenKind, SimpleTokenizer, @@ -323,6 +324,57 @@ 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 + dangling_comments(&self.comments[..end_of_line_split]), + // own line comments, which need to be indented + soft_block_indent(&dangling_comments(&self.comments[end_of_line_split..])), + self.closing + ])] + ) + } +} + #[cfg(test)] mod tests { use rustpython_parser::ast::ModModule; diff --git a/crates/ruff_python_formatter/src/expression/expr_call.rs b/crates/ruff_python_formatter/src/expression/expr_call.rs index 0febb6a89a..ad9b3a0a5f 100644 --- a/crates/ruff_python_formatter/src/expression/expr_call.rs +++ b/crates/ruff_python_formatter/src/expression/expr_call.rs @@ -1,11 +1,11 @@ use ruff_text_size::{TextRange, TextSize}; use rustpython_parser::ast::{Expr, ExprCall, Ranged}; +use crate::builders::empty_parenthesized_with_dangling_comments; use ruff_formatter::write; use ruff_python_ast::node::AnyNodeRef; use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer}; -use crate::comments::dangling_comments; use crate::expression::expr_generator_exp::GeneratorExpParentheses; use crate::expression::parentheses::{ parenthesized, NeedsParentheses, OptionalParentheses, Parentheses, @@ -34,14 +34,15 @@ impl FormatNodeRule for FormatExprCall { // ``` if args.is_empty() && keywords.is_empty() { let comments = f.context().comments().clone(); - let comments = comments.dangling_comments(item); return write!( f, [ func.format(), - text("("), - dangling_comments(comments), - text(")") + empty_parenthesized_with_dangling_comments( + text("("), + comments.dangling_comments(item), + text(")"), + ) ] ); } diff --git a/crates/ruff_python_formatter/src/expression/expr_dict.rs b/crates/ruff_python_formatter/src/expression/expr_dict.rs index 1e8b73e022..59d6b18093 100644 --- a/crates/ruff_python_formatter/src/expression/expr_dict.rs +++ b/crates/ruff_python_formatter/src/expression/expr_dict.rs @@ -1,4 +1,5 @@ -use crate::comments::{dangling_node_comments, leading_comments}; +use crate::builders::empty_parenthesized_with_dangling_comments; +use crate::comments::leading_comments; use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses}; use crate::prelude::*; use crate::FormatNodeRule; @@ -64,14 +65,13 @@ impl FormatNodeRule for FormatExprDict { debug_assert_eq!(keys.len(), values.len()); if values.is_empty() { - return write!( - f, - [ - &text("{"), - block_indent(&dangling_node_comments(item)), - &text("}"), - ] - ); + let comments = f.context().comments().clone(); + return empty_parenthesized_with_dangling_comments( + text("{"), + comments.dangling_comments(item), + text("}"), + ) + .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 b28085fc00..bd13210967 100644 --- a/crates/ruff_python_formatter/src/expression/expr_list.rs +++ b/crates/ruff_python_formatter/src/expression/expr_list.rs @@ -1,8 +1,7 @@ -use crate::comments::{dangling_comments, CommentLinePosition}; +use crate::builders::empty_parenthesized_with_dangling_comments; use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses}; use crate::prelude::*; use crate::FormatNodeRule; -use ruff_formatter::{format_args, write}; use ruff_python_ast::node::AnyNodeRef; use rustpython_parser::ast::{ExprList, Ranged}; @@ -20,30 +19,9 @@ impl FormatNodeRule for FormatExprList { let comments = f.context().comments().clone(); let dangling = comments.dangling_comments(item); - // The empty list is special because there can be dangling comments, and they can be in two - // positions: - // ```python - // a3 = [ # end-of-line - // # own line - // ] - // ``` - // In all other cases comments get assigned to a list element if elts.is_empty() { - let end_of_line_split = dangling.partition_point(|comment| { - comment.line_position() == CommentLinePosition::EndOfLine - }); - debug_assert!(dangling[end_of_line_split..] - .iter() - .all(|comment| comment.line_position() == CommentLinePosition::OwnLine)); - return write!( - f, - [group(&format_args![ - text("["), - dangling_comments(&dangling[..end_of_line_split]), - soft_block_indent(&dangling_comments(&dangling[end_of_line_split..])), - text("]") - ])] - ); + return empty_parenthesized_with_dangling_comments(text("["), dangling, text("]")) + .fmt(f); } debug_assert!( diff --git a/crates/ruff_python_formatter/src/expression/expr_tuple.rs b/crates/ruff_python_formatter/src/expression/expr_tuple.rs index 8e2128a6e7..5e89fd995c 100644 --- a/crates/ruff_python_formatter/src/expression/expr_tuple.rs +++ b/crates/ruff_python_formatter/src/expression/expr_tuple.rs @@ -5,8 +5,7 @@ use rustpython_parser::ast::{Expr, Ranged}; use ruff_formatter::{format_args, write, FormatRuleWithOptions}; use ruff_python_ast::node::AnyNodeRef; -use crate::builders::parenthesize_if_expands; -use crate::comments::{dangling_comments, CommentLinePosition}; +use crate::builders::{empty_parenthesized_with_dangling_comments, parenthesize_if_expands}; use crate::expression::parentheses::{ parenthesized, NeedsParentheses, OptionalParentheses, Parentheses, }; @@ -79,22 +78,12 @@ impl FormatNodeRule for FormatExprTuple { match elts.as_slice() { [] => { let comments = f.context().comments().clone(); - let dangling = comments.dangling_comments(item); - let end_of_line_split = dangling.partition_point(|comment| { - comment.line_position() == CommentLinePosition::EndOfLine - }); - debug_assert!(dangling[end_of_line_split..] - .iter() - .all(|comment| comment.line_position() == CommentLinePosition::OwnLine)); - write!( - f, - [group(&format_args![ - text("("), - dangling_comments(&dangling[..end_of_line_split]), - soft_block_indent(&dangling_comments(&dangling[end_of_line_split..])), - text(")") - ])] + return empty_parenthesized_with_dangling_comments( + text("("), + comments.dangling_comments(item), + text(")"), ) + .fmt(f); } [single] => match self.parentheses { TupleParentheses::Subscript diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__dict.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__dict.py.snap index 043bd0e952..300074af73 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__dict.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__dict.py.snap @@ -62,6 +62,9 @@ a = { # comment 3: True, } + +x={ # dangling end of line comment +} ``` ## Output @@ -126,6 +129,9 @@ a = { # comment 3: True, } + +x = { # dangling end of line comment +} ``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__delete.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__delete.py.snap index 9ccbf27444..b257c6486f 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__delete.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@statement__delete.py.snap @@ -76,6 +76,9 @@ del ( # Deleted ) # Completed # Done + +del ( # dangling end of line comment +) ``` ## Output @@ -211,6 +214,9 @@ del ( # Deleted ) # Completed # Done + +del ( # dangling end of line comment +) ```