Chore: Address PR review <3

Co-authored-by: Veetaha <gersoh3@gmail.com

marker_uilints/src/utils.rs

@@ -1,5 +1,5 @@
 use marker_api::prelude::*;
-use marker_utils::search::contains_return;
+use marker_utils::visitor::BoolTraversable;
 
 marker_api::declare_lint! {
     /// # What it does
@@ -13,7 +13,8 @@ pub fn check_item<'ast>(cx: &'ast AstContext<'ast>, item: ItemKind<'ast>) {
     let Some(ident) = fn_item.ident() else { return };
 
     if ident.name().starts_with("test_contains_return") {
-        let res = contains_return(cx, cx.body(fn_item.body_id().unwrap()).expr());
+        let body = cx.body(fn_item.body_id().unwrap());
+        let res = body.contains_return(cx);
 
         cx.emit_lint(
             TEST_CONTAINS_RETURN,

marker_utils/src/lib.rs

@@ -5,5 +5,4 @@
 #![allow(clippy::unused_self)] // `self` is needed to potentualy change the behavior later
 #![allow(clippy::trivially_copy_pass_by_ref)] // Needed to potentualy change the behavior later
 
-pub mod search;
 pub mod visitor;

marker_utils/src/visitor.rs

@@ -26,7 +26,7 @@ use marker_api::{
 ///         // The printed `x` is different from the `x` of `foo()`
 ///         // since it comes from the function parameter of `bar()`
 ///         println!("The magic number is {x}");
-///         // The `return` statement only effects the `bar` function.
+///         // The `return` statement only affects the `bar` function.
 ///         // `foo()` will just continue executing, when this is called
 ///         return;
 ///     }
@@ -35,7 +35,7 @@ use marker_api::{
 /// }
 /// ```
 ///
-/// The target scope, is checked when the respective `traverse_*` function is called
+/// The target scope is checked when the respective `traverse_*` function is called
 /// For example, [`traverse_body`] will visit a given body [`Body`], but will not enter
 /// nested bodies, unless [`AllBodies`](VisitorScope::AllBodies) is defined.
 #[non_exhaustive]
@@ -369,15 +369,68 @@ pub fn traverse_expr<'ast, B>(
 }
 
 /// This trait is implemented for nodes, that can be traversed by a [`Visitor`].
-pub trait Traverseable<'ast, B> {
+pub trait Traversable<'ast, B>
+where
+    Self: Sized + Copy,
+{
     /// This calls the `traverse_*` function for the implementing node.
     fn traverse(self, cx: &'ast AstContext<'ast>, visitor: &mut dyn Visitor<B>) -> ControlFlow<B>;
+
+    /// This function calls the given closure for every expression in the node. This
+    /// traversal will not enter any nested bodies.
+    ///
+    /// This function is a simple wrapper around the [`Visitor`] trait, that is good
+    /// for most use cases. For example, the following code counts the number of `if`s
+    /// in a body:
+    ///
+    /// ```
+    /// # use marker_api::prelude::*;
+    /// # use std::ops::ControlFlow;
+    /// # use marker_utils::visitor::Traversable;
+    /// fn count_ifs<'ast>(cx: &'ast AstContext<'ast>, body: &'ast Body<'ast>) -> u32 {
+    ///     let mut count = 0;
+    ///     let _: Option<()> = body.for_each_expr(
+    ///         cx,
+    ///         |expr| {
+    ///             if matches!(expr, ExprKind::If(_)) {
+    ///                 count += 1;
+    ///             }
+    ///             ControlFlow::Continue(())
+    ///         }
+    ///     );
+    ///     count
+    /// }
+    /// ```
+    fn for_each_expr<F: for<'a> FnMut(ExprKind<'a>) -> ControlFlow<B>>(
+        self,
+        cx: &'ast AstContext<'ast>,
+        f: F,
+    ) -> Option<B> {
+        struct ExprVisitor<F> {
+            f: F,
+        }
+        impl<B, F: for<'a> FnMut(ExprKind<'a>) -> ControlFlow<B>> Visitor<B> for ExprVisitor<F> {
+            fn visit_expr<'v_ast>(
+                &mut self,
+                _cx: &'v_ast AstContext<'v_ast>,
+                expr: ExprKind<'v_ast>,
+            ) -> ControlFlow<B> {
+                (self.f)(expr)
+            }
+        }
+        let mut visitor = ExprVisitor { f };
+
+        match self.traverse(cx, &mut visitor) {
+            ControlFlow::Continue(()) => None,
+            ControlFlow::Break(b) => Some(b),
+        }
+    }
 }
 
-/// This macro implements the [`Traverseable`] trait for a given node that implements `Copy`
+/// This macro implements the [`Traversable`] trait for a given node that implements `Copy`
 macro_rules! impl_traversable_for {
     ($ty:ty, $func:ident) => {
-        impl<'ast, B> Traverseable<'ast, B> for $ty {
+        impl<'ast, B> Traversable<'ast, B> for $ty {
             fn traverse(self, cx: &'ast AstContext<'ast>, visitor: &mut dyn Visitor<B>) -> ControlFlow<B> {
                 $func(cx, visitor, self)
             }
@@ -390,49 +443,26 @@ impl_traversable_for!(StmtKind<'ast>, traverse_stmt);
 impl_traversable_for!(ItemKind<'ast>, traverse_item);
 impl_traversable_for!(&'ast Body<'ast>, traverse_body);
 
-/// This function calls the given closure for every expression in the node. This
-/// traversal will not enter any nested bodies.
-///
-/// This function is a simple wrapper around the [`Visitor`] trait, that is good
-/// for most use cases. For example, the following code counts the number of `if`s
-/// in a body:
-///
-/// ```
-/// # use marker_api::prelude::*;
-/// # use std::ops::ControlFlow;
-/// # use marker_utils::visitor::for_each_expr;
-/// fn count_ifs<'ast>(cx: &'ast AstContext<'ast>, body: &'ast Body<'ast>) -> u32 {
-///     let mut count = 0;
-///     let _: Option<()> = for_each_expr(
-///         cx,
-///         body,
-///         |expr| {
-///             if matches!(expr, ExprKind::If(_)) {
-///                 count += 1;
-///             }
-///             ControlFlow::Continue(())
-///         }
-///     );
-///     count
-/// }
-/// ```
-pub fn for_each_expr<'ast, B, F: for<'a> FnMut(ExprKind<'a>) -> ControlFlow<B>>(
-    cx: &'ast AstContext<'ast>,
-    node: impl Traverseable<'ast, B>,
-    f: F,
-) -> Option<B> {
-    struct ExprVisitor<F> {
-        f: F,
-    }
-    impl<B, F: for<'a> FnMut(ExprKind<'a>) -> ControlFlow<B>> Visitor<B> for ExprVisitor<F> {
-        fn visit_expr<'v_ast>(&mut self, _cx: &'v_ast AstContext<'v_ast>, expr: ExprKind<'v_ast>) -> ControlFlow<B> {
-            (self.f)(expr)
-        }
-    }
-    let mut visitor = ExprVisitor { f };
-
-    match node.traverse(cx, &mut visitor) {
-        ControlFlow::Continue(()) => None,
-        ControlFlow::Break(b) => Some(b),
+/// This trait extends the [`Traversable`] trait with more functions, specific to
+/// the `bool` return type.
+pub trait BoolTraversable<'ast>: Traversable<'ast, bool> {
+    /// Checks if the given node contains an early return, in the form of an
+    /// [`ReturnExpr`](marker_api::ast::expr::ReturnExpr) or
+    /// [`QuestionMarkExpr`](marker_api::ast::expr::QuestionMarkExpr).
+    ///
+    /// This function is useful, for lints which suggest moving code snippets into
+    /// a closure or different function. Return statements might prevent the suggested
+    /// refactoring.
+    fn contains_return(&self, cx: &'ast AstContext<'ast>) -> bool {
+        self.for_each_expr(cx, |expr| {
+            if matches!(expr, ExprKind::Return(_) | ExprKind::QuestionMark(_)) {
+                ControlFlow::Break(true)
+            } else {
+                ControlFlow::Continue(())
+            }
+        })
+        .is_some()
     }
 }
+
+impl<'ast, T: Traversable<'ast, bool>> BoolTraversable<'ast> for T {}