From 88750f9ad7dc8e7b2138f9a4a960937317e4100d Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sat, 31 Aug 2019 08:16:04 +0200 Subject: [PATCH 1/3] Fix `extra_unused_lifetimes` false positive Fixes #4291 --- clippy_lints/src/lifetimes.rs | 6 ++++-- tests/ui/extra_unused_lifetimes.rs | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index 72e28d23ebdb..3a7863a02014 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/clippy_lints/src/lifetimes.rs @@ -9,7 +9,7 @@ use syntax::source_map::Span; use syntax::symbol::kw; use crate::reexport::*; -use crate::utils::{last_path_segment, span_lint}; +use crate::utils::{last_path_segment, span_lint, trait_ref_of_method}; declare_clippy_lint! { /// **What it does:** Checks for lifetime annotations which can be removed by @@ -66,7 +66,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Lifetimes { fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx ImplItem) { if let ImplItemKind::Method(ref sig, id) = item.node { - check_fn_inner(cx, &sig.decl, Some(id), &item.generics, item.span); + if trait_ref_of_method(cx, item.hir_id).is_none() { + check_fn_inner(cx, &sig.decl, Some(id), &item.generics, item.span); + } } } diff --git a/tests/ui/extra_unused_lifetimes.rs b/tests/ui/extra_unused_lifetimes.rs index 9f05f8c1ed02..66f6e67a75c2 100644 --- a/tests/ui/extra_unused_lifetimes.rs +++ b/tests/ui/extra_unused_lifetimes.rs @@ -61,4 +61,23 @@ impl X { fn explicit_self_with_lifetime<'a>(self: &'a Self) {} } +// Methods implementing traits must have matching lifetimes +mod issue4291 { + #[derive(Debug)] + pub struct Foo<'a>(&'a std::marker::PhantomData); + + #[derive(Debug)] + pub struct Bar<'a: 'b, 'b>(Foo<'a>, &'b std::marker::PhantomData); + + trait LT { + fn test<'a: 'b, 'b>(foo: &Foo<'a>, bar: &Bar<'a, 'b>); + } + + pub struct Baz; + + impl LT for Baz { + fn test<'a: 'b, 'b>(_foo: &Foo, _bar: &Bar) {} + } +} + fn main() {} From 4458bef5d153e81ebd5f21c29a75e37e44f997f1 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sun, 1 Sep 2019 07:55:29 +0200 Subject: [PATCH 2/3] Simplify issue-4291 test --- tests/ui/extra_unused_lifetimes.rs | 16 ++++------------ tests/ui/extra_unused_lifetimes.stderr | 8 +++++++- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/tests/ui/extra_unused_lifetimes.rs b/tests/ui/extra_unused_lifetimes.rs index 66f6e67a75c2..ba95fd63bf9a 100644 --- a/tests/ui/extra_unused_lifetimes.rs +++ b/tests/ui/extra_unused_lifetimes.rs @@ -63,20 +63,12 @@ impl X { // Methods implementing traits must have matching lifetimes mod issue4291 { - #[derive(Debug)] - pub struct Foo<'a>(&'a std::marker::PhantomData); - - #[derive(Debug)] - pub struct Bar<'a: 'b, 'b>(Foo<'a>, &'b std::marker::PhantomData); - - trait LT { - fn test<'a: 'b, 'b>(foo: &Foo<'a>, bar: &Bar<'a, 'b>); + trait BadTrait { + fn unused_lt<'a>(x: u8) {} } - pub struct Baz; - - impl LT for Baz { - fn test<'a: 'b, 'b>(_foo: &Foo, _bar: &Bar) {} + impl BadTrait for () { + fn unused_lt<'a>(_x: u8) {} } } diff --git a/tests/ui/extra_unused_lifetimes.stderr b/tests/ui/extra_unused_lifetimes.stderr index 6473a2fbcb28..ebdb8e749520 100644 --- a/tests/ui/extra_unused_lifetimes.stderr +++ b/tests/ui/extra_unused_lifetimes.stderr @@ -18,5 +18,11 @@ error: this lifetime isn't used in the function definition LL | fn x<'a>(&self) {} | ^^ -error: aborting due to 3 previous errors +error: this lifetime isn't used in the function definition + --> $DIR/extra_unused_lifetimes.rs:67:22 + | +LL | fn unused_lt<'a>(x: u8) {} + | ^^ + +error: aborting due to 4 previous errors From 2fdfd60569f57c650b2b7875a683ab6daf774faa Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Sun, 1 Sep 2019 08:11:40 +0200 Subject: [PATCH 3/3] Fix `needless_lifetimes` false positive --- clippy_lints/src/lifetimes.rs | 21 +++++++++++++++------ tests/ui/needless_lifetimes.rs | 11 +++++++++++ tests/ui/needless_lifetimes.stderr | 14 +++++++++++++- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index 3a7863a02014..e75e9c6541be 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/clippy_lints/src/lifetimes.rs @@ -60,15 +60,21 @@ declare_lint_pass!(Lifetimes => [NEEDLESS_LIFETIMES, EXTRA_UNUSED_LIFETIMES]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Lifetimes { fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) { if let ItemKind::Fn(ref decl, _, ref generics, id) = item.node { - check_fn_inner(cx, decl, Some(id), generics, item.span); + check_fn_inner(cx, decl, Some(id), generics, item.span, true); } } fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx ImplItem) { if let ImplItemKind::Method(ref sig, id) = item.node { - if trait_ref_of_method(cx, item.hir_id).is_none() { - check_fn_inner(cx, &sig.decl, Some(id), &item.generics, item.span); - } + let report_extra_lifetimes = trait_ref_of_method(cx, item.hir_id).is_none(); + check_fn_inner( + cx, + &sig.decl, + Some(id), + &item.generics, + item.span, + report_extra_lifetimes, + ); } } @@ -78,7 +84,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Lifetimes { TraitMethod::Required(_) => None, TraitMethod::Provided(id) => Some(id), }; - check_fn_inner(cx, &sig.decl, body, &item.generics, item.span); + check_fn_inner(cx, &sig.decl, body, &item.generics, item.span, true); } } } @@ -97,6 +103,7 @@ fn check_fn_inner<'a, 'tcx>( body: Option, generics: &'tcx Generics, span: Span, + report_extra_lifetimes: bool, ) { if in_external_macro(cx.sess(), span) || has_where_lifetimes(cx, &generics.where_clause) { return; @@ -146,7 +153,9 @@ fn check_fn_inner<'a, 'tcx>( (or replaced with `'_` if needed by type declaration)", ); } - report_extra_lifetimes(cx, decl, generics); + if report_extra_lifetimes { + self::report_extra_lifetimes(cx, decl, generics); + } } fn could_use_elision<'a, 'tcx>( diff --git a/tests/ui/needless_lifetimes.rs b/tests/ui/needless_lifetimes.rs index f585be2fd03f..f3fdd48633f8 100644 --- a/tests/ui/needless_lifetimes.rs +++ b/tests/ui/needless_lifetimes.rs @@ -248,4 +248,15 @@ fn out_return_type_lts<'a>(e: &'a str) -> Cow<'a> { unimplemented!() } +// Make sure we still warn on implementations +mod issue4291 { + trait BadTrait { + fn needless_lt<'a>(x: &'a u8) {} + } + + impl BadTrait for () { + fn needless_lt<'a>(_x: &'a u8) {} + } +} + fn main() {} diff --git a/tests/ui/needless_lifetimes.stderr b/tests/ui/needless_lifetimes.stderr index bbb69aeda1c4..ad55fc5f750d 100644 --- a/tests/ui/needless_lifetimes.stderr +++ b/tests/ui/needless_lifetimes.stderr @@ -118,5 +118,17 @@ LL | | unimplemented!() LL | | } | |_^ -error: aborting due to 15 previous errors +error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) + --> $DIR/needless_lifetimes.rs:254:9 + | +LL | fn needless_lt<'a>(x: &'a u8) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) + --> $DIR/needless_lifetimes.rs:258:9 + | +LL | fn needless_lt<'a>(_x: &'a u8) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 17 previous errors