From 59e70896c03905920a078d328d98387f4590818c Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 25 Aug 2023 00:06:56 -0400 Subject: [PATCH] Fix formatting of comments between function and arguments (#6826) ## Summary We now format comments between a function and its arguments as dangling. Like with other strange placements, I've biased towards preserving the existing formatting, rather than attempting to reorder the comments. Closes https://github.com/astral-sh/ruff/issues/6818. ## Test Plan `cargo test` Before: | project | similarity index | |--------------|------------------| | cpython | 0.76050 | | django | 0.99820 | | transformers | 0.99800 | | twine | 0.99876 | | typeshed | 0.99953 | | warehouse | 0.99615 | | zulip | 0.99729 | After: | project | similarity index | |--------------|------------------| | cpython | 0.76050 | | django | 0.99820 | | transformers | 0.99800 | | twine | 0.99876 | | typeshed | 0.99953 | | warehouse | 0.99615 | | zulip | 0.99729 | --- .../test/fixtures/ruff/expression/call.py | 41 +++++++++++ .../src/comments/format.rs | 13 +++- .../src/comments/placement.rs | 24 +++++++ .../src/expression/expr_attribute.rs | 26 ++----- .../src/expression/expr_call.rs | 42 ++++++++--- .../snapshots/format@expression__call.py.snap | 72 +++++++++++++++++++ 6 files changed, 186 insertions(+), 32 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/call.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/call.py index 6569ec3571..5dc7d1af44 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/call.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/call.py @@ -164,3 +164,44 @@ func( [] ) ) + +# Comments between the function and its arguments +aaa = ( + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + # awkward comment + () + .bbbbbbbbbbbbbbbb +) + +aaa = ( + # bar + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + # awkward comment + () + .bbbbbbbbbbbbbbbb +) + + +aaa = ( + # bar + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb # baz + # awkward comment + () + .bbbbbbbbbbbbbbbb +) + +aaa = ( + (foo # awkward comment + ) + () + .bbbbbbbbbbbbbbbb +) + +aaa = ( + ( + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + # awkward comment + ) + () + .bbbbbbbbbbbbbbbb +) diff --git a/crates/ruff_python_formatter/src/comments/format.rs b/crates/ruff_python_formatter/src/comments/format.rs index 850c32882f..7a36a3a406 100644 --- a/crates/ruff_python_formatter/src/comments/format.rs +++ b/crates/ruff_python_formatter/src/comments/format.rs @@ -5,7 +5,7 @@ use ruff_formatter::{format_args, write, FormatError, SourceCode}; use ruff_python_ast::node::{AnyNodeRef, AstNode}; use ruff_python_trivia::{lines_after, lines_after_ignoring_trivia, lines_before}; -use crate::comments::SourceComment; +use crate::comments::{CommentLinePosition, SourceComment}; use crate::context::NodeLevel; use crate::prelude::*; @@ -206,8 +206,15 @@ impl Format> for FormatDanglingComments<'_> { .iter() .filter(|comment| comment.is_unformatted()) { - if first && comment.line_position().is_end_of_line() { - write!(f, [space(), space()])?; + if first { + match comment.line_position { + CommentLinePosition::OwnLine => { + write!(f, [hard_line_break()])?; + } + CommentLinePosition::EndOfLine => { + write!(f, [space(), space()])?; + } + } } write!( diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index c0983ec19d..4fc1cf8c81 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -227,6 +227,7 @@ fn handle_enclosed_comment<'a>( } AnyNodeRef::StmtImportFrom(import_from) => handle_import_from_comment(comment, import_from), AnyNodeRef::StmtWith(with_) => handle_with_comment(comment, with_), + AnyNodeRef::ExprCall(_) => handle_call_comment(comment), AnyNodeRef::ExprConstant(_) => { if let Some(AnyNodeRef::ExprFString(fstring)) = comment.enclosing_parent() { CommentPlacement::dangling(fstring, comment) @@ -984,6 +985,29 @@ fn handle_dict_unpacking_comment<'a>( } } +/// Handle comments between a function call and its arguments. For example, attach the following as +/// dangling on the call: +/// ```python +/// ( +/// func +/// # dangling +/// () +/// ) +/// ``` +fn handle_call_comment(comment: DecoratedComment) -> CommentPlacement { + if comment.line_position().is_own_line() { + if comment.preceding_node().is_some_and(|preceding| { + comment.following_node().is_some_and(|following| { + preceding.end() < comment.start() && comment.end() < following.start() + }) + }) { + return CommentPlacement::dangling(comment.enclosing_node(), comment); + } + } + + CommentPlacement::Default(comment) +} + /// Own line comments coming after the node are always dangling comments /// ```python /// ( diff --git a/crates/ruff_python_formatter/src/expression/expr_attribute.rs b/crates/ruff_python_formatter/src/expression/expr_attribute.rs index b21b25701b..90d8c3c5d2 100644 --- a/crates/ruff_python_formatter/src/expression/expr_attribute.rs +++ b/crates/ruff_python_formatter/src/expression/expr_attribute.rs @@ -4,7 +4,7 @@ use ruff_python_ast::{Constant, Expr, ExprAttribute, ExprConstant, Ranged}; use ruff_python_trivia::{find_only_token_in_range, SimpleTokenKind}; use ruff_text_size::TextRange; -use crate::comments::{dangling_comments, trailing_comments, SourceComment}; +use crate::comments::{dangling_comments, SourceComment}; use crate::expression::parentheses::{ is_expression_parenthesized, NeedsParentheses, OptionalParentheses, Parentheses, }; @@ -88,10 +88,10 @@ impl FormatNodeRule for FormatExprAttribute { // ( // ( // a - // ) # `before_dot_end_of_line` - // # `before_dot_own_line` - // . # `after_dot_end_of_line` - // # `after_dot_own_line` + // ) # `before_dot` + // # `before_dot` + // . # `after_dot` + // # `after_dot` // b // ) // ``` @@ -110,24 +110,12 @@ impl FormatNodeRule for FormatExprAttribute { ) }; - let (before_dot_end_of_line, before_dot_own_line) = before_dot.split_at( - before_dot.partition_point(|comment| comment.line_position().is_end_of_line()), - ); - - let (after_dot_end_of_line, after_dot_own_line) = after_dot.split_at( - after_dot.partition_point(|comment| comment.line_position().is_end_of_line()), - ); - write!( f, [ - trailing_comments(before_dot_end_of_line), - (!before_dot.is_empty()).then_some(hard_line_break()), - dangling_comments(before_dot_own_line), + dangling_comments(before_dot), text("."), - trailing_comments(after_dot_end_of_line), - (!after_dot.is_empty()).then_some(hard_line_break()), - dangling_comments(after_dot_own_line), + dangling_comments(after_dot), attr.format() ] ) diff --git a/crates/ruff_python_formatter/src/expression/expr_call.rs b/crates/ruff_python_formatter/src/expression/expr_call.rs index 250dbe8d65..613fa3f091 100644 --- a/crates/ruff_python_formatter/src/expression/expr_call.rs +++ b/crates/ruff_python_formatter/src/expression/expr_call.rs @@ -1,10 +1,10 @@ -use crate::expression::CallChainLayout; - -use ruff_formatter::{format_args, write, FormatRuleWithOptions}; +use ruff_formatter::FormatRuleWithOptions; use ruff_python_ast::node::AnyNodeRef; use ruff_python_ast::{Expr, ExprCall}; +use crate::comments::{dangling_comments, SourceComment}; use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses}; +use crate::expression::CallChainLayout; use crate::prelude::*; use crate::FormatNodeRule; @@ -30,13 +30,25 @@ impl FormatNodeRule for FormatExprCall { arguments, } = item; + let comments = f.context().comments().clone(); + let dangling = comments.dangling(item); + let call_chain_layout = self.call_chain_layout.apply_in_node(item, f); - let fmt_func = format_with(|f| match func.as_ref() { - Expr::Attribute(expr) => expr.format().with_options(call_chain_layout).fmt(f), - Expr::Call(expr) => expr.format().with_options(call_chain_layout).fmt(f), - Expr::Subscript(expr) => expr.format().with_options(call_chain_layout).fmt(f), - _ => func.format().fmt(f), + let fmt_func = format_with(|f| { + // Format the function expression. + match func.as_ref() { + Expr::Attribute(expr) => expr.format().with_options(call_chain_layout).fmt(f), + Expr::Call(expr) => expr.format().with_options(call_chain_layout).fmt(f), + Expr::Subscript(expr) => expr.format().with_options(call_chain_layout).fmt(f), + _ => func.format().fmt(f), + }?; + + // Format comments between the function and its arguments. + dangling_comments(dangling).fmt(f)?; + + // Format the arguments. + arguments.format().fmt(f) }); // Allow to indent the parentheses while @@ -48,11 +60,19 @@ impl FormatNodeRule for FormatExprCall { if call_chain_layout == CallChainLayout::Fluent && self.call_chain_layout == CallChainLayout::Default { - group(&format_args![fmt_func, arguments.format()]).fmt(f) + group(&fmt_func).fmt(f) } else { - write!(f, [fmt_func, arguments.format()]) + fmt_func.fmt(f) } } + + fn fmt_dangling_comments( + &self, + _dangling_comments: &[SourceComment], + _f: &mut PyFormatter, + ) -> FormatResult<()> { + Ok(()) + } } impl NeedsParentheses for ExprCall { @@ -65,6 +85,8 @@ impl NeedsParentheses for ExprCall { == CallChainLayout::Fluent { OptionalParentheses::Multiline + } else if context.comments().has_dangling(self) { + OptionalParentheses::Always } else { self.func.needs_parentheses(self.into(), context) } diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap index d597fca638..eca6a3eea8 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap @@ -170,6 +170,47 @@ func( [] ) ) + +# Comments between the function and its arguments +aaa = ( + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + # awkward comment + () + .bbbbbbbbbbbbbbbb +) + +aaa = ( + # bar + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + # awkward comment + () + .bbbbbbbbbbbbbbbb +) + + +aaa = ( + # bar + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb # baz + # awkward comment + () + .bbbbbbbbbbbbbbbb +) + +aaa = ( + (foo # awkward comment + ) + () + .bbbbbbbbbbbbbbbb +) + +aaa = ( + ( + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + # awkward comment + ) + () + .bbbbbbbbbbbbbbbb +) ``` ## Output @@ -337,6 +378,37 @@ func( [] ) ) + +# Comments between the function and its arguments +aaa = ( + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + # awkward comment + ().bbbbbbbbbbbbbbbb +) + +aaa = ( + # bar + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + # awkward comment + ().bbbbbbbbbbbbbbbb +) + + +aaa = ( + # bar + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb # baz + # awkward comment + ().bbbbbbbbbbbbbbbb +) + +aaa = ( + foo # awkward comment +)().bbbbbbbbbbbbbbbb + +aaa = ( + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + # awkward comment +)().bbbbbbbbbbbbbbbb ```