From 44156f696247449375892864fe36ea7cb26763fa Mon Sep 17 00:00:00 2001
From: konstin <konstin@mailbox.org>
Date: Wed, 21 Jun 2023 11:52:13 +0200
Subject: [PATCH] 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.
---
 .../src/comments/placement.rs                 | 49 ++++++++++++-------
 .../src/comments/visitor.rs                   | 12 -----
 2 files changed, 30 insertions(+), 31 deletions(-)

diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs
index a467263e09..a1f9fa2cae 100644
--- a/crates/ruff_python_formatter/src/comments/placement.rs
+++ b/crates/ruff_python_formatter/src/comments/placement.rs
@@ -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());
diff --git a/crates/ruff_python_formatter/src/comments/visitor.rs b/crates/ruff_python_formatter/src/comments/visitor.rs
index 6102240a40..828cf6e10e 100644
--- a/crates/ruff_python_formatter/src/comments/visitor.rs
+++ b/crates/ruff_python_formatter/src/comments/visitor.rs
@@ -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)]