Attach dangling comments to the comprehension instead of the if or iter nodes (#7693)

This commit is contained in:
Micha Reiser 2023-09-29 11:45:01 +02:00 committed by GitHub
parent e62e245c61
commit e2ec42539b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 124 additions and 22 deletions

View file

@ -2083,7 +2083,7 @@ fn handle_comprehension_comment<'a>(
CommentPlacement::Default(comment)
} else {
// after the `for`
CommentPlacement::dangling(comment.enclosing_node(), comment)
CommentPlacement::dangling(comprehension, comment)
};
}
@ -2106,7 +2106,7 @@ fn handle_comprehension_comment<'a>(
// 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)
CommentPlacement::dangling(comprehension, comment)
} else {
// correctly trailing on the target
CommentPlacement::Default(comment)
@ -2126,7 +2126,7 @@ fn handle_comprehension_comment<'a>(
CommentPlacement::Default(comment)
} else {
// after the `in` but same line, turn into trailing on the `in` token
CommentPlacement::dangling(&comprehension.iter, comment)
CommentPlacement::dangling(comprehension, comment)
};
}
@ -2157,10 +2157,10 @@ fn handle_comprehension_comment<'a>(
);
if is_own_line {
if last_end < comment.start() && comment.start() < if_token.start() {
return CommentPlacement::dangling(if_node, comment);
return CommentPlacement::dangling(comprehension, comment);
}
} else if if_token.start() < comment.start() && comment.start() < if_node.start() {
return CommentPlacement::dangling(if_node, comment);
return CommentPlacement::dangling(comprehension, comment);
}
last_end = if_node.end();
}

View file

@ -20,10 +20,11 @@ impl FormatNodeRule<ExprCompare> for FormatExprCompare {
fn fmt_dangling_comments(
&self,
_dangling_comments: &[SourceComment],
dangling_comments: &[SourceComment],
_f: &mut PyFormatter,
) -> FormatResult<()> {
// Node can not have dangling comments
debug_assert!(dangling_comments.is_empty());
Ok(())
}
}

View file

@ -3,7 +3,7 @@ use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::ExprDictComp;
use ruff_text_size::Ranged;
use crate::comments::dangling_comments;
use crate::comments::{dangling_comments, SourceComment};
use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses};
use crate::prelude::*;
@ -58,6 +58,15 @@ impl FormatNodeRule<ExprDictComp> for FormatExprDictComp {
.with_dangling_comments(open_parenthesis_comments)]
)
}
fn fmt_dangling_comments(
&self,
_dangling_node_comments: &[SourceComment],
_f: &mut PyFormatter,
) -> FormatResult<()> {
// Handled as part of `fmt_fields`
Ok(())
}
}
impl NeedsParentheses for ExprDictComp {

View file

@ -1,5 +1,6 @@
use memchr::memchr2;
use crate::comments::SourceComment;
use ruff_formatter::FormatResult;
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::ExprFString;
@ -16,6 +17,15 @@ impl FormatNodeRule<ExprFString> for FormatExprFString {
fn fmt_fields(&self, item: &ExprFString, f: &mut PyFormatter) -> FormatResult<()> {
FormatString::new(&AnyString::FString(item)).fmt(f)
}
fn fmt_dangling_comments(
&self,
_dangling_node_comments: &[SourceComment],
_f: &mut PyFormatter,
) -> FormatResult<()> {
// Handled as part of `fmt_fields`
Ok(())
}
}
impl NeedsParentheses for ExprFString {

View file

@ -26,10 +26,11 @@ impl FormatNodeRule<ExprName> for FormatExprName {
fn fmt_dangling_comments(
&self,
_dangling_comments: &[SourceComment],
dangling_comments: &[SourceComment],
_f: &mut PyFormatter,
) -> FormatResult<()> {
// Node cannot have dangling comments
debug_assert!(dangling_comments.is_empty());
Ok(())
}
}

View file

@ -146,6 +146,15 @@ impl FormatNodeRule<ExprSlice> for FormatExprSlice {
}
Ok(())
}
fn fmt_dangling_comments(
&self,
_dangling_node_comments: &[SourceComment],
_f: &mut PyFormatter,
) -> FormatResult<()> {
// Handled as part of `fmt_fields`
Ok(())
}
}
/// We're in a slice, so we know there's a first colon, but with have to look into the source

View file

@ -85,6 +85,11 @@ where
dangling_node_comments: &[SourceComment],
f: &mut PyFormatter,
) -> FormatResult<()> {
debug_assert!(
dangling_node_comments.is_empty(),
"The node has dangling comments that need to be formatted manually. Add the special dangling comments handling to `fmt_fields` and override `fmt_dangling_comments` with an empty implementation that returns `Ok(())`."
);
dangling_comments(dangling_node_comments).fmt(f)
}

View file

@ -37,10 +37,11 @@ impl FormatNodeRule<ModModule> for FormatModModule {
fn fmt_dangling_comments(
&self,
_dangling_comments: &[SourceComment],
dangling_comments: &[SourceComment],
_f: &mut PyFormatter,
) -> FormatResult<()> {
// Handled as part of `fmt_fields`
// Node can't have dangling comments.
debug_assert!(dangling_comments.is_empty());
Ok(())
}
}

View file

@ -1,6 +1,7 @@
use ruff_formatter::{format_args, write, Buffer, FormatResult};
use ruff_formatter::{format_args, write, Buffer, FormatError, FormatResult};
use ruff_python_ast::{Comprehension, Expr};
use ruff_text_size::Ranged;
use ruff_python_trivia::{SimpleToken, SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::{Ranged, TextRange};
use crate::comments::{leading_comments, trailing_comments, SourceComment};
use crate::expression::expr_tuple::TupleParentheses;
@ -37,11 +38,35 @@ impl FormatNodeRule<Comprehension> for FormatComprehension {
let comments = f.context().comments().clone();
let dangling_item_comments = comments.dangling(item);
let (before_target_comments, before_in_comments) = dangling_item_comments.split_at(
let (before_target_comments, dangling_comments) = dangling_item_comments.split_at(
dangling_item_comments.partition_point(|comment| comment.end() < target.start()),
);
let trailing_in_comments = comments.dangling(iter);
let maybe_in_token = SimpleTokenizer::new(
f.context().source(),
TextRange::new(target.end(), iter.start()),
)
.skip_trivia()
.next();
let Some(
in_keyword @ SimpleToken {
kind: SimpleTokenKind::In,
..
},
) = maybe_in_token
else {
return Err(FormatError::syntax_error(
"Expected `in` keyword between the `target` and `iter`.",
));
};
let (before_in_comments, dangling_comments) = dangling_comments.split_at(
dangling_comments.partition_point(|comment| comment.end() < in_keyword.start()),
);
let (trailing_in_comments, dangling_if_comments) = dangling_comments
.split_at(dangling_comments.partition_point(|comment| comment.start() < iter.start()));
let in_spacer = format_with(|f| {
if before_in_comments.is_empty() {
@ -66,17 +91,23 @@ impl FormatNodeRule<Comprehension> for FormatComprehension {
iter.format(),
]
)?;
if !ifs.is_empty() {
let joined = format_with(|f| {
let mut joiner = f.join_with(soft_line_break_or_space());
for if_case in ifs {
let dangling_if_comments = comments.dangling(if_case);
let mut dangling_if_comments = dangling_if_comments;
for if_case in ifs {
let (if_comments, rest) = dangling_if_comments.split_at(
dangling_if_comments
.partition_point(|comment| comment.start() < if_case.start()),
);
let (own_line_if_comments, end_of_line_if_comments) = if_comments.split_at(
if_comments
.partition_point(|comment| comment.line_position().is_own_line()),
);
let (own_line_if_comments, end_of_line_if_comments) = dangling_if_comments
.split_at(
dangling_if_comments
.partition_point(|comment| comment.line_position().is_own_line()),
);
joiner.entry(&format_args!(
leading_comments(own_line_if_comments),
token("if"),
@ -84,7 +115,12 @@ impl FormatNodeRule<Comprehension> for FormatComprehension {
Spacer(if_case),
if_case.format(),
));
dangling_if_comments = rest;
}
debug_assert!(dangling_if_comments.is_empty());
joiner.finish()
});

View file

@ -1,3 +1,4 @@
use crate::comments::SourceComment;
use ruff_formatter::{format_args, Format, FormatResult};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::PatternMatchSequence;
@ -55,6 +56,15 @@ impl FormatNodeRule<PatternMatchSequence> for FormatPatternMatchSequence {
SequenceType::TupleNoParens => optional_parentheses(&items).fmt(f),
}
}
fn fmt_dangling_comments(
&self,
_dangling_node_comments: &[SourceComment],
_f: &mut PyFormatter,
) -> FormatResult<()> {
// Handled as part of `fmt_fields`
Ok(())
}
}
impl NeedsParentheses for PatternMatchSequence {