Skip to content

Commit

Permalink
Split needless_lifetime '_ suggestions into `elidable_lifetime_na…
Browse files Browse the repository at this point in the history
…mes` (#13960)

Fixes rust-lang/rust-clippy#13514

changelog: Added [`elidable_lifetime_names`] to `pedantic` (Split off
from [`needless_lifetime`] for suggestions with `'_`)
[#13960](rust-lang/rust-clippy#13960)

changelog: Enhancement: [`needless_lifetime`] No longer lints for
elidable lifetimes `'_`, use [`elidable_lifetime_names`] to lint these.
[#13960](rust-lang/rust-clippy#13960)
  • Loading branch information
xFrednet authored Feb 27, 2025
2 parents 52bf26e + efcf1f5 commit f50266a
Show file tree
Hide file tree
Showing 14 changed files with 688 additions and 638 deletions.
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 @@ -274,6 +274,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

0 comments on commit f50266a

Please sign in to comment.