diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 3e356afa2a4..d9f906619a0 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -4095,7 +4095,8 @@ fn lint_from_iter(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr< then { // `expr` implements `FromIterator` trait let iter_expr = sugg::Sugg::hir(cx, &args[0], "..").maybe_par(); - let sugg = format!("{}.collect::<{}>()", iter_expr, ty); + let turbofish = extract_turbofish(cx, expr, ty); + let sugg = format!("{}.collect::<{}>()", iter_expr, turbofish); span_lint_and_sugg( cx, FROM_ITER_INSTEAD_OF_COLLECT, @@ -4109,6 +4110,38 @@ fn lint_from_iter(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr< } } +fn extract_turbofish(cx: &LateContext<'_>, expr: &hir::Expr<'_>, ty: Ty<'tcx>) -> String { + if_chain! { + let call_site = expr.span.source_callsite(); + if let Ok(snippet) = cx.sess().source_map().span_to_snippet(call_site); + let snippet_split = snippet.split("::").collect::>(); + if let Some((_, elements)) = snippet_split.split_last(); + + then { + // is there a type specifier? (i.e.: like `` in `collections::BTreeSet::::`) + if let Some(type_specifier) = snippet_split.iter().find(|e| e.starts_with('<') && e.ends_with('>')) { + // remove the type specifier from the path elements + let without_ts = elements.iter().filter_map(|e| { + if e == type_specifier { None } else { Some((*e).to_string()) } + }).collect::>(); + // join and add the type specifier at the end (i.e.: `collections::BTreeSet`) + format!("{}{}", without_ts.join("::"), type_specifier) + } else { + // type is not explicitly specified so wildcards are needed + // i.e.: 2 wildcards in `std::collections::BTreeMap<&i32, &char>` + let ty_str = ty.to_string(); + let start = ty_str.find('<').unwrap_or(0); + let end = ty_str.find('>').unwrap_or_else(|| ty_str.len()); + let nb_wildcard = ty_str[start..end].split(',').count(); + let wildcards = format!("_{}", ", _".repeat(nb_wildcard - 1)); + format!("{}<{}>", elements.join("::"), wildcards) + } + } else { + ty.to_string() + } + } +} + fn fn_header_equals(expected: hir::FnHeader, actual: hir::FnHeader) -> bool { expected.constness == actual.constness && expected.unsafety == actual.unsafety diff --git a/tests/ui/from_iter_instead_of_collect.fixed b/tests/ui/from_iter_instead_of_collect.fixed index 96701e86395..b5f548810e6 100644 --- a/tests/ui/from_iter_instead_of_collect.fixed +++ b/tests/ui/from_iter_instead_of_collect.fixed @@ -8,27 +8,41 @@ use std::iter::FromIterator; fn main() { let iter_expr = std::iter::repeat(5).take(5); - let _ = iter_expr.collect::>(); + let _ = iter_expr.collect::>(); - let _ = vec![5, 5, 5, 5].iter().enumerate().collect::>(); + let _ = vec![5, 5, 5, 5].iter().enumerate().collect::>(); Vec::from_iter(vec![42u32]); let a = vec![0, 1, 2]; - assert_eq!(a, (0..3).collect::>()); + assert_eq!(a, (0..3).collect::>()); + assert_eq!(a, (0..3).collect::>()); - let mut b = (0..3).collect::>(); + let mut b = (0..3).collect::>(); b.push_back(4); + let mut b = (0..3).collect::>(); + b.push_back(4); + + { + use std::collections; + let mut b = (0..3).collect::>(); + b.push_back(4); + } + let values = [(0, 'a'), (1, 'b'), (2, 'c'), (3, 'd')]; - let bm = values.iter().cloned().collect::>(); - let mut bar = bm.range(0..2).collect::>(); + let bm = values.iter().cloned().collect::>(); + let mut bar = bm.range(0..2).collect::>(); bar.insert(&4, &'e'); - let mut bts = (0..3).collect::>(); + let mut bts = (0..3).collect::>(); bts.insert(2); { use std::collections; - let _ = (0..3).collect::>(); + let _ = (0..3).collect::>(); + let _ = (0..3).collect::>(); } + + for _i in [1, 2, 3].iter().collect::>() {} + for _i in [1, 2, 3].iter().collect::>() {} } diff --git a/tests/ui/from_iter_instead_of_collect.rs b/tests/ui/from_iter_instead_of_collect.rs index 211f57bc537..b842b5451d1 100644 --- a/tests/ui/from_iter_instead_of_collect.rs +++ b/tests/ui/from_iter_instead_of_collect.rs @@ -16,10 +16,20 @@ fn main() { let a = vec![0, 1, 2]; assert_eq!(a, Vec::from_iter(0..3)); + assert_eq!(a, Vec::::from_iter(0..3)); let mut b = VecDeque::from_iter(0..3); b.push_back(4); + let mut b = VecDeque::::from_iter(0..3); + b.push_back(4); + + { + use std::collections; + let mut b = collections::VecDeque::::from_iter(0..3); + b.push_back(4); + } + let values = [(0, 'a'), (1, 'b'), (2, 'c'), (3, 'd')]; let bm = BTreeMap::from_iter(values.iter().cloned()); let mut bar = BTreeMap::from_iter(bm.range(0..2)); @@ -30,5 +40,9 @@ fn main() { { use std::collections; let _ = collections::BTreeSet::from_iter(0..3); + let _ = collections::BTreeSet::::from_iter(0..3); } + + for _i in Vec::from_iter([1, 2, 3].iter()) {} + for _i in Vec::<&i32>::from_iter([1, 2, 3].iter()) {} } diff --git a/tests/ui/from_iter_instead_of_collect.stderr b/tests/ui/from_iter_instead_of_collect.stderr index 336e25a8adf..434734c9a21 100644 --- a/tests/ui/from_iter_instead_of_collect.stderr +++ b/tests/ui/from_iter_instead_of_collect.stderr @@ -2,7 +2,7 @@ error: usage of `FromIterator::from_iter` --> $DIR/from_iter_instead_of_collect.rs:11:13 | LL | let _ = Vec::from_iter(iter_expr); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter_expr.collect::>()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter_expr.collect::>()` | = note: `-D clippy::from-iter-instead-of-collect` implied by `-D warnings` @@ -10,43 +10,79 @@ error: usage of `FromIterator::from_iter` --> $DIR/from_iter_instead_of_collect.rs:13:13 | LL | let _ = HashMap::::from_iter(vec![5, 5, 5, 5].iter().enumerate()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `vec![5, 5, 5, 5].iter().enumerate().collect::>()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `vec![5, 5, 5, 5].iter().enumerate().collect::>()` error: usage of `FromIterator::from_iter` --> $DIR/from_iter_instead_of_collect.rs:18:19 | LL | assert_eq!(a, Vec::from_iter(0..3)); - | ^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::>()` + | ^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::>()` error: usage of `FromIterator::from_iter` - --> $DIR/from_iter_instead_of_collect.rs:20:17 + --> $DIR/from_iter_instead_of_collect.rs:19:19 + | +LL | assert_eq!(a, Vec::::from_iter(0..3)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::>()` + +error: usage of `FromIterator::from_iter` + --> $DIR/from_iter_instead_of_collect.rs:21:17 | LL | let mut b = VecDeque::from_iter(0..3); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::>()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::>()` error: usage of `FromIterator::from_iter` - --> $DIR/from_iter_instead_of_collect.rs:24:14 + --> $DIR/from_iter_instead_of_collect.rs:24:17 + | +LL | let mut b = VecDeque::::from_iter(0..3); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::>()` + +error: usage of `FromIterator::from_iter` + --> $DIR/from_iter_instead_of_collect.rs:29:21 + | +LL | let mut b = collections::VecDeque::::from_iter(0..3); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::>()` + +error: usage of `FromIterator::from_iter` + --> $DIR/from_iter_instead_of_collect.rs:34:14 | LL | let bm = BTreeMap::from_iter(values.iter().cloned()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `values.iter().cloned().collect::>()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `values.iter().cloned().collect::>()` error: usage of `FromIterator::from_iter` - --> $DIR/from_iter_instead_of_collect.rs:25:19 + --> $DIR/from_iter_instead_of_collect.rs:35:19 | LL | let mut bar = BTreeMap::from_iter(bm.range(0..2)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `bm.range(0..2).collect::>()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `bm.range(0..2).collect::>()` error: usage of `FromIterator::from_iter` - --> $DIR/from_iter_instead_of_collect.rs:28:19 + --> $DIR/from_iter_instead_of_collect.rs:38:19 | LL | let mut bts = BTreeSet::from_iter(0..3); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::>()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::>()` error: usage of `FromIterator::from_iter` - --> $DIR/from_iter_instead_of_collect.rs:32:17 + --> $DIR/from_iter_instead_of_collect.rs:42:17 | LL | let _ = collections::BTreeSet::from_iter(0..3); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::>()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::>()` -error: aborting due to 8 previous errors +error: usage of `FromIterator::from_iter` + --> $DIR/from_iter_instead_of_collect.rs:43:17 + | +LL | let _ = collections::BTreeSet::::from_iter(0..3); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::>()` + +error: usage of `FromIterator::from_iter` + --> $DIR/from_iter_instead_of_collect.rs:46:15 + | +LL | for _i in Vec::from_iter([1, 2, 3].iter()) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `[1, 2, 3].iter().collect::>()` + +error: usage of `FromIterator::from_iter` + --> $DIR/from_iter_instead_of_collect.rs:47:15 + | +LL | for _i in Vec::<&i32>::from_iter([1, 2, 3].iter()) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `[1, 2, 3].iter().collect::>()` + +error: aborting due to 14 previous errors