Skip to content

Commit

Permalink
Split needless_lifetime '_ suggestions into elidable_lifetime_names
Browse files Browse the repository at this point in the history
  • Loading branch information
Alexendoo committed Jan 7, 2025
1 parent 53994bd commit 5b9427e
Show file tree
Hide file tree
Showing 14 changed files with 456 additions and 377 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5458,6 +5458,7 @@ Released 2018-09-13
[`duplicated_attributes`]: https://rust-lang.github.io/rust-clippy/master/index.html#duplicated_attributes
[`duration_subsec`]: https://rust-lang.github.io/rust-clippy/master/index.html#duration_subsec
[`eager_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#eager_transmute
[`elidable_lifetime_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#elidable_lifetime_names
[`else_if_without_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#else_if_without_else
[`empty_docs`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_docs
[`empty_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_drop
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::let_underscore::LET_UNDERSCORE_MUST_USE_INFO,
crate::let_underscore::LET_UNDERSCORE_UNTYPED_INFO,
crate::let_with_type_underscore::LET_WITH_TYPE_UNDERSCORE_INFO,
crate::lifetimes::ELIDABLE_LIFETIME_NAMES_INFO,
crate::lifetimes::EXTRA_UNUSED_LIFETIMES_INFO,
crate::lifetimes::NEEDLESS_LIFETIMES_INFO,
crate::lines_filter_map_ok::LINES_FILTER_MAP_OK_INFO,
Expand Down
110 changes: 86 additions & 24 deletions clippy_lints/src/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,40 @@ declare_clippy_lint! {
would allow omitting them"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for lifetime annotations which can be replaced with anonymous lifetimes (`'_`).
///
/// ### Why is this bad?
/// The additional lifetimes can make the code look more complicated.
///
/// ### Known problems
/// - We bail out if the function has a `where` clause where lifetimes
/// are mentioned due to potential false positives.
///
/// ### Example
/// ```no_run
/// use std::str::Chars;
///
/// fn f<'a>(x: &'a str) -> Chars<'a> {
/// x.chars()
/// }
/// ```
///
/// Use instead:
/// ```no_run
/// use std::str::Chars;
///
/// fn f(x: &str) -> Chars<'_> {
/// x.chars()
/// }
/// ```
#[clippy::version = "1.84.0"]
pub ELIDABLE_LIFETIME_NAMES,
pedantic,
"lifetime name that can be replaced with the anonymous lifetime"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for lifetimes in generics that are never used
Expand Down Expand Up @@ -89,7 +123,11 @@ declare_clippy_lint! {
"unused lifetimes in function definitions"
}

declare_lint_pass!(Lifetimes => [NEEDLESS_LIFETIMES, EXTRA_UNUSED_LIFETIMES]);
declare_lint_pass!(Lifetimes => [
NEEDLESS_LIFETIMES,
ELIDABLE_LIFETIME_NAMES,
EXTRA_UNUSED_LIFETIMES,
]);

impl<'tcx> LateLintPass<'tcx> for Lifetimes {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
Expand Down Expand Up @@ -644,6 +682,15 @@ fn report_elidable_impl_lifetimes<'tcx>(
report_elidable_lifetimes(cx, impl_.generics, &elidable_lts, &usages, true);
}

#[derive(Copy, Clone)]
enum ElidableUsage {
/// Used in a ref (`&'a T`), can be removed
Ref(Span),
/// Used as a generic param (`T<'a>`) or an impl lifetime (`impl T + 'a`), can be replaced
/// with `'_`
Other(Span),
}

/// Generate diagnostic messages for elidable lifetimes.
fn report_elidable_lifetimes(
cx: &LateContext<'_>,
Expand All @@ -661,9 +708,29 @@ fn report_elidable_lifetimes(
.collect::<Vec<_>>()
.join(", ");

let elidable_usages: Vec<ElidableUsage> = usages
.iter()
.filter(|usage| named_lifetime(usage).is_some_and(|id| elidable_lts.contains(&id)))
.map(|usage| match cx.tcx.parent_hir_node(usage.hir_id) {
Node::Ty(Ty {
kind: TyKind::Ref(..), ..
}) => ElidableUsage::Ref(usage.ident.span),
_ => ElidableUsage::Other(usage.ident.span),
})
.collect();

let lint = if elidable_usages
.iter()
.any(|usage| matches!(usage, ElidableUsage::Other(_)))
{
ELIDABLE_LIFETIME_NAMES
} else {
NEEDLESS_LIFETIMES
};

span_lint_and_then(
cx,
NEEDLESS_LIFETIMES,
lint,
elidable_lts
.iter()
.map(|&lt| cx.tcx.def_span(lt))
Expand All @@ -683,7 +750,7 @@ fn report_elidable_lifetimes(
return;
};

if let Some(suggestions) = elision_suggestions(cx, generics, elidable_lts, usages) {
if let Some(suggestions) = elision_suggestions(cx, generics, elidable_lts, &elidable_usages) {
diag.multipart_suggestion("elide the lifetimes", suggestions, Applicability::MachineApplicable);
}
},
Expand All @@ -694,7 +761,7 @@ fn elision_suggestions(
cx: &LateContext<'_>,
generics: &Generics<'_>,
elidable_lts: &[LocalDefId],
usages: &[Lifetime],
usages: &[ElidableUsage],
) -> Option<Vec<(Span, String)>> {
let explicit_params = generics
.params
Expand Down Expand Up @@ -734,26 +801,21 @@ fn elision_suggestions(
.collect::<Option<Vec<_>>>()?
};

suggestions.extend(
usages
.iter()
.filter(|usage| named_lifetime(usage).is_some_and(|id| elidable_lts.contains(&id)))
.map(|usage| {
match cx.tcx.parent_hir_node(usage.hir_id) {
Node::Ty(Ty {
kind: TyKind::Ref(..), ..
}) => {
// expand `&'a T` to `&'a T`
// ^^ ^^^
let span = cx.sess().source_map().span_extend_while_whitespace(usage.ident.span);

(span, String::new())
},
// `T<'a>` and `impl Foo + 'a` should be replaced by `'_`
_ => (usage.ident.span, String::from("'_")),
}
}),
);
suggestions.extend(usages.iter().map(|&usage| {
match usage {
ElidableUsage::Ref(span) => {
// expand `&'a T` to `&'a T`
// ^^ ^^^
let span = cx.sess().source_map().span_extend_while_whitespace(span);

(span, String::new())
},
ElidableUsage::Other(span) => {
// `T<'a>` and `impl Foo + 'a` should be replaced by `'_`
(span, String::from("'_"))
},
}
}));

Some(suggestions)
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/crashes/needless_lifetimes_impl_trait.fixed
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![deny(clippy::needless_lifetimes)]
#![deny(clippy::elidable_lifetime_names)]
#![allow(dead_code)]

trait Foo {}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/crashes/needless_lifetimes_impl_trait.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![deny(clippy::needless_lifetimes)]
#![deny(clippy::elidable_lifetime_names)]
#![allow(dead_code)]

trait Foo {}
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/crashes/needless_lifetimes_impl_trait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ LL | impl<'a> Foo for Baz<'a> {}
note: the lint level is defined here
--> tests/ui/crashes/needless_lifetimes_impl_trait.rs:1:9
|
LL | #![deny(clippy::needless_lifetimes)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | #![deny(clippy::elidable_lifetime_names)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: elide the lifetimes
|
LL - impl<'a> Foo for Baz<'a> {}
Expand Down
108 changes: 108 additions & 0 deletions tests/ui/elidable_lifetime_names.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
#![warn(clippy::needless_lifetimes, clippy::elidable_lifetime_names)]

type Ref<'r> = &'r u8;

// No error; same lifetime on two params.
fn lifetime_param_1<'a>(_x: Ref<'a>, _y: &'a u8) {}

fn lifetime_param_2(_x: Ref<'_>, _y: &u8) {}

// No error; bounded lifetime.
fn lifetime_param_3<'a, 'b: 'a>(_x: Ref<'a>, _y: &'b u8) {}

// No error; bounded lifetime.
fn lifetime_param_4<'a, 'b>(_x: Ref<'a>, _y: &'b u8)
where
'b: 'a,
{
}

struct Lt<'a, I: 'static> {
x: &'a I,
}

// No error; fn bound references `'a`.
fn fn_bound<'a, F, I>(_m: Lt<'a, I>, _f: F) -> Lt<'a, I>
where
F: Fn(Lt<'a, I>) -> Lt<'a, I>,
{
unreachable!()
}

fn fn_bound_2<F, I>(_m: Lt<'_, I>, _f: F) -> Lt<'_, I>
where
for<'x> F: Fn(Lt<'x, I>) -> Lt<'x, I>,
{
unreachable!()
}

struct Foo<'a>(&'a u8);

fn struct_with_lt(_foo: Foo<'_>) -> &str {
unimplemented!()
}

// No warning; two input lifetimes (named on the reference, anonymous on `Foo`).
fn struct_with_lt2<'a>(_foo: &'a Foo) -> &'a str {
unimplemented!()
}

// No warning; two input lifetimes (anonymous on the reference, named on `Foo`).
fn struct_with_lt3<'a>(_foo: &Foo<'a>) -> &'a str {
unimplemented!()
}

// Warning; two input lifetimes, but the output lifetime is not elided, i.e., the following is
// valid:
// fn struct_with_lt4a<'a>(_foo: &'a Foo<'_>) -> &'a str
// ^^
fn struct_with_lt4a<'a>(_foo: &'a Foo<'_>) -> &'a str {
unimplemented!()
}

type FooAlias<'a> = Foo<'a>;

fn alias_with_lt(_foo: FooAlias<'_>) -> &str {
unimplemented!()
}

// No warning; two input lifetimes (named on the reference, anonymous on `FooAlias`).
fn alias_with_lt2<'a>(_foo: &'a FooAlias) -> &'a str {
unimplemented!()
}

// No warning; two input lifetimes (anonymous on the reference, named on `FooAlias`).
fn alias_with_lt3<'a>(_foo: &FooAlias<'a>) -> &'a str {
unimplemented!()
}

// Warning; two input lifetimes, but the output lifetime is not elided, i.e., the following is
// valid:
// fn alias_with_lt4a<'a>(_foo: &'a FooAlias<'_>) -> &'a str
// ^^
fn alias_with_lt4a<'a>(_foo: &'a FooAlias<'_>) -> &'a str {
unimplemented!()
}

// Issue #3284: give hint regarding lifetime in return type.
struct Cow<'a> {
x: &'a str,
}
fn out_return_type_lts(e: &str) -> Cow<'_> {
unimplemented!()
}

mod issue2944 {
trait Foo {}
struct Bar;
struct Baz<'a> {
bar: &'a Bar,
}

impl Foo for Baz<'_> {}
impl Bar {
fn baz(&self) -> impl Foo + '_ {
Baz { bar: self }
}
}
}
Loading

0 comments on commit 5b9427e

Please sign in to comment.