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
This commit is contained in:
Brent Westbrook 2025-11-14 17:38:27 -05:00
parent 4bc1271b34
commit f19c21914b
No known key found for this signature in database
7 changed files with 31 additions and 9 deletions

View file

@ -1206,11 +1206,12 @@ enum IndentMode {
impl<Context> Format<Context> for BlockIndent<'_, Context> { impl<Context> Format<Context> for BlockIndent<'_, Context> {
fn fmt(&self, f: &mut Formatter<Context>) -> FormatResult<()> { fn fmt(&self, f: &mut Formatter<Context>) -> FormatResult<()> {
token("<<>>").fmt(f)?;
let snapshot = f.snapshot(); let snapshot = f.snapshot();
f.write_element(FormatElement::Tag(StartIndent)); f.write_element(FormatElement::Tag(StartIndent));
match self.mode { match dbg!(self.mode) {
IndentMode::Soft => write!(f, [soft_line_break()])?, IndentMode::Soft => write!(f, [soft_line_break()])?,
IndentMode::Block => write!(f, [hard_line_break()])?, IndentMode::Block => write!(f, [hard_line_break()])?,
IndentMode::SoftLineOrSpace | IndentMode::SoftSpace => { IndentMode::SoftLineOrSpace | IndentMode::SoftSpace => {

View file

@ -390,10 +390,10 @@ impl<'a> Comments<'a> {
writeln!(output, "{:#?}", comment.debug(source_code)).unwrap(); writeln!(output, "{:#?}", comment.debug(source_code)).unwrap();
} }
assert!( // assert!(
output.is_empty(), // output.is_empty(),
"The following comments have not been formatted.\n{output}" // "The following comments have not been formatted.\n{output}"
); // );
} }
#[inline(always)] #[inline(always)]

View file

@ -1,8 +1,7 @@
use ruff_python_ast::AnyNodeRef; use ruff_python_ast::AnyNodeRef;
use ruff_python_ast::ExprUnaryOp; use ruff_python_ast::ExprUnaryOp;
use ruff_python_ast::UnaryOp; use ruff_python_ast::UnaryOp;
use ruff_python_trivia::SimpleTokenKind; use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_python_trivia::SimpleTokenizer;
use ruff_text_size::{Ranged, TextRange, TextSize}; use ruff_text_size::{Ranged, TextRange, TextSize};
use crate::comments::leading_comments; use crate::comments::leading_comments;
@ -17,6 +16,8 @@ pub struct FormatExprUnaryOp;
impl FormatNodeRule<ExprUnaryOp> for FormatExprUnaryOp { impl FormatNodeRule<ExprUnaryOp> for FormatExprUnaryOp {
fn fmt_fields(&self, item: &ExprUnaryOp, f: &mut PyFormatter) -> FormatResult<()> { fn fmt_fields(&self, item: &ExprUnaryOp, f: &mut PyFormatter) -> FormatResult<()> {
token("<cursor>").fmt(f)?;
dbg!("unary op");
let ExprUnaryOp { let ExprUnaryOp {
range: _, range: _,
node_index: _, node_index: _,

View file

@ -14,6 +14,7 @@ use ruff_text_size::Ranged;
use crate::builders::parenthesize_if_expands; use crate::builders::parenthesize_if_expands;
use crate::comments::{LeadingDanglingTrailingComments, leading_comments, trailing_comments}; use crate::comments::{LeadingDanglingTrailingComments, leading_comments, trailing_comments};
use crate::context::{NodeLevel, WithNodeLevel}; use crate::context::{NodeLevel, WithNodeLevel};
use crate::expression::expr_unary_op::operand_start;
use crate::expression::parentheses::{ use crate::expression::parentheses::{
NeedsParentheses, OptionalParentheses, Parentheses, Parenthesize, is_expression_parenthesized, NeedsParentheses, OptionalParentheses, Parentheses, Parenthesize, is_expression_parenthesized,
optional_parentheses, parenthesized, optional_parentheses, parenthesized,
@ -125,10 +126,12 @@ impl FormatRule<Expr, PyFormatContext<'_>> for FormatExpr {
let comments = f.context().comments().clone(); let comments = f.context().comments().clone();
let node_comments = comments.leading_dangling_trailing(expression); let node_comments = comments.leading_dangling_trailing(expression);
if !node_comments.has_leading() && !node_comments.has_trailing() { if !node_comments.has_leading() && !node_comments.has_trailing() {
dbg!("okay");
parenthesized("(", &format_expr, ")") parenthesized("(", &format_expr, ")")
.with_hugging(is_expression_huggable(expression, f.context())) .with_hugging(is_expression_huggable(expression, f.context()))
.fmt(f) .fmt(f)
} else { } else {
dbg!("disaster");
format_with_parentheses_comments(expression, &node_comments, f) format_with_parentheses_comments(expression, &node_comments, f)
} }
} else { } else {
@ -373,11 +376,23 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
let comments = f.context().comments().clone(); let comments = f.context().comments().clone();
let node_comments = comments.leading_dangling_trailing(*expression); 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 // 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 // ensures that we correctly handle parenthesized comments, and don't need to worry about
// them in the implementation below. // 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("<maybe cursor>").fmt(f)?;
return expression.format().with_options(Parentheses::Always).fmt(f); return expression.format().with_options(Parentheses::Always).fmt(f);
} }

View file

@ -179,12 +179,15 @@ impl<'ast> Format<PyFormatContext<'ast>> for FormatParenthesized<'_, 'ast> {
let indented = format_with(|f| { let indented = format_with(|f| {
let content = Arguments::from(&self.content); let content = Arguments::from(&self.content);
if self.comments.is_empty() { if self.comments.is_empty() {
dbg!("empty?");
if self.hug { if self.hug {
content.fmt(f) content.fmt(f)
} else { } else {
dbg!("no hug");
group(&soft_block_indent(&content)).fmt(f) group(&soft_block_indent(&content)).fmt(f)
} }
} else { } else {
dbg!("this is dangling");
group(&format_args![ group(&format_args![
dangling_open_parenthesis_comments(self.comments), dangling_open_parenthesis_comments(self.comments),
soft_block_indent(&content), soft_block_indent(&content),

View file

@ -403,6 +403,7 @@ impl<'ast> Format<PyFormatContext<'ast>> for FormatClauseHeader<'_, 'ast> {
if has_skip_comment(self.trailing_colon_comment, f.context().source()) { if has_skip_comment(self.trailing_colon_comment, f.context().source()) {
write_suppressed_clause_header(self.header, f)?; write_suppressed_clause_header(self.header, f)?;
} else { } 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 // 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. // 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() { let clause_end = if f.options().source_map_generation().is_enabled() {

View file

@ -22,7 +22,7 @@ impl FormatNodeRule<StmtIf> for FormatStmtIf {
} = item; } = item;
let comments = f.context().comments().clone(); let comments = f.context().comments().clone();
let trailing_colon_comment = comments.dangling(item); let trailing_colon_comment = dbg!(comments.dangling(item));
write!( write!(
f, f,
@ -33,6 +33,7 @@ impl FormatNodeRule<StmtIf> for FormatStmtIf {
&format_args![ &format_args![
token("if"), token("if"),
space(), space(),
token("<cursor>"),
maybe_parenthesize_expression(test, item, Parenthesize::IfBreaks), maybe_parenthesize_expression(test, item, Parenthesize::IfBreaks),
], ],
), ),