Auto merge of rust-lang#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: rust-lang#6648

changelog: none

clippy_lints/src/methods/mod.rs

@@ -4142,20 +4142,54 @@ fn lint_from_iter(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<
         if implements_trait(cx, ty, from_iter_id, &[]) && implements_trait(cx, arg_ty, iter_id, &[]);
         then {
             // `expr` implements `FromIterator` trait
-            let iter_expr = snippet(cx, args[0].span, "..");
+            let iter_expr = sugg::Sugg::hir(cx, &args[0], "..").maybe_par();
+            let turbofish = extract_turbofish(cx, expr, ty);
+            let sugg = format!("{}.collect::<{}>()", iter_expr, turbofish);
             span_lint_and_sugg(
                 cx,
                 FROM_ITER_INSTEAD_OF_COLLECT,
                 expr.span,
                 "usage of `FromIterator::from_iter`",
                 "use `.collect()` instead of `::from_iter()`",
-                format!("{}.collect()", iter_expr),
+                sugg,
                 Applicability::MaybeIncorrect,
             );
         }
     }
 }
 
+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::<Vec<_>>();
+        if let Some((_, elements)) = snippet_split.split_last();
+
+        then {
+            // is there a type specifier? (i.e.: like `<u32>` in `collections::BTreeSet::<u32>::`)
+            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::<Vec<_>>();
+                // join and add the type specifier at the end (i.e.: `collections::BTreeSet<u32>`)
+                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

tests/ui/from_iter_instead_of_collect.fixed

@@ -0,0 +1,48 @@
+// run-rustfix
+
+#![warn(clippy::from_iter_instead_of_collect)]
+#![allow(unused_imports)]
+
+use std::collections::{BTreeMap, BTreeSet, HashMap, VecDeque};
+use std::iter::FromIterator;
+
+fn main() {
+    let iter_expr = std::iter::repeat(5).take(5);
+    let _ = iter_expr.collect::<Vec<_>>();
+
+    let _ = vec![5, 5, 5, 5].iter().enumerate().collect::<HashMap<usize, &i8>>();
+
+    Vec::from_iter(vec![42u32]);
+
+    let a = vec![0, 1, 2];
+    assert_eq!(a, (0..3).collect::<Vec<_>>());
+    assert_eq!(a, (0..3).collect::<Vec<i32>>());
+
+    let mut b = (0..3).collect::<VecDeque<_>>();
+    b.push_back(4);
+
+    let mut b = (0..3).collect::<VecDeque<i32>>();
+    b.push_back(4);
+
+    {
+        use std::collections;
+        let mut b = (0..3).collect::<collections::VecDeque<i32>>();
+        b.push_back(4);
+    }
+
+    let values = [(0, 'a'), (1, 'b'), (2, 'c'), (3, 'd')];
+    let bm = values.iter().cloned().collect::<BTreeMap<_, _>>();
+    let mut bar = bm.range(0..2).collect::<BTreeMap<_, _>>();
+    bar.insert(&4, &'e');
+
+    let mut bts = (0..3).collect::<BTreeSet<_>>();
+    bts.insert(2);
+    {
+        use std::collections;
+        let _ = (0..3).collect::<collections::BTreeSet<_>>();
+        let _ = (0..3).collect::<collections::BTreeSet<u32>>();
+    }
+
+    for _i in [1, 2, 3].iter().collect::<Vec<_>>() {}
+    for _i in [1, 2, 3].iter().collect::<Vec<&i32>>() {}
+}

tests/ui/from_iter_instead_of_collect.rs

@@ -1,13 +1,48 @@
+// run-rustfix
+
 #![warn(clippy::from_iter_instead_of_collect)]
+#![allow(unused_imports)]
 
-use std::collections::HashMap;
+use std::collections::{BTreeMap, BTreeSet, HashMap, VecDeque};
 use std::iter::FromIterator;
 
 fn main() {
     let iter_expr = std::iter::repeat(5).take(5);
-    Vec::from_iter(iter_expr);
+    let _ = Vec::from_iter(iter_expr);
 
-    HashMap::<usize, &i8>::from_iter(vec![5, 5, 5, 5].iter().enumerate());
+    let _ = HashMap::<usize, &i8>::from_iter(vec![5, 5, 5, 5].iter().enumerate());
 
     Vec::from_iter(vec![42u32]);
+
+    let a = vec![0, 1, 2];
+    assert_eq!(a, Vec::from_iter(0..3));
+    assert_eq!(a, Vec::<i32>::from_iter(0..3));
+
+    let mut b = VecDeque::from_iter(0..3);
+    b.push_back(4);
+
+    let mut b = VecDeque::<i32>::from_iter(0..3);
+    b.push_back(4);
+
+    {
+        use std::collections;
+        let mut b = collections::VecDeque::<i32>::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));
+    bar.insert(&4, &'e');
+
+    let mut bts = BTreeSet::from_iter(0..3);
+    bts.insert(2);
+    {
+        use std::collections;
+        let _ = collections::BTreeSet::from_iter(0..3);
+        let _ = collections::BTreeSet::<u32>::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()) {}
 }

tests/ui/from_iter_instead_of_collect.stderr

@@ -1,16 +1,88 @@
 error: usage of `FromIterator::from_iter`
-  --> $DIR/from_iter_instead_of_collect.rs:8:5
+  --> $DIR/from_iter_instead_of_collect.rs:11:13
    |
-LL |     Vec::from_iter(iter_expr);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter_expr.collect()`
+LL |     let _ = Vec::from_iter(iter_expr);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter_expr.collect::<Vec<_>>()`
    |
    = note: `-D clippy::from-iter-instead-of-collect` implied by `-D warnings`
 
 error: usage of `FromIterator::from_iter`
-  --> $DIR/from_iter_instead_of_collect.rs:10:5
+  --> $DIR/from_iter_instead_of_collect.rs:13:13
    |
-LL |     HashMap::<usize, &i8>::from_iter(vec![5, 5, 5, 5].iter().enumerate());
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `vec![5, 5, 5, 5].iter().enumerate().collect()`
+LL |     let _ = HashMap::<usize, &i8>::from_iter(vec![5, 5, 5, 5].iter().enumerate());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `vec![5, 5, 5, 5].iter().enumerate().collect::<HashMap<usize, &i8>>()`
 
-error: aborting due to 2 previous errors
+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::<Vec<_>>()`
+
+error: usage of `FromIterator::from_iter`
+  --> $DIR/from_iter_instead_of_collect.rs:19:19
+   |
+LL |     assert_eq!(a, Vec::<i32>::from_iter(0..3));
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<Vec<i32>>()`
+
+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::<VecDeque<_>>()`
+
+error: usage of `FromIterator::from_iter`
+  --> $DIR/from_iter_instead_of_collect.rs:24:17
+   |
+LL |     let mut b = VecDeque::<i32>::from_iter(0..3);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<VecDeque<i32>>()`
+
+error: usage of `FromIterator::from_iter`
+  --> $DIR/from_iter_instead_of_collect.rs:29:21
+   |
+LL |         let mut b = collections::VecDeque::<i32>::from_iter(0..3);
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<collections::VecDeque<i32>>()`
+
+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::<BTreeMap<_, _>>()`
+
+error: usage of `FromIterator::from_iter`
+  --> $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::<BTreeMap<_, _>>()`
+
+error: usage of `FromIterator::from_iter`
+  --> $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::<BTreeSet<_>>()`
+
+error: usage of `FromIterator::from_iter`
+  --> $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::<collections::BTreeSet<_>>()`
+
+error: usage of `FromIterator::from_iter`
+  --> $DIR/from_iter_instead_of_collect.rs:43:17
+   |
+LL |         let _ = collections::BTreeSet::<u32>::from_iter(0..3);
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `(0..3).collect::<collections::BTreeSet<u32>>()`
+
+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::<Vec<_>>()`
+
+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::<Vec<&i32>>()`
+
+error: aborting due to 14 previous errors