Skip to content

Commit 297e743

Browse files
committed
Auto merge of rust-lang#7264 - yotamofek:from_iter_instead_collect_as_trait, r=llogiq
Fix invalid syntax in `from_iter_instead_of_collect` suggestion First attempt at contributing, hopefully this is a good start and can be improved. :) fixes rust-lang#7259 changelog: [`from_iter_instead_of_collect`] fix invalid suggestion involving "as Trait"
2 parents 8787186 + ae0d4da commit 297e743

File tree

4 files changed

+81
-34
lines changed

4 files changed

+81
-34
lines changed

clippy_lints/src/methods/from_iter_instead_of_collect.rs

+30-17
Original file line numberDiff line numberDiff line change
@@ -37,30 +37,43 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Exp
3737
}
3838

3939
fn extract_turbofish(cx: &LateContext<'_>, expr: &hir::Expr<'_>, ty: Ty<'tcx>) -> String {
40+
fn strip_angle_brackets(s: &str) -> Option<&str> {
41+
s.strip_prefix('<')?.strip_suffix('>')
42+
}
43+
4044
let call_site = expr.span.source_callsite();
4145
if_chain! {
4246
if let Ok(snippet) = cx.sess().source_map().span_to_snippet(call_site);
4347
let snippet_split = snippet.split("::").collect::<Vec<_>>();
4448
if let Some((_, elements)) = snippet_split.split_last();
4549

4650
then {
47-
// is there a type specifier? (i.e.: like `<u32>` in `collections::BTreeSet::<u32>::`)
48-
if let Some(type_specifier) = snippet_split.iter().find(|e| e.starts_with('<') && e.ends_with('>')) {
49-
// remove the type specifier from the path elements
50-
let without_ts = elements.iter().filter_map(|e| {
51-
if e == type_specifier { None } else { Some((*e).to_string()) }
52-
}).collect::<Vec<_>>();
53-
// join and add the type specifier at the end (i.e.: `collections::BTreeSet<u32>`)
54-
format!("{}{}", without_ts.join("::"), type_specifier)
55-
} else {
56-
// type is not explicitly specified so wildcards are needed
57-
// i.e.: 2 wildcards in `std::collections::BTreeMap<&i32, &char>`
58-
let ty_str = ty.to_string();
59-
let start = ty_str.find('<').unwrap_or(0);
60-
let end = ty_str.find('>').unwrap_or_else(|| ty_str.len());
61-
let nb_wildcard = ty_str[start..end].split(',').count();
62-
let wildcards = format!("_{}", ", _".repeat(nb_wildcard - 1));
63-
format!("{}<{}>", elements.join("::"), wildcards)
51+
if_chain! {
52+
if let [type_specifier, _] = snippet_split.as_slice();
53+
if let Some(type_specifier) = strip_angle_brackets(type_specifier);
54+
if let Some((type_specifier, ..)) = type_specifier.split_once(" as ");
55+
then {
56+
type_specifier.to_string()
57+
} else {
58+
// is there a type specifier? (i.e.: like `<u32>` in `collections::BTreeSet::<u32>::`)
59+
if let Some(type_specifier) = snippet_split.iter().find(|e| strip_angle_brackets(e).is_some()) {
60+
// remove the type specifier from the path elements
61+
let without_ts = elements.iter().filter_map(|e| {
62+
if e == type_specifier { None } else { Some((*e).to_string()) }
63+
}).collect::<Vec<_>>();
64+
// join and add the type specifier at the end (i.e.: `collections::BTreeSet<u32>`)
65+
format!("{}{}", without_ts.join("::"), type_specifier)
66+
} else {
67+
// type is not explicitly specified so wildcards are needed
68+
// i.e.: 2 wildcards in `std::collections::BTreeMap<&i32, &char>`
69+
let ty_str = ty.to_string();
70+
let start = ty_str.find('<').unwrap_or(0);
71+
let end = ty_str.find('>').unwrap_or_else(|| ty_str.len());
72+
let nb_wildcard = ty_str[start..end].split(',').count();
73+
let wildcards = format!("_{}", ", _".repeat(nb_wildcard - 1));
74+
format!("{}<{}>", elements.join("::"), wildcards)
75+
}
76+
}
6477
}
6578
} else {
6679
ty.to_string()

tests/ui/from_iter_instead_of_collect.fixed

+14
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,20 @@
66
use std::collections::{BTreeMap, BTreeSet, HashMap, VecDeque};
77
use std::iter::FromIterator;
88

9+
struct Foo(Vec<bool>);
10+
11+
impl FromIterator<bool> for Foo {
12+
fn from_iter<T: IntoIterator<Item = bool>>(_: T) -> Self {
13+
todo!()
14+
}
15+
}
16+
17+
impl<'a> FromIterator<&'a bool> for Foo {
18+
fn from_iter<T: IntoIterator<Item = &'a bool>>(iter: T) -> Self {
19+
iter.into_iter().copied().collect::<Self>()
20+
}
21+
}
22+
923
fn main() {
1024
let iter_expr = std::iter::repeat(5).take(5);
1125
let _ = iter_expr.collect::<Vec<_>>();

tests/ui/from_iter_instead_of_collect.rs

+14
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,20 @@
66
use std::collections::{BTreeMap, BTreeSet, HashMap, VecDeque};
77
use std::iter::FromIterator;
88

9+
struct Foo(Vec<bool>);
10+
11+
impl FromIterator<bool> for Foo {
12+
fn from_iter<T: IntoIterator<Item = bool>>(_: T) -> Self {
13+
todo!()
14+
}
15+
}
16+
17+
impl<'a> FromIterator<&'a bool> for Foo {
18+
fn from_iter<T: IntoIterator<Item = &'a bool>>(iter: T) -> Self {
19+
<Self as FromIterator<bool>>::from_iter(iter.into_iter().copied())
20+
}
21+
}
22+
923
fn main() {
1024
let iter_expr = std::iter::repeat(5).take(5);
1125
let _ = Vec::from_iter(iter_expr);
+23-17
Original file line numberDiff line numberDiff line change
@@ -1,88 +1,94 @@
11
error: usage of `FromIterator::from_iter`
2-
--> $DIR/from_iter_instead_of_collect.rs:11:13
2+
--> $DIR/from_iter_instead_of_collect.rs:19:9
33
|
4-
LL | let _ = Vec::from_iter(iter_expr);
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter_expr.collect::<Vec<_>>()`
4+
LL | <Self as FromIterator<bool>>::from_iter(iter.into_iter().copied())
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter.into_iter().copied().collect::<Self>()`
66
|
77
= note: `-D clippy::from-iter-instead-of-collect` implied by `-D warnings`
88

99
error: usage of `FromIterator::from_iter`
10-
--> $DIR/from_iter_instead_of_collect.rs:13:13
10+
--> $DIR/from_iter_instead_of_collect.rs:25:13
11+
|
12+
LL | let _ = Vec::from_iter(iter_expr);
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter_expr.collect::<Vec<_>>()`
14+
15+
error: usage of `FromIterator::from_iter`
16+
--> $DIR/from_iter_instead_of_collect.rs:27:13
1117
|
1218
LL | let _ = HashMap::<usize, &i8>::from_iter(vec![5, 5, 5, 5].iter().enumerate());
1319
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `vec![5, 5, 5, 5].iter().enumerate().collect::<HashMap<usize, &i8>>()`
1420

1521
error: usage of `FromIterator::from_iter`
16-
--> $DIR/from_iter_instead_of_collect.rs:18:19
22+
--> $DIR/from_iter_instead_of_collect.rs:32:19
1723
|
1824
LL | assert_eq!(a, Vec::from_iter(0..3));
1925
| ^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<Vec<_>>()`
2026

2127
error: usage of `FromIterator::from_iter`
22-
--> $DIR/from_iter_instead_of_collect.rs:19:19
28+
--> $DIR/from_iter_instead_of_collect.rs:33:19
2329
|
2430
LL | assert_eq!(a, Vec::<i32>::from_iter(0..3));
2531
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<Vec<i32>>()`
2632

2733
error: usage of `FromIterator::from_iter`
28-
--> $DIR/from_iter_instead_of_collect.rs:21:17
34+
--> $DIR/from_iter_instead_of_collect.rs:35:17
2935
|
3036
LL | let mut b = VecDeque::from_iter(0..3);
3137
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<VecDeque<_>>()`
3238

3339
error: usage of `FromIterator::from_iter`
34-
--> $DIR/from_iter_instead_of_collect.rs:24:17
40+
--> $DIR/from_iter_instead_of_collect.rs:38:17
3541
|
3642
LL | let mut b = VecDeque::<i32>::from_iter(0..3);
3743
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<VecDeque<i32>>()`
3844

3945
error: usage of `FromIterator::from_iter`
40-
--> $DIR/from_iter_instead_of_collect.rs:29:21
46+
--> $DIR/from_iter_instead_of_collect.rs:43:21
4147
|
4248
LL | let mut b = collections::VecDeque::<i32>::from_iter(0..3);
4349
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<collections::VecDeque<i32>>()`
4450

4551
error: usage of `FromIterator::from_iter`
46-
--> $DIR/from_iter_instead_of_collect.rs:34:14
52+
--> $DIR/from_iter_instead_of_collect.rs:48:14
4753
|
4854
LL | let bm = BTreeMap::from_iter(values.iter().cloned());
4955
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `values.iter().cloned().collect::<BTreeMap<_, _>>()`
5056

5157
error: usage of `FromIterator::from_iter`
52-
--> $DIR/from_iter_instead_of_collect.rs:35:19
58+
--> $DIR/from_iter_instead_of_collect.rs:49:19
5359
|
5460
LL | let mut bar = BTreeMap::from_iter(bm.range(0..2));
5561
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `bm.range(0..2).collect::<BTreeMap<_, _>>()`
5662

5763
error: usage of `FromIterator::from_iter`
58-
--> $DIR/from_iter_instead_of_collect.rs:38:19
64+
--> $DIR/from_iter_instead_of_collect.rs:52:19
5965
|
6066
LL | let mut bts = BTreeSet::from_iter(0..3);
6167
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<BTreeSet<_>>()`
6268

6369
error: usage of `FromIterator::from_iter`
64-
--> $DIR/from_iter_instead_of_collect.rs:42:17
70+
--> $DIR/from_iter_instead_of_collect.rs:56:17
6571
|
6672
LL | let _ = collections::BTreeSet::from_iter(0..3);
6773
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<collections::BTreeSet<_>>()`
6874

6975
error: usage of `FromIterator::from_iter`
70-
--> $DIR/from_iter_instead_of_collect.rs:43:17
76+
--> $DIR/from_iter_instead_of_collect.rs:57:17
7177
|
7278
LL | let _ = collections::BTreeSet::<u32>::from_iter(0..3);
7379
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<collections::BTreeSet<u32>>()`
7480

7581
error: usage of `FromIterator::from_iter`
76-
--> $DIR/from_iter_instead_of_collect.rs:46:15
82+
--> $DIR/from_iter_instead_of_collect.rs:60:15
7783
|
7884
LL | for _i in Vec::from_iter([1, 2, 3].iter()) {}
7985
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `[1, 2, 3].iter().collect::<Vec<_>>()`
8086

8187
error: usage of `FromIterator::from_iter`
82-
--> $DIR/from_iter_instead_of_collect.rs:47:15
88+
--> $DIR/from_iter_instead_of_collect.rs:61:15
8389
|
8490
LL | for _i in Vec::<&i32>::from_iter([1, 2, 3].iter()) {}
8591
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `[1, 2, 3].iter().collect::<Vec<&i32>>()`
8692

87-
error: aborting due to 14 previous errors
93+
error: aborting due to 15 previous errors
8894

0 commit comments

Comments
 (0)