Auto merge of #6952 - Jarcho:new_ret_no_self_fp, r=Manishearth

Fix `new_ret_no_self` false positive

fixes: #1724

changelog: Fix false positive with `new_ret_no_self` when returning `Self` with different generic arguments

3 files changed, 31 insertions, 4 deletions

diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index d77f9c03cae..fccdee07877 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -61,7 +61,7 @@ mod zst_offset;
 
 use bind_instead_of_map::BindInsteadOfMap;
 use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
-use clippy_utils::ty::{contains_ty, implements_trait, is_copy, is_type_diagnostic_item};
+use clippy_utils::ty::{contains_adt_constructor, contains_ty, implements_trait, is_copy, is_type_diagnostic_item};
 use clippy_utils::{contains_return, get_trait_def_id, in_macro, iter_input_pats, method_calls, paths, return_ty};
 use if_chain::if_chain;
 use rustc_hir as hir;
@@ -1916,7 +1916,11 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
             let ret_ty = return_ty(cx, impl_item.hir_id());
 
             // walk the return type and check for Self (this does not check associated types)
-            if contains_ty(ret_ty, self_ty) {
+            if let Some(self_adt) = self_ty.ty_adt_def() {
+                if contains_adt_constructor(ret_ty, self_adt) {
+                    return;
+                }
+            } else if contains_ty(ret_ty, self_ty) {
                 return;
             }
 
@@ -1926,7 +1930,11 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
                 for &(predicate, _span) in cx.tcx.explicit_item_bounds(def_id) {
                     if let ty::PredicateKind::Projection(projection_predicate) = predicate.kind().skip_binder() {
                         // walk the associated type and check for Self
-                        if contains_ty(projection_predicate.ty, self_ty) {
+                        if let Some(self_adt) = self_ty.ty_adt_def() {
+                            if contains_adt_constructor(projection_predicate.ty, self_adt) {
+                                return;
+                            }
+                        } else if contains_ty(projection_predicate.ty, self_ty) {
                             return;
                         }
                     }
diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs
index f4a1ae67da3..807cfbc4c7f 100644
--- a/clippy_utils/src/ty.rs
+++ b/clippy_utils/src/ty.rs
@@ -11,7 +11,7 @@ use rustc_hir::{TyKind, Unsafety};
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_lint::LateContext;
 use rustc_middle::ty::subst::{GenericArg, GenericArgKind};
-use rustc_middle::ty::{self, IntTy, Ty, TypeFoldable, UintTy};
+use rustc_middle::ty::{self, AdtDef, IntTy, Ty, TypeFoldable, UintTy};
 use rustc_span::sym;
 use rustc_span::symbol::Symbol;
 use rustc_span::DUMMY_SP;
@@ -43,6 +43,15 @@ pub fn contains_ty(ty: Ty<'_>, other_ty: Ty<'_>) -> bool {
     })
 }
 
+/// Walks into `ty` and returns `true` if any inner type is an instance of the given adt
+/// constructor.
+pub fn contains_adt_constructor(ty: Ty<'_>, adt: &AdtDef) -> bool {
+    ty.walk().any(|inner| match inner.unpack() {
+        GenericArgKind::Type(inner_ty) => inner_ty.ty_adt_def() == Some(adt),
+        GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
+    })
+}
+
 /// Returns true if ty has `iter` or `iter_mut` methods
 pub fn has_iter_method(cx: &LateContext<'_>, probably_ref_ty: Ty<'_>) -> Option<Symbol> {
     // FIXME: instead of this hard-coded list, we should check if `<adt>::iter`
diff --git a/tests/ui/new_ret_no_self.rs b/tests/ui/new_ret_no_self.rs
index e82873629a5..2f315ffe298 100644
--- a/tests/ui/new_ret_no_self.rs
+++ b/tests/ui/new_ret_no_self.rs
@@ -340,3 +340,13 @@ mod issue5435 {
         }
     }
 }
+
+// issue #1724
+struct RetOtherSelf<T>(T);
+struct RetOtherSelfWrapper<T>(T);
+
+impl RetOtherSelf<T> {
+    fn new(t: T) -> RetOtherSelf<RetOtherSelfWrapper<T>> {
+        RetOtherSelf(RetOtherSelfWrapper(t))
+    }
+}