format new dangling comments

This commit is contained in:
Brent Westbrook 2025-12-08 12:54:58 -05:00
parent 8ede14a083
commit e8540d9b08
No known key found for this signature in database
3 changed files with 174 additions and 71 deletions

View File

@ -1,7 +1,8 @@
use ast::helpers::comment_indentation_after; use ast::helpers::comment_indentation_after;
use ruff_python_ast::whitespace::indentation; use ruff_python_ast::whitespace::indentation;
use ruff_python_ast::{ 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::{ use ruff_python_trivia::{
BackwardsTokenizer, CommentRanges, SimpleToken, SimpleTokenKind, SimpleTokenizer, BackwardsTokenizer, CommentRanges, SimpleToken, SimpleTokenKind, SimpleTokenizer,
@ -205,6 +206,9 @@ fn handle_enclosed_comment<'a>(
}) })
} }
AnyNodeRef::Parameter(parameter) => handle_parameter_comment(comment, parameter, source), 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(_) => { AnyNodeRef::Arguments(_) | AnyNodeRef::TypeParams(_) | AnyNodeRef::PatternArguments(_) => {
handle_bracketed_end_of_line_comment(comment, source) handle_bracketed_end_of_line_comment(comment, source)
} }
@ -864,43 +868,63 @@ fn handle_parameter_comment<'a>(
assert_eq!(colon.kind(), SimpleTokenKind::Colon); 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. // The comment is before the colon, pull it out and make it a leading comment of the parameter.
CommentPlacement::leading(parameter, comment) CommentPlacement::leading(parameter, comment)
} else { } else {
CommentPlacement::Default(comment) 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. if comment.start() < parameter.name.start() {
// CommentPlacement::leading(parameter, comment)
// 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),
}
} else { } else {
CommentPlacement::Default(comment) 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), /// 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. /// and trailing end-of-line comments that are on the same line as the operator.
/// ///

View File

@ -35,8 +35,49 @@ impl FormatNodeRule<ExprLambda> for FormatExprLambda {
// In this context, a dangling comment can either be a comment between the `lambda` and the // 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. // parameters, or a comment between the parameters and the body.
let (dangling_before_parameters, dangling_after_parameters) = dangling let (dangling_before_parameters, dangling_after_parameters) =
.split_at(dangling.partition_point(|comment| comment.end() < parameters.start())); // 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() { if dangling_before_parameters.is_empty() {
write!(f, [space()])?; write!(f, [space()])?;

View File

@ -1513,7 +1513,7 @@ transform = (
): ):
pass pass
@@ -102,22 +83,25 @@ @@ -102,58 +83,63 @@
# Dangling comments without parameters. # Dangling comments without parameters.
( (
@ -1549,15 +1549,34 @@ transform = (
) )
( (
@@ -135,17 +119,18 @@ - lambda
- # comment
- *x: x
+ lambda *x: (
+ # comment
+ x
+ )
)
( (
lambda - lambda
# comment 1 - # comment
- *x, **y: x
+ lambda *x, **y: (
+ # comment
+ x
+ )
)
(
- lambda
- # comment 1
- *x: - *x:
- # comment 2 - # comment 2
- # comment 3 - # comment 3
- x - x
+ *x: ( + lambda *x: (
+ # comment 1
+ # comment 2 + # comment 2
+ # comment 3 + # comment 3
+ x + x
@ -1565,17 +1584,29 @@ transform = (
) )
( (
lambda # comment 1 - lambda # comment 1
- *x: # comment 2 - *x: # comment 2
- # comment 3 - # comment 3
- x - x
+ *x: ( # comment 2 # comment 3 + lambda *x: ( # comment 1 # comment 2 # comment 3
+ x + x
+ ) + )
) )
lambda *x: 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 x
) )
) )
@@ -192,11 +181,12 @@
( (
lambda # 1 - lambda # 1
# 2 - # 2
- x: # 3 - x: # 3
- # 4 - # 4
- # 5 - # 5
- # 6 - # 6
- x - x
+ x: ( # 3 + lambda x: ( # 1
+ # 2
+ # 3
+ # 4 + # 4
+ # 5 + # 5
+ # 6 + # 6
@ -1640,7 +1673,7 @@ transform = (
) )
( (
@@ -204,9 +194,10 @@ @@ -204,9 +195,10 @@
# 2 # 2
x, # 3 x, # 3
# 4 # 4
@ -1654,7 +1687,7 @@ transform = (
) )
( (
@@ -218,71 +209,79 @@ @@ -218,71 +210,79 @@
# Leading # Leading
lambda x: ( lambda x: (
@ -1793,7 +1826,7 @@ transform = (
# Regression tests for https://github.com/astral-sh/ruff/issues/8179 # Regression tests for https://github.com/astral-sh/ruff/issues/8179
@@ -291,9 +290,9 @@ @@ -291,9 +291,9 @@
c, c,
d, d,
e, e,
@ -1806,7 +1839,7 @@ transform = (
) )
@@ -302,15 +301,9 @@ @@ -302,15 +302,9 @@
c, c,
d, d,
e, e,
@ -1825,7 +1858,7 @@ transform = (
g=10, g=10,
) )
@@ -320,9 +313,9 @@ @@ -320,9 +314,9 @@
c, c,
d, d,
e, e,
@ -1838,7 +1871,7 @@ transform = (
) )
@@ -338,9 +331,9 @@ @@ -338,9 +332,9 @@
class C: class C:
function_dict: Dict[Text, Callable[[CRFToken], Any]] = { function_dict: Dict[Text, Callable[[CRFToken], Any]] = {
@ -1851,7 +1884,7 @@ transform = (
} }
@@ -352,42 +345,40 @@ @@ -352,42 +346,40 @@
def foo(): def foo():
if True: if True:
if True: if True:
@ -1910,7 +1943,7 @@ transform = (
CREATE TABLE {table} AS CREATE TABLE {table} AS
SELECT ROW_NUMBER() OVER () AS id, {var} SELECT ROW_NUMBER() OVER () AS id, {var}
FROM ( FROM (
@@ -402,18 +393,19 @@ @@ -402,18 +394,19 @@
long_assignment_target.with_attribute.and_a_slice[with_an_index] = ( long_assignment_target.with_attribute.and_a_slice[with_an_index] = (
# 1 # 1
# 2 # 2
@ -1937,7 +1970,7 @@ transform = (
) )
very_long_variable_name_x, very_long_variable_name_y = ( 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, lambda b: b * another_very_long_expression_here,
) )
@ -1948,7 +1981,7 @@ transform = (
x, more_args, additional_parameters x, more_args, additional_parameters
) )
) )
@@ -458,12 +450,12 @@ @@ -458,12 +451,12 @@
[ [
# Not fluent # Not fluent
param( param(
@ -1963,7 +1996,7 @@ transform = (
), ),
param( param(
lambda left, right: ( lambda left, right: (
@@ -472,9 +464,9 @@ @@ -472,9 +465,9 @@
), ),
param(lambda left, right: ibis.timestamp("2017-04-01").cast(dt.date)), param(lambda left, right: ibis.timestamp("2017-04-01").cast(dt.date)),
param( param(
@ -1976,7 +2009,7 @@ transform = (
), ),
# This is too long on one line in the lambda body and gets wrapped # This is too long on one line in the lambda body and gets wrapped
# inside the body. # inside the body.
@@ -508,16 +500,18 @@ @@ -508,16 +501,18 @@
] ]
# adds parentheses around the body # adds parentheses around the body
@ -1998,7 +2031,7 @@ transform = (
lambda x, y, z: ( lambda x, y, z: (
x + y + z x + y + z
@@ -528,7 +522,7 @@ @@ -528,7 +523,7 @@
x + y + z # trailing eol body x + y + z # trailing eol body
) )
@ -2007,7 +2040,7 @@ transform = (
lambda x, y, z: ( lambda x, y, z: (
# leading body # leading body
@@ -540,21 +534,23 @@ @@ -540,21 +535,23 @@
) )
( (
@ -2041,7 +2074,7 @@ transform = (
# dangling header comment # dangling header comment
source_bucket source_bucket
if name == source_bucket_name if name == source_bucket_name
@@ -562,8 +558,7 @@ @@ -562,8 +559,7 @@
) )
( (
@ -2051,7 +2084,7 @@ transform = (
source_bucket source_bucket
if name == source_bucket_name if name == source_bucket_name
else storage.Bucket(mock_service, destination_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 - lambda
# comment - # comment
- *args, **kwargs: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) - *args, **kwargs: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs)
- + 1 - + 1
+ *args, **kwargs: ( + lambda *args, **kwargs: (
+ # comment
+ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1 + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1
+ ) + )
) )
( (
lambda # comment - lambda # comment
- *args, **kwargs: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) - *args, **kwargs: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs)
- + 1 - + 1
+ *args, **kwargs: ( + lambda *args, **kwargs: ( # comment
+ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1 + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1
+ ) + )
) )
( (
lambda # comment 1 - lambda # comment 1
# comment 2 - # comment 2
- *args, **kwargs: # comment 3 - *args, **kwargs: # comment 3
- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1 - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1
+ *args, **kwargs: ( # comment 3 + lambda *args, **kwargs: ( # comment 1
+ # comment 2
+ # comment 3
+ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1 + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1
+ ) + )
) )
( (
lambda # comment 1 - lambda # comment 1
- *args, **kwargs: # comment 3 - *args, **kwargs: # comment 3
- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1 - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1
+ *args, **kwargs: ( # comment 3 + lambda *args, **kwargs: ( # comment 1 # comment 3
+ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1 + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(*args, **kwargs) + 1
+ ) + )
) )
( (
@@ -666,19 +674,20 @@ @@ -666,19 +672,20 @@
# 2 # 2
left, # 3 left, # 3
# 4 # 4
@ -2225,7 +2263,7 @@ transform = (
) )
) )
) )
@@ -696,46 +705,50 @@ @@ -696,46 +703,50 @@
foo( foo(
lambda from_ts, # but still wrap the body if it gets too long lambda from_ts, # but still wrap the body if it gets too long
to_ts, to_ts,