[unconditional_recursion]: catch From -> Into -> From

Author: y21

clippy_lints/src/unconditional_recursion.rs

@@ -1,5 +1,5 @@
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::{expr_or_init, get_trait_def_id, path_def_id};
+use clippy_utils::{expr_or_init, fn_def_id_with_node_args, path_def_id};
 use rustc_ast::BinOpKind;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir as hir;
@@ -19,11 +19,11 @@ use rustc_trait_selection::traits::error_reporting::suggestions::ReturnsVisitor;
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Checks that there isn't an infinite recursion in `PartialEq` trait
-    /// implementation.
+    /// Checks that there isn't an infinite recursion in trait
+    /// implementations.
     ///
     /// ### Why is this bad?
-    /// This is a hard to find infinite recursion which will crashing any code
+    /// This is a hard to find infinite recursion that will crash any code.
     /// using it.
     ///
     /// ### Example
@@ -201,7 +201,6 @@ fn check_partial_eq(cx: &LateContext<'_>, method_span: Span, method_def_id: Loca
     }
 }
 
-#[allow(clippy::unnecessary_def_path)]
 fn check_to_string(cx: &LateContext<'_>, method_span: Span, method_def_id: LocalDefId, name: Ident, expr: &Expr<'_>) {
     let args = cx
         .tcx
@@ -224,7 +223,7 @@ fn check_to_string(cx: &LateContext<'_>, method_span: Span, method_def_id: Local
         && let Some(trait_) = impl_.of_trait
         && let Some(trait_def_id) = trait_.trait_def_id()
         // The trait is `ToString`.
-        && Some(trait_def_id) == get_trait_def_id(cx, &["alloc", "string", "ToString"])
+        && cx.tcx.is_diagnostic_item(sym::ToString, trait_def_id)
     {
         let is_bad = match expr.kind {
             ExprKind::MethodCall(segment, _receiver, &[_arg], _) if segment.ident.name == name.name => {
@@ -291,7 +290,6 @@ where
         self.map
     }
 
-    #[allow(clippy::unnecessary_def_path)]
     fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
         if self.found_default_call {
             return;
@@ -303,7 +301,7 @@ where
             && is_default_method_on_current_ty(self.cx.tcx, qpath, self.implemented_ty_id)
             && let Some(method_def_id) = path_def_id(self.cx, f)
             && let Some(trait_def_id) = self.cx.tcx.trait_of_item(method_def_id)
-            && Some(trait_def_id) == get_trait_def_id(self.cx, &["core", "default", "Default"])
+            && self.cx.tcx.is_diagnostic_item(sym::Default, trait_def_id)
         {
             self.found_default_call = true;
             span_error(self.cx, self.method_span, expr);
@@ -312,10 +310,9 @@ where
 }
 
 impl UnconditionalRecursion {
-    #[allow(clippy::unnecessary_def_path)]
     fn init_default_impl_for_type_if_needed(&mut self, cx: &LateContext<'_>) {
         if self.default_impl_for_type.is_empty()
-            && let Some(default_trait_id) = get_trait_def_id(cx, &["core", "default", "Default"])
+            && let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
         {
             let impls = cx.tcx.trait_impls_of(default_trait_id);
             for (ty, impl_def_ids) in impls.non_blanket_impls() {
@@ -394,6 +391,34 @@ impl UnconditionalRecursion {
     }
 }
 
+fn check_from(cx: &LateContext<'_>, method_span: Span, method_def_id: LocalDefId, expr: &Expr<'_>) {
+    let Some(sig) = cx
+        .typeck_results()
+        .liberated_fn_sigs()
+        .get(cx.tcx.local_def_id_to_hir_id(method_def_id))
+    else {
+        return;
+    };
+
+    // Check if we are calling `Into::into` where the node args match with our `From::from` signature:
+    // From::from signature: fn(S1) -> S2
+    // <S1 as Into<S2>>::into(s1), node_args=[S1, S2]
+    // If they do match, then it must mean that it is the blanket impl,
+    // which calls back into our `From::from` again (`Into` is not specializable).
+    // rustc's unconditional_recursion already catches calling `From::from` directly
+    if let Some((fn_def_id, node_args)) = fn_def_id_with_node_args(cx, expr)
+        && let [s1, s2] = **node_args
+        && let (Some(s1), Some(s2)) = (s1.as_type(), s2.as_type())
+        && let Some(trait_def_id) = cx.tcx.trait_of_item(fn_def_id)
+        && cx.tcx.is_diagnostic_item(sym::Into, trait_def_id)
+        && get_impl_trait_def_id(cx, method_def_id) == cx.tcx.get_diagnostic_item(sym::From)
+        && s1 == sig.inputs()[0]
+        && s2 == sig.output()
+    {
+        span_error(cx, method_span, expr);
+    }
+}
+
 impl<'tcx> LateLintPass<'tcx> for UnconditionalRecursion {
     fn check_fn(
         &mut self,
@@ -410,10 +435,11 @@ impl<'tcx> LateLintPass<'tcx> for UnconditionalRecursion {
             // Doesn't have a conditional return.
             && !has_conditional_return(body, expr)
         {
-            if name.name == sym::eq || name.name == sym::ne {
-                check_partial_eq(cx, method_span, method_def_id, name, expr);
-            } else if name.name == sym::to_string {
-                check_to_string(cx, method_span, method_def_id, name, expr);
+            match name.name {
+                sym::eq | sym::ne => check_partial_eq(cx, method_span, method_def_id, name, expr),
+                sym::to_string => check_to_string(cx, method_span, method_def_id, name, expr),
+                sym::from => check_from(cx, method_span, method_def_id, expr),
+                _ => {},
             }
             self.check_default_new(cx, decl, body, method_span, method_def_id);
         }

clippy_utils/src/lib.rs

@@ -2249,8 +2249,21 @@ pub fn fn_has_unsatisfiable_preds(cx: &LateContext<'_>, did: DefId) -> bool {
 
 /// Returns the `DefId` of the callee if the given expression is a function or method call.
 pub fn fn_def_id(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<DefId> {
+    fn_def_id_with_node_args(cx, expr).map(|(did, _)| did)
+}
+
+/// Returns the `DefId` of the callee if the given expression is a function or method call,
+/// as well as its node args.
+pub fn fn_def_id_with_node_args<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &Expr<'_>,
+) -> Option<(DefId, rustc_ty::GenericArgsRef<'tcx>)> {
+    let typeck = cx.typeck_results();
     match &expr.kind {
-        ExprKind::MethodCall(..) => cx.typeck_results().type_dependent_def_id(expr.hir_id),
+        ExprKind::MethodCall(..) => Some((
+            typeck.type_dependent_def_id(expr.hir_id)?,
+            typeck.node_args(expr.hir_id),
+        )),
         ExprKind::Call(
             Expr {
                 kind: ExprKind::Path(qpath),
@@ -2262,9 +2275,9 @@ pub fn fn_def_id(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<DefId> {
             // Only return Fn-like DefIds, not the DefIds of statics/consts/etc that contain or
             // deref to fn pointers, dyn Fn, impl Fn - #8850
             if let Res::Def(DefKind::Fn | DefKind::Ctor(..) | DefKind::AssocFn, id) =
-                cx.typeck_results().qpath_res(qpath, *path_hir_id)
+                typeck.qpath_res(qpath, *path_hir_id)
             {
-                Some(id)
+                Some((id, typeck.node_args(*path_hir_id)))
             } else {
                 None
             }

tests/ui/unconditional_recursion.rs

@@ -1,7 +1,11 @@
 //@no-rustfix
 
 #![warn(clippy::unconditional_recursion)]
-#![allow(clippy::partialeq_ne_impl, clippy::default_constructed_unit_structs)]
+#![allow(
+    clippy::partialeq_ne_impl,
+    clippy::default_constructed_unit_structs,
+    clippy::only_used_in_recursion
+)]
 
 enum Foo {
     A,
@@ -350,4 +354,48 @@ mod issue12154 {
     }
 }
 
+// From::from -> Into::into -> From::from
+struct BadFromTy1<'a>(&'a ());
+struct BadIntoTy1<'b>(&'b ());
+impl<'a> From<BadFromTy1<'a>> for BadIntoTy1<'static> {
+    fn from(f: BadFromTy1<'a>) -> Self {
+        f.into()
+    }
+}
+
+// Using UFCS syntax
+struct BadFromTy2<'a>(&'a ());
+struct BadIntoTy2<'b>(&'b ());
+impl<'a> From<BadFromTy2<'a>> for BadIntoTy2<'static> {
+    fn from(f: BadFromTy2<'a>) -> Self {
+        Into::into(f)
+    }
+}
+
+// Different Into impl (<i16 as Into<i32>>), so no infinite recursion
+struct BadFromTy3;
+impl From<BadFromTy3> for i32 {
+    fn from(f: BadFromTy3) -> Self {
+        Into::into(1i16)
+    }
+}
+
+// A conditional return that ends the recursion
+struct BadFromTy4;
+impl From<BadFromTy4> for i32 {
+    fn from(f: BadFromTy4) -> Self {
+        if true {
+            return 42;
+        }
+        f.into()
+    }
+}
+
+// Types differ in refs, don't lint
+impl From<&BadFromTy4> for i32 {
+    fn from(f: &BadFromTy4) -> Self {
+        BadFromTy4.into()
+    }
+}
+
 fn main() {}