Auto merge of #8737 - smoelius:extra-impl-lifetimes, r=giraffate

Extend `extra_unused_lifetimes` to handle impl lifetimes

Fixes #6437 (cc: `@carols10cents)`

changelog: fix #6437

Author: bors

clippy_lints/src/lifetimes.rs

@@ -1,16 +1,19 @@
 use clippy_utils::diagnostics::span_lint;
 use clippy_utils::trait_ref_of_method;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
+use rustc_hir::intravisit::nested_filter::{self as hir_nested_filter, NestedFilter};
 use rustc_hir::intravisit::{
-    walk_fn_decl, walk_generic_param, walk_generics, walk_item, walk_param_bound, walk_poly_trait_ref, walk_ty, Visitor,
+    walk_fn_decl, walk_generic_param, walk_generics, walk_impl_item_ref, walk_item, walk_param_bound,
+    walk_poly_trait_ref, walk_trait_ref, walk_ty, Visitor,
 };
 use rustc_hir::FnRetTy::Return;
 use rustc_hir::{
-    BareFnTy, BodyId, FnDecl, GenericArg, GenericBound, GenericParam, GenericParamKind, Generics, ImplItem,
+    BareFnTy, BodyId, FnDecl, GenericArg, GenericBound, GenericParam, GenericParamKind, Generics, Impl, ImplItem,
     ImplItemKind, Item, ItemKind, LangItem, Lifetime, LifetimeName, ParamName, PolyTraitRef, TraitBoundModifier,
     TraitFn, TraitItem, TraitItemKind, Ty, TyKind, WhereClause, WherePredicate,
 };
 use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::hir::nested_filter as middle_nested_filter;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::source_map::Span;
 use rustc_span::symbol::{kw, Ident, Symbol};
@@ -84,6 +87,8 @@ impl<'tcx> LateLintPass<'tcx> for Lifetimes {
     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
         if let ItemKind::Fn(ref sig, ref generics, id) = item.kind {
             check_fn_inner(cx, sig.decl, Some(id), None, generics, item.span, true);
+        } else if let ItemKind::Impl(ref impl_) = item.kind {
+            report_extra_impl_lifetimes(cx, impl_);
         }
     }
 
@@ -194,8 +199,7 @@ fn explicit_self_type<'tcx>(cx: &LateContext<'tcx>, func: &FnDecl<'tcx>, ident:
             visitor.visit_ty(self_ty);
 
             !visitor.all_lts().is_empty()
-        }
-        else {
+        } else {
             false
         }
     }
@@ -481,11 +485,29 @@ fn has_where_lifetimes<'tcx>(cx: &LateContext<'tcx>, where_clause: &'tcx WhereCl
     false
 }
 
-struct LifetimeChecker {
+struct LifetimeChecker<'cx, 'tcx, F> {
+    cx: &'cx LateContext<'tcx>,
     map: FxHashMap<Symbol, Span>,
+    phantom: std::marker::PhantomData<F>,
 }
 
-impl<'tcx> Visitor<'tcx> for LifetimeChecker {
+impl<'cx, 'tcx, F> LifetimeChecker<'cx, 'tcx, F> {
+    fn new(cx: &'cx LateContext<'tcx>, map: FxHashMap<Symbol, Span>) -> LifetimeChecker<'cx, 'tcx, F> {
+        Self {
+            cx,
+            map,
+            phantom: std::marker::PhantomData,
+        }
+    }
+}
+
+impl<'cx, 'tcx, F> Visitor<'tcx> for LifetimeChecker<'cx, 'tcx, F>
+where
+    F: NestedFilter<'tcx>,
+{
+    type Map = rustc_middle::hir::map::Map<'tcx>;
+    type NestedFilter = F;
+
     // for lifetimes as parameters of generics
     fn visit_lifetime(&mut self, lifetime: &'tcx Lifetime) {
         self.map.remove(&lifetime.name.ident().name);
@@ -501,6 +523,10 @@ impl<'tcx> Visitor<'tcx> for LifetimeChecker {
             walk_generic_param(self, param);
         }
     }
+
+    fn nested_visit_map(&mut self) -> Self::Map {
+        self.cx.tcx.hir()
+    }
 }
 
 fn report_extra_lifetimes<'tcx>(cx: &LateContext<'tcx>, func: &'tcx FnDecl<'_>, generics: &'tcx Generics<'_>) {
@@ -512,7 +538,7 @@ fn report_extra_lifetimes<'tcx>(cx: &LateContext<'tcx>, func: &'tcx FnDecl<'_>,
             _ => None,
         })
         .collect();
-    let mut checker = LifetimeChecker { map: hs };
+    let mut checker = LifetimeChecker::<hir_nested_filter::None>::new(cx, hs);
 
     walk_generics(&mut checker, generics);
     walk_fn_decl(&mut checker, func);
@@ -527,6 +553,32 @@ fn report_extra_lifetimes<'tcx>(cx: &LateContext<'tcx>, func: &'tcx FnDecl<'_>,
     }
 }
 
+fn report_extra_impl_lifetimes<'tcx>(cx: &LateContext<'tcx>, impl_: &'tcx Impl<'_>) {
+    let hs = impl_
+        .generics
+        .params
+        .iter()
+        .filter_map(|par| match par.kind {
+            GenericParamKind::Lifetime { .. } => Some((par.name.ident().name, par.span)),
+            _ => None,
+        })
+        .collect();
+    let mut checker = LifetimeChecker::<middle_nested_filter::All>::new(cx, hs);
+
+    walk_generics(&mut checker, &impl_.generics);
+    if let Some(ref trait_ref) = impl_.of_trait {
+        walk_trait_ref(&mut checker, trait_ref);
+    }
+    walk_ty(&mut checker, impl_.self_ty);
+    for item in impl_.items {
+        walk_impl_item_ref(&mut checker, item);
+    }
+
+    for &v in checker.map.values() {
+        span_lint(cx, EXTRA_UNUSED_LIFETIMES, v, "this lifetime isn't used in the impl");
+    }
+}
+
 struct BodyLifetimeChecker {
     lifetimes_used_in_body: bool,
 }

tests/ui/crashes/ice-2865.rs

@@ -1,4 +1,4 @@
-#[allow(dead_code)]
+#![allow(dead_code, clippy::extra_unused_lifetimes)]
 
 /// Test for https://github.com/rust-lang/rust-clippy/issues/2865
 

tests/ui/crashes/ice-3151.rs

@@ -1,4 +1,4 @@
-/// Test for https://github.com/rust-lang/rust-clippy/issues/2865
+/// Test for https://github.com/rust-lang/rust-clippy/issues/3151
 
 #[derive(Clone)]
 pub struct HashMap<V, S> {

tests/ui/extra_unused_lifetimes.rs

@@ -72,4 +72,46 @@ mod issue4291 {
     }
 }
 
+mod issue6437 {
+    pub struct Scalar;
+
+    impl<'a> std::ops::AddAssign<&Scalar> for &mut Scalar {
+        fn add_assign(&mut self, _rhs: &Scalar) {
+            unimplemented!();
+        }
+    }
+
+    impl<'b> Scalar {
+        pub fn something<'c>() -> Self {
+            Self
+        }
+    }
+}
+
+// https://github.com/rust-lang/rust-clippy/pull/8737#pullrequestreview-951268213
+mod first_case {
+    use serde::de::Visitor;
+    pub trait Expected {
+        fn fmt(&self, formatter: &mut std::fmt::Formatter);
+    }
+
+    impl<'de, T> Expected for T
+    where
+        T: Visitor<'de>,
+    {
+        fn fmt(&self, formatter: &mut std::fmt::Formatter) {}
+    }
+}
+
+// https://github.com/rust-lang/rust-clippy/pull/8737#pullrequestreview-951268213
+mod second_case {
+    pub trait Source {
+        fn hey();
+    }
+
+    impl<'a, T: Source + ?Sized + 'a> Source for Box<T> {
+        fn hey() {}
+    }
+}
+
 fn main() {}

tests/ui/extra_unused_lifetimes.stderr

@@ -24,5 +24,23 @@ error: this lifetime isn't used in the function definition
 LL |         fn unused_lt<'a>(x: u8) {}
    |                      ^^
 
-error: aborting due to 4 previous errors
+error: this lifetime isn't used in the impl
+  --> $DIR/extra_unused_lifetimes.rs:78:10
+   |
+LL |     impl<'a> std::ops::AddAssign<&Scalar> for &mut Scalar {
+   |          ^^
+
+error: this lifetime isn't used in the impl
+  --> $DIR/extra_unused_lifetimes.rs:84:10
+   |
+LL |     impl<'b> Scalar {
+   |          ^^
+
+error: this lifetime isn't used in the function definition
+  --> $DIR/extra_unused_lifetimes.rs:85:26
+   |
+LL |         pub fn something<'c>() -> Self {
+   |                          ^^
+
+error: aborting due to 7 previous errors
 

tests/ui/impl.rs

@@ -1,4 +1,4 @@
-#![allow(dead_code)]
+#![allow(dead_code, clippy::extra_unused_lifetimes)]
 #![warn(clippy::multiple_inherent_impl)]
 
 struct MyStruct;

tests/ui/new_without_default.rs

@@ -1,4 +1,4 @@
-#![allow(dead_code, clippy::missing_safety_doc)]
+#![allow(dead_code, clippy::missing_safety_doc, clippy::extra_unused_lifetimes)]
 #![warn(clippy::new_without_default)]
 
 pub struct Foo;