diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index b09c23f31e97..54b80bf3b27d 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/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, + phantom: std::marker::PhantomData, } -impl<'tcx> Visitor<'tcx> for LifetimeChecker { +impl<'cx, 'tcx, F> LifetimeChecker<'cx, 'tcx, F> { + fn new(cx: &'cx LateContext<'tcx>, map: FxHashMap) -> 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::::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::::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, } diff --git a/tests/ui/crashes/ice-2865.rs b/tests/ui/crashes/ice-2865.rs index 6b1ceb505693..c62981396016 100644 --- a/tests/ui/crashes/ice-2865.rs +++ b/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 diff --git a/tests/ui/crashes/ice-3151.rs b/tests/ui/crashes/ice-3151.rs index fef4d7db84dd..268ba86fc7aa 100644 --- a/tests/ui/crashes/ice-3151.rs +++ b/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 { diff --git a/tests/ui/extra_unused_lifetimes.rs b/tests/ui/extra_unused_lifetimes.rs index 150acfbfee75..f76127a7105f 100644 --- a/tests/ui/extra_unused_lifetimes.rs +++ b/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 { + fn hey() {} + } +} + fn main() {} diff --git a/tests/ui/extra_unused_lifetimes.stderr b/tests/ui/extra_unused_lifetimes.stderr index ebdb8e749520..68f02c7ad0f4 100644 --- a/tests/ui/extra_unused_lifetimes.stderr +++ b/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 diff --git a/tests/ui/impl.rs b/tests/ui/impl.rs index 394437750153..aea52a852f98 100644 --- a/tests/ui/impl.rs +++ b/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; diff --git a/tests/ui/new_without_default.rs b/tests/ui/new_without_default.rs index e94f99c95f48..538927b18055 100644 --- a/tests/ui/new_without_default.rs +++ b/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;