Formatter: Comprehension leading if comments

[MichaReiser]

This is not a bug per se but something I found rather confusing. The comprehension comment placement associates leading and trailing comments on the ifs as dangling comments of the condition node:

// Handle comments inside comprehensions, e.g.
//
// ```python
// [
//      a
//      b
//      c
//      # leading becomes dangling on if
//      if  # trailing becomes dangling on if
//      d
// ]
// ```

ruff/crates/ruff_python_formatter/src/comments/placement.rs

Lines 1244 to 1369 in 96d310f

	// Handle comments inside comprehensions, e.g.
	//
	// ```python
	// [
	// a
	// for # dangling on the comprehension
	// b
	// # dangling on the comprehension
	// in # dangling on comprehension.iter
	// # leading on the iter
	// c
	// # dangling on comprehension.if.n
	// if # dangling on comprehension.if.n
	// d
	// ]
	// ```
	fn handle_comprehension_comment<'a>(
	comment: DecoratedComment<'a>,
	comprehension: &'a Comprehension,
	locator: &Locator,
	) -> CommentPlacement<'a> {
	let is_own_line = comment.line_position().is_own_line();
	// Comments between the `for` and target
	// ```python
	// [
	// a
	// for # attach as dangling on the comprehension
	// b in c
	// ]
	// ```
	if comment.slice().end() < comprehension.target.range().start() {
	return if is_own_line {
	// own line comments are correctly assigned as leading the target
	CommentPlacement::Default(comment)
	} else {
	// after the `for`
	CommentPlacement::dangling(comment.enclosing_node(), comment)
	};
	}
	let in_token = find_only_token_in_range(
	TextRange::new(
	comprehension.target.range().end(),
	comprehension.iter.range().start(),
	),
	SimpleTokenKind::In,
	locator,
	);
	// Comments between the target and the `in`
	// ```python
	// [
	// a for b
	// # attach as dangling on the target
	// # (to be rendered as leading on the "in")
	// in c
	// ]
	// ```
	if comment.slice().start() < in_token.start() {
	// attach as dangling comments on the target
	// (to be rendered as leading on the "in")
	return if is_own_line {
	CommentPlacement::dangling(comment.enclosing_node(), comment)
	} else {
	// correctly trailing on the target
	CommentPlacement::Default(comment)
	};
	}
	// Comments between the `in` and the iter
	// ```python
	// [
	// a for b
	// in # attach as dangling on the iter
	// c
	// ]
	// ```
	if comment.slice().start() < comprehension.iter.range().start() {
	return if is_own_line {
	CommentPlacement::Default(comment)
	} else {
	// after the `in` but same line, turn into trailing on the `in` token
	CommentPlacement::dangling(&comprehension.iter, comment)
	};
	}
	let mut last_end = comprehension.iter.range().end();
	for if_node in &comprehension.ifs {
	// ```python
	// [
	// a
	// for
	// c
	// in
	// e
	// # above if <-- find these own-line between previous and `if` token
	// if # if <-- find these end-of-line between `if` and if node (`f`)
	// # above f <-- already correctly assigned as leading `f`
	// f # f <-- already correctly assigned as trailing `f`
	// # above if2
	// if # if2
	// # above g
	// g # g
	// ]
	// ```
	let if_token = find_only_token_in_range(
	TextRange::new(last_end, if_node.range().start()),
	SimpleTokenKind::If,
	locator,
	);
	if is_own_line {
	if last_end < comment.slice().start() && comment.slice().start() < if_token.start() {
	return CommentPlacement::dangling(if_node, comment);
	}
	} else if if_token.start() < comment.slice().start()
	&& comment.slice().start() < if_node.range().start()
	{
	return CommentPlacement::dangling(if_node, comment);
	}
	last_end = if_node.range().end();
	}
	CommentPlacement::Default(comment)
	}

This is confusing because nodes that otherwise cannot have dangling comments now suddenly have dangling comments. I ran into this because I added assertions to fmt_dangling_comments in ExprName and ExprCompare to verify that the dangling comments are empty (because that's what the comment says) and got surprised by the assertions failing.

The more common approach is to make the comments dangling comments on the comprehension and then find them again during formatting. This is probably a bit more work. Let's see how it turns out.

[charliermarsh]

I can take a look at this.

[charliermarsh]

Gonna prioritize some other things over this but agree we should do it.