From c6426f5dd7c3cde75e6faf83d477cd016c5b44e5 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sun, 19 Oct 2025 17:52:30 +0200 Subject: [PATCH 1/2] Revert "fix(elidable_lifetime_names): avoid overlapping spans in suggestions" This reverts commit 25881bfc3400005b047c8eb553fb7bb2fab25950. --- clippy_lints/src/lifetimes.rs | 97 ++++++++--------------------------- 1 file changed, 22 insertions(+), 75 deletions(-) diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index 519ec228c884..2779052e17a2 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/clippy_lints/src/lifetimes.rs @@ -856,89 +856,36 @@ fn elision_suggestions( .filter(|param| !param.is_elided_lifetime() && !param.is_impl_trait()) .collect::>(); - 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(¶m.def_id) { - unreachable!("handled by `elidable_lts.len() == explicit_params.len()`") + elidable_lts + .iter() + .map(|&id| { + let pos = explicit_params.iter().position(|param| param.def_id == id)?; + let param = explicit_params.get(pos)?; + + let span = if let Some(next) = explicit_params.get(pos + 1) { + // 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(¶m.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(¶m.def_id)) - else { - // there were no lifetime param that couldn't be elided - unreachable!("handled by `elidable_lts.len() == explicit_params.len()`") + // `pos` should be at least 1 here, because the param in position 0 would either have a `next` + // param or would have taken the `elidable_lts.len() == explicit_params.len()` branch. + let prev = explicit_params.get(pos - 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::>>()? }; suggestions.extend(usages.iter().map(|&usage| { From 17f194238d696fb7bcb8f293fe70bcab1cf01469 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sun, 19 Oct 2025 18:32:06 +0200 Subject: [PATCH 2/2] Adapt the logic from `extra_unused_type_parameters` .. and add some docs to both of them --- .../src/extra_unused_type_parameters.rs | 5 ++++ clippy_lints/src/lifetimes.rs | 25 ++++++++++++++----- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/extra_unused_type_parameters.rs b/clippy_lints/src/extra_unused_type_parameters.rs index c0b0fd88d9e1..fb98cb30ae90 100644 --- a/clippy_lints/src/extra_unused_type_parameters.rs +++ b/clippy_lints/src/extra_unused_type_parameters.rs @@ -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 params 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 = None; extra_params .iter() diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index 2779052e17a2..727e9b172a87 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/clippy_lints/src/lifetimes.rs @@ -863,20 +863,33 @@ fn elision_suggestions( // ^^^^ vec![(generics.span, String::new())] } else { + // 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 = None; elidable_lts .iter() + .rev() .map(|&id| { - let pos = explicit_params.iter().position(|param| param.def_id == id)?; - let param = explicit_params.get(pos)?; + let (idx, param) = explicit_params.iter().find_position(|param| param.def_id == id)?; - let span = if let Some(next) = explicit_params.get(pos + 1) { + 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 { - // `pos` should be at least 1 here, because the param in position 0 would either have a `next` - // param or would have taken the `elidable_lts.len() == explicit_params.len()` branch. - let prev = explicit_params.get(pos - 1)?; + // 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>() {} // ^^^^