Skip to content

refactor(elidable_lifetimes): harmonize suggestion-building logic with extra_unused_type_parameters #15908

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

refactor(elidable_lifetimes): harmonize suggestion-building logic with extra_unused_type_parameters #15908

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c6426f5
Revert "fix(elidable_lifetime_names): avoid overlapping spans in sugg…
ada4a Oct 19, 2025
d666c68
Adapt the logic from `extra_unused_type_parameters`
ada4a Oct 19, 2025
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
5 changes: 5 additions & 0 deletions clippy_lints/src/extra_unused_type_parameters.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@ impl<'cx, 'tcx> TypeWalker<'cx, 'tcx> {
let spans = if explicit_params.len() == extra_params.len() {
vec![self.generics.span] // Remove the entire list of generics
} else {
// 1. Start from the last extra param
// 2. While the pars preceding it are also extra, construct spans going from the current param to
// the comma before it
// 3. Once this chain of extra params stops, switch to constructing spans going from the current
// param to the comma _after_ it
let mut end: Option<LocalDefId> = None;
extra_params
.iter()
Expand Down
110 changes: 35 additions & 75 deletions clippy_lints/src/lifetimes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -856,89 +856,49 @@ fn elision_suggestions(
.filter(|param| !param.is_elided_lifetime() && !param.is_impl_trait())
.collect::<Vec<_>>();

if !elidable_lts
.iter()
.all(|lt| explicit_params.iter().any(|param| param.def_id == *lt))
{
return None;
}

let mut suggestions = if elidable_lts.is_empty() {
vec![]
} else if elidable_lts.len() == explicit_params.len() {
let mut suggestions = if elidable_lts.len() == explicit_params.len() {
// if all the params are elided remove the whole generic block
//
// fn x<'a>() {}
// ^^^^
vec![(generics.span, String::new())]
} else {
match &explicit_params[..] {
// no params, nothing to elide
[] => unreachable!("handled by `elidable_lts.is_empty()`"),
[param] => {
if elidable_lts.contains(&param.def_id) {
unreachable!("handled by `elidable_lts.len() == explicit_params.len()`")
// 1. Start from the last elidable lifetime
// 2. While the lifetimes preceding it are also elidable, construct spans going from the current
// lifetime to the comma before it
// 3. Once this chain of elidable lifetimes stops, switch to constructing spans going from the
// current lifetime to the comma _after_ it
let mut end: Option<LocalDefId> = None;
elidable_lts
.iter()
.rev()
.map(|&id| {
let (idx, param) = explicit_params.iter().find_position(|param| param.def_id == id)?;

let span = if let Some(next) = explicit_params.get(idx + 1)
&& end != Some(next.def_id)
{
// Extend the current span forward, up until the next param in the list.
// fn x<'prev, 'a, 'next>() {}
// ^^^^
param.span.until(next.span)
} else {
unreachable!("handled by `elidable_lts.is_empty()`")
}
},
[_, _, ..] => {
// Given a list like `<'a, 'b, 'c, 'd, ..>`,
//
// If there is a cluster of elidable lifetimes at the beginning, say `'a` and `'b`, we should
// suggest removing them _and_ the trailing comma. The span for that is `a.span.until(c.span)`:
// <'a, 'b, 'c, 'd, ..> => <'a, 'b, 'c, 'd, ..>
// ^^ ^^ ^^^^^^^^
//
// And since we know that `'c` isn't elidable--otherwise it would've been in the cluster--we can go
// over all the lifetimes after it, and for each elidable one, add a suggestion spanning the
// lifetime itself and the comma before, because each individual suggestion is guaranteed to leave
// the list valid:
// <.., 'c, 'd, 'e, 'f, 'g, ..> => <.., 'c, 'd, 'e, 'f, 'g, ..>
// ^^ ^^ ^^ ^^^^ ^^^^^^^^
//
// In case there is no such starting cluster, we only need to do the second part of the algorithm:
// <'a, 'b, 'c, 'd, 'e, 'f, 'g, ..> => <'a, 'b , 'c, 'd, 'e, 'f, 'g, ..>
// ^^ ^^ ^^ ^^ ^^^^^^^^^ ^^^^^^^^

// Split off the starting cluster
// TODO: use `slice::split_once` once stabilized (github.com/rust-lang/rust/issues/112811):
// ```
// let Some(split) = explicit_params.split_once(|param| !elidable_lts.contains(&param.def_id)) else {
// // there were no lifetime param that couldn't be elided
// unreachable!("handled by `elidable_lts.len() == explicit_params.len()`")
// };
// match split { /* .. */ }
// ```
let Some(split_pos) = explicit_params
.iter()
.position(|param| !elidable_lts.contains(&param.def_id))
else {
// there were no lifetime param that couldn't be elided
unreachable!("handled by `elidable_lts.len() == explicit_params.len()`")
// Extend the current span back to include the comma following the previous
// param. If the span of the next param in the list has already been
// extended, we continue the chain. This is why we're iterating in reverse.
end = Some(param.def_id);

// `idx` will never be 0, else we'd be removing the entire list of generics
let prev = explicit_params.get(idx - 1)?;

// fn x<'prev, 'a>() {}
// ^^^^
param.span.with_lo(prev.span.hi())
};
let split = explicit_params
.split_at_checked(split_pos)
.expect("got `split_pos` from `position` on the same Vec");

match split {
([..], []) => unreachable!("handled by `elidable_lts.len() == explicit_params.len()`"),
([], [_]) => unreachable!("handled by `explicit_params.len() == 1`"),
(cluster, rest @ [rest_first, ..]) => {
// the span for the cluster
(cluster.first().map(|fw| fw.span.until(rest_first.span)).into_iter())
// the span for the remaining lifetimes (calculations independent of the cluster)
.chain(
rest.array_windows()
.filter(|[_, curr]| elidable_lts.contains(&curr.def_id))
.map(|[prev, curr]| curr.span.with_lo(prev.span.hi())),
)
.map(|sp| (sp, String::new()))
.collect()
},
}
},
}

Some((span, String::new()))
})
.collect::<Option<Vec<_>>>()?
};

suggestions.extend(usages.iter().map(|&usage| {
Expand Down