Skip to content

Commit f66cbd1

Browse files
committed
Add new lint: direct-recursion Lint
changelog: new lint: [`direct_recursion`]
1 parent f2922e7 commit f66cbd1

File tree

6 files changed

+424
-0
lines changed

6 files changed

+424
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5721,6 +5721,7 @@ Released 2018-09-13
57215721
[`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord
57225722
[`derive_partial_eq_without_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_partial_eq_without_eq
57235723
[`derived_hash_with_manual_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derived_hash_with_manual_eq
5724+
[`direct_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#direct_recursion
57245725
[`disallowed_macros`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_macros
57255726
[`disallowed_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method
57265727
[`disallowed_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
107107
crate::derive::DERIVE_PARTIAL_EQ_WITHOUT_EQ_INFO,
108108
crate::derive::EXPL_IMPL_CLONE_ON_COPY_INFO,
109109
crate::derive::UNSAFE_DERIVE_DESERIALIZE_INFO,
110+
crate::direct_recursion::DIRECT_RECURSION_INFO,
110111
crate::disallowed_macros::DISALLOWED_MACROS_INFO,
111112
crate::disallowed_methods::DISALLOWED_METHODS_INFO,
112113
crate::disallowed_names::DISALLOWED_NAMES_INFO,

clippy_lints/src/direct_recursion.rs

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
use clippy_utils::diagnostics::{span_lint, span_lint_hir};
2+
use clippy_utils::get_parent_expr;
3+
use rustc_hir::def::{DefKind, Res};
4+
use rustc_hir::{Body, Expr, ExprKind, QPath};
5+
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_middle::ty::Instance;
7+
use rustc_session::impl_lint_pass;
8+
use rustc_span::def_id::DefId;
9+
10+
declare_clippy_lint! {
11+
/// ### What it does
12+
/// Checks for functions that call themselves from their body.
13+
///
14+
/// ### Why restrict this?
15+
/// In Safety Critical contexts, recursive calls can lead to catastrophic
16+
/// crashes if they happen to overflow the stack.
17+
///
18+
/// In most contexts, this is not an issue, so this lint is allow-by-default.
19+
///
20+
/// ### Notes
21+
///
22+
/// #### Triggers
23+
/// There are two things that trigger this lint:
24+
/// - Function calls from a function (or method) to itself,
25+
/// - Function pointer bindings from a function (or method) to itself.
26+
///
27+
/// #### Independent of control flow
28+
/// This lint triggers whenever the conditions above are met, regardless of
29+
/// control flow and other such constructs.
30+
///
31+
/// #### Blessing a recursive call
32+
/// The user may choose to bless a recursive call or binding using the
33+
/// attribute #[clippy::allowed_recursion]
34+
///
35+
/// #### Indirect calls
36+
/// This lint **does not** detect indirect recursive calls.
37+
///
38+
/// ### Examples
39+
/// This function will trigger the lint:
40+
/// ```no_run
41+
/// fn i_call_myself_in_a_bounded_way(bound: u8) {
42+
/// if bound > 0 {
43+
/// // This line will trigger the lint
44+
/// i_call_myself_in_a_bounded_way(bound - 1);
45+
/// }
46+
/// }
47+
/// ```
48+
/// Using #[clippy::allowed_recursion] lets it pass:
49+
/// ```no_run
50+
/// fn i_call_myself_in_a_bounded_way(bound: u8) {
51+
/// if bound > 0 {
52+
/// #[clippy::allowed_recursion]
53+
/// i_call_myself_in_a_bounded_way(bound - 1);
54+
/// }
55+
/// }
56+
/// ```
57+
/// This triggers the lint when `fibo` is bound to a function pointer
58+
/// inside `fibo`'s body
59+
/// ```no_run
60+
/// fn fibo(a: u32) -> u32 {
61+
/// if a < 2 { a } else { (a - 2..a).map(fibo).sum() }
62+
/// }
63+
/// ```
64+
#[clippy::version = "1.89.0"]
65+
pub DIRECT_RECURSION,
66+
restriction,
67+
"functions shall not call themselves directly"
68+
}
69+
70+
#[derive(Default)]
71+
pub struct DirectRecursion {
72+
fn_id_stack: Vec<DefId>,
73+
}
74+
75+
impl_lint_pass!(DirectRecursion => [DIRECT_RECURSION]);
76+
77+
impl<'tcx> LateLintPass<'tcx> for DirectRecursion {
78+
/// Whenever we enter a Body, we push its owner's `DefId` into the stack
79+
fn check_body(&mut self, cx: &LateContext<'tcx>, body: &Body<'_>) {
80+
self.fn_id_stack
81+
.push(cx.tcx.hir_body_owner_def_id(body.id()).to_def_id());
82+
}
83+
84+
/// We then revert this when we exit said `Body`
85+
fn check_body_post(&mut self, _: &LateContext<'tcx>, _: &Body<'_>) {
86+
_ = self.fn_id_stack.pop();
87+
}
88+
89+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
90+
match expr.kind {
91+
ExprKind::MethodCall(_, _, _, _) => {
92+
let typeck = cx.typeck_results();
93+
if let Some(basic_id) = typeck.type_dependent_def_id(expr.hir_id) {
94+
// This finds default Trait implementations of methods
95+
if self.fn_id_stack.contains(&basic_id) {
96+
span_lint(cx, DIRECT_RECURSION, expr.span, "this method contains a call to itself");
97+
}
98+
// Whereas this finds non-default implementations
99+
else if let args = typeck.node_args(expr.hir_id)
100+
&& let Ok(Some(fn_def)) = Instance::try_resolve(cx.tcx, cx.typing_env(), basic_id, args)
101+
&& let type_resolved_def_id = fn_def.def_id()
102+
&& self.fn_id_stack.contains(&type_resolved_def_id)
103+
{
104+
span_lint(cx, DIRECT_RECURSION, expr.span, "this method contains a call to itself");
105+
}
106+
}
107+
},
108+
ExprKind::Path(QPath::TypeRelative(_, _)) => {
109+
// I'm still not sure this is proper.
110+
// It definitely finds the right `DefId`, though.
111+
let typeck = cx.typeck_results();
112+
if let Some(id) = typeck.type_dependent_def_id(expr.hir_id)
113+
&& let args = typeck.node_args(expr.hir_id)
114+
&& let Ok(Some(fn_def)) = Instance::try_resolve(cx.tcx, cx.typing_env(), id, args)
115+
{
116+
let type_resolved_def_id = fn_def.def_id();
117+
118+
if self.fn_id_stack.contains(&type_resolved_def_id) {
119+
emit_lint(cx, expr);
120+
}
121+
}
122+
},
123+
// This branch takes care of finding bindings of function and method names
124+
// into fn pointers.
125+
ExprKind::Path(QPath::Resolved(_, path)) => {
126+
// Now we know that this Path is fully resolved.
127+
// We now must check if it points to a function or a method's definition.
128+
if let Res::Def(DefKind::Fn | DefKind::AssocFn, fn_path_id) = path.res
129+
// 1) Now we know that the path we've found is of a function or method definition.
130+
//
131+
// 2) We will now check if it corresponds to the path of a function we're inside
132+
// of.
133+
//
134+
// 3) Thankfully, we've kept track of the functions that surround us, in
135+
//`self.fn_id_stack`.
136+
//
137+
// 4) If the path that we've captured from `expr` coincides with one of the functions
138+
// in the stack, then we know we have a recursive loop.
139+
140+
&& self.fn_id_stack.contains(&fn_path_id)
141+
{
142+
emit_lint(cx, expr);
143+
}
144+
},
145+
_ => {},
146+
}
147+
}
148+
}
149+
150+
fn emit_lint<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
151+
let (node_id, span, msg) = if let Some(parent_expr) = get_parent_expr(cx, expr)
152+
&& let ExprKind::Call(func, _) = parent_expr.kind
153+
&& func.hir_id == expr.hir_id
154+
{
155+
(
156+
parent_expr.hir_id,
157+
parent_expr.span,
158+
"this function contains a call to itself",
159+
)
160+
} else {
161+
(expr.hir_id, expr.span, "this function creates a reference to itself")
162+
};
163+
span_lint_hir(cx, DIRECT_RECURSION, node_id, span, msg);
164+
}

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ mod default_union_representation;
114114
mod dereference;
115115
mod derivable_impls;
116116
mod derive;
117+
mod direct_recursion;
117118
mod disallowed_macros;
118119
mod disallowed_methods;
119120
mod disallowed_names;
@@ -951,5 +952,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
951952
store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf)));
952953
store.register_late_pass(|_| Box::new(infallible_try_from::InfallibleTryFrom));
953954
store.register_late_pass(|_| Box::new(coerce_container_to_any::CoerceContainerToAny));
955+
store.register_late_pass(|_| Box::<direct_recursion::DirectRecursion>::default());
954956
// add lints here, do not remove this comment, it's used in `new_lint`
955957
}

tests/ui/direct_recursion.rs

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
#![deny(clippy::direct_recursion)]
2+
#![feature(stmt_expr_attributes)]
3+
4+
// Basic Cases //
5+
6+
#[allow(unconditional_recursion, reason = "We're not testing for that lint")]
7+
fn i_call_myself_always() {
8+
i_call_myself_always();
9+
//~^ direct_recursion
10+
}
11+
12+
fn i_call_myself_conditionally(do_i: bool) {
13+
if do_i {
14+
i_call_myself_conditionally(false);
15+
//~^ direct_recursion
16+
}
17+
}
18+
19+
// Basic Counterexamples //
20+
21+
fn i_get_called_by_others() {}
22+
23+
fn i_call_something_else() {
24+
i_get_called_by_others();
25+
}
26+
27+
// Elaborate Cases //
28+
29+
// Case 1: Blessing //
30+
// Here we check that we're allowed to bless specific recursive calls.
31+
// A fine-grained control of where to allow recursion is desirable.
32+
// This is a test of such a feature.
33+
fn i_call_myself_in_a_bounded_way(bound: u8) {
34+
if bound > 0 {
35+
#[expect(clippy::direct_recursion)]
36+
i_call_myself_in_a_bounded_way(bound - 1);
37+
}
38+
}
39+
40+
// Case 2: Blessing is Bounded //
41+
// Here we check that blessing a specific recursive call doesn't
42+
// let other recursive calls go through.
43+
fn i_have_one_blessing_but_two_calls(bound: u8) {
44+
if bound > 25 {
45+
#[expect(clippy::direct_recursion)]
46+
i_have_one_blessing_but_two_calls(bound - 1);
47+
} else if bound > 0 {
48+
// "WIP: we still need to audit this part of the function"
49+
i_have_one_blessing_but_two_calls(bound - 2)
50+
//~^ direct_recursion
51+
}
52+
}
53+
54+
// Case 3: Blessing is Recursive //
55+
// Here we check that blessing a specific expression will
56+
// bless everything inside of that expression as well.
57+
fn ackermann(m: u64, n: u64) -> u64 {
58+
if m == 0 {
59+
n + 1
60+
} else if n == 0 {
61+
#[expect(clippy::direct_recursion)]
62+
ackermann(m - 1, 1)
63+
} else {
64+
#[expect(clippy::direct_recursion)]
65+
ackermann(m, ackermann(m + 1, n))
66+
}
67+
}
68+
69+
// Case 4: Linting is Recursive //
70+
// Here we check that linting a specific expression will
71+
// not block other expressions inside of it from being linted.
72+
fn ackermann_2_electric_recursion(m: u64, n: u64) -> u64 {
73+
if m == 0 {
74+
n + 1
75+
} else if n == 0 {
76+
#[expect(clippy::direct_recursion)]
77+
ackermann_2_electric_recursion(m - 1, 1)
78+
} else {
79+
ackermann_2_electric_recursion(
80+
//~^ direct_recursion
81+
m,
82+
ackermann_2_electric_recursion(m + 1, n),
83+
//~^ direct_recursion
84+
)
85+
}
86+
}
87+
88+
// Case 5: Nesting Functions //
89+
// Here we check that nested functions calling their parents are still
90+
// linted
91+
fn grand_parent() {
92+
fn parent() {
93+
fn child() {
94+
parent();
95+
//~^ direct_recursion
96+
grand_parent();
97+
//~^ direct_recursion
98+
}
99+
grand_parent();
100+
//~^ direct_recursion
101+
}
102+
}
103+
104+
// Case 6: Binding of path to a Fn pointer //
105+
// Here we check that we are able to detect bindings of function names
106+
// as edges for the function call graph.
107+
fn fibo(a: u32) -> u32 {
108+
if a < 2 { a } else { (a - 2..a).map(fibo).sum() }
109+
//~^ direct_recursion
110+
}
111+
112+
// Case 7: Linting on Associated Function Implementations //
113+
// Here we check that different implementations of the same trait don't go
114+
// linting calls to functions of implementations that are not their own.
115+
trait RecSum {
116+
fn rec_sum(n: u32) -> u32;
117+
}
118+
119+
struct Summer;
120+
121+
// Recursive Call: should be linted
122+
impl RecSum for Summer {
123+
fn rec_sum(n: u32) -> u32 {
124+
if n == 0 {
125+
0
126+
} else {
127+
// Notice how this is a recursive call, whereas the next one isn't
128+
n + Self::rec_sum(n - 1)
129+
//~^ direct_recursion
130+
}
131+
}
132+
}
133+
134+
struct Winter;
135+
136+
// Not a Recursive Call: should be ignored.
137+
impl RecSum for Winter {
138+
fn rec_sum(n: u32) -> u32 {
139+
// This should NOT trigger the lint, because even though it's calling the same
140+
// function (or "the same symbol"), it's not recursively calling its own implementation.
141+
if n == 0 { 0 } else { n + Summer::rec_sum(n - 1) }
142+
}
143+
}
144+
145+
// Case 8: Linting on Default Trait Method Implementations //
146+
// Here we check that recursion in trait methods is also captured by the lint
147+
trait MyTrait {
148+
fn myfun(&self, num: i32) {
149+
if num > 0 {
150+
self.myfun(num - 1);
151+
//~^ direct_recursion
152+
}
153+
}
154+
}
155+
156+
// Case 9: Linting on Trait Method Implementations //
157+
158+
struct T(u32);
159+
160+
trait W {
161+
fn f(&self);
162+
}
163+
164+
impl W for T {
165+
fn f(&self) {
166+
if self.0 > 0 {
167+
self.f()
168+
//~^ direct_recursion
169+
}
170+
}
171+
}

0 commit comments

Comments
 (0)