From cc9e84c14475f158458e4bd3f262a30e43d08b8b Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 15 Sep 2023 20:34:09 -0400 Subject: [PATCH] Format trailing operator comments as dangling (#7427) ## Summary Given a trailing operator comment in a unary expression, like: ```python if ( not # comment a): ... ``` We were attaching these to the operand (`a`), but formatting them in the unary operator via special handling. Parents shouldn't format the comments of their children, so this instead attaches them as dangling comments on the unary expression. (No intended change in formatting.) --- .../test/fixtures/ruff/expression/unary.py | 5 ++ .../src/comments/placement.rs | 41 ++++++++++++++ .../src/expression/expr_unary_op.rs | 54 ++++++------------- .../format@expression__unary.py.snap | 10 ++++ 4 files changed, 73 insertions(+), 37 deletions(-) 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 9b08dc9baa..89ca82644a 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 @@ -146,3 +146,8 @@ if ( # comment a): ... + +if ( + not # comment + a): + ... diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 005cfb002d..6557a4fb9b 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -208,6 +208,7 @@ fn handle_enclosed_comment<'a>( AnyNodeRef::PatternKeyword(pattern_keyword) => { handle_pattern_keyword_comment(comment, pattern_keyword, locator) } + AnyNodeRef::ExprUnaryOp(unary_op) => handle_unary_op_comment(comment, unary_op, locator), AnyNodeRef::ExprNamedExpr(_) => handle_named_expr_comment(comment, locator), AnyNodeRef::ExprDict(_) => handle_dict_unpacking_comment(comment, locator) .or_else(|comment| handle_bracketed_end_of_line_comment(comment, locator)), @@ -1583,6 +1584,46 @@ fn handle_named_expr_comment<'a>( } } +/// Attach trailing end-of-line comments on the operator as dangling comments on the enclosing +/// node. +/// +/// For example, given: +/// ```python +/// ( +/// not # comment +/// True +/// ) +/// ``` +/// +/// The `# comment` will be attached as a dangling comment on the enclosing node, to ensure that +/// it remains on the same line as the operator. +fn handle_unary_op_comment<'a>( + comment: DecoratedComment<'a>, + unary_op: &'a ast::ExprUnaryOp, + locator: &Locator, +) -> CommentPlacement<'a> { + if comment.line_position().is_own_line() { + return CommentPlacement::Default(comment); + } + + if comment.start() > unary_op.operand.start() { + return CommentPlacement::Default(comment); + } + + let tokenizer = SimpleTokenizer::new( + locator.contents(), + TextRange::new(comment.start(), unary_op.operand.start()), + ); + if tokenizer + .skip_trivia() + .any(|token| token.kind == SimpleTokenKind::LParen) + { + return CommentPlacement::Default(comment); + } + + CommentPlacement::dangling(comment.enclosing_node(), comment) +} + /// Attach an end-of-line comment immediately following an open bracket as a dangling comment on /// enclosing node. /// diff --git a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs index 2a75c243a4..3405fa3e5c 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -1,11 +1,11 @@ use ruff_python_ast::node::AnyNodeRef; use ruff_python_ast::ExprUnaryOp; use ruff_python_ast::UnaryOp; -use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer}; -use ruff_text_size::{Ranged, TextLen, TextRange}; -use crate::comments::trailing_comments; -use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses}; +use crate::comments::{trailing_comments, SourceComment}; +use crate::expression::parentheses::{ + is_expression_parenthesized, NeedsParentheses, OptionalParentheses, +}; use crate::prelude::*; #[derive(Default)] @@ -29,21 +29,14 @@ impl FormatNodeRule for FormatExprUnaryOp { token(operator).fmt(f)?; let comments = f.context().comments().clone(); + let dangling = comments.dangling(item); // Split off the comments that follow after the operator and format them as trailing comments. // ```python // (not # comment // a) // ``` - let leading_operand_comments = comments.leading(operand.as_ref()); - let trailing_operator_comments_end = - leading_operand_comments.partition_point(|p| p.line_position().is_end_of_line()); - let (trailing_operator_comments, leading_operand_comments) = - leading_operand_comments.split_at(trailing_operator_comments_end); - - if !trailing_operator_comments.is_empty() { - trailing_comments(trailing_operator_comments).fmt(f)?; - } + trailing_comments(dangling).fmt(f)?; // Insert a line break if the operand has comments but itself is not parenthesized. // ```python @@ -52,8 +45,8 @@ impl FormatNodeRule for FormatExprUnaryOp { // # comment // a) // ``` - if !leading_operand_comments.is_empty() - && !is_operand_parenthesized(item, f.context().source()) + if comments.has_leading(operand.as_ref()) + && !is_expression_parenthesized(operand.as_ref().into(), f.context().source()) { hard_line_break().fmt(f)?; } else if op.is_not() { @@ -62,6 +55,14 @@ impl FormatNodeRule for FormatExprUnaryOp { operand.format().fmt(f) } + + fn fmt_dangling_comments( + &self, + _dangling_comments: &[SourceComment], + _f: &mut PyFormatter, + ) -> FormatResult<()> { + Ok(()) + } } impl NeedsParentheses for ExprUnaryOp { @@ -71,31 +72,10 @@ impl NeedsParentheses for ExprUnaryOp { context: &PyFormatContext, ) -> OptionalParentheses { // We preserve the parentheses of the operand. It should not be necessary to break this expression. - if is_operand_parenthesized(self, context.source()) { + if is_expression_parenthesized(self.operand.as_ref().into(), context.source()) { OptionalParentheses::Never } else { OptionalParentheses::Multiline } } } - -fn is_operand_parenthesized(unary: &ExprUnaryOp, source: &str) -> bool { - let operator_len = match unary.op { - UnaryOp::Invert => '~'.text_len(), - UnaryOp::Not => "not".text_len(), - UnaryOp::UAdd => '+'.text_len(), - UnaryOp::USub => '-'.text_len(), - }; - - let trivia_range = TextRange::new(unary.start() + operator_len, unary.operand.start()); - - if let Some(token) = SimpleTokenizer::new(source, trivia_range) - .skip_trivia() - .next() - { - debug_assert_eq!(token.kind(), SimpleTokenKind::LParen); - true - } else { - false - } -} 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 b9d8bd127e..6e87a9a5e7 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 @@ -152,6 +152,11 @@ if ( # comment a): ... + +if ( + not # comment + a): + ... ``` ## Output @@ -317,6 +322,11 @@ if ( a ): ... + +if ( + not a # comment +): + ... ```