From 9fda4d1ab28c610e9d6ee67752ac8a20f9172f8a Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 14 Nov 2025 14:47:55 -0500 Subject: [PATCH 01/18] failed attempt at re-partitioning dangling comments see https://github.com/astral-sh/ruff/pull/21410#issuecomment-3534322840, but the short summary is that `if` (and likely other) statement formatting code that uses `maybe_parenthesize` checks if the condition has any leading or trailing comments, so if we try to smuggle the comments in as dangling comments, it thinks the expression won't break, so it doesn't add parentheses when formatting a case like this: ```py if ( not # comment a): pass ``` and we end up with a syntax error: ```py if not a: pass ``` There may be some other way around this, but this is why I'm giving up for now. It really feels like we want another CommentPlacement variant or some kind of dangling tag, like Micha mentioned when I met with him. --- .../src/comments/placement.rs | 4 +- .../src/expression/expr_unary_op.rs | 50 +++++++++++++++---- 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 2bd7402a314199..fb55dabb280bd3 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1924,8 +1924,10 @@ fn handle_unary_op_comment<'a>( .find(|token| token.kind == SimpleTokenKind::LParen) .map_or(unary_op.operand.start(), |lparen| lparen.start()); if comment.end() < up_to { - CommentPlacement::leading(unary_op, comment) + eprintln!("leading: {}", &source[comment.range()]); + CommentPlacement::dangling(unary_op, comment) } else { + eprintln!("default: {}", &source[comment.range()]); CommentPlacement::Default(comment) } } 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 a8454cda1eb41c..cccb9c5d39a488 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -1,7 +1,12 @@ use ruff_python_ast::AnyNodeRef; use ruff_python_ast::ExprUnaryOp; use ruff_python_ast::UnaryOp; +use ruff_python_trivia::SimpleTokenKind; +use ruff_python_trivia::SimpleTokenizer; +use ruff_text_size::Ranged; +use ruff_text_size::TextRange; +use crate::comments::leading_comments; use crate::comments::trailing_comments; use crate::expression::parentheses::{ NeedsParentheses, OptionalParentheses, Parentheses, is_expression_parenthesized, @@ -20,24 +25,49 @@ impl FormatNodeRule for FormatExprUnaryOp { operand, } = item; - let operator = match op { - UnaryOp::Invert => "~", - UnaryOp::Not => "not", - UnaryOp::UAdd => "+", - UnaryOp::USub => "-", - }; - - token(operator).fmt(f)?; - let comments = f.context().comments().clone(); let dangling = comments.dangling(item); + let mut tokenizer = SimpleTokenizer::new( + f.context().source(), + TextRange::new(item.start(), operand.start()), + ) + .skip_trivia(); + let op_token = tokenizer.next(); + debug_assert!(op_token.is_some_and(|token| matches!( + token.kind, + SimpleTokenKind::Tilde + | SimpleTokenKind::Not + | SimpleTokenKind::Plus + | SimpleTokenKind::Minus + ))); + let up_to = tokenizer + .find(|token| token.kind == SimpleTokenKind::LParen) + .map_or(operand.start(), |lparen| lparen.start()); + + let pivot = dangling.partition_point(|comment| comment.end() < up_to); + dbg!(pivot); + let leading = &dangling[..pivot]; + // Split off the comments that follow after the operator and format them as trailing comments. // ```python // (not # comment // a) // ``` - trailing_comments(dangling).fmt(f)?; + if !leading.is_empty() { + hard_line_break().fmt(f)?; + leading_comments(&dangling[..pivot]).fmt(f)?; + } + trailing_comments(&dangling[pivot..]).fmt(f)?; + + let operator = match op { + UnaryOp::Invert => "~", + UnaryOp::Not => "not", + UnaryOp::UAdd => "+", + UnaryOp::USub => "-", + }; + + token(operator).fmt(f)?; // Insert a line break if the operand has comments but itself is not parenthesized. // ```python From e7a838a01217d0a9798bd5bcf97f4e78dedb2898 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 14 Nov 2025 16:01:11 -0500 Subject: [PATCH 02/18] looking very reasonable --- .../src/expression/expr_unary_op.rs | 47 ++++++++++++------- 1 file changed, 29 insertions(+), 18 deletions(-) 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 cccb9c5d39a488..15d62c69499cbf 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -3,8 +3,7 @@ use ruff_python_ast::ExprUnaryOp; use ruff_python_ast::UnaryOp; use ruff_python_trivia::SimpleTokenKind; use ruff_python_trivia::SimpleTokenizer; -use ruff_text_size::Ranged; -use ruff_text_size::TextRange; +use ruff_text_size::{Ranged, TextRange, TextSize}; use crate::comments::leading_comments; use crate::comments::trailing_comments; @@ -28,22 +27,7 @@ impl FormatNodeRule for FormatExprUnaryOp { let comments = f.context().comments().clone(); let dangling = comments.dangling(item); - let mut tokenizer = SimpleTokenizer::new( - f.context().source(), - TextRange::new(item.start(), operand.start()), - ) - .skip_trivia(); - let op_token = tokenizer.next(); - debug_assert!(op_token.is_some_and(|token| matches!( - token.kind, - SimpleTokenKind::Tilde - | SimpleTokenKind::Not - | SimpleTokenKind::Plus - | SimpleTokenKind::Minus - ))); - let up_to = tokenizer - .find(|token| token.kind == SimpleTokenKind::LParen) - .map_or(operand.start(), |lparen| lparen.start()); + let up_to = operand_start(item, f.context()); let pivot = dangling.partition_point(|comment| comment.end() < up_to); dbg!(pivot); @@ -113,6 +97,13 @@ impl NeedsParentheses for ExprUnaryOp { context.source(), ) { OptionalParentheses::Never + } else if context + .comments() + .dangling(self) + .iter() + .any(|comment| comment.end() < operand_start(self, context)) + { + OptionalParentheses::Multiline } else if context.comments().has(self.operand.as_ref()) { OptionalParentheses::Always } else { @@ -120,3 +111,23 @@ impl NeedsParentheses for ExprUnaryOp { } } } + +/// Returns the start of `unary_op`'s operand, or its leading parenthesis, if it has one. +fn operand_start(unary_op: &ExprUnaryOp, context: &PyFormatContext<'_>) -> TextSize { + let mut tokenizer = SimpleTokenizer::new( + context.source(), + TextRange::new(unary_op.start(), unary_op.operand.start()), + ) + .skip_trivia(); + let op_token = tokenizer.next(); + debug_assert!(op_token.is_some_and(|token| matches!( + token.kind, + SimpleTokenKind::Tilde + | SimpleTokenKind::Not + | SimpleTokenKind::Plus + | SimpleTokenKind::Minus + ))); + tokenizer + .find(|token| token.kind == SimpleTokenKind::LParen) + .map_or(unary_op.operand.start(), |lparen| lparen.start()) +} From 7b1da06433f61f4a5af10c2457859b3b91e5b7fb Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 14 Nov 2025 16:14:05 -0500 Subject: [PATCH 03/18] grab the tests from the other PR, update needs_parentheses this is very close, but now I have an extra newline in a few cases --- .../test/fixtures/ruff/expression/unary.py | 16 +++++++ .../src/comments/placement.rs | 20 +-------- .../src/expression/expr_unary_op.rs | 13 +++--- .../format@expression__unary.py.snap | 44 +++++++++++++++++-- 4 files changed, 65 insertions(+), 28 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 5c8ab8d8b31ab5..ea17e4a66f8c07 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 @@ -193,3 +193,19 @@ def foo(): not (aaaaaaaaaaaaaaaaaaaaa[bbbbbbbb, ccccccc]) and dddddddddd < eeeeeeeeeeeeeee ): pass + +# Regression tests for https://github.com/astral-sh/ruff/issues/19226 +if '' and (not # +0): + pass + +if '' and (not # +(0) +): + pass + +if '' and (not + ( # + 0 +)): + pass diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index fb55dabb280bd3..c94383d6340446 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -13,6 +13,7 @@ use std::cmp::Ordering; use crate::comments::visitor::{CommentPlacement, DecoratedComment}; use crate::expression::expr_slice::{ExprSliceCommentSection, assign_comment_in_slice}; +use crate::expression::expr_unary_op::operand_start; use crate::expression::parentheses::is_expression_parenthesized; use crate::other::parameters::{ assign_argument_separator_comment_placement, find_parameter_separators, @@ -1907,27 +1908,10 @@ fn handle_unary_op_comment<'a>( unary_op: &'a ast::ExprUnaryOp, source: &str, ) -> CommentPlacement<'a> { - let mut tokenizer = SimpleTokenizer::new( - source, - TextRange::new(unary_op.start(), unary_op.operand.start()), - ) - .skip_trivia(); - let op_token = tokenizer.next(); - debug_assert!(op_token.is_some_and(|token| matches!( - token.kind, - SimpleTokenKind::Tilde - | SimpleTokenKind::Not - | SimpleTokenKind::Plus - | SimpleTokenKind::Minus - ))); - let up_to = tokenizer - .find(|token| token.kind == SimpleTokenKind::LParen) - .map_or(unary_op.operand.start(), |lparen| lparen.start()); + let up_to = operand_start(unary_op, source); if comment.end() < up_to { - eprintln!("leading: {}", &source[comment.range()]); CommentPlacement::dangling(unary_op, comment) } else { - eprintln!("default: {}", &source[comment.range()]); CommentPlacement::Default(comment) } } 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 15d62c69499cbf..6c30e00196cad1 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -27,10 +27,9 @@ impl FormatNodeRule for FormatExprUnaryOp { let comments = f.context().comments().clone(); let dangling = comments.dangling(item); - let up_to = operand_start(item, f.context()); + let up_to = operand_start(item, f.context().source()); let pivot = dangling.partition_point(|comment| comment.end() < up_to); - dbg!(pivot); let leading = &dangling[..pivot]; // Split off the comments that follow after the operator and format them as trailing comments. @@ -39,9 +38,9 @@ impl FormatNodeRule for FormatExprUnaryOp { // a) // ``` if !leading.is_empty() { - hard_line_break().fmt(f)?; - leading_comments(&dangling[..pivot]).fmt(f)?; + // hard_line_break().fmt(f)?; } + leading_comments(leading).fmt(f)?; trailing_comments(&dangling[pivot..]).fmt(f)?; let operator = match op { @@ -101,7 +100,7 @@ impl NeedsParentheses for ExprUnaryOp { .comments() .dangling(self) .iter() - .any(|comment| comment.end() < operand_start(self, context)) + .any(|comment| comment.end() < operand_start(self, context.source())) { OptionalParentheses::Multiline } else if context.comments().has(self.operand.as_ref()) { @@ -113,9 +112,9 @@ impl NeedsParentheses for ExprUnaryOp { } /// Returns the start of `unary_op`'s operand, or its leading parenthesis, if it has one. -fn operand_start(unary_op: &ExprUnaryOp, context: &PyFormatContext<'_>) -> TextSize { +pub(crate) fn operand_start(unary_op: &ExprUnaryOp, source: &str) -> TextSize { let mut tokenizer = SimpleTokenizer::new( - context.source(), + source, TextRange::new(unary_op.start(), unary_op.operand.start()), ) .skip_trivia(); 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 67f54f9da90a78..76f2dbc555b549 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 @@ -1,7 +1,6 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unary.py -snapshot_kind: text --- ## Input ```python @@ -200,6 +199,22 @@ def foo(): not (aaaaaaaaaaaaaaaaaaaaa[bbbbbbbb, ccccccc]) and dddddddddd < eeeeeeeeeeeeeee ): pass + +# Regression tests for https://github.com/astral-sh/ruff/issues/19226 +if '' and (not # +0): + pass + +if '' and (not # +(0) +): + pass + +if '' and (not + ( # + 0 +)): + pass ``` ## Output @@ -318,7 +333,8 @@ if ( ## Trailing operator comments -if ( # comment +if ( + # comment not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): @@ -367,7 +383,8 @@ if ( ): pass -if ( # comment +if ( + # comment not a ): pass @@ -415,4 +432,25 @@ def foo(): not (aaaaaaaaaaaaaaaaaaaaa[bbbbbbbb, ccccccc]) and dddddddddd < eeeeeeeeeeeeeee ): pass + + +# Regression tests for https://github.com/astral-sh/ruff/issues/19226 +if "" and ( + # + not 0 +): + pass + +if "" and ( + # + not (0) +): + pass + +if "" and ( + not ( # + 0 + ) +): + pass ``` From d9486aa469f05d9406996f5d6e4dd48b51b2650f Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 14 Nov 2025 16:59:47 -0500 Subject: [PATCH 04/18] remove commented code --- crates/ruff_python_formatter/src/expression/expr_unary_op.rs | 3 --- 1 file changed, 3 deletions(-) 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 6c30e00196cad1..2aa6a95af2a162 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -37,9 +37,6 @@ impl FormatNodeRule for FormatExprUnaryOp { // (not # comment // a) // ``` - if !leading.is_empty() { - // hard_line_break().fmt(f)?; - } leading_comments(leading).fmt(f)?; trailing_comments(&dangling[pivot..]).fmt(f)?; From e4dc4b2acb3c69c50151147d3c51e05f6eee751e Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 14 Nov 2025 17:38:27 -0500 Subject: [PATCH 05/18] heavy debugging, I think I found the root cause missing the leading comments is causing the whole thing to get indented deep in the if formatting --- crates/ruff_formatter/src/builders.rs | 3 ++- .../ruff_python_formatter/src/comments/mod.rs | 8 ++++---- .../src/expression/expr_unary_op.rs | 5 +++-- .../ruff_python_formatter/src/expression/mod.rs | 17 ++++++++++++++++- .../src/expression/parentheses.rs | 3 +++ .../src/statement/clause.rs | 1 + .../src/statement/stmt_if.rs | 3 ++- 7 files changed, 31 insertions(+), 9 deletions(-) diff --git a/crates/ruff_formatter/src/builders.rs b/crates/ruff_formatter/src/builders.rs index ab60103d996186..f6d803f1738b11 100644 --- a/crates/ruff_formatter/src/builders.rs +++ b/crates/ruff_formatter/src/builders.rs @@ -1206,11 +1206,12 @@ enum IndentMode { impl Format for BlockIndent<'_, Context> { fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { + token("<<>>").fmt(f)?; let snapshot = f.snapshot(); f.write_element(FormatElement::Tag(StartIndent)); - match self.mode { + match dbg!(self.mode) { IndentMode::Soft => write!(f, [soft_line_break()])?, IndentMode::Block => write!(f, [hard_line_break()])?, IndentMode::SoftLineOrSpace | IndentMode::SoftSpace => { diff --git a/crates/ruff_python_formatter/src/comments/mod.rs b/crates/ruff_python_formatter/src/comments/mod.rs index 07fc789f34dac7..24a7e3979bcc24 100644 --- a/crates/ruff_python_formatter/src/comments/mod.rs +++ b/crates/ruff_python_formatter/src/comments/mod.rs @@ -390,10 +390,10 @@ impl<'a> Comments<'a> { writeln!(output, "{:#?}", comment.debug(source_code)).unwrap(); } - assert!( - output.is_empty(), - "The following comments have not been formatted.\n{output}" - ); + // assert!( + // output.is_empty(), + // "The following comments have not been formatted.\n{output}" + // ); } #[inline(always)] 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 2aa6a95af2a162..866b91ee77be75 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -1,8 +1,7 @@ use ruff_python_ast::AnyNodeRef; use ruff_python_ast::ExprUnaryOp; use ruff_python_ast::UnaryOp; -use ruff_python_trivia::SimpleTokenKind; -use ruff_python_trivia::SimpleTokenizer; +use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer}; use ruff_text_size::{Ranged, TextRange, TextSize}; use crate::comments::leading_comments; @@ -17,6 +16,8 @@ pub struct FormatExprUnaryOp; impl FormatNodeRule for FormatExprUnaryOp { fn fmt_fields(&self, item: &ExprUnaryOp, f: &mut PyFormatter) -> FormatResult<()> { + token("").fmt(f)?; + dbg!("unary op"); let ExprUnaryOp { range: _, node_index: _, diff --git a/crates/ruff_python_formatter/src/expression/mod.rs b/crates/ruff_python_formatter/src/expression/mod.rs index a320a1edf54b96..0da86821e95192 100644 --- a/crates/ruff_python_formatter/src/expression/mod.rs +++ b/crates/ruff_python_formatter/src/expression/mod.rs @@ -14,6 +14,7 @@ use ruff_text_size::Ranged; use crate::builders::parenthesize_if_expands; use crate::comments::{LeadingDanglingTrailingComments, leading_comments, trailing_comments}; use crate::context::{NodeLevel, WithNodeLevel}; +use crate::expression::expr_unary_op::operand_start; use crate::expression::parentheses::{ NeedsParentheses, OptionalParentheses, Parentheses, Parenthesize, is_expression_parenthesized, optional_parentheses, parenthesized, @@ -125,10 +126,12 @@ impl FormatRule> for FormatExpr { let comments = f.context().comments().clone(); let node_comments = comments.leading_dangling_trailing(expression); if !node_comments.has_leading() && !node_comments.has_trailing() { + dbg!("okay"); parenthesized("(", &format_expr, ")") .with_hugging(is_expression_huggable(expression, f.context())) .fmt(f) } else { + dbg!("disaster"); format_with_parentheses_comments(expression, &node_comments, f) } } else { @@ -373,11 +376,23 @@ impl Format> for MaybeParenthesizeExpression<'_> { let comments = f.context().comments().clone(); let node_comments = comments.leading_dangling_trailing(*expression); + let up_to = expression + .as_unary_op_expr() + .map(|unary_op| operand_start(unary_op, f.context().source())); // 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 node_comments.has_leading() || node_comments.has_trailing_own_line() { + if node_comments.has_leading() + || node_comments.has_trailing_own_line() + || dbg!(up_to.is_some_and(|up_to| { + node_comments + .dangling + .iter() + .any(|comment| comment.end() < up_to) + })) + { + token("").fmt(f)?; return expression.format().with_options(Parentheses::Always).fmt(f); } diff --git a/crates/ruff_python_formatter/src/expression/parentheses.rs b/crates/ruff_python_formatter/src/expression/parentheses.rs index a76f8a0aec7c75..523ab95c33f7b9 100644 --- a/crates/ruff_python_formatter/src/expression/parentheses.rs +++ b/crates/ruff_python_formatter/src/expression/parentheses.rs @@ -179,12 +179,15 @@ impl<'ast> Format> for FormatParenthesized<'_, 'ast> { let indented = format_with(|f| { let content = Arguments::from(&self.content); if self.comments.is_empty() { + dbg!("empty?"); if self.hug { content.fmt(f) } else { + dbg!("no hug"); group(&soft_block_indent(&content)).fmt(f) } } else { + dbg!("this is dangling"); group(&format_args![ dangling_open_parenthesis_comments(self.comments), soft_block_indent(&content), diff --git a/crates/ruff_python_formatter/src/statement/clause.rs b/crates/ruff_python_formatter/src/statement/clause.rs index 1554c30d0fbb75..8851bac3b8419e 100644 --- a/crates/ruff_python_formatter/src/statement/clause.rs +++ b/crates/ruff_python_formatter/src/statement/clause.rs @@ -403,6 +403,7 @@ impl<'ast> Format> for FormatClauseHeader<'_, 'ast> { if has_skip_comment(self.trailing_colon_comment, f.context().source()) { write_suppressed_clause_header(self.header, f)?; } else { + dbg!("last branch in clause header"); // Write a source map entry for the colon for range formatting to support formatting the clause header without // the clause body. Avoid computing `self.header.range()` otherwise because it's somewhat involved. let clause_end = if f.options().source_map_generation().is_enabled() { diff --git a/crates/ruff_python_formatter/src/statement/stmt_if.rs b/crates/ruff_python_formatter/src/statement/stmt_if.rs index 9b080ddc6e87a9..edf0cb96bdff3f 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_if.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_if.rs @@ -22,7 +22,7 @@ impl FormatNodeRule for FormatStmtIf { } = item; let comments = f.context().comments().clone(); - let trailing_colon_comment = comments.dangling(item); + let trailing_colon_comment = dbg!(comments.dangling(item)); write!( f, @@ -33,6 +33,7 @@ impl FormatNodeRule for FormatStmtIf { &format_args![ token("if"), space(), + token(""), maybe_parenthesize_expression(test, item, Parenthesize::IfBreaks), ], ), From 2d9b3fe60791d2fbd130dddf8776d74c83298c8d Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 17 Nov 2025 10:26:43 -0500 Subject: [PATCH 06/18] revert debugging code, get something working? Co-authored-by: Micha Reiser --- crates/ruff_formatter/src/builders.rs | 3 +- .../ruff_python_formatter/src/comments/mod.rs | 8 +-- .../src/comments/placement.rs | 2 +- .../src/expression/expr_unary_op.rs | 50 ++++++++++++------- .../src/expression/mod.rs | 17 +------ .../src/expression/parentheses.rs | 3 -- .../src/statement/clause.rs | 1 - .../src/statement/stmt_if.rs | 3 +- .../format@expression__unary.py.snap | 42 ++++++++++------ ...s__expression_parentheses_comments.py.snap | 13 +++-- 10 files changed, 77 insertions(+), 65 deletions(-) diff --git a/crates/ruff_formatter/src/builders.rs b/crates/ruff_formatter/src/builders.rs index f6d803f1738b11..ab60103d996186 100644 --- a/crates/ruff_formatter/src/builders.rs +++ b/crates/ruff_formatter/src/builders.rs @@ -1206,12 +1206,11 @@ enum IndentMode { impl Format for BlockIndent<'_, Context> { fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { - token("<<>>").fmt(f)?; let snapshot = f.snapshot(); f.write_element(FormatElement::Tag(StartIndent)); - match dbg!(self.mode) { + match self.mode { IndentMode::Soft => write!(f, [soft_line_break()])?, IndentMode::Block => write!(f, [hard_line_break()])?, IndentMode::SoftLineOrSpace | IndentMode::SoftSpace => { diff --git a/crates/ruff_python_formatter/src/comments/mod.rs b/crates/ruff_python_formatter/src/comments/mod.rs index 24a7e3979bcc24..07fc789f34dac7 100644 --- a/crates/ruff_python_formatter/src/comments/mod.rs +++ b/crates/ruff_python_formatter/src/comments/mod.rs @@ -390,10 +390,10 @@ impl<'a> Comments<'a> { writeln!(output, "{:#?}", comment.debug(source_code)).unwrap(); } - // assert!( - // output.is_empty(), - // "The following comments have not been formatted.\n{output}" - // ); + assert!( + output.is_empty(), + "The following comments have not been formatted.\n{output}" + ); } #[inline(always)] diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index c94383d6340446..57477d44453a57 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1909,7 +1909,7 @@ fn handle_unary_op_comment<'a>( source: &str, ) -> CommentPlacement<'a> { let up_to = operand_start(unary_op, source); - if comment.end() < up_to { + if comment.end() < up_to && comment.line_position().is_end_of_line() { CommentPlacement::dangling(unary_op, comment) } else { CommentPlacement::Default(comment) 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 866b91ee77be75..cdaa9e12290093 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -1,6 +1,7 @@ use ruff_python_ast::AnyNodeRef; use ruff_python_ast::ExprUnaryOp; use ruff_python_ast::UnaryOp; +use ruff_python_ast::parenthesize::parenthesized_range; use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer}; use ruff_text_size::{Ranged, TextRange, TextSize}; @@ -16,8 +17,6 @@ pub struct FormatExprUnaryOp; impl FormatNodeRule for FormatExprUnaryOp { fn fmt_fields(&self, item: &ExprUnaryOp, f: &mut PyFormatter) -> FormatResult<()> { - token("").fmt(f)?; - dbg!("unary op"); let ExprUnaryOp { range: _, node_index: _, @@ -25,30 +24,32 @@ impl FormatNodeRule for FormatExprUnaryOp { operand, } = item; + let operator = match op { + UnaryOp::Invert => "~", + UnaryOp::Not => "not", + UnaryOp::UAdd => "+", + UnaryOp::USub => "-", + }; + + token(operator).fmt(f)?; + let comments = f.context().comments().clone(); let dangling = comments.dangling(item); - let up_to = operand_start(item, f.context().source()); - - let pivot = dangling.partition_point(|comment| comment.end() < up_to); - let leading = &dangling[..pivot]; + let idx = dangling.partition_point(|comment| comment.line_position().is_end_of_line()); + let (leading, trailing) = dangling.split_at(idx); // Split off the comments that follow after the operator and format them as trailing comments. // ```python // (not # comment // a) // ``` - leading_comments(leading).fmt(f)?; - trailing_comments(&dangling[pivot..]).fmt(f)?; + trailing_comments(trailing).fmt(f)?; - let operator = match op { - UnaryOp::Invert => "~", - UnaryOp::Not => "not", - UnaryOp::UAdd => "+", - UnaryOp::USub => "-", - }; - - token(operator).fmt(f)?; + if !leading.is_empty() { + hard_line_break().fmt(f)?; + leading_comments(leading).fmt(f)?; + } // Insert a line break if the operand has comments but itself is not parenthesized. // ```python @@ -57,15 +58,30 @@ impl FormatNodeRule for FormatExprUnaryOp { // # comment // a) // ``` + let range = parenthesized_range( + operand.into(), + item.into(), + comments.ranges(), + f.context().source(), + ); + // look at leading comments (on the operand) and see if any of them come before the starting + // range of the parentheses; if so, insert hard line break, otherwise space + let has_leading_comments_before_parens = range.is_some_and(|range| { + comments + .leading(operand.as_ref()) + .iter() + .any(|comment| comment.start() < range.start()) + }); if comments.has_leading(operand.as_ref()) && !is_expression_parenthesized( operand.as_ref().into(), f.context().comments().ranges(), f.context().source(), ) + || has_leading_comments_before_parens { hard_line_break().fmt(f)?; - } else if op.is_not() { + } else if op.is_not() && leading.is_empty() { space().fmt(f)?; } diff --git a/crates/ruff_python_formatter/src/expression/mod.rs b/crates/ruff_python_formatter/src/expression/mod.rs index 0da86821e95192..a320a1edf54b96 100644 --- a/crates/ruff_python_formatter/src/expression/mod.rs +++ b/crates/ruff_python_formatter/src/expression/mod.rs @@ -14,7 +14,6 @@ use ruff_text_size::Ranged; use crate::builders::parenthesize_if_expands; use crate::comments::{LeadingDanglingTrailingComments, leading_comments, trailing_comments}; use crate::context::{NodeLevel, WithNodeLevel}; -use crate::expression::expr_unary_op::operand_start; use crate::expression::parentheses::{ NeedsParentheses, OptionalParentheses, Parentheses, Parenthesize, is_expression_parenthesized, optional_parentheses, parenthesized, @@ -126,12 +125,10 @@ impl FormatRule> for FormatExpr { let comments = f.context().comments().clone(); let node_comments = comments.leading_dangling_trailing(expression); if !node_comments.has_leading() && !node_comments.has_trailing() { - dbg!("okay"); parenthesized("(", &format_expr, ")") .with_hugging(is_expression_huggable(expression, f.context())) .fmt(f) } else { - dbg!("disaster"); format_with_parentheses_comments(expression, &node_comments, f) } } else { @@ -376,23 +373,11 @@ impl Format> for MaybeParenthesizeExpression<'_> { let comments = f.context().comments().clone(); let node_comments = comments.leading_dangling_trailing(*expression); - let up_to = expression - .as_unary_op_expr() - .map(|unary_op| operand_start(unary_op, f.context().source())); // 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 node_comments.has_leading() - || node_comments.has_trailing_own_line() - || dbg!(up_to.is_some_and(|up_to| { - node_comments - .dangling - .iter() - .any(|comment| comment.end() < up_to) - })) - { - token("").fmt(f)?; + if node_comments.has_leading() || node_comments.has_trailing_own_line() { return expression.format().with_options(Parentheses::Always).fmt(f); } diff --git a/crates/ruff_python_formatter/src/expression/parentheses.rs b/crates/ruff_python_formatter/src/expression/parentheses.rs index 523ab95c33f7b9..a76f8a0aec7c75 100644 --- a/crates/ruff_python_formatter/src/expression/parentheses.rs +++ b/crates/ruff_python_formatter/src/expression/parentheses.rs @@ -179,15 +179,12 @@ impl<'ast> Format> for FormatParenthesized<'_, 'ast> { let indented = format_with(|f| { let content = Arguments::from(&self.content); if self.comments.is_empty() { - dbg!("empty?"); if self.hug { content.fmt(f) } else { - dbg!("no hug"); group(&soft_block_indent(&content)).fmt(f) } } else { - dbg!("this is dangling"); group(&format_args![ dangling_open_parenthesis_comments(self.comments), soft_block_indent(&content), diff --git a/crates/ruff_python_formatter/src/statement/clause.rs b/crates/ruff_python_formatter/src/statement/clause.rs index 8851bac3b8419e..1554c30d0fbb75 100644 --- a/crates/ruff_python_formatter/src/statement/clause.rs +++ b/crates/ruff_python_formatter/src/statement/clause.rs @@ -403,7 +403,6 @@ impl<'ast> Format> for FormatClauseHeader<'_, 'ast> { if has_skip_comment(self.trailing_colon_comment, f.context().source()) { write_suppressed_clause_header(self.header, f)?; } else { - dbg!("last branch in clause header"); // Write a source map entry for the colon for range formatting to support formatting the clause header without // the clause body. Avoid computing `self.header.range()` otherwise because it's somewhat involved. let clause_end = if f.options().source_map_generation().is_enabled() { diff --git a/crates/ruff_python_formatter/src/statement/stmt_if.rs b/crates/ruff_python_formatter/src/statement/stmt_if.rs index edf0cb96bdff3f..9b080ddc6e87a9 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_if.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_if.rs @@ -22,7 +22,7 @@ impl FormatNodeRule for FormatStmtIf { } = item; let comments = f.context().comments().clone(); - let trailing_colon_comment = dbg!(comments.dangling(item)); + let trailing_colon_comment = comments.dangling(item); write!( f, @@ -33,7 +33,6 @@ impl FormatNodeRule for FormatStmtIf { &format_args![ token("if"), space(), - token(""), maybe_parenthesize_expression(test, item, Parenthesize::IfBreaks), ], ), 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 76f2dbc555b549..4f17b76ce800e6 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 @@ -265,31 +265,35 @@ if +( pass if ( + not # comment - not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( + ~ # comment - ~aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( + - # comment - -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( + + # comment - +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass @@ -298,8 +302,9 @@ if ( if ( # unary comment + not # operand comment - not ( + ( # comment aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb @@ -334,31 +339,35 @@ if ( ## Trailing operator comments if ( + not # comment - not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( + ~ # comment - ~aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( + - # comment - -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( + + # comment - +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass @@ -378,14 +387,16 @@ if ( pass if ( + not # comment - not a + a ): pass if ( + not # comment - not a + a ): pass @@ -402,9 +413,10 @@ if True: # Regression test for: https://github.com/astral-sh/ruff/issues/7448 x = ( # a + not # b # c - not ( # d + ( # d # e True ) @@ -436,14 +448,16 @@ def foo(): # Regression tests for https://github.com/astral-sh/ruff/issues/19226 if "" and ( + not # - not 0 + 0 ): pass if "" and ( + not # - not (0) + (0) ): pass diff --git a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap index 5e8e826231f3bc..0961da8482d47a 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/parentheses/expression_parentheses_comments.py -snapshot_kind: text --- ## Input ```python @@ -179,13 +178,15 @@ nested_parentheses4 = [ x = ( # unary comment + not # in-between comment - not ( + ( # leading inner "a" ), + not # in-between comment - not ( + ( # leading inner "b" ), @@ -194,8 +195,9 @@ x = ( "c" ), # 1 + not # 2 - not ( # 3 + ( # 3 # 4 "d" ), @@ -203,8 +205,9 @@ x = ( if ( # unary comment + not # in-between comment - not ( + ( # leading inner 1 ) From c7fc1212e5b12b73e40cd76c5157c5b423f4403c Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 17 Nov 2025 10:34:44 -0500 Subject: [PATCH 07/18] update comments and range variable name --- .../src/expression/expr_unary_op.rs | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) 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 cdaa9e12290093..34ba72b6b2c3cf 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -51,22 +51,31 @@ impl FormatNodeRule for FormatExprUnaryOp { leading_comments(leading).fmt(f)?; } - // Insert a line break if the operand has comments but itself is not parenthesized. + // Insert a line break if the operand has comments but itself is not parenthesized or if the + // operand is parenthesized but has a leading comment before the parentheses. // ```python // if ( // not // # comment - // a) + // a): + // pass + // + // if 1 and ( + // not + // # comment + // ( + // a + // ) + // ): + // pass // ``` - let range = parenthesized_range( + let parenthesized_operand_range = parenthesized_range( operand.into(), item.into(), comments.ranges(), f.context().source(), ); - // look at leading comments (on the operand) and see if any of them come before the starting - // range of the parentheses; if so, insert hard line break, otherwise space - let has_leading_comments_before_parens = range.is_some_and(|range| { + let has_leading_comments_before_parens = parenthesized_operand_range.is_some_and(|range| { comments .leading(operand.as_ref()) .iter() From 60317c830f76b7b64ce39a73e70a18f9ae40f3ef Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 17 Nov 2025 10:37:38 -0500 Subject: [PATCH 08/18] avoid computing operand_start for each dangling comment --- .../src/expression/expr_unary_op.rs | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) 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 34ba72b6b2c3cf..24ed4140377d00 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -112,21 +112,28 @@ impl NeedsParentheses for ExprUnaryOp { context: &PyFormatContext, ) -> OptionalParentheses { if parent.is_expr_await() { - OptionalParentheses::Always - } else if is_expression_parenthesized( + return OptionalParentheses::Always; + } + + if is_expression_parenthesized( self.operand.as_ref().into(), context.comments().ranges(), context.source(), ) { - OptionalParentheses::Never - } else if context + return OptionalParentheses::Never; + } + + let operand_start = operand_start(self, context.source()); + if context .comments() .dangling(self) .iter() - .any(|comment| comment.end() < operand_start(self, context.source())) + .any(|comment| comment.end() < operand_start) { - OptionalParentheses::Multiline - } else if context.comments().has(self.operand.as_ref()) { + return OptionalParentheses::Multiline; + } + + if context.comments().has(self.operand.as_ref()) { OptionalParentheses::Always } else { self.operand.needs_parentheses(self.into(), context) From 7e193df267945a14559fc39e1ce8f4348d2ed41d Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 17 Nov 2025 10:49:17 -0500 Subject: [PATCH 09/18] try dangling as trailing --- .../src/expression/expr_unary_op.rs | 13 ++------ .../format@expression__unary.py.snap | 31 +++++-------------- ...s__expression_parentheses_comments.py.snap | 8 ++--- 3 files changed, 12 insertions(+), 40 deletions(-) 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 24ed4140377d00..18459ba5aa22b0 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -5,7 +5,6 @@ use ruff_python_ast::parenthesize::parenthesized_range; use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer}; use ruff_text_size::{Ranged, TextRange, TextSize}; -use crate::comments::leading_comments; use crate::comments::trailing_comments; use crate::expression::parentheses::{ NeedsParentheses, OptionalParentheses, Parentheses, is_expression_parenthesized, @@ -36,20 +35,12 @@ impl FormatNodeRule for FormatExprUnaryOp { let comments = f.context().comments().clone(); let dangling = comments.dangling(item); - let idx = dangling.partition_point(|comment| comment.line_position().is_end_of_line()); - let (leading, trailing) = dangling.split_at(idx); - // Split off the comments that follow after the operator and format them as trailing comments. // ```python // (not # comment // a) // ``` - trailing_comments(trailing).fmt(f)?; - - if !leading.is_empty() { - hard_line_break().fmt(f)?; - leading_comments(leading).fmt(f)?; - } + trailing_comments(dangling).fmt(f)?; // Insert a line break if the operand has comments but itself is not parenthesized or if the // operand is parenthesized but has a leading comment before the parentheses. @@ -90,7 +81,7 @@ impl FormatNodeRule for FormatExprUnaryOp { || has_leading_comments_before_parens { hard_line_break().fmt(f)?; - } else if op.is_not() && leading.is_empty() { + } else if op.is_not() { space().fmt(f)?; } 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 4f17b76ce800e6..747474811a9972 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 @@ -339,35 +339,27 @@ if ( ## Trailing operator comments if ( - not - # comment - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # comment + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( - ~ - # comment - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + ~aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # comment + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( - - - # comment - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # comment + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( - + - # comment - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # comment + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass @@ -394,9 +386,7 @@ if ( pass if ( - not - # comment - a + not a # comment ): pass @@ -413,8 +403,7 @@ if True: # Regression test for: https://github.com/astral-sh/ruff/issues/7448 x = ( # a - not - # b + not # b # c ( # d # e @@ -448,16 +437,12 @@ def foo(): # Regression tests for https://github.com/astral-sh/ruff/issues/19226 if "" and ( - not - # - 0 + not 0 # ): pass if "" and ( - not - # - (0) + not (0) # ): pass diff --git a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap index 0961da8482d47a..16152c56701401 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap @@ -184,9 +184,7 @@ x = ( # leading inner "a" ), - not - # in-between comment - ( + not ( # in-between comment # leading inner "b" ), @@ -195,9 +193,7 @@ x = ( "c" ), # 1 - not - # 2 - ( # 3 + not ( # 2 # 3 # 4 "d" ), From 31d43c582202b18e34c5c2b16c97759d77002919 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 17 Nov 2025 11:46:42 -0500 Subject: [PATCH 10/18] update handle_unary_op_comment docs --- .../ruff_python_formatter/src/comments/placement.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 57477d44453a57..3704434424cd02 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1891,9 +1891,11 @@ fn handle_lambda_comment<'a>( CommentPlacement::Default(comment) } -/// Move comment between a unary op and its operand before the unary op by marking them as trailing. +/// Move an end-of-line comment between a unary op and its operand after the operand by marking +/// it as dangling. /// /// For example, given: +/// /// ```python /// ( /// not # comment @@ -1901,8 +1903,13 @@ fn handle_lambda_comment<'a>( /// ) /// ``` /// -/// 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. +/// the `# comment` will be attached as a dangling comment on the unary op and formatted as: +/// +/// ```python +/// ( +/// not True # comment +/// ) +/// ``` fn handle_unary_op_comment<'a>( comment: DecoratedComment<'a>, unary_op: &'a ast::ExprUnaryOp, From a50e006c23d53947d72f106bf366e87c2b82e945 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 17 Nov 2025 11:49:01 -0500 Subject: [PATCH 11/18] reuse leading_operand_comments --- .../ruff_python_formatter/src/expression/expr_unary_op.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 18459ba5aa22b0..a4c5fb428c2868 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -66,13 +66,13 @@ impl FormatNodeRule for FormatExprUnaryOp { comments.ranges(), f.context().source(), ); + let leading_operand_comments = comments.leading(operand.as_ref()); let has_leading_comments_before_parens = parenthesized_operand_range.is_some_and(|range| { - comments - .leading(operand.as_ref()) + leading_operand_comments .iter() .any(|comment| comment.start() < range.start()) }); - if comments.has_leading(operand.as_ref()) + if !leading_operand_comments.is_empty() && !is_expression_parenthesized( operand.as_ref().into(), f.context().comments().ranges(), From bd612c4d3a019a3b7d8dcde6afe1216529d4fac6 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 17 Nov 2025 12:00:48 -0500 Subject: [PATCH 12/18] simplify has_dangling check, move it before Never case --- .../src/expression/expr_unary_op.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) 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 a4c5fb428c2868..65724815d63590 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -106,6 +106,10 @@ impl NeedsParentheses for ExprUnaryOp { return OptionalParentheses::Always; } + if context.comments().has_dangling(self) { + return OptionalParentheses::Multiline; + } + if is_expression_parenthesized( self.operand.as_ref().into(), context.comments().ranges(), @@ -114,16 +118,6 @@ impl NeedsParentheses for ExprUnaryOp { return OptionalParentheses::Never; } - let operand_start = operand_start(self, context.source()); - if context - .comments() - .dangling(self) - .iter() - .any(|comment| comment.end() < operand_start) - { - return OptionalParentheses::Multiline; - } - if context.comments().has(self.operand.as_ref()) { OptionalParentheses::Always } else { From 91cb799f0bda025af019886d4e730e88b90d4f7f Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 17 Nov 2025 12:02:41 -0500 Subject: [PATCH 13/18] revert some now-unnecessary changes operand_start was now used in only one place again, and the intermediate variable in NeedsParentheses was no longer needed --- .../src/comments/placement.rs | 18 +++++++- .../src/expression/expr_unary_op.rs | 41 ++++--------------- 2 files changed, 23 insertions(+), 36 deletions(-) diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 3704434424cd02..f28f9b18a868a9 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -13,7 +13,6 @@ use std::cmp::Ordering; use crate::comments::visitor::{CommentPlacement, DecoratedComment}; use crate::expression::expr_slice::{ExprSliceCommentSection, assign_comment_in_slice}; -use crate::expression::expr_unary_op::operand_start; use crate::expression::parentheses::is_expression_parenthesized; use crate::other::parameters::{ assign_argument_separator_comment_placement, find_parameter_separators, @@ -1915,7 +1914,22 @@ fn handle_unary_op_comment<'a>( unary_op: &'a ast::ExprUnaryOp, source: &str, ) -> CommentPlacement<'a> { - let up_to = operand_start(unary_op, source); + let mut tokenizer = SimpleTokenizer::new( + source, + TextRange::new(unary_op.start(), unary_op.operand.start()), + ) + .skip_trivia(); + let op_token = tokenizer.next(); + debug_assert!(op_token.is_some_and(|token| matches!( + token.kind, + SimpleTokenKind::Tilde + | SimpleTokenKind::Not + | SimpleTokenKind::Plus + | SimpleTokenKind::Minus + ))); + let up_to = tokenizer + .find(|token| token.kind == SimpleTokenKind::LParen) + .map_or(unary_op.operand.start(), |lparen| lparen.start()); if comment.end() < up_to && comment.line_position().is_end_of_line() { CommentPlacement::dangling(unary_op, comment) } else { 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 65724815d63590..a1676f5e0a4b8a 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -2,8 +2,7 @@ use ruff_python_ast::AnyNodeRef; use ruff_python_ast::ExprUnaryOp; use ruff_python_ast::UnaryOp; use ruff_python_ast::parenthesize::parenthesized_range; -use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer}; -use ruff_text_size::{Ranged, TextRange, TextSize}; +use ruff_text_size::Ranged; use crate::comments::trailing_comments; use crate::expression::parentheses::{ @@ -103,45 +102,19 @@ impl NeedsParentheses for ExprUnaryOp { context: &PyFormatContext, ) -> OptionalParentheses { if parent.is_expr_await() { - return OptionalParentheses::Always; - } - - if context.comments().has_dangling(self) { - return OptionalParentheses::Multiline; - } - - if is_expression_parenthesized( + OptionalParentheses::Always + } else if context.comments().has_dangling(self) { + OptionalParentheses::Multiline + } else if is_expression_parenthesized( self.operand.as_ref().into(), context.comments().ranges(), context.source(), ) { - return OptionalParentheses::Never; - } - - if context.comments().has(self.operand.as_ref()) { + OptionalParentheses::Never + } else if context.comments().has(self.operand.as_ref()) { OptionalParentheses::Always } else { self.operand.needs_parentheses(self.into(), context) } } } - -/// Returns the start of `unary_op`'s operand, or its leading parenthesis, if it has one. -pub(crate) fn operand_start(unary_op: &ExprUnaryOp, source: &str) -> TextSize { - let mut tokenizer = SimpleTokenizer::new( - source, - TextRange::new(unary_op.start(), unary_op.operand.start()), - ) - .skip_trivia(); - let op_token = tokenizer.next(); - debug_assert!(op_token.is_some_and(|token| matches!( - token.kind, - SimpleTokenKind::Tilde - | SimpleTokenKind::Not - | SimpleTokenKind::Plus - | SimpleTokenKind::Minus - ))); - tokenizer - .find(|token| token.kind == SimpleTokenKind::LParen) - .map_or(unary_op.operand.start(), |lparen| lparen.start()) -} From c0d4d91a3d66d2015dc3b9401c81971cae2027fb Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 18 Nov 2025 08:46:50 -0500 Subject: [PATCH 14/18] convert back to guarded returns --- .../src/expression/expr_unary_op.rs | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) 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 a1676f5e0a4b8a..77273d13891359 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -102,19 +102,25 @@ impl NeedsParentheses for ExprUnaryOp { context: &PyFormatContext, ) -> OptionalParentheses { if parent.is_expr_await() { - OptionalParentheses::Always - } else if context.comments().has_dangling(self) { - OptionalParentheses::Multiline - } else if is_expression_parenthesized( + return OptionalParentheses::Always; + } + + if context.comments().has_dangling(self) { + return OptionalParentheses::Multiline; + } + + if is_expression_parenthesized( self.operand.as_ref().into(), context.comments().ranges(), context.source(), ) { - OptionalParentheses::Never - } else if context.comments().has(self.operand.as_ref()) { - OptionalParentheses::Always - } else { - self.operand.needs_parentheses(self.into(), context) + return OptionalParentheses::Never; + } + + if context.comments().has(self.operand.as_ref()) { + return OptionalParentheses::Always; } + + self.operand.needs_parentheses(self.into(), context) } } From 58cd40f4422eee2704b8cd92a96d27cd2f40632e Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 18 Nov 2025 08:56:22 -0500 Subject: [PATCH 15/18] factor out needs_line_break, fix parenthesize bug --- .../test/fixtures/ruff/expression/unary.py | 6 +++ .../src/expression/expr_unary_op.rs | 49 +++++++++++-------- .../format@expression__unary.py.snap | 17 +++++-- 3 files changed, 48 insertions(+), 24 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 ea17e4a66f8c07..90d80540b0347f 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 @@ -209,3 +209,9 @@ def foo(): 0 )): pass + +if ( + not + # comment + (a)): + pass 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 77273d13891359..f8db8b0b5c1f47 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -59,26 +59,7 @@ impl FormatNodeRule for FormatExprUnaryOp { // ): // pass // ``` - let parenthesized_operand_range = parenthesized_range( - operand.into(), - item.into(), - comments.ranges(), - f.context().source(), - ); - let leading_operand_comments = comments.leading(operand.as_ref()); - let has_leading_comments_before_parens = parenthesized_operand_range.is_some_and(|range| { - leading_operand_comments - .iter() - .any(|comment| comment.start() < range.start()) - }); - if !leading_operand_comments.is_empty() - && !is_expression_parenthesized( - operand.as_ref().into(), - f.context().comments().ranges(), - f.context().source(), - ) - || has_leading_comments_before_parens - { + if needs_line_break(item, f.context()) { hard_line_break().fmt(f)?; } else if op.is_not() { space().fmt(f)?; @@ -105,7 +86,7 @@ impl NeedsParentheses for ExprUnaryOp { return OptionalParentheses::Always; } - if context.comments().has_dangling(self) { + if needs_line_break(self, context) { return OptionalParentheses::Multiline; } @@ -124,3 +105,29 @@ impl NeedsParentheses for ExprUnaryOp { self.operand.needs_parentheses(self.into(), context) } } + +/// Returns `true` if the unary operator will have a hard line break between the operator and its +/// operand and thus requires parentheses. +fn needs_line_break(item: &ExprUnaryOp, context: &PyFormatContext) -> bool { + let comments = context.comments(); + let parenthesized_operand_range = parenthesized_range( + item.operand.as_ref().into(), + item.into(), + comments.ranges(), + context.source(), + ); + let leading_operand_comments = comments.leading(item.operand.as_ref()); + let has_leading_comments_before_parens = parenthesized_operand_range.is_some_and(|range| { + leading_operand_comments + .iter() + .any(|comment| comment.start() < range.start()) + }); + + !leading_operand_comments.is_empty() + && !is_expression_parenthesized( + item.operand.as_ref().into(), + context.comments().ranges(), + context.source(), + ) + || has_leading_comments_before_parens +} 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 747474811a9972..95a5ca84dd9794 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 @@ -215,6 +215,12 @@ if '' and (not 0 )): pass + +if ( + not + # comment + (a)): + pass ``` ## Output @@ -385,9 +391,7 @@ if ( ): pass -if ( - not a # comment -): +if not a: # comment pass # Regression test for: https://github.com/astral-sh/ruff/issues/7423 @@ -452,4 +456,11 @@ if "" and ( ) ): pass + +if ( + not + # comment + (a) +): + pass ``` From d89e1c27b78c7ffa1100203dea51ea80067c6f91 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 18 Nov 2025 09:03:39 -0500 Subject: [PATCH 16/18] add a few more tests --- .../test/fixtures/ruff/expression/unary.py | 33 +++++++++ .../format@expression__unary.py.snap | 71 +++++++++++++++++++ 2 files changed, 104 insertions(+) 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 90d80540b0347f..a444088b0a108e 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 @@ -215,3 +215,36 @@ def foo(): # comment (a)): pass + +if not ( # comment + a): + pass + +if not ( + # comment + (a)): + pass + +if not ( + # comment + a): + pass + +not (# comment + (a)) + +(-#comment + (a)) + +if ( # a + # b + not # c + # d + ( # e + # f + a # g + # h + ) # i + # j +): + pass 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 95a5ca84dd9794..9dffb4e517f411 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 @@ -221,6 +221,39 @@ if ( # comment (a)): pass + +if not ( # comment + a): + pass + +if not ( + # comment + (a)): + pass + +if not ( + # comment + a): + pass + +not (# comment + (a)) + +(-#comment + (a)) + +if ( # a + # b + not # c + # d + ( # e + # f + a # g + # h + ) # i + # j +): + pass ``` ## Output @@ -463,4 +496,42 @@ if ( (a) ): pass + +if not ( # comment + a +): + pass + +if not ( + # comment + a +): + pass + +if not ( + # comment + a +): + pass + +not ( # comment + a +) + +( + -(a) # comment +) + +if ( # a + # b + not # c + # d + ( # e + # f + a # g + # h + ) # i + # j +): + pass ``` From 9d86a5c58e0d5e1df6bd13d99c0dd893bb5ee286 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 18 Nov 2025 10:29:36 -0500 Subject: [PATCH 17/18] needs_line_break => OptionalParentheses::Always --- crates/ruff_python_formatter/src/expression/expr_unary_op.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 f8db8b0b5c1f47..5cc741018fc9c4 100644 --- a/crates/ruff_python_formatter/src/expression/expr_unary_op.rs +++ b/crates/ruff_python_formatter/src/expression/expr_unary_op.rs @@ -87,7 +87,7 @@ impl NeedsParentheses for ExprUnaryOp { } if needs_line_break(self, context) { - return OptionalParentheses::Multiline; + return OptionalParentheses::Always; } if is_expression_parenthesized( From 9e161513f811e3c571c27e69f5058799c26cd884 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 18 Nov 2025 10:29:55 -0500 Subject: [PATCH 18/18] add co-author Co-authored-by: Takayuki Maeda