fix(use_self): don't early-return if the outer type has no lifetimes (#15611)

Fixes https://github.com/rust-lang/rust-clippy/issues/13277

Unfortunately breaks another case, which has only been working thanks to
the now fixed bug -- see last commit.

changelog: [`use_self`]: don't early-return if the outer type has no
lifetimes

8 files changed, 92 insertions, 87 deletions

diff --git a/clippy_lints/src/matches/needless_match.rs b/clippy_lints/src/matches/needless_match.rs
index b04db03f8d2..3a2097c3df2 100644
--- a/clippy_lints/src/matches/needless_match.rs
+++ b/clippy_lints/src/matches/needless_match.rs
@@ -1,7 +1,7 @@
 use super::NEEDLESS_MATCH;
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_applicability;
-use clippy_utils::ty::{is_type_diagnostic_item, same_type_and_consts};
+use clippy_utils::ty::{is_type_diagnostic_item, same_type_modulo_regions};
 use clippy_utils::{
     SpanlessEq, eq_expr_value, get_parent_expr_for_hir, higher, is_else_clause, is_res_lang_ctor, over, path_res,
     peel_blocks_with_stmt,
@@ -122,7 +122,7 @@ fn expr_ty_matches_p_ty(cx: &LateContext<'_>, expr: &Expr<'_>, p_expr: &Expr<'_>
         // Compare match_expr ty with local in `let local = match match_expr {..}`
         Node::LetStmt(local) => {
             let results = cx.typeck_results();
-            return same_type_and_consts(results.node_type(local.hir_id), results.expr_ty(expr));
+            return same_type_modulo_regions(results.node_type(local.hir_id), results.expr_ty(expr));
         },
         // compare match_expr ty with RetTy in `fn foo() -> RetTy`
         Node::Item(item) => {
@@ -133,7 +133,7 @@ fn expr_ty_matches_p_ty(cx: &LateContext<'_>, expr: &Expr<'_>, p_expr: &Expr<'_>
                     .instantiate_identity()
                     .output()
                     .skip_binder();
-                return same_type_and_consts(output, cx.typeck_results().expr_ty(expr));
+                return same_type_modulo_regions(output, cx.typeck_results().expr_ty(expr));
             }
         },
         // check the parent expr for this whole block `{ match match_expr {..} }`
diff --git a/clippy_lints/src/operators/erasing_op.rs b/clippy_lints/src/operators/erasing_op.rs
index e3fc8d8fea7..8f5ee390f72 100644
--- a/clippy_lints/src/operators/erasing_op.rs
+++ b/clippy_lints/src/operators/erasing_op.rs
@@ -1,6 +1,6 @@
 use clippy_utils::consts::{ConstEvalCtxt, Constant};
 use clippy_utils::diagnostics::span_lint;
-use clippy_utils::ty::same_type_and_consts;
+use clippy_utils::ty::same_type_modulo_regions;
 
 use rustc_hir::{BinOpKind, Expr};
 use rustc_lint::LateContext;
@@ -29,7 +29,7 @@ pub(super) fn check<'tcx>(
 fn different_types(tck: &TypeckResults<'_>, input: &Expr<'_>, output: &Expr<'_>) -> bool {
     let input_ty = tck.expr_ty(input).peel_refs();
     let output_ty = tck.expr_ty(output).peel_refs();
-    !same_type_and_consts(input_ty, output_ty)
+    !same_type_modulo_regions(input_ty, output_ty)
 }
 
 fn check_op<'tcx>(
diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs
index aeda864b7eb..8210320591c 100644
--- a/clippy_lints/src/use_self.rs
+++ b/clippy_lints/src/use_self.rs
@@ -2,20 +2,21 @@ use clippy_config::Conf;
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::is_from_proc_macro;
 use clippy_utils::msrvs::{self, Msrv};
-use clippy_utils::ty::{same_type_and_consts, ty_from_hir_ty};
+use clippy_utils::ty::{same_type_modulo_regions, ty_from_hir_ty};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::Applicability;
 use rustc_hir::def::{CtorOf, DefKind, Res};
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::intravisit::{InferKind, Visitor, VisitorExt, walk_ty};
 use rustc_hir::{
-    self as hir, AmbigArg, Expr, ExprKind, FnRetTy, FnSig, GenericArgsParentheses, GenericParam, GenericParamKind,
-    HirId, Impl, ImplItemKind, Item, ItemKind, Pat, PatExpr, PatExprKind, PatKind, Path, QPath, Ty, TyKind,
+    self as hir, AmbigArg, Expr, ExprKind, FnRetTy, FnSig, GenericArgsParentheses, GenericParamKind, HirId, Impl,
+    ImplItemKind, Item, ItemKind, Pat, PatExpr, PatExprKind, PatKind, Path, QPath, Ty, TyKind,
 };
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::Ty as MiddleTy;
 use rustc_session::impl_lint_pass;
 use rustc_span::Span;
+use std::iter;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -101,17 +102,11 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
             let types_to_skip = generics
                 .params
                 .iter()
-                .filter_map(|param| match param {
-                    GenericParam {
-                        kind:
-                            GenericParamKind::Const {
-                                ty: Ty { hir_id, .. }, ..
-                            },
-                        ..
-                    } => Some(*hir_id),
+                .filter_map(|param| match param.kind {
+                    GenericParamKind::Const { ty, .. } => Some(ty.hir_id),
                     _ => None,
                 })
-                .chain(std::iter::once(self_ty.hir_id))
+                .chain([self_ty.hir_id])
                 .collect();
             StackItem::Check {
                 impl_id: item.owner_id.def_id,
@@ -210,11 +205,11 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
             && !types_to_skip.contains(&hir_ty.hir_id)
             && let ty = ty_from_hir_ty(cx, hir_ty.as_unambig_ty())
             && let impl_ty = cx.tcx.type_of(impl_id).instantiate_identity()
-            && same_type_and_consts(ty, impl_ty)
+            && same_type_modulo_regions(ty, impl_ty)
             // Ensure the type we encounter and the one from the impl have the same lifetime parameters. It may be that
-            // the lifetime parameters of `ty` are elided (`impl<'a> Foo<'a> { fn new() -> Self { Foo{..} } }`, in
+            // the lifetime parameters of `ty` are elided (`impl<'a> Foo<'a> { fn new() -> Self { Foo{..} } }`), in
             // which case we must still trigger the lint.
-            && (has_no_lifetime(ty) || same_lifetimes(ty, impl_ty))
+            && same_lifetimes(ty, impl_ty)
             && self.msrv.meets(cx, msrvs::TYPE_ALIAS_ENUM_VARIANTS)
         {
             span_lint(cx, hir_ty.span);
@@ -227,18 +222,16 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
             && cx.typeck_results().expr_ty(expr) == cx.tcx.type_of(impl_id).instantiate_identity()
             && self.msrv.meets(cx, msrvs::TYPE_ALIAS_ENUM_VARIANTS)
         {
-        } else {
-            return;
-        }
-        match expr.kind {
-            ExprKind::Struct(QPath::Resolved(_, path), ..) => check_path(cx, path),
-            ExprKind::Call(fun, _) => {
-                if let ExprKind::Path(QPath::Resolved(_, path)) = fun.kind {
-                    check_path(cx, path);
-                }
-            },
-            ExprKind::Path(QPath::Resolved(_, path)) => check_path(cx, path),
-            _ => (),
+            match expr.kind {
+                ExprKind::Struct(QPath::Resolved(_, path), ..) => check_path(cx, path),
+                ExprKind::Call(fun, _) => {
+                    if let ExprKind::Path(QPath::Resolved(_, path)) = fun.kind {
+                        check_path(cx, path);
+                    }
+                },
+                ExprKind::Path(QPath::Resolved(_, path)) => check_path(cx, path),
+                _ => (),
+            }
         }
     }
 
@@ -308,36 +301,20 @@ fn lint_path_to_variant(cx: &LateContext<'_>, path: &Path<'_>) {
     }
 }
 
-/// Returns `true` if types `a` and `b` have the same lifetime parameters, otherwise returns
-/// `false`.
+/// Checks whether types `a` and `b` have the same lifetime parameters.
 ///
 /// This function does not check that types `a` and `b` are the same types.
 fn same_lifetimes<'tcx>(a: MiddleTy<'tcx>, b: MiddleTy<'tcx>) -> bool {
     use rustc_middle::ty::{Adt, GenericArgKind};
-    match (&a.kind(), &b.kind()) {
-        (&Adt(_, args_a), &Adt(_, args_b)) => {
-            args_a
-                .iter()
-                .zip(args_b.iter())
-                .all(|(arg_a, arg_b)| match (arg_a.kind(), arg_b.kind()) {
-                    // TODO: Handle inferred lifetimes
-                    (GenericArgKind::Lifetime(inner_a), GenericArgKind::Lifetime(inner_b)) => inner_a == inner_b,
-                    (GenericArgKind::Type(type_a), GenericArgKind::Type(type_b)) => same_lifetimes(type_a, type_b),
-                    _ => true,
-                })
+    match (a.kind(), b.kind()) {
+        (Adt(_, args_a), Adt(_, args_b)) => {
+            iter::zip(*args_a, *args_b).all(|(arg_a, arg_b)| match (arg_a.kind(), arg_b.kind()) {
+                // TODO: Handle inferred lifetimes
+                (GenericArgKind::Lifetime(inner_a), GenericArgKind::Lifetime(inner_b)) => inner_a == inner_b,
+                (GenericArgKind::Type(type_a), GenericArgKind::Type(type_b)) => same_lifetimes(type_a, type_b),
+                _ => true,
+            })
         },
         _ => a == b,
     }
 }
-
-/// Returns `true` if `ty` has no lifetime parameter, otherwise returns `false`.
-fn has_no_lifetime(ty: MiddleTy<'_>) -> bool {
-    use rustc_middle::ty::{Adt, GenericArgKind};
-    match ty.kind() {
-        &Adt(_, args) => !args
-            .iter()
-            // TODO: Handle inferred lifetimes
-            .any(|arg| matches!(arg.kind(), GenericArgKind::Lifetime(..))),
-        _ => true,
-    }
-}
diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs
index 70ae982a445..5274fdbd1dc 100644
--- a/clippy_lints/src/useless_conversion.rs
+++ b/clippy_lints/src/useless_conversion.rs
@@ -1,7 +1,7 @@
 use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::source::{snippet, snippet_with_context};
 use clippy_utils::sugg::{DiagExt as _, Sugg};
-use clippy_utils::ty::{get_type_diagnostic_name, is_copy, is_type_diagnostic_item, same_type_and_consts};
+use clippy_utils::ty::{get_type_diagnostic_name, is_copy, is_type_diagnostic_item, same_type_modulo_regions};
 use clippy_utils::{
     get_parent_expr, is_inherent_method_call, is_trait_item, is_trait_method, is_ty_alias, path_to_local, sym,
 };
@@ -184,7 +184,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
                     && (is_trait_item(cx, arg, sym::Into) || is_trait_item(cx, arg, sym::From))
                     && let ty::FnDef(_, args) = cx.typeck_results().expr_ty(arg).kind()
                     && let &[from_ty, to_ty] = args.into_type_list(cx.tcx).as_slice()
-                    && same_type_and_consts(from_ty, to_ty)
+                    && same_type_modulo_regions(from_ty, to_ty)
                 {
                     span_lint_and_then(
                         cx,
@@ -207,7 +207,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
                 if is_trait_method(cx, e, sym::Into) && name.ident.name == sym::into {
                     let a = cx.typeck_results().expr_ty(e);
                     let b = cx.typeck_results().expr_ty(recv);
-                    if same_type_and_consts(a, b) {
+                    if same_type_modulo_regions(a, b) {
                         let mut app = Applicability::MachineApplicable;
                         let sugg = snippet_with_context(cx, recv.span, e.span.ctxt(), "<expr>", &mut app).0;
                         span_lint_and_sugg(
@@ -324,7 +324,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
                     // If the types are identical then .into_iter() can be removed, unless the type
                     // implements Copy, in which case .into_iter() returns a copy of the receiver and
                     // cannot be safely omitted.
-                    if same_type_and_consts(a, b) && !is_copy(cx, b) {
+                    if same_type_modulo_regions(a, b) && !is_copy(cx, b) {
                         // Below we check if the parent method call meets the following conditions:
                         // 1. First parameter is `&mut self` (requires mutable reference)
                         // 2. Second parameter implements the `FnMut` trait (e.g., Iterator::any)
@@ -371,7 +371,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
                     && is_type_diagnostic_item(cx, a, sym::Result)
                     && let ty::Adt(_, args) = a.kind()
                     && let Some(a_type) = args.types().next()
-                    && same_type_and_consts(a_type, b)
+                    && same_type_modulo_regions(a_type, b)
                 {
                     span_lint_and_help(
                         cx,
@@ -396,7 +396,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
                         && is_type_diagnostic_item(cx, a, sym::Result)
                         && let ty::Adt(_, args) = a.kind()
                         && let Some(a_type) = args.types().next()
-                        && same_type_and_consts(a_type, b)
+                        && same_type_modulo_regions(a_type, b)
                     {
                         let hint = format!("consider removing `{}()`", snippet(cx, path.span, "TryFrom::try_from"));
                         span_lint_and_help(
@@ -407,7 +407,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
                             None,
                             hint,
                         );
-                    } else if name == sym::from_fn && same_type_and_consts(a, b) {
+                    } else if name == sym::from_fn && same_type_modulo_regions(a, b) {
                         let mut app = Applicability::MachineApplicable;
                         let sugg = Sugg::hir_with_context(cx, arg, e.span.ctxt(), "<expr>", &mut app).maybe_paren();
                         let sugg_msg = format!("consider removing `{}()`", snippet(cx, path.span, "From::from"));
diff --git a/clippy_utils/src/ty/mod.rs b/clippy_utils/src/ty/mod.rs
index 8e302f9d2ad..8fbaac58288 100644
--- a/clippy_utils/src/ty/mod.rs
+++ b/clippy_utils/src/ty/mod.rs
@@ -513,25 +513,31 @@ pub fn walk_ptrs_ty_depth(ty: Ty<'_>) -> (Ty<'_>, usize) {
     inner(ty, 0)
 }
 
-/// Returns `true` if types `a` and `b` are same types having same `Const` generic args,
-/// otherwise returns `false`
-pub fn same_type_and_consts<'tcx>(a: Ty<'tcx>, b: Ty<'tcx>) -> bool {
+/// Checks whether `a` and `b` are same types having same `Const` generic args, but ignores
+/// lifetimes.
+///
+/// For example, the function would return `true` for
+/// - `u32` and `u32`
+/// - `[u8; N]` and `[u8; M]`, if `N=M`
+/// - `Option<T>` and `Option<U>`, if `same_type_modulo_regions(T, U)` holds
+/// - `&'a str` and `&'b str`
+///
+/// and `false` for:
+/// - `Result<u32, String>` and `Result<usize, String>`
+pub fn same_type_modulo_regions<'tcx>(a: Ty<'tcx>, b: Ty<'tcx>) -> bool {
     match (&a.kind(), &b.kind()) {
         (&ty::Adt(did_a, args_a), &ty::Adt(did_b, args_b)) => {
             if did_a != did_b {
                 return false;
             }
 
-            args_a
-                .iter()
-                .zip(args_b.iter())
-                .all(|(arg_a, arg_b)| match (arg_a.kind(), arg_b.kind()) {
-                    (GenericArgKind::Const(inner_a), GenericArgKind::Const(inner_b)) => inner_a == inner_b,
-                    (GenericArgKind::Type(type_a), GenericArgKind::Type(type_b)) => {
-                        same_type_and_consts(type_a, type_b)
-                    },
-                    _ => true,
-                })
+            iter::zip(*args_a, *args_b).all(|(arg_a, arg_b)| match (arg_a.kind(), arg_b.kind()) {
+                (GenericArgKind::Const(inner_a), GenericArgKind::Const(inner_b)) => inner_a == inner_b,
+                (GenericArgKind::Type(type_a), GenericArgKind::Type(type_b)) => {
+                    same_type_modulo_regions(type_a, type_b)
+                },
+                _ => true,
+            })
         },
         _ => a == b,
     }
diff --git a/tests/ui/use_self.fixed b/tests/ui/use_self.fixed
index cccb6bffabb..075e31d202b 100644
--- a/tests/ui/use_self.fixed
+++ b/tests/ui/use_self.fixed
@@ -530,8 +530,8 @@ mod issue7206 {
 
     impl<'a> S2<S<'a>> {
         fn new_again() -> Self {
-            Self::new()
-            //~^ use_self
+            S2::new()
+            // FIXME: ^Broken by PR #15611
         }
     }
 }
@@ -755,3 +755,17 @@ mod crash_check_13128 {
         }
     }
 }
+
+mod issue_13277 {
+    trait Foo {
+        type Item<'foo>;
+    }
+    struct Bar<'b> {
+        content: &'b str,
+    }
+    impl<'b> Foo for Option<Bar<'b>> {
+        // when checking whether `Option<Bar<'foo>>` has a lifetime, check not only the outer
+        // `Option<T>`, but also the inner `Bar<'foo>`
+        type Item<'foo> = Option<Bar<'foo>>;
+    }
+}
diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs
index 09288677aa7..6fbba0bbc55 100644
--- a/tests/ui/use_self.rs
+++ b/tests/ui/use_self.rs
@@ -531,7 +531,7 @@ mod issue7206 {
     impl<'a> S2<S<'a>> {
         fn new_again() -> Self {
             S2::new()
-            //~^ use_self
+            // FIXME: ^Broken by PR #15611
         }
     }
 }
@@ -755,3 +755,17 @@ mod crash_check_13128 {
         }
     }
 }
+
+mod issue_13277 {
+    trait Foo {
+        type Item<'foo>;
+    }
+    struct Bar<'b> {
+        content: &'b str,
+    }
+    impl<'b> Foo for Option<Bar<'b>> {
+        // when checking whether `Option<Bar<'foo>>` has a lifetime, check not only the outer
+        // `Option<T>`, but also the inner `Bar<'foo>`
+        type Item<'foo> = Option<Bar<'foo>>;
+    }
+}
diff --git a/tests/ui/use_self.stderr b/tests/ui/use_self.stderr
index 781327696ac..5f65c53ea25 100644
--- a/tests/ui/use_self.stderr
+++ b/tests/ui/use_self.stderr
@@ -170,12 +170,6 @@ LL |             A::new::<submod::B>(submod::B {})
    |             ^ help: use the applicable keyword: `Self`
 
 error: unnecessary structure name repetition
-  --> tests/ui/use_self.rs:533:13
-   |
-LL |             S2::new()
-   |             ^^ help: use the applicable keyword: `Self`
-
-error: unnecessary structure name repetition
   --> tests/ui/use_self.rs:571:17
    |
 LL |                 Foo::Bar => unimplemented!(),
@@ -259,5 +253,5 @@ error: unnecessary structure name repetition
 LL |                 E::A => {},
    |                 ^ help: use the applicable keyword: `Self`
 
-error: aborting due to 43 previous errors
+error: aborting due to 42 previous errors