Auto merge of #6571 - ThibsG:BoxedLocalTrait, r=phansch

Fix FP for `boxed_local` lint in default trait fn impl

Fix FP on default trait function implementation on `boxed_local` lint.

Maybe I checked too much when looking if `self` is carrying `Self` in its bound type.
I can't find a good test case for this, so it could be too much conservative.
Let me know if you think only detecting `self` parameter is enough.

Fixes: #4804

changelog: none

3 files changed, 62 insertions, 5 deletions

diff --git a/clippy_lints/src/escape.rs b/clippy_lints/src/escape.rs
index d2dcb3e5c46..9fcd17a756a 100644
--- a/clippy_lints/src/escape.rs
+++ b/clippy_lints/src/escape.rs
@@ -1,15 +1,16 @@
 use rustc_hir::intravisit;
-use rustc_hir::{self, Body, FnDecl, HirId, HirIdSet, ItemKind, Node};
+use rustc_hir::{self, AssocItemKind, Body, FnDecl, HirId, HirIdSet, ItemKind, Node};
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty::{self, Ty};
+use rustc_middle::ty::{self, TraitRef, Ty};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::source_map::Span;
+use rustc_span::symbol::kw;
 use rustc_target::abi::LayoutOf;
 use rustc_target::spec::abi::Abi;
 use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
 
-use crate::utils::span_lint;
+use crate::utils::{contains_ty, span_lint};
 
 #[derive(Copy, Clone)]
 pub struct BoxedLocal {
@@ -51,6 +52,7 @@ fn is_non_trait_box(ty: Ty<'_>) -> bool {
 struct EscapeDelegate<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
     set: HirIdSet,
+    trait_self_ty: Option<Ty<'a>>,
     too_large_for_stack: u64,
 }
 
@@ -72,19 +74,34 @@ impl<'tcx> LateLintPass<'tcx> for BoxedLocal {
             }
         }
 
-        // If the method is an impl for a trait, don't warn.
         let parent_id = cx.tcx.hir().get_parent_item(hir_id);
         let parent_node = cx.tcx.hir().find(parent_id);
 
+        let mut trait_self_ty = None;
         if let Some(Node::Item(item)) = parent_node {
+            // If the method is an impl for a trait, don't warn.
             if let ItemKind::Impl { of_trait: Some(_), .. } = item.kind {
                 return;
             }
+
+            // find `self` ty for this trait if relevant
+            if let ItemKind::Trait(_, _, _, _, items) = item.kind {
+                for trait_item in items {
+                    if trait_item.id.hir_id == hir_id {
+                        // be sure we have `self` parameter in this function
+                        if let AssocItemKind::Fn { has_self: true } = trait_item.kind {
+                            trait_self_ty =
+                                Some(TraitRef::identity(cx.tcx, trait_item.id.hir_id.owner.to_def_id()).self_ty());
+                        }
+                    }
+                }
+            }
         }
 
         let mut v = EscapeDelegate {
             cx,
             set: HirIdSet::default(),
+            trait_self_ty,
             too_large_for_stack: self.too_large_for_stack,
         };
 
@@ -153,6 +170,14 @@ impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> {
                     return;
                 }
 
+                // skip if there is a `self` parameter binding to a type
+                // that contains `Self` (i.e.: `self: Box<Self>`), see #4804
+                if let Some(trait_self_ty) = self.trait_self_ty {
+                    if map.name(cmt.hir_id) == kw::SelfLower && contains_ty(cmt.place.ty(), trait_self_ty) {
+                        return;
+                    }
+                }
+
                 if is_non_trait_box(cmt.place.ty()) && !self.is_large_box(cmt.place.ty()) {
                     self.set.insert(cmt.hir_id);
                 }
diff --git a/tests/ui/escape_analysis.rs b/tests/ui/escape_analysis.rs
index 07004489610..d26f48fc68f 100644
--- a/tests/ui/escape_analysis.rs
+++ b/tests/ui/escape_analysis.rs
@@ -182,3 +182,23 @@ pub extern "C" fn do_not_warn_me(_c_pointer: Box<String>) -> () {}
 
 #[rustfmt::skip] // Forces rustfmt to not add ABI
 pub extern fn do_not_warn_me_no_abi(_c_pointer: Box<String>) -> () {}
+
+// Issue #4804 - default implementation in trait
+mod issue4804 {
+    trait DefaultTraitImplTest {
+        // don't warn on `self`
+        fn default_impl(self: Box<Self>) -> u32 {
+            5
+        }
+
+        // warn on `x: Box<u32>`
+        fn default_impl_x(self: Box<Self>, x: Box<u32>) -> u32 {
+            4
+        }
+    }
+
+    trait WarnTrait {
+        // warn on `x: Box<u32>`
+        fn foo(x: Box<u32>) {}
+    }
+}
diff --git a/tests/ui/escape_analysis.stderr b/tests/ui/escape_analysis.stderr
index c86a769a3da..4a82b4419f9 100644
--- a/tests/ui/escape_analysis.stderr
+++ b/tests/ui/escape_analysis.stderr
@@ -12,5 +12,17 @@ error: local variable doesn't need to be boxed here
 LL | pub fn new(_needs_name: Box<PeekableSeekable<&()>>) -> () {}
    |            ^^^^^^^^^^^
 
-error: aborting due to 2 previous errors
+error: local variable doesn't need to be boxed here
+  --> $DIR/escape_analysis.rs:195:44
+   |
+LL |         fn default_impl_x(self: Box<Self>, x: Box<u32>) -> u32 {
+   |                                            ^
+
+error: local variable doesn't need to be boxed here
+  --> $DIR/escape_analysis.rs:202:16
+   |
+LL |         fn foo(x: Box<u32>) {}
+   |                ^
+
+error: aborting due to 4 previous errors