Fix invalid syntax in from_iter_instead_of_collect suggestion with …

…"as Trait"
rust-lang · May 22, 2021 · ae0d4da · ae0d4da
1 parent f694f85
commit ae0d4da
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 34 deletions.
diff --git a/clippy_lints/src/methods/from_iter_instead_of_collect.rs b/clippy_lints/src/methods/from_iter_instead_of_collect.rs
@@ -37,30 +37,43 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Exp
 }
 
 fn extract_turbofish(cx: &LateContext<'_>, expr: &hir::Expr<'_>, ty: Ty<'tcx>) -> String {
+    fn strip_angle_brackets(s: &str) -> Option<&str> {
+        s.strip_prefix('<')?.strip_suffix('>')
+    }
+
     let call_site = expr.span.source_callsite();
     if_chain! {
         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)
+            if_chain! {
+                if let [type_specifier, _] = snippet_split.as_slice();
+                if let Some(type_specifier) = strip_angle_brackets(type_specifier);
+                if let Some((type_specifier, ..)) = type_specifier.split_once(" as ");
+                then {
+                    type_specifier.to_string()
+                } else {
+                    // is there a type specifier? (i.e.: like `<u32>` in `collections::BTreeSet::<u32>::`)
+                    if let Some(type_specifier) = snippet_split.iter().find(|e| strip_angle_brackets(e).is_some()) {
+                        // 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()

diff --git a/tests/ui/from_iter_instead_of_collect.fixed b/tests/ui/from_iter_instead_of_collect.fixed
@@ -6,6 +6,20 @@
 use std::collections::{BTreeMap, BTreeSet, HashMap, VecDeque};
 use std::iter::FromIterator;
 
+struct Foo(Vec<bool>);
+
+impl FromIterator<bool> for Foo {
+    fn from_iter<T: IntoIterator<Item = bool>>(_: T) -> Self {
+        todo!()
+    }
+}
+
+impl<'a> FromIterator<&'a bool> for Foo {
+    fn from_iter<T: IntoIterator<Item = &'a bool>>(iter: T) -> Self {
+        iter.into_iter().copied().collect::<Self>()
+    }
+}
+
 fn main() {
     let iter_expr = std::iter::repeat(5).take(5);
     let _ = iter_expr.collect::<Vec<_>>();

diff --git a/tests/ui/from_iter_instead_of_collect.rs b/tests/ui/from_iter_instead_of_collect.rs
@@ -6,6 +6,20 @@
 use std::collections::{BTreeMap, BTreeSet, HashMap, VecDeque};
 use std::iter::FromIterator;
 
+struct Foo(Vec<bool>);
+
+impl FromIterator<bool> for Foo {
+    fn from_iter<T: IntoIterator<Item = bool>>(_: T) -> Self {
+        todo!()
+    }
+}
+
+impl<'a> FromIterator<&'a bool> for Foo {
+    fn from_iter<T: IntoIterator<Item = &'a bool>>(iter: T) -> Self {
+        <Self as FromIterator<bool>>::from_iter(iter.into_iter().copied())
+    }
+}
+
 fn main() {
     let iter_expr = std::iter::repeat(5).take(5);
     let _ = Vec::from_iter(iter_expr);

diff --git a/tests/ui/from_iter_instead_of_collect.stderr b/tests/ui/from_iter_instead_of_collect.stderr
@@ -1,88 +1,94 @@
 error: usage of `FromIterator::from_iter`
-  --> $DIR/from_iter_instead_of_collect.rs:11:13
+  --> $DIR/from_iter_instead_of_collect.rs:19:9
    |
-LL |     let _ = Vec::from_iter(iter_expr);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter_expr.collect::<Vec<_>>()`
+LL |         <Self as FromIterator<bool>>::from_iter(iter.into_iter().copied())
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter.into_iter().copied().collect::<Self>()`
    |
    = 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:13:13
+  --> $DIR/from_iter_instead_of_collect.rs:25:13
+   |
+LL |     let _ = Vec::from_iter(iter_expr);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter_expr.collect::<Vec<_>>()`
+
+error: usage of `FromIterator::from_iter`
+  --> $DIR/from_iter_instead_of_collect.rs:27:13
    |
 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: usage of `FromIterator::from_iter`
-  --> $DIR/from_iter_instead_of_collect.rs:18:19
+  --> $DIR/from_iter_instead_of_collect.rs:32: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
+  --> $DIR/from_iter_instead_of_collect.rs:33: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
+  --> $DIR/from_iter_instead_of_collect.rs:35: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
+  --> $DIR/from_iter_instead_of_collect.rs:38: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
+  --> $DIR/from_iter_instead_of_collect.rs:43: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
+  --> $DIR/from_iter_instead_of_collect.rs:48: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
+  --> $DIR/from_iter_instead_of_collect.rs:49: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
+  --> $DIR/from_iter_instead_of_collect.rs:52: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
+  --> $DIR/from_iter_instead_of_collect.rs:56: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
+  --> $DIR/from_iter_instead_of_collect.rs:57: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
+  --> $DIR/from_iter_instead_of_collect.rs:60: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
+  --> $DIR/from_iter_instead_of_collect.rs:61: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
+error: aborting due to 15 previous errors