Skip to content

Commit 5f225b1

Browse files
Generalize bracketed end-of-line comment handling (#6315)
Micha suggested this in #6274 (comment), and it allows us to unify the implementations for arguments and type params.
1 parent 8276b26 commit 5f225b1

File tree

1 file changed

+34
-88
lines changed

1 file changed

+34
-88
lines changed

crates/ruff_python_formatter/src/comments/placement.rs

Lines changed: 34 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ use std::cmp::Ordering;
33
use ruff_python_ast::node::AnyNodeRef;
44
use ruff_python_ast::whitespace::indentation;
55
use ruff_python_ast::{
6-
self as ast, Arguments, Comprehension, Expr, ExprAttribute, ExprBinOp, ExprIfExp, ExprSlice,
7-
ExprStarred, MatchCase, Parameters, Ranged, TypeParams,
6+
self as ast, Comprehension, Expr, ExprAttribute, ExprBinOp, ExprIfExp, ExprSlice, ExprStarred,
7+
MatchCase, Parameters, Ranged,
88
};
99
use ruff_python_trivia::{
1010
indentation_at_offset, PythonWhitespace, SimpleToken, SimpleTokenKind, SimpleTokenizer,
@@ -46,7 +46,9 @@ pub(super) fn place_comment<'a>(
4646
AnyNodeRef::Parameters(arguments) => {
4747
handle_parameters_separator_comment(comment, arguments, locator)
4848
}
49-
AnyNodeRef::Arguments(arguments) => handle_arguments_comment(comment, arguments),
49+
AnyNodeRef::Arguments(_) | AnyNodeRef::TypeParams(_) => {
50+
handle_bracketed_end_of_line_comment(comment, locator)
51+
}
5052
AnyNodeRef::Comprehension(comprehension) => {
5153
handle_comprehension_comment(comment, comprehension, locator)
5254
}
@@ -85,7 +87,6 @@ pub(super) fn place_comment<'a>(
8587
handle_leading_class_with_decorators_comment(comment, class_def)
8688
}
8789
AnyNodeRef::StmtImportFrom(import_from) => handle_import_from_comment(comment, import_from),
88-
AnyNodeRef::TypeParams(type_params) => handle_type_params_comment(comment, type_params),
8990
_ => CommentPlacement::Default(comment),
9091
}
9192
}
@@ -609,51 +610,6 @@ fn handle_own_line_comment_after_branch<'a>(
609610
}
610611
}
611612

612-
/// Attach an enclosed end-of-line comment to a set of [`TypeParams`].
613-
///
614-
/// For example, given:
615-
/// ```python
616-
/// type foo[ # comment
617-
/// bar,
618-
/// ] = ...
619-
/// ```
620-
///
621-
/// The comment will be attached to the [`TypeParams`] node as a dangling comment, to ensure
622-
/// that it remains on the same line as open bracket.
623-
fn handle_type_params_comment<'a>(
624-
comment: DecoratedComment<'a>,
625-
type_params: &'a TypeParams,
626-
) -> CommentPlacement<'a> {
627-
// The comment needs to be on the same line, but before the first type param. For example, we want
628-
// to treat this as a dangling comment:
629-
// ```python
630-
// type foo[ # comment
631-
// bar,
632-
// baz,
633-
// qux,
634-
// ]
635-
// ```
636-
// However, this should _not_ be treated as a dangling comment:
637-
// ```python
638-
// type foo[bar, # comment
639-
// baz,
640-
// qux,
641-
// ] = ...
642-
// ```
643-
// Thus, we check whether the comment is an end-of-line comment _between_ the start of the
644-
// statement and the first type param. If so, the only possible position is immediately following
645-
// the open parenthesis.
646-
if comment.line_position().is_end_of_line() {
647-
if let Some(first_type_param) = type_params.type_params.first() {
648-
if type_params.start() < comment.start() && comment.end() < first_type_param.start() {
649-
return CommentPlacement::dangling(comment.enclosing_node(), comment);
650-
}
651-
}
652-
}
653-
654-
CommentPlacement::Default(comment)
655-
}
656-
657613
/// Attaches comments for the positional-only parameters separator `/` or the keywords-only
658614
/// parameters separator `*` as dangling comments to the enclosing [`Parameters`] node.
659615
///
@@ -1226,9 +1182,10 @@ fn find_only_token_in_range(
12261182
token
12271183
}
12281184

1229-
/// Attach an enclosed end-of-line comment to a set of [`Arguments`].
1185+
/// Attach an end-of-line comment immediately following an open bracket as a dangling comment on
1186+
/// enclosing node.
12301187
///
1231-
/// For example, given:
1188+
/// For example, given the following function call:
12321189
/// ```python
12331190
/// foo( # comment
12341191
/// bar,
@@ -1237,47 +1194,36 @@ fn find_only_token_in_range(
12371194
///
12381195
/// The comment will be attached to the [`Arguments`] node as a dangling comment, to ensure
12391196
/// that it remains on the same line as open parenthesis.
1240-
fn handle_arguments_comment<'a>(
1197+
///
1198+
/// Similarly, given:
1199+
/// ```python
1200+
/// type foo[ # comment
1201+
/// bar,
1202+
/// ] = ...
1203+
/// ```
1204+
///
1205+
/// The comment will be attached to the [`TypeParams`] node as a dangling comment, to ensure
1206+
/// that it remains on the same line as open bracket.
1207+
fn handle_bracketed_end_of_line_comment<'a>(
12411208
comment: DecoratedComment<'a>,
1242-
arguments: &'a Arguments,
1209+
locator: &Locator,
12431210
) -> CommentPlacement<'a> {
1244-
// The comment needs to be on the same line, but before the first argument. For example, we want
1245-
// to treat this as a dangling comment:
1246-
// ```python
1247-
// foo( # comment
1248-
// bar,
1249-
// baz,
1250-
// qux,
1251-
// )
1252-
// ```
1253-
// However, this should _not_ be treated as a dangling comment:
1254-
// ```python
1255-
// foo(bar, # comment
1256-
// baz,
1257-
// qux,
1258-
// )
1259-
// ```
1260-
// Thus, we check whether the comment is an end-of-line comment _between_ the start of the
1261-
// statement and the first argument. If so, the only possible position is immediately following
1262-
// the open parenthesis.
12631211
if comment.line_position().is_end_of_line() {
1264-
let first_argument = match (arguments.args.as_slice(), arguments.keywords.as_slice()) {
1265-
([first_arg, ..], [first_keyword, ..]) => {
1266-
if first_arg.start() < first_keyword.start() {
1267-
Some(first_arg.range())
1268-
} else {
1269-
Some(first_keyword.range())
1270-
}
1271-
}
1272-
([first_arg, ..], []) => Some(first_arg.range()),
1273-
([], [first_keyword, ..]) => Some(first_keyword.range()),
1274-
([], []) => None,
1275-
};
1212+
// Ensure that there are no tokens between the open bracket and the comment.
1213+
let mut lexer = SimpleTokenizer::new(
1214+
locator.contents(),
1215+
TextRange::new(comment.enclosing_node().start(), comment.start()),
1216+
)
1217+
.skip_trivia()
1218+
.skip_while(|t| {
1219+
matches!(
1220+
t.kind(),
1221+
SimpleTokenKind::LParen | SimpleTokenKind::LBrace | SimpleTokenKind::LBracket
1222+
)
1223+
});
12761224

1277-
if let Some(first_argument) = first_argument {
1278-
if arguments.start() < comment.start() && comment.end() < first_argument.start() {
1279-
return CommentPlacement::dangling(comment.enclosing_node(), comment);
1280-
}
1225+
if lexer.next().is_none() {
1226+
return CommentPlacement::dangling(comment.enclosing_node(), comment);
12811227
}
12821228
}
12831229

0 commit comments

Comments
 (0)