Build complete usable type from a type-relative prefix (#14586)

blyxyas · web-flow · commit 222660bb36f3 · 2025-04-17T00:09:51.000Z
Instead of looking for angle brackets in the source code, use the HIR and Ty interfaces to either copy the original type, or complete it with `_` placeholders if all type and const generic arguments are inferred. Fixes #14581 changelog: [`from_iter_instead_of_collect`]: show correct type in suggestion
diff --git 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
@@ -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
@@ -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
@@ -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
 
