Skip to content

Commit 3784cdf

Browse files
borsbors
committed
Auto merge of #6657 - ThibsG:FromIterParens, r=llogiq
Fix suggestions that need parens in `from_iter_instead_of_collect` lint Fixes broken suggestions that need parens (i.e.: range) Fixes: #6648 changelog: none
2 parents a6d6b1b + b932587 commit 3784cdf

File tree

4 files changed

+201
-12
lines changed

4 files changed

+201
-12
lines changed

Diff for: ‎clippy_lints/src/methods/mod.rs

+36-2
Original file line numberDiff line numberDiff line change
@@ -4142,20 +4142,54 @@ fn lint_from_iter(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<
41424142
if implements_trait(cx, ty, from_iter_id, &[]) && implements_trait(cx, arg_ty, iter_id, &[]);
41434143
then {
41444144
// `expr` implements `FromIterator` trait
4145-
let iter_expr = snippet(cx, args[0].span, "..");
4145+
let iter_expr = sugg::Sugg::hir(cx, &args[0], "..").maybe_par();
4146+
let turbofish = extract_turbofish(cx, expr, ty);
4147+
let sugg = format!("{}.collect::<{}>()", iter_expr, turbofish);
41464148
span_lint_and_sugg(
41474149
cx,
41484150
FROM_ITER_INSTEAD_OF_COLLECT,
41494151
expr.span,
41504152
"usage of `FromIterator::from_iter`",
41514153
"use `.collect()` instead of `::from_iter()`",
4152-
format!("{}.collect()", iter_expr),
4154+
sugg,
41534155
Applicability::MaybeIncorrect,
41544156
);
41554157
}
41564158
}
41574159
}
41584160

4161+
fn extract_turbofish(cx: &LateContext<'_>, expr: &hir::Expr<'_>, ty: Ty<'tcx>) -> String {
4162+
if_chain! {
4163+
let call_site = expr.span.source_callsite();
4164+
if let Ok(snippet) = cx.sess().source_map().span_to_snippet(call_site);
4165+
let snippet_split = snippet.split("::").collect::<Vec<_>>();
4166+
if let Some((_, elements)) = snippet_split.split_last();
4167+
4168+
then {
4169+
// is there a type specifier? (i.e.: like `<u32>` in `collections::BTreeSet::<u32>::`)
4170+
if let Some(type_specifier) = snippet_split.iter().find(|e| e.starts_with('<') && e.ends_with('>')) {
4171+
// remove the type specifier from the path elements
4172+
let without_ts = elements.iter().filter_map(|e| {
4173+
if e == type_specifier { None } else { Some((*e).to_string()) }
4174+
}).collect::<Vec<_>>();
4175+
// join and add the type specifier at the end (i.e.: `collections::BTreeSet<u32>`)
4176+
format!("{}{}", without_ts.join("::"), type_specifier)
4177+
} else {
4178+
// type is not explicitly specified so wildcards are needed
4179+
// i.e.: 2 wildcards in `std::collections::BTreeMap<&i32, &char>`
4180+
let ty_str = ty.to_string();
4181+
let start = ty_str.find('<').unwrap_or(0);
4182+
let end = ty_str.find('>').unwrap_or_else(|| ty_str.len());
4183+
let nb_wildcard = ty_str[start..end].split(',').count();
4184+
let wildcards = format!("_{}", ", _".repeat(nb_wildcard - 1));
4185+
format!("{}<{}>", elements.join("::"), wildcards)
4186+
}
4187+
} else {
4188+
ty.to_string()
4189+
}
4190+
}
4191+
}
4192+
41594193
fn fn_header_equals(expected: hir::FnHeader, actual: hir::FnHeader) -> bool {
41604194
expected.constness == actual.constness
41614195
&& expected.unsafety == actual.unsafety

Diff for: ‎tests/ui/from_iter_instead_of_collect.fixed

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::from_iter_instead_of_collect)]
4+
#![allow(unused_imports)]
5+
6+
use std::collections::{BTreeMap, BTreeSet, HashMap, VecDeque};
7+
use std::iter::FromIterator;
8+
9+
fn main() {
10+
let iter_expr = std::iter::repeat(5).take(5);
11+
let _ = iter_expr.collect::<Vec<_>>();
12+
13+
let _ = vec![5, 5, 5, 5].iter().enumerate().collect::<HashMap<usize, &i8>>();
14+
15+
Vec::from_iter(vec![42u32]);
16+
17+
let a = vec![0, 1, 2];
18+
assert_eq!(a, (0..3).collect::<Vec<_>>());
19+
assert_eq!(a, (0..3).collect::<Vec<i32>>());
20+
21+
let mut b = (0..3).collect::<VecDeque<_>>();
22+
b.push_back(4);
23+
24+
let mut b = (0..3).collect::<VecDeque<i32>>();
25+
b.push_back(4);
26+
27+
{
28+
use std::collections;
29+
let mut b = (0..3).collect::<collections::VecDeque<i32>>();
30+
b.push_back(4);
31+
}
32+
33+
let values = [(0, 'a'), (1, 'b'), (2, 'c'), (3, 'd')];
34+
let bm = values.iter().cloned().collect::<BTreeMap<_, _>>();
35+
let mut bar = bm.range(0..2).collect::<BTreeMap<_, _>>();
36+
bar.insert(&4, &'e');
37+
38+
let mut bts = (0..3).collect::<BTreeSet<_>>();
39+
bts.insert(2);
40+
{
41+
use std::collections;
42+
let _ = (0..3).collect::<collections::BTreeSet<_>>();
43+
let _ = (0..3).collect::<collections::BTreeSet<u32>>();
44+
}
45+
46+
for _i in [1, 2, 3].iter().collect::<Vec<_>>() {}
47+
for _i in [1, 2, 3].iter().collect::<Vec<&i32>>() {}
48+
}

Diff for: ‎tests/ui/from_iter_instead_of_collect.rs

+38-3
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,48 @@
1+
// run-rustfix
2+
13
#![warn(clippy::from_iter_instead_of_collect)]
4+
#![allow(unused_imports)]
25

3-
use std::collections::HashMap;
6+
use std::collections::{BTreeMap, BTreeSet, HashMap, VecDeque};
47
use std::iter::FromIterator;
58

69
fn main() {
710
let iter_expr = std::iter::repeat(5).take(5);
8-
Vec::from_iter(iter_expr);
11+
let _ = Vec::from_iter(iter_expr);
912

10-
HashMap::<usize, &i8>::from_iter(vec![5, 5, 5, 5].iter().enumerate());
13+
let _ = HashMap::<usize, &i8>::from_iter(vec![5, 5, 5, 5].iter().enumerate());
1114

1215
Vec::from_iter(vec![42u32]);
16+
17+
let a = vec![0, 1, 2];
18+
assert_eq!(a, Vec::from_iter(0..3));
19+
assert_eq!(a, Vec::<i32>::from_iter(0..3));
20+
21+
let mut b = VecDeque::from_iter(0..3);
22+
b.push_back(4);
23+
24+
let mut b = VecDeque::<i32>::from_iter(0..3);
25+
b.push_back(4);
26+
27+
{
28+
use std::collections;
29+
let mut b = collections::VecDeque::<i32>::from_iter(0..3);
30+
b.push_back(4);
31+
}
32+
33+
let values = [(0, 'a'), (1, 'b'), (2, 'c'), (3, 'd')];
34+
let bm = BTreeMap::from_iter(values.iter().cloned());
35+
let mut bar = BTreeMap::from_iter(bm.range(0..2));
36+
bar.insert(&4, &'e');
37+
38+
let mut bts = BTreeSet::from_iter(0..3);
39+
bts.insert(2);
40+
{
41+
use std::collections;
42+
let _ = collections::BTreeSet::from_iter(0..3);
43+
let _ = collections::BTreeSet::<u32>::from_iter(0..3);
44+
}
45+
46+
for _i in Vec::from_iter([1, 2, 3].iter()) {}
47+
for _i in Vec::<&i32>::from_iter([1, 2, 3].iter()) {}
1348
}

Diff for: ‎tests/ui/from_iter_instead_of_collect.stderr

+79-7
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,88 @@
11
error: usage of `FromIterator::from_iter`
2-
--> $DIR/from_iter_instead_of_collect.rs:8:5
2+
--> $DIR/from_iter_instead_of_collect.rs:11:13
33
|
4-
LL | Vec::from_iter(iter_expr);
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter_expr.collect()`
4+
LL | let _ = Vec::from_iter(iter_expr);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter_expr.collect::<Vec<_>>()`
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:10:5
10+
--> $DIR/from_iter_instead_of_collect.rs:13:13
1111
|
12-
LL | HashMap::<usize, &i8>::from_iter(vec![5, 5, 5, 5].iter().enumerate());
13-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `vec![5, 5, 5, 5].iter().enumerate().collect()`
12+
LL | let _ = HashMap::<usize, &i8>::from_iter(vec![5, 5, 5, 5].iter().enumerate());
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `vec![5, 5, 5, 5].iter().enumerate().collect::<HashMap<usize, &i8>>()`
1414

15-
error: aborting due to 2 previous errors
15+
error: usage of `FromIterator::from_iter`
16+
--> $DIR/from_iter_instead_of_collect.rs:18:19
17+
|
18+
LL | assert_eq!(a, Vec::from_iter(0..3));
19+
| ^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<Vec<_>>()`
20+
21+
error: usage of `FromIterator::from_iter`
22+
--> $DIR/from_iter_instead_of_collect.rs:19:19
23+
|
24+
LL | assert_eq!(a, Vec::<i32>::from_iter(0..3));
25+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<Vec<i32>>()`
26+
27+
error: usage of `FromIterator::from_iter`
28+
--> $DIR/from_iter_instead_of_collect.rs:21:17
29+
|
30+
LL | let mut b = VecDeque::from_iter(0..3);
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<VecDeque<_>>()`
32+
33+
error: usage of `FromIterator::from_iter`
34+
--> $DIR/from_iter_instead_of_collect.rs:24:17
35+
|
36+
LL | let mut b = VecDeque::<i32>::from_iter(0..3);
37+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<VecDeque<i32>>()`
38+
39+
error: usage of `FromIterator::from_iter`
40+
--> $DIR/from_iter_instead_of_collect.rs:29:21
41+
|
42+
LL | let mut b = collections::VecDeque::<i32>::from_iter(0..3);
43+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<collections::VecDeque<i32>>()`
44+
45+
error: usage of `FromIterator::from_iter`
46+
--> $DIR/from_iter_instead_of_collect.rs:34:14
47+
|
48+
LL | let bm = BTreeMap::from_iter(values.iter().cloned());
49+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `values.iter().cloned().collect::<BTreeMap<_, _>>()`
50+
51+
error: usage of `FromIterator::from_iter`
52+
--> $DIR/from_iter_instead_of_collect.rs:35:19
53+
|
54+
LL | let mut bar = BTreeMap::from_iter(bm.range(0..2));
55+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `bm.range(0..2).collect::<BTreeMap<_, _>>()`
56+
57+
error: usage of `FromIterator::from_iter`
58+
--> $DIR/from_iter_instead_of_collect.rs:38:19
59+
|
60+
LL | let mut bts = BTreeSet::from_iter(0..3);
61+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<BTreeSet<_>>()`
62+
63+
error: usage of `FromIterator::from_iter`
64+
--> $DIR/from_iter_instead_of_collect.rs:42:17
65+
|
66+
LL | let _ = collections::BTreeSet::from_iter(0..3);
67+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<collections::BTreeSet<_>>()`
68+
69+
error: usage of `FromIterator::from_iter`
70+
--> $DIR/from_iter_instead_of_collect.rs:43:17
71+
|
72+
LL | let _ = collections::BTreeSet::<u32>::from_iter(0..3);
73+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<collections::BTreeSet<u32>>()`
74+
75+
error: usage of `FromIterator::from_iter`
76+
--> $DIR/from_iter_instead_of_collect.rs:46:15
77+
|
78+
LL | for _i in Vec::from_iter([1, 2, 3].iter()) {}
79+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `[1, 2, 3].iter().collect::<Vec<_>>()`
80+
81+
error: usage of `FromIterator::from_iter`
82+
--> $DIR/from_iter_instead_of_collect.rs:47:15
83+
|
84+
LL | for _i in Vec::<&i32>::from_iter([1, 2, 3].iter()) {}
85+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `[1, 2, 3].iter().collect::<Vec<&i32>>()`
86+
87+
error: aborting due to 14 previous errors
1688

0 commit comments

Comments
 (0)