From 2bd64095a6f9e8bd047dc88e93a63fe888240bad Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 9 Dec 2025 16:35:14 -0500 Subject: [PATCH 1/7] revert FormatParameters changes --- .../src/other/parameters.rs | 26 ------------------- 1 file changed, 26 deletions(-) diff --git a/crates/ruff_python_formatter/src/other/parameters.rs b/crates/ruff_python_formatter/src/other/parameters.rs index 2e12f48456..1c6682bab1 100644 --- a/crates/ruff_python_formatter/src/other/parameters.rs +++ b/crates/ruff_python_formatter/src/other/parameters.rs @@ -241,32 +241,6 @@ impl FormatNodeRule for FormatParameters { let num_parameters = item.len(); if self.parentheses == ParametersParentheses::Never { - // In a lambda, format any leading comments on the first parameter outside of the - // parameters group so that the parameters don't break. For example, format this input: - // - // ```py - // ( - // lambda - // * # this becomes a leading comment on *x - // x, **y: - // x - // ) - // ``` - // - // as: - // - // ```py - // ( - // lambda - // # this becomes a leading comment on *x - // *x, **y: x - // ) - // ``` - if let Some(first) = item.iter().next() - && comments.has_leading(first.as_parameter()) - { - leading_node_comments(first.as_parameter()).fmt(f)?; - } write!(f, [group(&format_inner), dangling_comments(dangling)]) } else if num_parameters == 0 { let mut f = WithNodeLevel::new(NodeLevel::ParenthesizedExpression, f); From d722155762a27e877d0410515d87053251ff39e2 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 9 Dec 2025 16:45:26 -0500 Subject: [PATCH 2/7] make comments leading on parameter_s_ --- .../src/comments/placement.rs | 34 +++++++++++++++---- .../src/expression/expr_lambda.rs | 6 +--- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 28397b6dcf..3ad3df2653 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -857,6 +857,19 @@ fn handle_parameter_comment<'a>( parameter: &'a Parameter, source: &str, ) -> CommentPlacement<'a> { + let parent = if let Some(parent @ AnyNodeRef::Parameters(parameters)) = + comment.enclosing_parent() + && parameters + .iter() + .next() + .is_some_and(|first| first.range() == parameter.range()) + && !are_parameters_parenthesized(parameters, source) + { + Some(parent) + } else { + None + }; + if parameter.annotation().is_some() { let colon = first_non_trivia_token(parameter.name.end(), source).expect( "A annotated parameter should have a colon following its name when it is valid syntax.", @@ -865,13 +878,22 @@ fn handle_parameter_comment<'a>( assert_eq!(colon.kind(), SimpleTokenKind::Colon); if comment.start() < colon.start() { - // The comment is before the colon, pull it out and make it a leading comment of the parameter. - CommentPlacement::leading(parameter, comment) + // The comment is before the colon, pull it out and make it a leading comment of the + // parameter, or of the whole parameters list, if it's the first parameter. + if let Some(parent) = parent { + CommentPlacement::leading(parent, comment) + } else { + CommentPlacement::leading(parameter, comment) + } } else { CommentPlacement::Default(comment) } } else if comment.start() < parameter.name.start() { - CommentPlacement::leading(parameter, comment) + if let Some(parent) = parent { + CommentPlacement::leading(parent, comment) + } else { + CommentPlacement::leading(parameter, comment) + } } else { CommentPlacement::Default(comment) } @@ -1835,10 +1857,8 @@ fn handle_lambda_comment<'a>( // ) // ``` if comment.start() < parameters.start() { - return if let Some(first) = parameters.iter().next() - && comment.line_position().is_own_line() - { - CommentPlacement::leading(first.as_parameter(), comment) + return if parameters.iter().next().is_some() && comment.line_position().is_own_line() { + CommentPlacement::leading(parameters, comment) } else { CommentPlacement::dangling(comment.enclosing_node(), comment) }; diff --git a/crates/ruff_python_formatter/src/expression/expr_lambda.rs b/crates/ruff_python_formatter/src/expression/expr_lambda.rs index 335f112323..ab2b054891 100644 --- a/crates/ruff_python_formatter/src/expression/expr_lambda.rs +++ b/crates/ruff_python_formatter/src/expression/expr_lambda.rs @@ -86,11 +86,7 @@ impl FormatNodeRule for FormatExprLambda { // *x: x // ) // ``` - if parameters - .iter() - .next() - .is_some_and(|parameter| comments.has_leading(parameter.as_parameter())) - { + if comments.has_leading(&**parameters) { hard_line_break().fmt(f)?; } else { write!(f, [space()])?; From f639389e6a1b01794ec9b0082d3ab449d75d1b97 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 9 Dec 2025 16:48:15 -0500 Subject: [PATCH 3/7] simplify check with parameters.start() == parameter.start() and fix some names the start check handles both the `are_parameters_parenthesized` check (because lambda parameters cannot be parenthesized and thus nothing can come between the start of the parameters and the first parameter) and the comparison with first.range() since the parameters start where the first parameter starts --- .../src/comments/placement.rs | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 3ad3df2653..b0c49974f3 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -857,15 +857,10 @@ fn handle_parameter_comment<'a>( parameter: &'a Parameter, source: &str, ) -> CommentPlacement<'a> { - let parent = if let Some(parent @ AnyNodeRef::Parameters(parameters)) = - comment.enclosing_parent() - && parameters - .iter() - .next() - .is_some_and(|first| first.range() == parameter.range()) - && !are_parameters_parenthesized(parameters, source) + let parameters = if let Some(AnyNodeRef::Parameters(parameters)) = comment.enclosing_parent() + && parameters.start() == parameter.start() { - Some(parent) + Some(parameters) } else { None }; @@ -880,17 +875,13 @@ fn handle_parameter_comment<'a>( if comment.start() < colon.start() { // The comment is before the colon, pull it out and make it a leading comment of the // parameter, or of the whole parameters list, if it's the first parameter. - if let Some(parent) = parent { - CommentPlacement::leading(parent, comment) - } else { - CommentPlacement::leading(parameter, comment) - } + CommentPlacement::leading(parameter, comment) } else { CommentPlacement::Default(comment) } } else if comment.start() < parameter.name.start() { - if let Some(parent) = parent { - CommentPlacement::leading(parent, comment) + if let Some(parameters) = parameters { + CommentPlacement::leading(parameters, comment) } else { CommentPlacement::leading(parameter, comment) } From 9d683da964b4864c6a7f6442e6f80790ed6ad894 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 9 Dec 2025 16:51:55 -0500 Subject: [PATCH 4/7] simplify a bit further --- .../src/comments/placement.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index b0c49974f3..af0a5d5f2e 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -857,14 +857,6 @@ fn handle_parameter_comment<'a>( parameter: &'a Parameter, source: &str, ) -> CommentPlacement<'a> { - let parameters = if let Some(AnyNodeRef::Parameters(parameters)) = comment.enclosing_parent() - && parameters.start() == parameter.start() - { - Some(parameters) - } else { - None - }; - if parameter.annotation().is_some() { let colon = first_non_trivia_token(parameter.name.end(), source).expect( "A annotated parameter should have a colon following its name when it is valid syntax.", @@ -880,7 +872,13 @@ fn handle_parameter_comment<'a>( CommentPlacement::Default(comment) } } else if comment.start() < parameter.name.start() { - if let Some(parameters) = parameters { + // For lambdas, where the parameters cannot be parenthesized and the first parameter thus + // starts at the same position as the parent parameters, mark a comment before the first + // parameter as leading on the parameters rather than the individual parameter to prevent + // the whole parameter list from breaking. + if let Some(AnyNodeRef::Parameters(parameters)) = comment.enclosing_parent() + && parameters.start() == parameter.start() + { CommentPlacement::leading(parameters, comment) } else { CommentPlacement::leading(parameter, comment) From 2da4798def0b62820c6641ba763dd6dc87391cf0 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 9 Dec 2025 16:53:44 -0500 Subject: [PATCH 5/7] update comments --- crates/ruff_python_formatter/src/comments/placement.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index af0a5d5f2e..3348154bfb 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -865,8 +865,7 @@ fn handle_parameter_comment<'a>( assert_eq!(colon.kind(), SimpleTokenKind::Colon); if comment.start() < colon.start() { - // The comment is before the colon, pull it out and make it a leading comment of the - // parameter, or of the whole parameters list, if it's the first parameter. + // The comment is before the colon, pull it out and make it a leading comment of the parameter. CommentPlacement::leading(parameter, comment) } else { CommentPlacement::Default(comment) @@ -876,6 +875,9 @@ fn handle_parameter_comment<'a>( // starts at the same position as the parent parameters, mark a comment before the first // parameter as leading on the parameters rather than the individual parameter to prevent // the whole parameter list from breaking. + // + // Note that this check is not needed above because lambda parameters cannot have + // annotations. if let Some(AnyNodeRef::Parameters(parameters)) = comment.enclosing_parent() && parameters.start() == parameter.start() { From acc49ac1e7325bc42559cb12e7eb220a14a9b721 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 9 Dec 2025 17:17:26 -0500 Subject: [PATCH 6/7] remove redundant parameters check I believe parameters itself would be None if there were no next parameter --- crates/ruff_python_formatter/src/comments/placement.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 3348154bfb..76449285be 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1848,7 +1848,7 @@ fn handle_lambda_comment<'a>( // ) // ``` if comment.start() < parameters.start() { - return if parameters.iter().next().is_some() && comment.line_position().is_own_line() { + return if comment.line_position().is_own_line() { CommentPlacement::leading(parameters, comment) } else { CommentPlacement::dangling(comment.enclosing_node(), comment) From 2e0ee2e0a99a7a1118e86d9610f78dc98284e8fb Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 9 Dec 2025 17:20:18 -0500 Subject: [PATCH 7/7] update lambda comment --- crates/ruff_python_formatter/src/expression/expr_lambda.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ruff_python_formatter/src/expression/expr_lambda.rs b/crates/ruff_python_formatter/src/expression/expr_lambda.rs index ab2b054891..f91666ecf7 100644 --- a/crates/ruff_python_formatter/src/expression/expr_lambda.rs +++ b/crates/ruff_python_formatter/src/expression/expr_lambda.rs @@ -32,8 +32,8 @@ impl FormatNodeRule for FormatExprLambda { .split_at(dangling.partition_point(|comment| comment.end() < parameters.start())); if dangling_before_parameters.is_empty() { - // If the first parameter has a leading comment, insert a hard line break. This - // comment is associated as a leading comment on the first parameter: + // If the parameters have a leading comment, insert a hard line break. This + // comment is associated as a leading comment on the parameters: // // ```py // (