E583 fix(elidable_lifetime_names): avoid overlapping spans in suggestions · rust-lang/rust-clippy@aa1cea1 · GitHub
[go: up one dir, main page]
Skip to content

Commit aa1cea1

Browse files
ada4aada4a
committed
fix(elidable_lifetime_names): avoid overlapping spans in suggestions
1 parent 382c964 commit aa1cea1

File tree

7 files changed

+424
-24
lines changed

7 files changed

+424
-24
lines changed

‎clippy_lints/src/lifetimes.rs‎

Lines changed: 87 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -856,36 +856,100 @@ fn elision_suggestions(
856856
.filter(|param| !param.is_elided_lifetime() && !param.is_impl_trait())
857857
.collect::<Vec<_>>();
858858

859-
let mut suggestions = if elidable_lts.len() == explicit_params.len() {
859+
if !elidable_lts
860+
.iter()
861+
.all(|lt| explicit_params.iter().any(|param| param.def_id == *lt))
862+
{
863+
return None;
864+
}
865+
866+
let mut suggestions = if elidable_lts.is_empty() {
867+
vec![]
868+
} else if elidable_lts.len() == explicit_params.len() {
860869
// if all the params are elided remove the whole generic block
861870
//
862871
// fn x<'a>() {}
863872
// ^^^^
864873
vec![(generics.span, String::new())]
865874
} else {
866-
elidable_lts
867-
.iter()
868-
.map(|&id| {
869-
let pos = explicit_params.iter().position(|param| param.def_id == id)?;
870-
let param = explicit_params.get(pos)?;
871-
872-
let span = if let Some(next) = explicit_params.get(pos + 1) {
873-
// fn x<'prev, 'a, 'next>() {}
874-
// ^^^^
875-
param.span.until(next.span)
875+
match &explicit_params[..] {
876+
// no params, nothing to elide
877+
[] => unreachable!("handled by `elidable_lts.is_empty()`"),
878+
[param] => {
879+
if elidable_lts.contains(&param.def_id) {
880+
unreachable!("handled by `elidable_lts.len() == explicit_params.len()`")
876881
} else {
877-
// `pos` should be at least 1 here, because the param in position 0 would either have a `next`
878-
// param or would have taken the `elidable_lts.len() == explicit_params.len()` branch.
879-
let prev = explicit_params.get(pos - 1)?;
880-
881-
// fn x<'prev, 'a>() {}
882-
// ^^^^
883-
param.span.with_lo(prev.span.hi())
884-
};
885-
886-
Some((span, String::new()))
887-
})
888-
.collect::<Option<Vec<_>>>()?
882+
unreachable!("handled by `elidable_lts.is_empty()`")
883+
}
884+
},
885+
[first, second] => {
886+
// NOTE: we could theoretically remove this case for the next one,
887+
// but the latter likely has quite a lot of overhead, so don't
888+
match (
889+
elidable_lts.contains(&first.def_id),
890+
elidable_lts.contains(&second.def_id),
891+
) {
892+
(false, false) => unreachable!("handled by `elidable_lts.is_empty()`"),
893+
(true, true) => unreachable!("handled by `elidable_lts.len() == explicit_params.len()`"),
894+
895+
// <'a, 'b> -> <'a, 'b>
896+
// ^^ ^^^^
897+
(false, true) => vec![(second.span.with_lo(first.span.hi()), String::new())],
898+
// <'a, 'b> -> <'a, 'b>
899+
// ^^ ^^^^
900+
(true, false) => vec![(first.span.until(second.span), String::new())],
901+
}
902+
},
903+
// Time for the big guns
904+
[_, _, ..] => {
905+
// Given a list like `<'a, 'b, 'c, 'd, ..>`,
906+
//
907+
// If there is a cluster of elidable lifetimes at the beginning, say `'a` and `'b`, we should
908+
// suggest removing them _and_ the trailing comma. The span for that is `a.span.until(c.span)`:
909+
// <'a, 'b, 'c, 'd, ..> => <'a, 'b, 'c, 'd, ..>
910+
// ^^ ^^ ^^^^^^^^
911+
//
912+
// And since we know that `'c` isn't elidable--otherwise it would've been in the cluster--we can go
913+
// over all the lifetimes after it, and for each elidable one, add a suggestion spanning the
914+
// lifetime itself and the comma before, because each individual suggestion is guaranteed to leave
915+
// the list valid:
916+
// <.., 'c, 'd, 'e, 'f, 'g, ..> => <.., 'c, 'd, 'e, 'f, 'g, ..>
917+
// ^^ ^^ ^^ ^^^^ ^^^^^^^^
918+
//
919+
// In case there is no such starting cluster, we only need to do the second part of the algorithm:
920+
// <'a, 'b, 'c, 'd, 'e, 'f, 'g, ..> => <'a, 'b , 'c, 'd, 'e, 'f, 'g, ..>
921+
// ^^ ^^ ^^ ^^ ^^^^^^^^^ ^^^^^^^^
922+
923+
// Split off the starting cluster
924+
if let Some(split_pos) = explicit_params
925+
.iter()
926+
.position(|param| !elidable_lts.contains(&param.def_id))
927+
{
928+
match explicit_params
929+
.split_at_checked(split_pos)
930+
.expect("got `split_pos` from `position` on the same Vec")
931+
{
932+
([..], []) => unreachable!("handled by `elidable_lts.len() == explicit_params.len()`"),
933+
([], [_]) => unreachable!("handled by `explicit_params.len() == 1`"),
934+
(cluster, rest @ [rest_first, ..]) => {
935+
// the span for the cluster
936+
(cluster.first().map(|fw| fw.span.until(rest_first.span)).into_iter())
937+
// the span for the remaining lifetimes (calculations independent of the cluster)
938+
.chain(
939+
rest.array_windows()
940+
.filter(|[_, curr]| elidable_lts.contains(&curr.def_id))
941+
.map(|[prev, curr]| curr.span.with_lo(prev.span.hi())),
942+
)
943+
.map(|sp| (sp, String::new()))
944+
.collect()
945+
},
946+
}
947+
} else {
948+
// there were no lifetime param that couldn't be elided
949+
unreachable!("handled by `elidable_lts.len() == explicit_params.len()`")
950+
}
951+
},
952+
}
889953
};
890954

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

‎tests/ui/crashes/ice-15666.fixed‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#![warn(clippy::elidable_lifetime_names)]
2+
3+
struct UnitVariantAccess<'a, 'b, 's>(&'a &'b &'s ());
4+
trait Trait<'de> {}
5+
impl<'de> Trait<'de> for UnitVariantAccess<'_, 'de, '_> {}
6+
//~^ elidable_lifetime_names

‎tests/ui/crashes/ice-15666.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#![warn(clippy::elidable_lifetime_names)]
2+
3+
struct UnitVariantAccess<'a, 'b, 's>(&'a &'b &'s ());
4+
trait Trait<'de> {}
5+
impl<'de, 'a, 's> Trait<'de> for UnitVariantAccess<'a, 'de, 's> {}
6+
//~^ elidable_lifetime_names

‎tests/ui/crashes/ice-15666.stderr‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: the following explicit lifetimes could be elided: 'a, 's
2+
--> tests/ui/crashes/ice-15666.rs:5:11
3+
|
4+
LL | impl<'de, 'a, 's> Trait<'de> for UnitVariantAccess<'a, 'de, 's> {}
5+
| ^^ ^^ ^^ ^^
6+
|
7+
= note: `-D clippy::elidable-lifetime-names` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::elidable_lifetime_names)]`
9+
help: elide the lifetimes
10+
|
11+
LL - impl<'de, 'a, 's> Trait<'de> for UnitVariantAccess<'a, 'de, 's> {}
12+
LL + impl<'de> Trait<'de> for UnitVariantAccess<'_, 'de, '_> {}
13+
|
14+
15+
error: aborting due to 1 previous error
16+

‎tests/ui/elidable_lifetime_names.fixed‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,3 +192,85 @@ mod issue13923 {
192192
x.b
193193
}
194194
}
195+
196+
fn issue15666_original() {
197+
struct UnitVariantAccess<'a, 'b, 's>(&'a &'b &'s ());
198+
199+
trait Trait<'de> {}
200+
201+
//~v elidable_lifetime_names
202+
impl<'de> Trait<'de> for UnitVariantAccess<'_, 'de, '_> {}
203+
// ^^ ^^ ^^ ^^
204+
}
205+
206+
#[allow(clippy::upper_case_acronyms)]
207+
fn issue15666() {
208+
struct S1<'a>(&'a ());
209+
struct S2<'a, 'b>(&'a &'b ());
210+
struct S3<'a, 'b, 'c>(&'a &'b &'c ());
211+
212+
trait T {}
213+
trait TA<'a> {}
214+
trait TB<'b> {}
215+
trait TC<'c> {}
216+
trait TAB<'a, 'b> {}
217+
trait TAC<'a, 'c> {}
218+
trait TBC<'b, 'c> {}
219+
trait TABC<'a, 'b, 'c> {}
220+
221+
// 1 lifetime
222+
223+
impl<'a> TA<'a> for S1<'a> {}
224+
225+
//~v elidable_lifetime_names
226+
impl T for S1<'_> {}
227+
// ^^
228+
229+
// 2 lifetimes
230+
231+
impl<'a, 'b> TAB<'a, 'b> for S2<'a, 'b> {}
232+
233+
//~v elidable_lifetime_names
234+
impl<'a> TA<'a> for S2<'a, '_> {}
235+
// ^^
236+
237+
//~v elidable_lifetime_names
238+
impl<'b> TB<'b> for S2<'_, 'b> {}
239+
// ^^
240+
241+
//~v elidable_lifetime_names
242+
impl T for S2<'_, '_> {}
243+
// ^^ ^^
244+
245+
// 3 lifetimes
246+
247+
impl<'a, 'b, 'c> TABC<'a, 'b, 'c> for S3<'a, 'b, 'c> {}
248+
249+
//~v elidable_lifetime_names
250+
impl<'a, 'b> TAB<'a, 'b> for S3<'a, 'b, '_> {}
251+
// ^^
252+
253+
//~v elidable_lifetime_names
254+
impl<'a, 'c> TAC<'a, 'c> for S3<'a, '_, 'c> {}
255+
// ^^
256+
257+
//~v elidable_lifetime_names
258+
impl<'a> TA<'a> for S3<'a, '_, '_> {}
259+
// ^^ ^^
260+
261+
//~v elidable_lifetime_names
262+
impl<'b, 'c> TBC<'b, 'c> for S3<'_, 'b, 'c> {}
263+
// ^^
264+
265+
//~v elidable_lifetime_names
266+
impl<'b> TB<'b> for S3<'_, 'b, '_> {}
267+
// ^^ ^^
268+
269+
//~v elidable_lifetime_names
270+
impl<'c> TC<'c> for S3<'_, '_, 'c> {}
271+
// ^^ ^^
272+
273+
//~v elidable_lifetime_names
274+
impl T for S3<'_, '_, '_> {}
275+
// ^^ ^^ ^^
276+
}

‎tests/ui/elidable_lifetime_names.rs‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,3 +192,85 @@ mod issue13923 {
192192
x.b
193193
}
194< BD9E /td>194
}
195+
196+
fn issue15666_original() {
197+
struct UnitVariantAccess<'a, 'b, 's>(&'a &'b &'s ());
198+
199+
trait Trait<'de> {}
200+
201+
//~v elidable_lifetime_names
202+
impl<'de, 'a, 's> Trait<'de> for UnitVariantAccess<'a, 'de, 's> {}
203+
// ^^ ^^ ^^ ^^
204+
}
205+
206+
#[allow(clippy::upper_case_acronyms)]
207+
fn issue15666() {
208+
struct S1<'a>(&'a ());
209+
struct S2<'a, 'b>(&'a &'b ());
210+
struct S3<'a, 'b, 'c>(&'a &'b &'c ());
211+
212+
trait T {}
213+
trait TA<'a> {}
214+
trait TB<'b> {}
215+
trait TC<'c> {}
216+
trait TAB<'a, 'b> {}
217+
trait TAC<'a, 'c> {}
218+
trait TBC<'b, 'c> {}
219+
trait TABC<'a, 'b, 'c> {}
220+
221+
// 1 lifetime
222+
223+
impl<'a> TA<'a> for S1<'a> {}
224+
225+
//~v elidable_lifetime_names
226+
impl<'a> T for S1<'a> {}
227+
// ^^
228+
229+
// 2 lifetimes
230+
231+
impl<'a, 'b> TAB<'a, 'b> for S2<'a, 'b> {}
232+
233+
//~v elidable_lifetime_names
234+
impl<'a, 'b> TA<'a> for S2<'a, 'b> {}
235+
// ^^
236+
237+
//~v elidable_lifetime_names
238+
impl<'a, 'b> TB<'b> for S2<'a, 'b> {}
239+
// ^^
240+
241+
//~v elidable_lifetime_names
242+
impl<'a, 'b> T for S2<'a, 'b> {}
243+
// ^^ ^^
244+
245+
// 3 lifetimes
246+
247+
impl<'a, 'b, 'c> TABC<'a, 'b, 'c> for S3<'a, 'b, 'c> {}
248+
249+
//~v elidable_lifetime_names
250+
impl<'a, 'b, 'c> TAB<'a, 'b> for S3<'a, 'b, 'c> {}
251+
// ^^
252+
253+
//~v elidable_lifetime_names
254+
impl<'a, 'b, 'c> TAC<'a, 'c> for S3<'a, 'b, 'c> {}
255+
// ^^
256+
257+
//~v elidable_lifetime_names
258+
impl<'a, 'b, 'c> TA<'a> for S3<'a, 'b, 'c> {}
259+
// ^^ ^^
260+
261+
//~v elidable_lifetime_names
262+
impl<'a, 'b, 'c> TBC<'b, 'c> for S3<'a, 'b, 'c> {}
263+
// ^^
264+
265+
//~v elidable_lifetime_names
266+
impl<'a, 'b, 'c> TB<'b> for S3<'a, 'b, 'c> {}
267+
// ^^ ^^
268+
269+
//~v elidable_lifetime_names
270+
impl<'a, 'b, 'c> TC<'c> for S3<'a, 'b, 'c> {}
271+
// ^^ ^^
272+
273+
//~v elidable_lifetime_names
274+
impl<'a, 'b, 'c> T for S3<'a, 'b, 'c> {}
275+
// ^^ ^^ ^^
276+
}

0 commit comments

Comments
 (0)
0