diff options
| -rw-r--r-- | clippy_lints/src/methods/from_iter_instead_of_collect.rs | 95 | ||||
| -rw-r--r-- | tests/ui/from_iter_instead_of_collect.fixed | 43 | ||||
| -rw-r--r-- | tests/ui/from_iter_instead_of_collect.rs | 43 | ||||
| -rw-r--r-- | tests/ui/from_iter_instead_of_collect.stderr | 56 |
4 files changed, 188 insertions, 49 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 1f26b44767b..045363058d1 100644 --- a/clippy_lints/src/methods/from_iter_instead_of_collect.rs +++ b/clippy_lints/src/methods/from_iter_instead_of_collect.rs @@ -1,25 +1,31 @@ +use std::fmt::Write as _; + use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::SpanRangeExt; +use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::implements_trait; use clippy_utils::{is_path_diagnostic_item, sugg}; use rustc_errors::Applicability; -use rustc_hir as hir; +use rustc_hir::def::Res; +use rustc_hir::{self as hir, Expr, ExprKind, GenericArg, QPath, TyKind}; use rustc_lint::LateContext; -use rustc_middle::ty::Ty; +use rustc_middle::ty::GenericParamDefKind; use rustc_span::sym; use super::FROM_ITER_INSTEAD_OF_COLLECT; -pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>], func: &hir::Expr<'_>) { +pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, args: &[Expr<'_>], func: &Expr<'_>) { if is_path_diagnostic_item(cx, func, sym::from_iter_fn) - && let ty = cx.typeck_results().expr_ty(expr) && let arg_ty = cx.typeck_results().expr_ty(&args[0]) && let Some(iter_id) = cx.tcx.get_diagnostic_item(sym::Iterator) && implements_trait(cx, arg_ty, iter_id, &[]) { - // `expr` implements `FromIterator` trait + let mut app = Applicability::MaybeIncorrect; + let turbofish = match func.kind { + ExprKind::Path(QPath::TypeRelative(hir_ty, _)) => build_full_type(cx, hir_ty, &mut app), + ExprKind::Path(QPath::Resolved(Some(self_ty), _)) => build_full_type(cx, self_ty, &mut app), + _ => return, + }; let iter_expr = sugg::Sugg::hir(cx, &args[0], "..").maybe_paren(); - let turbofish = extract_turbofish(cx, expr, ty); let sugg = format!("{iter_expr}.collect::<{turbofish}>()"); span_lint_and_sugg( cx, @@ -28,54 +34,47 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Exp "usage of `FromIterator::from_iter`", "use `.collect()` instead of `::from_iter()`", sugg, - Applicability::MaybeIncorrect, + app, ); } } -fn extract_turbofish(cx: &LateContext<'_>, expr: &hir::Expr<'_>, ty: Ty<'_>) -> String { - fn strip_angle_brackets(s: &str) -> Option<&str> { - s.strip_prefix('<')?.strip_suffix('>') - } - - let call_site = expr.span.source_callsite(); - if let Some(snippet) = call_site.get_source_text(cx) - && let snippet_split = snippet.split("::").collect::<Vec<_>>() - && let Some((_, elements)) = snippet_split.split_last() +/// Build a type which can be used in a turbofish syntax from `hir_ty`, either by copying the +/// existing generic arguments with the exception of elided lifetimes, or by inserting placeholders +/// for types and consts without default values. +fn build_full_type(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, app: &mut Applicability) -> String { + if let TyKind::Path(ty_qpath) = hir_ty.kind + && let QPath::Resolved(None, ty_path) = &ty_qpath + && let Res::Def(_, ty_did) = ty_path.res { - if let [type_specifier, _] = snippet_split.as_slice() - && let Some(type_specifier) = strip_angle_brackets(type_specifier) - && let Some((type_specifier, ..)) = type_specifier.split_once(" as ") - { - type_specifier.to_string() + let mut ty_str = itertools::join(ty_path.segments.iter().map(|s| s.ident), "::"); + let mut first = true; + let mut append = |arg: &str| { + write!(&mut ty_str, "{}{arg}", [", ", "<"][usize::from(first)]).unwrap(); + first = false; + }; + if let Some(args) = ty_path.segments.last().and_then(|segment| segment.args) { + args.args + .iter() + .filter(|arg| !matches!(arg, GenericArg::Lifetime(lt) if lt.is_elided())) + .for_each(|arg| append(&snippet_with_applicability(cx, arg.span().source_callsite(), "_", app))); } 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!("{}{type_specifier}", without_ts.join("::")) - } 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(ty_str.len()); - let nb_wildcard = ty_str[start..end].split(',').count(); - let wildcards = format!("_{}", ", _".repeat(nb_wildcard - 1)); - format!("{}<{wildcards}>", elements.join("::")) - } + cx.tcx + .generics_of(ty_did) + .own_params + .iter() + .filter(|param| { + matches!( + param.kind, + GenericParamDefKind::Type { has_default: false, .. } + | GenericParamDefKind::Const { has_default: false, .. } + ) + }) + .for_each(|_| append("_")); } + ty_str.push_str([">", ""][usize::from(first)]); + ty_str } else { - ty.to_string() + snippet_with_applicability(cx, hir_ty.span.source_callsite(), "_", app).into() } } diff --git a/tests/ui/from_iter_instead_of_collect.fixed b/tests/ui/from_iter_instead_of_collect.fixed index 8618004efb8..be98b227795 100644 --- a/tests/ui/from_iter_instead_of_collect.fixed +++ b/tests/ui/from_iter_instead_of_collect.fixed @@ -73,3 +73,46 @@ fn main() { for _i in [1, 2, 3].iter().collect::<Vec<&i32>>() {} //~^ from_iter_instead_of_collect } + +fn issue14581() { + let nums = [0, 1, 2]; + let _ = &nums.iter().map(|&num| char::from_u32(num).unwrap()).collect::<String>(); + //~^ from_iter_instead_of_collect +} + +fn test_implicit_generic_args(iter: impl Iterator<Item = &'static i32> + Copy) { + struct S<'l, T = i32, const A: usize = 3, const B: usize = 3> { + a: [&'l T; A], + b: [&'l T; B], + } + + impl<'l, T, const A: usize, const B: usize> FromIterator<&'l T> for S<'l, T, A, B> { + fn from_iter<I: IntoIterator<Item = &'l T>>(_: I) -> Self { + todo!() + } + } + + let _ = iter.collect::<S<'static, i32, 7>>(); + //~^ from_iter_instead_of_collect + + let _ = iter.collect::<S<'static, i32>>(); + //~^ from_iter_instead_of_collect + + let _ = iter.collect::<S<'static, _, 7>>(); + //~^ from_iter_instead_of_collect + + let _ = iter.collect::<S<'static, _, 7, 8>>(); + //~^ from_iter_instead_of_collect + + let _ = iter.collect::<S<_, 7, 8>>(); + //~^ from_iter_instead_of_collect + + let _ = iter.collect::<S<i32>>(); + //~^ from_iter_instead_of_collect + + let _ = iter.collect::<S<i32>>(); + //~^ from_iter_instead_of_collect + + let _ = iter.collect::<S>(); + //~^ from_iter_instead_of_collect +} diff --git a/tests/ui/from_iter_instead_of_collect.rs b/tests/ui/from_iter_instead_of_collect.rs index c46397e8ff5..ce20fef2ac3 100644 --- a/tests/ui/from_iter_instead_of_collect.rs +++ b/tests/ui/from_iter_instead_of_collect.rs @@ -73,3 +73,46 @@ fn main() { for _i in Vec::<&i32>::from_iter([1, 2, 3].iter()) {} //~^ from_iter_instead_of_collect } + +fn issue14581() { + let nums = [0, 1, 2]; + let _ = &String::from_iter(nums.iter().map(|&num| char::from_u32(num).unwrap())); + //~^ from_iter_instead_of_collect +} + +fn test_implicit_generic_args(iter: impl Iterator<Item = &'static i32> + Copy) { + struct S<'l, T = i32, const A: usize = 3, const B: usize = 3> { + a: [&'l T; A], + b: [&'l T; B], + } + + impl<'l, T, const A: usize, const B: usize> FromIterator<&'l T> for S<'l, T, A, B> { + fn from_iter<I: IntoIterator<Item = &'l T>>(_: I) -> Self { + todo!() + } + } + + let _ = <S<'static, i32, 7>>::from_iter(iter); + //~^ from_iter_instead_of_collect + + let _ = <S<'static, i32>>::from_iter(iter); + //~^ from_iter_instead_of_collect + + let _ = <S<'static, _, 7>>::from_iter(iter); + //~^ from_iter_instead_of_collect + + let _ = <S<'static, _, 7, 8>>::from_iter(iter); + //~^ from_iter_instead_of_collect + + let _ = <S<'_, _, 7, 8>>::from_iter(iter); + //~^ from_iter_instead_of_collect + + let _ = <S<i32>>::from_iter(iter); + //~^ from_iter_instead_of_collect + + let _ = <S<'_, i32>>::from_iter(iter); + //~^ from_iter_instead_of_collect + + let _ = <S>::from_iter(iter); + //~^ from_iter_instead_of_collect +} diff --git a/tests/ui/from_iter_instead_of_collect.stderr b/tests/ui/from_iter_instead_of_collect.stderr index b46d97af152..ec11a375c0d 100644 --- a/tests/ui/from_iter_instead_of_collect.stderr +++ b/tests/ui/from_iter_instead_of_collect.stderr @@ -91,5 +91,59 @@ error: usage of `FromIterator::from_iter` 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 15 previous errors +error: usage of `FromIterator::from_iter` + --> tests/ui/from_iter_instead_of_collect.rs:79:14 + | +LL | let _ = &String::from_iter(nums.iter().map(|&num| char::from_u32(num).unwrap())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `nums.iter().map(|&num| char::from_u32(num).unwrap()).collect::<String>()` + +error: usage of `FromIterator::from_iter` + --> tests/ui/from_iter_instead_of_collect.rs:95:13 + | +LL | let _ = <S<'static, i32, 7>>::from_iter(iter); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter.collect::<S<'static, i32, 7>>()` + +error: usage of `FromIterator::from_iter` + --> tests/ui/from_iter_instead_of_collect.rs:98:13 + | +LL | let _ = <S<'static, i32>>::from_iter(iter); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter.collect::<S<'static, i32>>()` + +error: usage of `FromIterator::from_iter` + --> tests/ui/from_iter_instead_of_collect.rs:101:13 + | +LL | let _ = <S<'static, _, 7>>::from_iter(iter); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter.collect::<S<'static, _, 7>>()` + +error: usage of `FromIterator::from_iter` + --> tests/ui/from_iter_instead_of_collect.rs:104:13 + | +LL | let _ = <S<'static, _, 7, 8>>::from_iter(iter); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter.collect::<S<'static, _, 7, 8>>()` + +error: usage of `FromIterator::from_iter` + --> tests/ui/from_iter_instead_of_collect.rs:107:13 + | +LL | let _ = <S<'_, _, 7, 8>>::from_iter(iter); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter.collect::<S<_, 7, 8>>()` + +error: usage of `FromIterator::from_iter` + --> tests/ui/from_iter_instead_of_collect.rs:110:13 + | +LL | let _ = <S<i32>>::from_iter(iter); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter.collect::<S<i32>>()` + +error: usage of `FromIterator::from_iter` + --> tests/ui/from_iter_instead_of_collect.rs:113:13 + | +LL | let _ = <S<'_, i32>>::from_iter(iter); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter.collect::<S<i32>>()` + +error: usage of `FromIterator::from_iter` + --> tests/ui/from_iter_instead_of_collect.rs:116:13 + | +LL | let _ = <S>::from_iter(iter); + | ^^^^^^^^^^^^^^^^^^^^ help: use `.collect()` instead of `::from_iter()`: `iter.collect::<S>()` + +error: aborting due to 24 previous errors |