From bd38ff3c9be59c378dc17b04b2f93f396591e389 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 9 Dec 2023 00:12:07 +0100 Subject: [PATCH] Add new `unconditional_recursion` lint --- clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/unconditional_recursion.rs | 124 ++++++++++++++++++++ 3 files changed, 127 insertions(+) create mode 100644 clippy_lints/src/unconditional_recursion.rs diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index d3105ed9d185..ec372d860a30 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -675,6 +675,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::types::REDUNDANT_ALLOCATION_INFO, crate::types::TYPE_COMPLEXITY_INFO, crate::types::VEC_BOX_INFO, + crate::unconditional_recursion::UNCONDITIONAL_RECURSION_INFO, crate::undocumented_unsafe_blocks::UNDOCUMENTED_UNSAFE_BLOCKS_INFO, crate::undocumented_unsafe_blocks::UNNECESSARY_SAFETY_COMMENT_INFO, crate::unicode::INVISIBLE_CHARACTERS_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index e4ff7ebb0f72..b81e77a48809 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -325,6 +325,7 @@ mod trait_bounds; mod transmute; mod tuple_array_conversions; mod types; +mod unconditional_recursion; mod undocumented_unsafe_blocks; mod unicode; mod uninhabited_references; @@ -1075,6 +1076,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(repeat_vec_with_capacity::RepeatVecWithCapacity)); store.register_late_pass(|_| Box::new(uninhabited_references::UninhabitedReferences)); store.register_late_pass(|_| Box::new(ineffective_open_options::IneffectiveOpenOptions)); + store.register_late_pass(|_| Box::new(unconditional_recursion::UnconditionalRecursion)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/unconditional_recursion.rs b/clippy_lints/src/unconditional_recursion.rs new file mode 100644 index 000000000000..e267c6d85346 --- /dev/null +++ b/clippy_lints/src/unconditional_recursion.rs @@ -0,0 +1,124 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::{get_parent_as_impl, get_trait_def_id, path_res}; +use rustc_ast::BinOpKind; +use rustc_hir::def::Res; +use rustc_hir::def_id::{DefId, LocalDefId}; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{Body, Expr, ExprKind, FnDecl}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{self, Ty}; +use rustc_session::declare_lint_pass; +use rustc_span::{sym, Span}; + +declare_clippy_lint! { + /// ### What it does + /// Checks that there isn't an infinite recursion in `PartialEq` trait + /// implementation. + /// + /// ### Why is this bad? + /// This is a hard to find infinite recursion which will crashing any code + /// using it. + /// + /// ### Example + /// ```no_run + /// enum Foo { + /// A, + /// B, + /// } + /// + /// impl PartialEq for Foo { + /// fn eq(&self, other: &Self) -> bool { + /// self == other // bad! + /// } + /// } + /// ``` + /// Use instead: + /// + /// In such cases, either use `#[derive(PartialEq)]` or don't implement it. + #[clippy::version = "1.76.0"] + pub UNCONDITIONAL_RECURSION, + suspicious, + "detect unconditional recursion in some traits implementation" +} + +declare_lint_pass!(UnconditionalRecursion => [UNCONDITIONAL_RECURSION]); + +fn get_ty_def_id(ty: Ty<'_>) -> Option { + match ty.peel_refs().kind() { + ty::Adt(adt, _) => Some(adt.did()), + ty::Foreign(def_id) => Some(*def_id), + _ => None, + } +} + +fn is_local(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + matches!(path_res(cx, expr), Res::Local(_)) +} + +impl<'tcx> LateLintPass<'tcx> for UnconditionalRecursion { + #[allow(clippy::unnecessary_def_path)] + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + kind: FnKind<'tcx>, + _decl: &'tcx FnDecl<'tcx>, + body: &'tcx Body<'tcx>, + method_span: Span, + def_id: LocalDefId, + ) { + // We don't check code generated from (proc) macro. + if method_span.from_expansion() { + return; + } + if let FnKind::Method(name, _) = kind + && let [self_arg, other_arg] = cx + .tcx + .instantiate_bound_regions_with_erased(cx.tcx.fn_sig(def_id).skip_binder()) + .inputs() + && let Some(self_arg) = get_ty_def_id(*self_arg) + && let Some(other_arg) = get_ty_def_id(*other_arg) + && self_arg == other_arg + && let hir_id = cx.tcx.local_def_id_to_hir_id(def_id) + && let Some(impl_) = get_parent_as_impl(cx.tcx, hir_id) + && let Some(trait_) = impl_.of_trait + && let Some(trait_def_id) = trait_.trait_def_id() + && Some(trait_def_id) == get_trait_def_id(cx, &["core", "cmp", "PartialEq"]) + { + let to_check_op = if name.name == sym::eq { + BinOpKind::Eq + } else { + BinOpKind::Ne + }; + let expr = body.value.peel_blocks(); + let is_bad = match expr.kind { + ExprKind::Binary(op, left, right) if op.node == to_check_op => { + is_local(cx, left) && is_local(cx, right) + }, + ExprKind::MethodCall(segment, receiver, &[arg], _) if segment.ident.name == name.name => { + if is_local(cx, receiver) + && is_local(cx, &arg) + && let Some(fn_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) + && let Some(trait_id) = cx.tcx.trait_of_item(fn_id) + && trait_id == trait_def_id + { + true + } else { + false + } + }, + _ => false, + }; + if is_bad { + span_lint_and_then( + cx, + UNCONDITIONAL_RECURSION, + method_span, + "function cannot return without recursing", + |diag| { + diag.span_note(expr.span, "recursive call site"); + }, + ); + } + } + } +}