From ae0d4da764fd751494fb270fd04c2c686eac1302 Mon Sep 17 00:00:00 2001 From: Yotam Ofek Date: Sat, 22 May 2021 21:47:11 +0300 Subject: [PATCH] Fix invalid syntax in `from_iter_instead_of_collect` suggestion with "as Trait" --- .../methods/from_iter_instead_of_collect.rs | 47 ++++++++++++------- tests/ui/from_iter_instead_of_collect.fixed | 14 ++++++ tests/ui/from_iter_instead_of_collect.rs | 14 ++++++ tests/ui/from_iter_instead_of_collect.stderr | 40 +++++++++------- 4 files changed, 81 insertions(+), 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 index 28d0e8cd4ae..b4188d9ed30 100644 --- a/clippy_lints/src/methods/from_iter_instead_of_collect.rs +++ b/clippy_lints/src/methods/from_iter_instead_of_collect.rs @@ -37,6 +37,10 @@ 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); @@ -44,23 +48,32 @@ fn extract_turbofish(cx: &LateContext<'_>, expr: &hir::Expr<'_>, ty: Ty<'tcx>) - 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) + 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 `` in `collections::BTreeSet::::`) + 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::>(); + // 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() diff --git a/tests/ui/from_iter_instead_of_collect.fixed b/tests/ui/from_iter_instead_of_collect.fixed index b5f548810e6..12db43b5343 100644 --- 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); + +impl FromIterator for Foo { + fn from_iter>(_: T) -> Self { + todo!() + } +} + +impl<'a> FromIterator<&'a bool> for Foo { + fn from_iter>(iter: T) -> Self { + iter.into_iter().copied().collect::() + } +} + fn main() { let iter_expr = std::iter::repeat(5).take(5); let _ = iter_expr.collect::>(); diff --git a/tests/ui/from_iter_instead_of_collect.rs b/tests/ui/from_iter_instead_of_collect.rs index b842b5451d1..f5ec190e0cd 100644 --- 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); + +impl FromIterator for Foo { + fn from_iter>(_: T) -> Self { + todo!() + } +} + +impl<'a> FromIterator<&'a bool> for Foo { + fn from_iter>(iter: T) -> Self { + >::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 index 434734c9a21..8f08ac8c3ff 100644 --- 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::>()` +LL | >::from_iter(iter.into_iter().copied()) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter.into_iter().copied().collect::()` | = 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::>()` + +error: usage of `FromIterator::from_iter` + --> $DIR/from_iter_instead_of_collect.rs:27: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::>()` 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::>()` 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::::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 + --> $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::>()` 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::::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 + --> $DIR/from_iter_instead_of_collect.rs:43: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 + --> $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::>()` 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::>()` 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::>()` 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::>()` 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::::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 + --> $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::>()` 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::>()` -error: aborting due to 14 previous errors +error: aborting due to 15 previous errors