Rollup merge of #116717 - estebank:issue-9082, r=oli-obk

Special case iterator chain checks for suggestion

When encountering method call chains of `Iterator`, check for trailing `;` in the body of closures passed into `Iterator::map`, as well as calls to `<T as Clone>::clone` when `T` is a type param and `T: !Clone`.

Fix #9082.

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -313,6 +313,18 @@ pub trait TypeErrCtxtExt<'tcx> {
         predicate: ty::Predicate<'tcx>,
         call_hir_id: HirId,
     );
+
+    fn look_for_iterator_item_mistakes(
+        &self,
+        assocs_in_this_method: &[Option<(Span, (DefId, Ty<'tcx>))>],
+        typeck_results: &TypeckResults<'tcx>,
+        type_diffs: &[TypeError<'tcx>],
+        param_env: ty::ParamEnv<'tcx>,
+        path_segment: &hir::PathSegment<'_>,
+        args: &[hir::Expr<'_>],
+        err: &mut Diagnostic,
+    );
+
     fn point_at_chain(
         &self,
         expr: &hir::Expr<'_>,
@@ -321,6 +333,7 @@ pub trait TypeErrCtxtExt<'tcx> {
         param_env: ty::ParamEnv<'tcx>,
         err: &mut Diagnostic,
     );
+
     fn probe_assoc_types_at_expr(
         &self,
         type_diffs: &[TypeError<'tcx>],
@@ -3612,6 +3625,109 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
         }
     }
 
+    fn look_for_iterator_item_mistakes(
+        &self,
+        assocs_in_this_method: &[Option<(Span, (DefId, Ty<'tcx>))>],
+        typeck_results: &TypeckResults<'tcx>,
+        type_diffs: &[TypeError<'tcx>],
+        param_env: ty::ParamEnv<'tcx>,
+        path_segment: &hir::PathSegment<'_>,
+        args: &[hir::Expr<'_>],
+        err: &mut Diagnostic,
+    ) {
+        let tcx = self.tcx;
+        // Special case for iterator chains, we look at potential failures of `Iterator::Item`
+        // not being `: Clone` and `Iterator::map` calls with spurious trailing `;`.
+        for entry in assocs_in_this_method {
+            let Some((_span, (def_id, ty))) = entry else {
+                continue;
+            };
+            for diff in type_diffs {
+                let Sorts(expected_found) = diff else {
+                    continue;
+                };
+                if tcx.is_diagnostic_item(sym::IteratorItem, *def_id)
+                    && path_segment.ident.name == sym::map
+                    && self.can_eq(param_env, expected_found.found, *ty)
+                    && let [arg] = args
+                    && let hir::ExprKind::Closure(closure) = arg.kind
+                {
+                    let body = tcx.hir().body(closure.body);
+                    if let hir::ExprKind::Block(block, None) = body.value.kind
+                        && let None = block.expr
+                        && let [.., stmt] = block.stmts
+                        && let hir::StmtKind::Semi(expr) = stmt.kind
+                        // FIXME: actually check the expected vs found types, but right now
+                        // the expected is a projection that we need to resolve.
+                        // && let Some(tail_ty) = typeck_results.expr_ty_opt(expr)
+                        && expected_found.found.is_unit()
+                    {
+                        err.span_suggestion_verbose(
+                            expr.span.shrink_to_hi().with_hi(stmt.span.hi()),
+                            "consider removing this semicolon",
+                            String::new(),
+                            Applicability::MachineApplicable,
+                        );
+                    }
+                    let expr = if let hir::ExprKind::Block(block, None) = body.value.kind
+                        && let Some(expr) = block.expr
+                    {
+                        expr
+                    } else {
+                        body.value
+                    };
+                    if let hir::ExprKind::MethodCall(path_segment, rcvr, [], span) = expr.kind
+                        && path_segment.ident.name == sym::clone
+                        && let Some(expr_ty) = typeck_results.expr_ty_opt(expr)
+                        && let Some(rcvr_ty) = typeck_results.expr_ty_opt(rcvr)
+                        && self.can_eq(param_env, expr_ty, rcvr_ty)
+                        && let ty::Ref(_, ty, _) = expr_ty.kind()
+                    {
+                        err.span_label(
+                            span,
+                            format!(
+                                "this method call is cloning the reference `{expr_ty}`, not \
+                                 `{ty}` which doesn't implement `Clone`",
+                            ),
+                        );
+                        let ty::Param(..) = ty.kind() else {
+                            continue;
+                        };
+                        let hir = tcx.hir();
+                        let node = hir.get_by_def_id(hir.get_parent_item(expr.hir_id).def_id);
+
+                        let pred = ty::Binder::dummy(ty::TraitPredicate {
+                            trait_ref: ty::TraitRef::from_lang_item(
+                                tcx,
+                                LangItem::Clone,
+                                span,
+                                [*ty],
+                            ),
+                            polarity: ty::ImplPolarity::Positive,
+                        });
+                        let Some(generics) = node.generics() else {
+                            continue;
+                        };
+                        let Some(body_id) = node.body_id() else {
+                            continue;
+                        };
+                        suggest_restriction(
+                            tcx,
+                            hir.body_owner_def_id(body_id),
+                            &generics,
+                            &format!("type parameter `{ty}`"),
+                            err,
+                            node.fn_sig(),
+                            None,
+                            pred,
+                            None,
+                        );
+                    }
+                }
+            }
+        }
+    }
+
     fn point_at_chain(
         &self,
         expr: &hir::Expr<'_>,
@@ -3631,13 +3747,22 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
         let mut prev_ty = self.resolve_vars_if_possible(
             typeck_results.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(tcx)),
         );
-        while let hir::ExprKind::MethodCall(_path_segment, rcvr_expr, _args, span) = expr.kind {
+        while let hir::ExprKind::MethodCall(path_segment, rcvr_expr, args, span) = expr.kind {
             // Point at every method call in the chain with the resulting type.
             // vec![1, 2, 3].iter().map(mapper).sum<i32>()
             //               ^^^^^^ ^^^^^^^^^^^
             expr = rcvr_expr;
             let assocs_in_this_method =
                 self.probe_assoc_types_at_expr(&type_diffs, span, prev_ty, expr.hir_id, param_env);
+            self.look_for_iterator_item_mistakes(
+                &assocs_in_this_method,
+                typeck_results,
+                &type_diffs,
+                param_env,
+                path_segment,
+                args,
+                err,
+            );
             assocs.push(assocs_in_this_method);
             prev_ty = self.resolve_vars_if_possible(
                 typeck_results.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(tcx)),

tests/ui/iterators/invalid-iterator-chain-fixable.fixed

@@ -0,0 +1,42 @@
+// run-rustfix
+use std::collections::hash_set::Iter;
+use std::collections::HashSet;
+
+fn iter_to_vec<'b, X>(i: Iter<'b, X>) -> Vec<X> where X: Clone {
+    let i = i.map(|x| x.clone());
+    i.collect() //~ ERROR E0277
+}
+
+fn main() {
+    let v = vec![(0, 0)];
+    let scores = v
+        .iter()
+        .map(|(a, b)| {
+            a + b
+        });
+    println!("{}", scores.sum::<i32>()); //~ ERROR E0277
+    println!(
+        "{}",
+        vec![0, 1]
+            .iter()
+            .map(|x| x * 2)
+            .map(|x| { x })
+            .map(|x| { x })
+            .sum::<i32>(), //~ ERROR E0277
+    );
+    println!("{}", vec![0, 1].iter().map(|x| { x }).sum::<i32>()); //~ ERROR E0277
+    let a = vec![0];
+    let b = a.into_iter();
+    let c = b.map(|x| x + 1);
+    let d = c.filter(|x| *x > 10 );
+    let e = d.map(|x| {
+        x + 1
+    });
+    let f = e.filter(|_| false);
+    let g: Vec<i32> = f.collect(); //~ ERROR E0277
+    println!("{g:?}");
+
+    let mut s = HashSet::new();
+    s.insert(1u8);
+    println!("{:?}", iter_to_vec(s.iter()));
+}

tests/ui/iterators/invalid-iterator-chain-fixable.rs

@@ -0,0 +1,42 @@
+// run-rustfix
+use std::collections::hash_set::Iter;
+use std::collections::HashSet;
+
+fn iter_to_vec<'b, X>(i: Iter<'b, X>) -> Vec<X> {
+    let i = i.map(|x| x.clone());
+    i.collect() //~ ERROR E0277
+}
+
+fn main() {
+    let v = vec![(0, 0)];
+    let scores = v
+        .iter()
+        .map(|(a, b)| {
+            a + b;
+        });
+    println!("{}", scores.sum::<i32>()); //~ ERROR E0277
+    println!(
+        "{}",
+        vec![0, 1]
+            .iter()
+            .map(|x| x * 2)
+            .map(|x| { x; })
+            .map(|x| { x })
+            .sum::<i32>(), //~ ERROR E0277
+    );
+    println!("{}", vec![0, 1].iter().map(|x| { x; }).sum::<i32>()); //~ ERROR E0277
+    let a = vec![0];
+    let b = a.into_iter();
+    let c = b.map(|x| x + 1);
+    let d = c.filter(|x| *x > 10 );
+    let e = d.map(|x| {
+        x + 1;
+    });
+    let f = e.filter(|_| false);
+    let g: Vec<i32> = f.collect(); //~ ERROR E0277
+    println!("{g:?}");
+
+    let mut s = HashSet::new();
+    s.insert(1u8);
+    println!("{:?}", iter_to_vec(s.iter()));
+}

tests/ui/iterators/invalid-iterator-chain-fixable.stderr

@@ -0,0 +1,157 @@
+error[E0277]: a value of type `Vec<X>` cannot be built from an iterator over elements of type `&X`
+  --> $DIR/invalid-iterator-chain-fixable.rs:7:7
+   |
+LL |     let i = i.map(|x| x.clone());
+   |                         ------- this method call is cloning the reference `&X`, not `X` which doesn't implement `Clone`
+LL |     i.collect()
+   |       ^^^^^^^ value of type `Vec<X>` cannot be built from `std::iter::Iterator<Item=&X>`
+   |
+   = help: the trait `FromIterator<&X>` is not implemented for `Vec<X>`
+   = help: the trait `FromIterator<X>` is implemented for `Vec<X>`
+   = help: for that trait implementation, expected `X`, found `&X`
+note: the method call chain might not have had the expected associated types
+  --> $DIR/invalid-iterator-chain-fixable.rs:5:26
+   |
+LL | fn iter_to_vec<'b, X>(i: Iter<'b, X>) -> Vec<X> {
+   |                          ^^^^^^^^^^^ `Iterator::Item` is `&X` here
+LL |     let i = i.map(|x| x.clone());
+   |               ------------------ `Iterator::Item` remains `&X` here
+note: required by a bound in `collect`
+  --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
+help: consider further restricting type parameter `X`
+   |
+LL | fn iter_to_vec<'b, X>(i: Iter<'b, X>) -> Vec<X> where X: Clone {
+   |                                                 ++++++++++++++
+
+error[E0277]: a value of type `i32` cannot be made by summing an iterator over elements of type `()`
+  --> $DIR/invalid-iterator-chain-fixable.rs:17:33
+   |
+LL |     println!("{}", scores.sum::<i32>());
+   |                           ---   ^^^ value of type `i32` cannot be made by summing a `std::iter::Iterator<Item=()>`
+   |                           |
+   |                           required by a bound introduced by this call
+   |
+   = help: the trait `Sum<()>` is not implemented for `i32`
+   = help: the following other types implement trait `Sum<A>`:
+             <i32 as Sum>
+             <i32 as Sum<&'a i32>>
+note: the method call chain might not have had the expected associated types
+  --> $DIR/invalid-iterator-chain-fixable.rs:14:10
+   |
+LL |       let v = vec![(0, 0)];
+   |               ------------ this expression has type `Vec<({integer}, {integer})>`
+LL |       let scores = v
+LL |           .iter()
+   |            ------ `Iterator::Item` is `&({integer}, {integer})` here
+LL |           .map(|(a, b)| {
+   |  __________^
+LL | |             a + b;
+LL | |         });
+   | |__________^ `Iterator::Item` changed to `()` here
+note: required by a bound in `std::iter::Iterator::sum`
+  --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
+help: consider removing this semicolon
+   |
+LL -             a + b;
+LL +             a + b
+   |
+
+error[E0277]: a value of type `i32` cannot be made by summing an iterator over elements of type `()`
+  --> $DIR/invalid-iterator-chain-fixable.rs:25:20
+   |
+LL |             .sum::<i32>(),
+   |              ---   ^^^ value of type `i32` cannot be made by summing a `std::iter::Iterator<Item=()>`
+   |              |
+   |              required by a bound introduced by this call
+   |
+   = help: the trait `Sum<()>` is not implemented for `i32`
+   = help: the following other types implement trait `Sum<A>`:
+             <i32 as Sum>
+             <i32 as Sum<&'a i32>>
+note: the method call chain might not have had the expected associated types
+  --> $DIR/invalid-iterator-chain-fixable.rs:23:14
+   |
+LL |         vec![0, 1]
+   |         ---------- this expression has type `Vec<{integer}>`
+LL |             .iter()
+   |              ------ `Iterator::Item` is `&{integer}` here
+LL |             .map(|x| x * 2)
+   |              -------------- `Iterator::Item` changed to `{integer}` here
+LL |             .map(|x| { x; })
+   |              ^^^^^^^^^^^^^^^ `Iterator::Item` changed to `()` here
+LL |             .map(|x| { x })
+   |              -------------- `Iterator::Item` remains `()` here
+note: required by a bound in `std::iter::Iterator::sum`
+  --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
+help: consider removing this semicolon
+   |
+LL -             .map(|x| { x; })
+LL +             .map(|x| { x })
+   |
+
+error[E0277]: a value of type `i32` cannot be made by summing an iterator over elements of type `()`
+  --> $DIR/invalid-iterator-chain-fixable.rs:27:60
+   |
+LL |     println!("{}", vec![0, 1].iter().map(|x| { x; }).sum::<i32>());
+   |                                                      ---   ^^^ value of type `i32` cannot be made by summing a `std::iter::Iterator<Item=()>`
+   |                                                      |
+   |                                                      required by a bound introduced by this call
+   |
+   = help: the trait `Sum<()>` is not implemented for `i32`
+   = help: the following other types implement trait `Sum<A>`:
+             <i32 as Sum>
+             <i32 as Sum<&'a i32>>
+note: the method call chain might not have had the expected associated types
+  --> $DIR/invalid-iterator-chain-fixable.rs:27:38
+   |
+LL |     println!("{}", vec![0, 1].iter().map(|x| { x; }).sum::<i32>());
+   |                    ---------- ------ ^^^^^^^^^^^^^^^ `Iterator::Item` changed to `()` here
+   |                    |          |
+   |                    |          `Iterator::Item` is `&{integer}` here
+   |                    this expression has type `Vec<{integer}>`
+note: required by a bound in `std::iter::Iterator::sum`
+  --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
+help: consider removing this semicolon
+   |
+LL -     println!("{}", vec![0, 1].iter().map(|x| { x; }).sum::<i32>());
+LL +     println!("{}", vec![0, 1].iter().map(|x| { x }).sum::<i32>());
+   |
+
+error[E0277]: a value of type `Vec<i32>` cannot be built from an iterator over elements of type `()`
+  --> $DIR/invalid-iterator-chain-fixable.rs:36:25
+   |
+LL |     let g: Vec<i32> = f.collect();
+   |                         ^^^^^^^ value of type `Vec<i32>` cannot be built from `std::iter::Iterator<Item=()>`
+   |
+   = help: the trait `FromIterator<()>` is not implemented for `Vec<i32>`
+   = help: the trait `FromIterator<i32>` is implemented for `Vec<i32>`
+   = help: for that trait implementation, expected `i32`, found `()`
+note: the method call chain might not have had the expected associated types
+  --> $DIR/invalid-iterator-chain-fixable.rs:32:15
+   |
+LL |       let a = vec![0];
+   |               ------- this expression has type `Vec<{integer}>`
+LL |       let b = a.into_iter();
+   |                 ----------- `Iterator::Item` is `{integer}` here
+LL |       let c = b.map(|x| x + 1);
+   |                 -------------- `Iterator::Item` remains `{integer}` here
+LL |       let d = c.filter(|x| *x > 10 );
+   |                 -------------------- `Iterator::Item` remains `{integer}` here
+LL |       let e = d.map(|x| {
+   |  _______________^
+LL | |         x + 1;
+LL | |     });
+   | |______^ `Iterator::Item` changed to `()` here
+LL |       let f = e.filter(|_| false);
+   |                 ----------------- `Iterator::Item` remains `()` here
+note: required by a bound in `collect`
+  --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
+help: consider removing this semicolon
+   |
+LL -         x + 1;
+LL +         x + 1
+   |
+
+error: aborting due to 5 previous errors
+
+For more information about this error, try `rustc --explain E0277`.

tests/ui/iterators/invalid-iterator-chain.stderr

@@ -1,6 +1,8 @@
 error[E0277]: a value of type `Vec<X>` cannot be built from an iterator over elements of type `&X`
   --> $DIR/invalid-iterator-chain.rs:6:7
    |
+LL |     let i = i.map(|x| x.clone());
+   |                         ------- this method call is cloning the reference `&X`, not `X` which doesn't implement `Clone`
 LL |     i.collect()
    |       ^^^^^^^ value of type `Vec<X>` cannot be built from `std::iter::Iterator<Item=&X>`
    |
@@ -16,6 +18,10 @@ LL |     let i = i.map(|x| x.clone());
    |               ------------------ `Iterator::Item` remains `&X` here
 note: required by a bound in `collect`
   --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
+help: consider further restricting type parameter `X`
+   |
+LL | fn iter_to_vec<'b, X>(i: Iter<'b, X>) -> Vec<X> where X: Clone {
+   |                                                 ++++++++++++++
 
 error[E0277]: a value of type `i32` cannot be made by summing an iterator over elements of type `()`
   --> $DIR/invalid-iterator-chain.rs:15:33
@@ -43,6 +49,11 @@ LL | |         });
    | |__________^ `Iterator::Item` changed to `()` here
 note: required by a bound in `std::iter::Iterator::sum`
   --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
+help: consider removing this semicolon
+   |
+LL -             a + b;
+LL +             a + b
+   |
 
 error[E0277]: a value of type `i32` cannot be made by summing an iterator over elements of type `()`
   --> $DIR/invalid-iterator-chain.rs:26:20
@@ -77,6 +88,11 @@ LL |             .map(|x| { x; })
    |              ^^^^^^^^^^^^^^^ `Iterator::Item` changed to `()` here
 note: required by a bound in `std::iter::Iterator::sum`
   --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
+help: consider removing this semicolon
+   |
+LL -             .map(|x| { x; })
+LL +             .map(|x| { x })
+   |
 
 error[E0277]: a value of type `i32` cannot be made by summing an iterator over elements of type `f64`
   --> $DIR/invalid-iterator-chain.rs:36:20
@@ -130,6 +146,11 @@ LL |     println!("{}", vec![0, 1].iter().map(|x| { x; }).sum::<i32>());
    |                    this expression has type `Vec<{integer}>`
 note: required by a bound in `std::iter::Iterator::sum`
   --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
+help: consider removing this semicolon
+   |
+LL -     println!("{}", vec![0, 1].iter().map(|x| { x; }).sum::<i32>());
+LL +     println!("{}", vec![0, 1].iter().map(|x| { x }).sum::<i32>());
+   |
 
 error[E0277]: a value of type `i32` cannot be made by summing an iterator over elements of type `&()`
   --> $DIR/invalid-iterator-chain.rs:39:46
@@ -182,6 +203,11 @@ LL |       let f = e.filter(|_| false);
    |                 ----------------- `Iterator::Item` remains `()` here
 note: required by a bound in `collect`
   --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
+help: consider removing this semicolon
+   |
+LL -         x + 1;
+LL +         x + 1
+   |
 
 error: aborting due to 7 previous errors