Add direct-recursion Lint

CHANGELOG.md

@@ -5721,6 +5721,7 @@ Released 2018-09-13
 [`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord
 [`derive_partial_eq_without_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_partial_eq_without_eq
 [`derived_hash_with_manual_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derived_hash_with_manual_eq
+[`direct_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#direct_recursion
 [`disallowed_macros`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_macros
 [`disallowed_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method
 [`disallowed_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods

clippy_lints/src/declared_lints.rs

@@ -107,6 +107,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::derive::DERIVE_PARTIAL_EQ_WITHOUT_EQ_INFO,
     crate::derive::EXPL_IMPL_CLONE_ON_COPY_INFO,
     crate::derive::UNSAFE_DERIVE_DESERIALIZE_INFO,
+    crate::direct_recursion::DIRECT_RECURSION_INFO,
     crate::disallowed_macros::DISALLOWED_MACROS_INFO,
     crate::disallowed_methods::DISALLOWED_METHODS_INFO,
     crate::disallowed_names::DISALLOWED_NAMES_INFO,

clippy_lints/src/direct_recursion.rs

@@ -0,0 +1,164 @@
+use clippy_utils::diagnostics::{span_lint, span_lint_hir};
+use clippy_utils::get_parent_expr;
+use rustc_hir::def::{DefKind, Res};
+use rustc_hir::{Body, Expr, ExprKind, QPath};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::Instance;
+use rustc_session::impl_lint_pass;
+use rustc_span::def_id::DefId;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for functions that call themselves from their body.
+    ///
+    /// ### Why restrict this?
+    /// In Safety Critical contexts, recursive calls can lead to catastrophic
+    /// crashes if they happen to overflow the stack.
+    ///
+    /// In most contexts, this is not an issue, so this lint is allow-by-default.
+    ///
+    /// ### Notes
+    ///
+    /// #### Triggers
+    /// There are two things that trigger this lint:
+    /// - Function calls from a function (or method) to itself,
+    /// - Function pointer bindings from a function (or method) to itself.
+    ///
+    /// #### Independent of control flow
+    /// This lint triggers whenever the conditions above are met, regardless of
+    /// control flow and other such constructs.
+    ///
+    /// #### Blessing a recursive call
+    /// The user may choose to bless a recursive call or binding using the
+    /// attribute #[clippy::allowed_recursion]
+    ///
+    /// #### Indirect calls
+    /// This lint **does not** detect indirect recursive calls.
+    ///
+    /// ### Examples
+    /// This function will trigger the lint:
+    /// ```no_run
+    /// fn i_call_myself_in_a_bounded_way(bound: u8) {
+    ///     if bound > 0 {
+    ///         // This line will trigger the lint
+    ///         i_call_myself_in_a_bounded_way(bound - 1);
+    ///     }
+    /// }
+    /// ```
+    /// Using #[clippy::allowed_recursion] lets it pass:
+    /// ```no_run
+    /// fn i_call_myself_in_a_bounded_way(bound: u8) {
+    ///     if bound > 0 {
+    ///         #[clippy::allowed_recursion]
+    ///         i_call_myself_in_a_bounded_way(bound - 1);
+    ///     }
+    /// }
+    /// ```
+    /// This triggers the lint when `fibo` is bound to a function pointer
+    /// inside `fibo`'s body
+    /// ```no_run
+    /// fn fibo(a: u32) -> u32 {
+    ///     if a < 2 { a } else { (a - 2..a).map(fibo).sum() }
+    /// }
+    /// ```
+    #[clippy::version = "1.89.0"]
+    pub DIRECT_RECURSION,
+    restriction,
+    "functions shall not call themselves directly"
+}
+
+#[derive(Default)]
+pub struct DirectRecursion {
+    fn_id_stack: Vec<DefId>,
+}
+
+impl_lint_pass!(DirectRecursion => [DIRECT_RECURSION]);
+
+impl<'tcx> LateLintPass<'tcx> for DirectRecursion {
+    /// Whenever we enter a Body, we push its owner's `DefId` into the stack
+    fn check_body(&mut self, cx: &LateContext<'tcx>, body: &Body<'_>) {
+        self.fn_id_stack
+            .push(cx.tcx.hir_body_owner_def_id(body.id()).to_def_id());
+    }
+
+    /// We then revert this when we exit said `Body`
+    fn check_body_post(&mut self, _: &LateContext<'tcx>, _: &Body<'_>) {
+        _ = self.fn_id_stack.pop();
+    }
+
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
+        match expr.kind {
+            ExprKind::MethodCall(_, _, _, _) => {
+                let typeck = cx.typeck_results();
+                if let Some(basic_id) = typeck.type_dependent_def_id(expr.hir_id) {
+                    // This finds default Trait implementations of methods
+                    if self.fn_id_stack.contains(&basic_id) {
+                        span_lint(cx, DIRECT_RECURSION, expr.span, "this method contains a call to itself");
+                    }
+                    // Whereas this finds non-default implementations
+                    else if let args = typeck.node_args(expr.hir_id)
+                        && let Ok(Some(fn_def)) = Instance::try_resolve(cx.tcx, cx.typing_env(), basic_id, args)
+                        && let type_resolved_def_id = fn_def.def_id()
+                        && self.fn_id_stack.contains(&type_resolved_def_id)
+                    {
+                        span_lint(cx, DIRECT_RECURSION, expr.span, "this method contains a call to itself");
+                    }
+                }
+            },
+            ExprKind::Path(QPath::TypeRelative(_, _)) => {
+                // I'm still not sure this is proper.
+                // It definitely finds the right `DefId`, though.
+                let typeck = cx.typeck_results();
+                if let Some(id) = typeck.type_dependent_def_id(expr.hir_id)
+                    && let args = typeck.node_args(expr.hir_id)
+                    && let Ok(Some(fn_def)) = Instance::try_resolve(cx.tcx, cx.typing_env(), id, args)
+                {
+                    let type_resolved_def_id = fn_def.def_id();
+
+                    if self.fn_id_stack.contains(&type_resolved_def_id) {
+                        emit_lint(cx, expr);
+                    }
+                }
+            },
+            // This branch takes care of finding bindings of function and method names
+            // into fn pointers.
+            ExprKind::Path(QPath::Resolved(_, path)) => {
+                // Now we know that this Path is fully resolved.
+                // We now must check if it points to a function or a method's definition.
+                if let Res::Def(DefKind::Fn | DefKind::AssocFn, fn_path_id) = path.res
+                // 1) Now we know that the path we've found is of a function or method definition.
+                //
+                // 2) We will now check if it corresponds to the path of a function we're inside
+                // of.
+                //
+                // 3) Thankfully, we've kept track of the functions that surround us, in
+                //`self.fn_id_stack`.
+                //
+                // 4) If the path that we've captured from `expr` coincides with one of the functions
+                // in the stack, then we know we have a recursive loop.
+
+                && self.fn_id_stack.contains(&fn_path_id)
+                {
+                    emit_lint(cx, expr);
+                }
+            },
+            _ => {},
+        }
+    }
+}
+
+fn emit_lint<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
+    let (node_id, span, msg) = if let Some(parent_expr) = get_parent_expr(cx, expr)
+        && let ExprKind::Call(func, _) = parent_expr.kind
+        && func.hir_id == expr.hir_id
+    {
+        (
+            parent_expr.hir_id,
+            parent_expr.span,
+            "this function contains a call to itself",
+        )
+    } else {
+        (expr.hir_id, expr.span, "this function creates a reference to itself")
+    };
+    span_lint_hir(cx, DIRECT_RECURSION, node_id, span, msg);
+}

clippy_lints/src/lib.rs

@@ -114,6 +114,7 @@ mod default_union_representation;
 mod dereference;
 mod derivable_impls;
 mod derive;
+mod direct_recursion;
 mod disallowed_macros;
 mod disallowed_methods;
 mod disallowed_names;
@@ -951,5 +952,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf)));
     store.register_late_pass(|_| Box::new(infallible_try_from::InfallibleTryFrom));
     store.register_late_pass(|_| Box::new(coerce_container_to_any::CoerceContainerToAny));
+    store.register_late_pass(|_| Box::<direct_recursion::DirectRecursion>::default());
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

tests/ui/direct_recursion.rs

@@ -0,0 +1,171 @@
+#![deny(clippy::direct_recursion)]
+#![feature(stmt_expr_attributes)]
+
+// Basic Cases //
+
+#[allow(unconditional_recursion, reason = "We're not testing for that lint")]
+fn i_call_myself_always() {
+    i_call_myself_always();
+    //~^ direct_recursion
+}
+
+fn i_call_myself_conditionally(do_i: bool) {
+    if do_i {
+        i_call_myself_conditionally(false);
+        //~^ direct_recursion
+    }
+}
+
+// Basic Counterexamples //
+
+fn i_get_called_by_others() {}
+
+fn i_call_something_else() {
+    i_get_called_by_others();
+}
+
+// Elaborate Cases //
+
+// Case 1: Blessing //
+// Here we check that we're allowed to bless specific recursive calls.
+// A fine-grained control of where to allow recursion is desirable.
+// This is a test of such a feature.
+fn i_call_myself_in_a_bounded_way(bound: u8) {
+    if bound > 0 {
+        #[expect(clippy::direct_recursion)]
+        i_call_myself_in_a_bounded_way(bound - 1);
+    }
+}
+
+// Case 2: Blessing is Bounded //
+// Here we check that blessing a specific recursive call doesn't
+// let other recursive calls go through.
+fn i_have_one_blessing_but_two_calls(bound: u8) {
+    if bound > 25 {
+        #[expect(clippy::direct_recursion)]
+        i_have_one_blessing_but_two_calls(bound - 1);
+    } else if bound > 0 {
+        // "WIP: we still need to audit this part of the function"
+        i_have_one_blessing_but_two_calls(bound - 2)
+        //~^ direct_recursion
+    }
+}
+
+// Case 3: Blessing is Recursive //
+// Here we check that blessing a specific expression will
+// bless everything inside of that expression as well.
+fn ackermann(m: u64, n: u64) -> u64 {
+    if m == 0 {
+        n + 1
+    } else if n == 0 {
+        #[expect(clippy::direct_recursion)]
+        ackermann(m - 1, 1)
+    } else {
+        #[expect(clippy::direct_recursion)]
+        ackermann(m, ackermann(m + 1, n))
+    }
+}
+
+// Case 4: Linting is Recursive //
+// Here we check that linting a specific expression will
+// not block other expressions inside of it from being linted.
+fn ackermann_2_electric_recursion(m: u64, n: u64) -> u64 {
+    if m == 0 {
+        n + 1
+    } else if n == 0 {
+        #[expect(clippy::direct_recursion)]
+        ackermann_2_electric_recursion(m - 1, 1)
+    } else {
+        ackermann_2_electric_recursion(
+            //~^ direct_recursion
+            m,
+            ackermann_2_electric_recursion(m + 1, n),
+            //~^ direct_recursion
+        )
+    }
+}
+
+// Case 5: Nesting Functions //
+// Here we check that nested functions calling their parents are still
+// linted
+fn grand_parent() {
+    fn parent() {
+        fn child() {
+            parent();
+            //~^ direct_recursion
+            grand_parent();
+            //~^ direct_recursion
+        }
+        grand_parent();
+        //~^ direct_recursion
+    }
+}
+
+// Case 6: Binding of path to a Fn pointer //
+// Here we check that we are able to detect bindings of function names
+// as edges for the function call graph.
+fn fibo(a: u32) -> u32 {
+    if a < 2 { a } else { (a - 2..a).map(fibo).sum() }
+    //~^ direct_recursion
+}
+
+// Case 7: Linting on Associated Function Implementations //
+// Here we check that different implementations of the same trait don't go
+// linting calls to functions of implementations that are not their own.
+trait RecSum {
+    fn rec_sum(n: u32) -> u32;
+}
+
+struct Summer;
+
+// Recursive Call: should be linted
+impl RecSum for Summer {
+    fn rec_sum(n: u32) -> u32 {
+        if n == 0 {
+            0
+        } else {
+            // Notice how this is a recursive call, whereas the next one isn't
+            n + Self::rec_sum(n - 1)
+            //~^ direct_recursion
+        }
+    }
+}
+
+struct Winter;
+
+// Not a Recursive Call: should be ignored.
+impl RecSum for Winter {
+    fn rec_sum(n: u32) -> u32 {
+        // This should NOT trigger the lint, because even though it's calling the same
+        // function (or "the same symbol"), it's not recursively calling its own implementation.
+        if n == 0 { 0 } else { n + Summer::rec_sum(n - 1) }
+    }
+}
+
+// Case 8: Linting on Default Trait Method Implementations //
+// Here we check that recursion in trait methods is also captured by the lint
+trait MyTrait {
+    fn myfun(&self, num: i32) {
+        if num > 0 {
+            self.myfun(num - 1);
+            //~^ direct_recursion
+        }
+    }
+}
+
+// Case 9: Linting on Trait Method Implementations //
+
+struct T(u32);
+
+trait W {
+    fn f(&self);
+}
+
+impl W for T {
+    fn f(&self) {
+        if self.0 > 0 {
+            self.f()
+            //~^ direct_recursion
+        }
+    }
+}