Skip to content

Split needless_lifetime '_ suggestions into elidable_lifetime_names #13960

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5592,6 +5592,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 @@ -273,6 +273,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
112 changes: 86 additions & 26 deletions clippy_lints/src/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ declare_clippy_lint! {
/// them leads to more readable code.
///
/// ### Known problems
/// - We bail out if the function has a `where` clause where lifetimes
/// are mentioned due to potential false positives.
/// This lint ignores functions with `where` clauses that reference
/// lifetimes to prevent false positives.
///
/// ### Example
/// ```no_run
Expand All @@ -62,6 +62,38 @@ 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
/// This lint ignores functions with `where` clauses that reference
/// lifetimes to prevent 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 @@ -104,7 +136,11 @@ impl Lifetimes {
}
}

impl_lint_pass!(Lifetimes => [NEEDLESS_LIFETIMES, EXTRA_UNUSED_LIFETIMES]);
impl_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 @@ -746,6 +782,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 @@ -763,9 +808,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 @@ -785,7 +850,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 @@ -796,7 +861,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 @@ -836,26 +901,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
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 All @@ -10,11 +10,11 @@ struct Baz<'a> {
}

impl Foo for Baz<'_> {}
//~^ needless_lifetimes
//~^ elidable_lifetime_names

impl Bar {
fn baz(&self) -> impl Foo + '_ {
//~^ needless_lifetimes
//~^ elidable_lifetime_names

Baz { bar: self }
}
Expand Down
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 All @@ -10,11 +10,11 @@ struct Baz<'a> {
}

impl<'a> Foo for Baz<'a> {}
//~^ needless_lifetimes
//~^ elidable_lifetime_names

impl Bar {
fn baz<'a>(&'a self) -> impl Foo + 'a {
//~^ needless_lifetimes
//~^ elidable_lifetime_names

Baz { bar: self }
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
error: the following explicit lifetimes could be elided: 'a
--> tests/ui/crashes/needless_lifetimes_impl_trait.rs:12:6
--> tests/ui/crashes/elidable_lifetime_names_impl_trait.rs:12:6
|
LL | impl<'a> Foo for Baz<'a> {}
| ^^ ^^
|
note: the lint level is defined here
--> tests/ui/crashes/needless_lifetimes_impl_trait.rs:1:9
--> tests/ui/crashes/elidable_lifetime_names_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> {}
LL + impl Foo for Baz<'_> {}
|

error: the following explicit lifetimes could be elided: 'a
--> tests/ui/crashes/needless_lifetimes_impl_trait.rs:16:12
--> tests/ui/crashes/elidable_lifetime_names_impl_trait.rs:16:12
|
LL | fn baz<'a>(&'a self) -> impl Foo + 'a {
| ^^ ^^ ^^
Expand Down
Loading