Improve debuggability of place_comment (#5209)

## Summary

I found it hard to figure out which function decides placement for a
specific comment. An explicit loop makes this easier to debug

## Test Plan

There should be no functional changes, no changes to the formatting of
the fixtures.
This commit is contained in:
konstin 2023-06-21 11:52:13 +02:00 committed by GitHub
parent f551c9aad2
commit 44156f6962
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 30 additions and 31 deletions

View file

@ -14,25 +14,30 @@ use crate::trivia::{SimpleTokenizer, Token, TokenKind};
/// Implements the custom comment placement logic.
pub(super) fn place_comment<'a>(
comment: DecoratedComment<'a>,
mut comment: DecoratedComment<'a>,
locator: &Locator,
) -> CommentPlacement<'a> {
handle_in_between_except_handlers_or_except_handler_and_else_or_finally_comment(
comment, locator,
)
.or_else(|comment| handle_match_comment(comment, locator))
.or_else(|comment| handle_in_between_bodies_own_line_comment(comment, locator))
.or_else(|comment| handle_in_between_bodies_end_of_line_comment(comment, locator))
.or_else(|comment| handle_trailing_body_comment(comment, locator))
.or_else(handle_trailing_end_of_line_body_comment)
.or_else(|comment| handle_trailing_end_of_line_condition_comment(comment, locator))
.or_else(|comment| {
handle_module_level_own_line_comment_before_class_or_function_comment(comment, locator)
})
.or_else(|comment| handle_positional_only_arguments_separator_comment(comment, locator))
.or_else(|comment| handle_trailing_binary_expression_left_or_operator_comment(comment, locator))
.or_else(handle_leading_function_with_decorators_comment)
.or_else(|comment| handle_dict_unpacking_comment(comment, locator))
static HANDLERS: &[for<'a> fn(DecoratedComment<'a>, &Locator) -> CommentPlacement<'a>] = &[
handle_in_between_except_handlers_or_except_handler_and_else_or_finally_comment,
handle_match_comment,
handle_in_between_bodies_own_line_comment,
handle_in_between_bodies_end_of_line_comment,
handle_trailing_body_comment,
handle_trailing_end_of_line_body_comment,
handle_trailing_end_of_line_condition_comment,
handle_module_level_own_line_comment_before_class_or_function_comment,
handle_positional_only_arguments_separator_comment,
handle_trailing_binary_expression_left_or_operator_comment,
handle_leading_function_with_decorators_comment,
handle_dict_unpacking_comment,
];
for handler in HANDLERS {
comment = match handler(comment, locator) {
CommentPlacement::Default(comment) => comment,
placement => return placement,
};
}
CommentPlacement::Default(comment)
}
/// Handles leading comments in front of a match case or a trailing comment of the `match` statement.
@ -483,7 +488,10 @@ fn handle_trailing_body_comment<'a>(
/// if something.changed:
/// do.stuff() # trailing comment
/// ```
fn handle_trailing_end_of_line_body_comment(comment: DecoratedComment<'_>) -> CommentPlacement<'_> {
fn handle_trailing_end_of_line_body_comment<'a>(
comment: DecoratedComment<'a>,
_locator: &Locator,
) -> CommentPlacement<'a> {
// Must be an end of line comment
if comment.line_position().is_own_line() {
return CommentPlacement::Default(comment);
@ -868,7 +876,10 @@ fn find_pos_only_slash_offset(
/// def test():
/// ...
/// ```
fn handle_leading_function_with_decorators_comment(comment: DecoratedComment) -> CommentPlacement {
fn handle_leading_function_with_decorators_comment<'a>(
comment: DecoratedComment<'a>,
_locator: &Locator,
) -> CommentPlacement<'a> {
let is_preceding_decorator = comment
.preceding_node()
.map_or(false, |node| node.is_decorator());

View file

@ -615,18 +615,6 @@ impl<'a> CommentPlacement<'a> {
comment: comment.into(),
}
}
/// Returns the placement if it isn't [`CommentPlacement::Default`], otherwise calls `f` and returns the result.
#[inline]
pub(super) fn or_else<F>(self, f: F) -> Self
where
F: FnOnce(DecoratedComment<'a>) -> CommentPlacement<'a>,
{
match self {
CommentPlacement::Default(comment) => f(comment),
placement => placement,
}
}
}
#[derive(Copy, Clone, Eq, PartialEq, Debug)]