diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index a730b52bbc..9af7b31bcf 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1,7 +1,8 @@ use ast::helpers::comment_indentation_after; use ruff_python_ast::whitespace::indentation; use ruff_python_ast::{ - self as ast, AnyNodeRef, Comprehension, Expr, ModModule, Parameter, Parameters, StringLike, + self as ast, AnyNodeRef, Comprehension, Expr, ModModule, Parameter, ParameterWithDefault, + Parameters, StringLike, }; use ruff_python_trivia::{ BackwardsTokenizer, CommentRanges, SimpleToken, SimpleTokenKind, SimpleTokenizer, @@ -205,6 +206,9 @@ fn handle_enclosed_comment<'a>( }) } AnyNodeRef::Parameter(parameter) => handle_parameter_comment(comment, parameter, source), + AnyNodeRef::ParameterWithDefault(parameter) => { + handle_parameter_with_default_comment(comment, parameter, source) + } AnyNodeRef::Arguments(_) | AnyNodeRef::TypeParams(_) | AnyNodeRef::PatternArguments(_) => { handle_bracketed_end_of_line_comment(comment, source) } @@ -864,43 +868,63 @@ fn handle_parameter_comment<'a>( assert_eq!(colon.kind(), SimpleTokenKind::Colon); - if comment.start() < colon.start() { + return 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) } else { CommentPlacement::Default(comment) + }; + } + + // If the comment falls before the parameter, and the grandparent node is a lambda (the + // parent node is always `Parameters`), make the comment a dangling lambda parameter, so + // that it can be formatted with the other dangling lambda comments in the lambda header. + // + // This is especially important for niche cases like: + // + // ```py + // ( + // lambda # comment 1 + // * # comment 2 + // x: # comment 3 + // x + // ) + // ``` + // + // where `# comment 2` otherwise becomes a leading comment on `*x` in the first format and a + // dangling comment on the lambda in the second. Instead, make it a dangling comment on the + // enclosing lambda from the start. + match comment.enclosing_grandparent() { + Some(AnyNodeRef::ExprLambda(lambda)) => return CommentPlacement::dangling(lambda, comment), + Some(AnyNodeRef::StmtExpr(ast::StmtExpr { value, .. })) if value.is_lambda_expr() => { + return CommentPlacement::dangling(&**value, comment); } - } else if comment.start() < parameter.name.start() { - // If the comment falls before the parameter, and the grandparent node is a lambda (the - // parent node is always `Parameters`), make the comment a dangling lambda parameter, so - // that it can be formatted with the other dangling lambda comments in the lambda header. - // - // This is especially important for niche cases like: - // - // ```py - // ( - // lambda # comment 1 - // * # comment 2 - // x: # comment 3 - // x - // ) - // ``` - // - // where `# comment 2` otherwise becomes a leading comment on `*x` in the first format and a - // dangling comment on the lambda in the second. Instead, make it a dangling comment on the - // enclosing lambda from the start. - match comment.enclosing_grandparent() { - Some(AnyNodeRef::ExprLambda(lambda)) => CommentPlacement::dangling(lambda, comment), - Some(AnyNodeRef::StmtExpr(ast::StmtExpr { value, .. })) if value.is_lambda_expr() => { - CommentPlacement::dangling(&**value, comment) - } - _ => CommentPlacement::leading(parameter, comment), - } + _ => {} + } + + if comment.start() < parameter.name.start() { + CommentPlacement::leading(parameter, comment) } else { CommentPlacement::Default(comment) } } +/// Handles parameters with defaults inside of lambda expressions by making them dangling comments +/// on the lambda itself. +fn handle_parameter_with_default_comment<'a>( + comment: DecoratedComment<'a>, + _parameter: &'a ParameterWithDefault, + _source: &str, +) -> CommentPlacement<'a> { + match comment.enclosing_grandparent() { + Some(AnyNodeRef::ExprLambda(lambda)) => CommentPlacement::dangling(lambda, comment), + Some(AnyNodeRef::StmtExpr(ast::StmtExpr { value, .. })) if value.is_lambda_expr() => { + CommentPlacement::dangling(&**value, comment) + } + _ => CommentPlacement::Default(comment), + } +} + /// Handles comments between the left side and the operator of a binary expression (trailing comments of the left), /// and trailing end-of-line comments that are on the same line as the operator. /// diff --git a/crates/ruff_python_formatter/src/expression/expr_lambda.rs b/crates/ruff_python_formatter/src/expression/expr_lambda.rs index de363516b6..18ab36997b 100644 --- a/crates/ruff_python_formatter/src/expression/expr_lambda.rs +++ b/crates/ruff_python_formatter/src/expression/expr_lambda.rs @@ -35,8 +35,49 @@ impl FormatNodeRule for FormatExprLambda { // In this context, a dangling comment can either be a comment between the `lambda` and the // parameters, or a comment between the parameters and the body. - let (dangling_before_parameters, dangling_after_parameters) = dangling - .split_at(dangling.partition_point(|comment| comment.end() < parameters.start())); + let (dangling_before_parameters, dangling_after_parameters) = + // To prevent an instability in cases like: + // + // ```py + // ( + // lambda # comment 1 + // * # comment 2 + // x: # comment 3 + // x + // ) + // ``` + // + // `# comment 1` and `# comment 2` also become dangling comments on the lambda, so + // in preview, we include these in `dangling_after_parameters`, as long as the + // parameter list doesn't include any additional comments. + // + // This ends up formatted as: + // + // ```py + // ( + // lambda *x: ( # comment 1 # comment 2 # comment 3 + // x + // ) + // ) + // ``` + // + // instead of the stable formatting: + // + // ```py + // ( + // lambda # comment 1 + // *x: # comment 2 + // # comment 3 + // x + // ) + // ``` + if preview && !parameters_have_comments { + ([].as_slice(), dangling) + } else { + dangling.split_at( + dangling.partition_point(|comment| comment.end() < parameters.start()), + ) + }; if dangling_before_parameters.is_empty() { write!(f, [space()])?; diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__lambda.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__lambda.py.snap index 90aa73cc51..2aee975e1c 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__lambda.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__lambda.py.snap @@ -1513,7 +1513,7 @@ transform = ( ): pass -@@ -102,22 +83,25 @@ +@@ -102,58 +83,63 @@ # Dangling comments without parameters. ( @@ -1549,15 +1549,34 @@ transform = ( ) ( -@@ -135,17 +119,18 @@ +- lambda +- # comment +- *x: x ++ lambda *x: ( ++ # comment ++ x ++ ) + ) + ( - lambda - # comment 1 +- lambda +- # comment +- *x, **y: x ++ lambda *x, **y: ( ++ # comment ++ x ++ ) + ) + + ( +- lambda +- # comment 1 - *x: - # comment 2 - # comment 3 - x -+ *x: ( ++ lambda *x: ( ++ # comment 1 + # comment 2 + # comment 3 + x @@ -1565,17 +1584,29 @@ transform = ( ) ( - lambda # comment 1 +- lambda # comment 1 - *x: # comment 2 - # comment 3 - x -+ *x: ( # comment 2 # comment 3 ++ lambda *x: ( # comment 1 # comment 2 # comment 3 + x + ) ) lambda *x: x -@@ -161,30 +146,34 @@ + + ( +- lambda +- # comment +- *x: x ++ lambda *x: ( ++ # comment ++ x ++ ) + ) + + lambda: ( # comment +@@ -161,42 +147,47 @@ ) ( @@ -1622,16 +1653,18 @@ transform = ( x ) ) -@@ -192,11 +181,12 @@ + ( - lambda # 1 - # 2 +- lambda # 1 +- # 2 - x: # 3 - # 4 - # 5 - # 6 - x -+ x: ( # 3 ++ lambda x: ( # 1 ++ # 2 ++ # 3 + # 4 + # 5 + # 6 @@ -1640,7 +1673,7 @@ transform = ( ) ( -@@ -204,9 +194,10 @@ +@@ -204,9 +195,10 @@ # 2 x, # 3 # 4 @@ -1654,7 +1687,7 @@ transform = ( ) ( -@@ -218,71 +209,79 @@ +@@ -218,71 +210,79 @@ # Leading lambda x: ( @@ -1793,7 +1826,7 @@ transform = ( # Regression tests for https://github.com/astral-sh/ruff/issues/8179 -@@ -291,9 +290,9 @@ +@@ -291,9 +291,9 @@ c, d, e, @@ -1806,7 +1839,7 @@ transform = ( ) -@@ -302,15 +301,9 @@ +@@ -302,15 +302,9 @@ c, d, e, @@ -1825,7 +1858,7 @@ transform = ( g=10, ) -@@ -320,9 +313,9 @@ +@@ -320,9 +314,9 @@ c, d, e, @@ -1838,7 +1871,7 @@ transform = ( ) -@@ -338,9 +331,9 @@ +@@ -338,9 +332,9 @@ class C: function_dict: Dict[Text, Callable[[CRFToken], Any]] = { @@ -1851,7 +1884,7 @@ transform = ( } -@@ -352,42 +345,40 @@ +@@ -352,42 +346,40 @@ def foo(): if True: if True: @@ -1910,7 +1943,7 @@ transform = ( CREATE TABLE {table} AS SELECT ROW_NUMBER() OVER () AS id, {var} FROM ( -@@ -402,18 +393,19 @@ +@@ -402,18 +394,19 @@ long_assignment_target.with_attribute.and_a_slice[with_an_index] = ( # 1 # 2 @@ -1937,7 +1970,7 @@ transform = ( ) very_long_variable_name_x, very_long_variable_name_y = ( -@@ -421,8 +413,8 @@ +@@ -421,8 +414,8 @@ lambda b: b * another_very_long_expression_here, ) @@ -1948,7 +1981,7 @@ transform = ( x, more_args, additional_parameters ) ) -@@ -458,12 +450,12 @@ +@@ -458,12 +451,12 @@ [ # Not fluent param( @@ -1963,7 +1996,7 @@ transform = ( ), param( lambda left, right: ( -@@ -472,9 +464,9 @@ +@@ -472,9 +465,9 @@ ), param(lambda left, right: ibis.timestamp("2017-04-01").cast(dt.date)), param( @@ -1976,7 +2009,7 @@ transform = ( ), # This is too long on one line in the lambda body and gets wrapped # inside the body. -@@ -508,16 +500,18 @@ +@@ -508,16 +501,18 @@ ] # adds parentheses around the body @@ -1998,7 +2031,7 @@ transform = ( lambda x, y, z: ( x + y + z -@@ -528,7 +522,7 @@ +@@ -528,7 +523,7 @@ x + y + z # trailing eol body ) @@ -2007,7 +2040,7 @@ transform = ( lambda x, y, z: ( # leading body -@@ -540,21 +534,23 @@ +@@ -540,21 +535,23 @@ ) ( @@ -2041,7 +2074,7 @@ transform = ( # dangling header comment source_bucket if name == source_bucket_name -@@ -562,8 +558,7 @@ +@@ -562,8 +559,7 @@ ) ( @@ -2051,7 +2084,7 @@ transform = ( source_bucket if name == source_bucket_name else storage.Bucket(mock_service, destination_bucket_name) -@@ -571,61 +566,70 @@ +@@ -571,61 +567,70 @@ ) ( @@ -2154,47 +2187,52 @@ transform = ( ) ( -@@ -638,27 +642,31 @@ +@@ -636,29 +641,30 @@ + ) + ( - lambda - # comment +- lambda +- # comment - *args, **kwargs: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) - + 1 -+ *args, **kwargs: ( ++ lambda *args, **kwargs: ( ++ # comment + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1 + ) ) ( - lambda # comment +- lambda # comment - *args, **kwargs: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) - + 1 -+ *args, **kwargs: ( ++ lambda *args, **kwargs: ( # comment + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1 + ) ) ( - lambda # comment 1 - # comment 2 +- lambda # comment 1 +- # comment 2 - *args, **kwargs: # comment 3 - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1 -+ *args, **kwargs: ( # comment 3 ++ lambda *args, **kwargs: ( # comment 1 ++ # comment 2 ++ # comment 3 + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1 + ) ) ( - lambda # comment 1 +- lambda # comment 1 - *args, **kwargs: # comment 3 - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1 -+ *args, **kwargs: ( # comment 3 ++ lambda *args, **kwargs: ( # comment 1 # comment 3 + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1 + ) ) ( -@@ -666,19 +674,20 @@ +@@ -666,19 +672,20 @@ # 2 left, # 3 # 4 @@ -2225,7 +2263,7 @@ transform = ( ) ) ) -@@ -696,46 +705,50 @@ +@@ -696,46 +703,50 @@ foo( lambda from_ts, # but still wrap the body if it gets too long to_ts,