Suggest .iter() for shared projections

* Suggest `.iter()` for shared projections
* address few nits
* a few improvements

compiler/rustc_hir_typeck/src/expr_use_visitor.rs

@@ -2,7 +2,7 @@
 //! normal visitor, which just walks the entire body in one shot, the
 //! `ExprUseVisitor` determines how expressions are being used.
 //!
-//! In the compiler, this is only used for upvar inference, but there
+//! In the compiler, this is only used for upvar inference and diagnostics, but there
 //! are many uses within clippy.
 
 use std::cell::{Ref, RefCell};
@@ -1855,3 +1855,27 @@ fn is_multivariant_adt(&self, ty: Ty<'tcx>, span: Span) -> bool {
         }
     }
 }
+
+struct ExprPlaceDelegate;
+
+impl<'tcx> Delegate<'tcx> for ExprPlaceDelegate {
+    fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {}
+
+    fn use_cloned(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {}
+
+    fn borrow(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId, _: ty::BorrowKind) {}
+
+    fn mutate(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {}
+
+    fn fake_read(&mut self, _: &PlaceWithHirId<'tcx>, _: FakeReadCause, _: HirId) {}
+}
+
+/// Categorizes `expr` as a place for diagnostic suggestions.
+///
+/// This should be used for diagnostics purpose only.
+pub(crate) fn expr_place<'tcx>(
+    fcx: &FnCtxt<'_, 'tcx>,
+    expr: &hir::Expr<'_>,
+) -> Result<PlaceWithHirId<'tcx>, ErrorGuaranteed> {
+    ExprUseVisitor::new(fcx, ExprPlaceDelegate).cat_expr(expr)
+}

compiler/rustc_hir_typeck/src/method/suggest.rs

@@ -48,6 +48,7 @@
 use super::probe::{AutorefOrPtrAdjustment, IsSuggestion, Mode, ProbeScope};
 use super::{CandidateSource, MethodError, NoMatchData};
 use crate::errors::{self, CandidateTraitNote, NoAssociatedItem};
+use crate::expr_use_visitor::expr_place;
 use crate::method::probe::UnsatisfiedPredicates;
 use crate::{Expectation, FnCtxt};
 
@@ -189,6 +190,70 @@ fn predicate_bounds_generic_param<'tcx>(
         false
     }
 
+    // Pick the iterator method to suggest: `.into_iter()` by default, and
+    // `.iter()`/`.iter_mut()` for projections through references.
+    fn preferred_iterator_method(
+        &self,
+        source: SelfSource<'tcx>,
+        rcvr_ty: Ty<'tcx>,
+    ) -> Option<Symbol> {
+        let SelfSource::MethodCall(rcvr_expr) = source else {
+            return Some(sym::into_iter);
+        };
+
+        let rcvr_expr = rcvr_expr.peel_drop_temps().peel_blocks();
+        let Ok(place_with_id) = expr_place(self, rcvr_expr) else {
+            return None;
+        };
+
+        let mut projection_mutability = None;
+        for pointer_ty in place_with_id.place.deref_tys() {
+            match self.structurally_resolve_type(rcvr_expr.span, pointer_ty).kind() {
+                ty::Ref(.., Mutability::Not) => {
+                    projection_mutability = Some(Mutability::Not);
+                    break;
+                }
+                ty::Ref(.., Mutability::Mut) => {
+                    projection_mutability.get_or_insert(Mutability::Mut);
+                }
+                ty::RawPtr(..) => return None,
+                _ => {}
+            }
+        }
+
+        // Keep `.into_iter()` for receivers like `&Vec<_>`; only projections that
+        // dereference a reference need to switch to `iter`/`iter_mut`.
+        let Some(projection_mutability) = projection_mutability else {
+            return Some(sym::into_iter);
+        };
+
+        let call_expr = self.tcx.hir_expect_expr(self.tcx.parent_hir_id(rcvr_expr.hir_id));
+        // `IntoIterator` does not imply inherent `iter`/`iter_mut` methods.
+        let has_method = |method_name| {
+            self.lookup_probe_for_diagnostic(
+                Ident::with_dummy_span(method_name),
+                rcvr_ty,
+                call_expr,
+                ProbeScope::TraitsInScope,
+                None,
+            )
+            .is_ok()
+        };
+
+        match projection_mutability {
+            Mutability::Not => has_method(sym::iter).then_some(sym::iter),
+            Mutability::Mut => {
+                if has_method(sym::iter_mut) {
+                    Some(sym::iter_mut)
+                } else if has_method(sym::iter) {
+                    Some(sym::iter)
+                } else {
+                    None
+                }
+            }
+        }
+    }
+
     #[instrument(level = "debug", skip(self))]
     pub(crate) fn report_method_error(
         &self,
@@ -855,10 +920,12 @@ fn suggest_unsatisfied_ty_or_trait(
         } else if self.impl_into_iterator_should_be_iterator(rcvr_ty, span, unsatisfied_predicates)
         {
             err.span_label(span, format!("`{rcvr_ty}` is not an iterator"));
-            if !span.in_external_macro(self.tcx.sess.source_map()) {
+            if !span.in_external_macro(self.tcx.sess.source_map())
+                && let Some(method_name) = self.preferred_iterator_method(source, rcvr_ty)
+            {
                 err.multipart_suggestion(
-                    "call `.into_iter()` first",
-                    vec![(span.shrink_to_lo(), format!("into_iter()."))],
+                    format!("call `.{method_name}()` first"),
+                    vec![(span.shrink_to_lo(), format!("{method_name}()."))],
                     Applicability::MaybeIncorrect,
                 );
             }

tests/ui/did_you_mean/collect-without-into-iter-call.rs

@@ -1,12 +1,14 @@
-// Tests that the compiler suggests an `into_iter` call when an `Iterator` method
-// is called on something that implements `IntoIterator`
+// Tests that the compiler suggests an iterator method when an `Iterator` method
+// is called on something that implements `IntoIterator`.
 
 fn main() {
     let items = items();
     let other_items = items.map(|i| i + 1);
     //~^ ERROR no method named `map` found for opaque type `impl IntoIterator<Item = i32>` in the current scope
+    //~| HELP: call `.into_iter()` first
     let vec: Vec<i32> = items.collect();
     //~^ ERROR no method named `collect` found for opaque type `impl IntoIterator<Item = i32>` in the current scope
+    //~| HELP: call `.into_iter()` first
 }
 
 fn items() -> impl IntoIterator<Item = i32> {
@@ -16,4 +18,36 @@ fn items() -> impl IntoIterator<Item = i32> {
 fn process(items: impl IntoIterator<Item = String>) -> Vec<String> {
     items.collect()
     //~^ ERROR no method named `collect` found for type parameter `impl IntoIterator<Item = String>` in the current scope
+    //~| HELP: call `.into_iter()` first
+}
+
+// Regression test for https://github.com/rust-lang/rust/issues/155365
+struct Demo {
+    contents: Vec<u32>,
+}
+
+impl Demo {
+    fn count_odds(&self) -> usize {
+        self.contents.filter(|v| *v % 2 == 1).count()
+        //~^ ERROR no method named `filter` found for struct `Vec<u32>` in the current scope
+        //~| HELP: call `.iter()` first
+    }
+
+    fn increment(&mut self) {
+        self.contents.for_each(|v| *v += 1)
+        //~^ ERROR no method named `for_each` found for struct `Vec<u32>` in the current scope
+        //~| HELP: call `.iter_mut()` first
+    }
+}
+
+fn count_odds_param(contents: &Vec<u32>) -> usize {
+    contents.filter(|v| *v % 2 == 1).count()
+    //~^ ERROR no method named `filter` found for reference `&Vec<u32>` in the current scope
+    //~| HELP: call `.into_iter()` first
+}
+
+fn count_odds_explicit_deref(contents: &Vec<u32>) -> usize {
+    (*contents).filter(|v| *v % 2 == 1).count()
+    //~^ ERROR no method named `filter` found for struct `Vec<u32>` in the current scope
+    //~| HELP: call `.iter()` first
 }

tests/ui/did_you_mean/collect-without-into-iter-call.stderr

@@ -10,7 +10,7 @@ LL |     let other_items = items.into_iter().map(|i| i + 1);
    |                             ++++++++++++
 
 error[E0599]: no method named `collect` found for opaque type `impl IntoIterator<Item = i32>` in the current scope
-  --> $DIR/collect-without-into-iter-call.rs:8:31
+  --> $DIR/collect-without-into-iter-call.rs:9:31
    |
 LL |     let vec: Vec<i32> = items.collect();
    |                               ^^^^^^^ `impl IntoIterator<Item = i32>` is not an iterator
@@ -21,7 +21,7 @@ LL |     let vec: Vec<i32> = items.into_iter().collect();
    |                               ++++++++++++
 
 error[E0599]: no method named `collect` found for type parameter `impl IntoIterator<Item = String>` in the current scope
-  --> $DIR/collect-without-into-iter-call.rs:17:11
+  --> $DIR/collect-without-into-iter-call.rs:19:11
    |
 LL |     items.collect()
    |           ^^^^^^^ `impl IntoIterator<Item = String>` is not an iterator
@@ -31,6 +31,50 @@ help: call `.into_iter()` first
 LL |     items.into_iter().collect()
    |           ++++++++++++
 
-error: aborting due to 3 previous errors
+error[E0599]: no method named `filter` found for struct `Vec<u32>` in the current scope
+  --> $DIR/collect-without-into-iter-call.rs:31:23
+   |
+LL |         self.contents.filter(|v| *v % 2 == 1).count()
+   |                       ^^^^^^ `Vec<u32>` is not an iterator
+   |
+help: call `.iter()` first
+   |
+LL |         self.contents.iter().filter(|v| *v % 2 == 1).count()
+   |                       +++++++
+
+error[E0599]: no method named `for_each` found for struct `Vec<u32>` in the current scope
+  --> $DIR/collect-without-into-iter-call.rs:37:23
+   |
+LL |         self.contents.for_each(|v| *v += 1)
+   |                       ^^^^^^^^ `Vec<u32>` is not an iterator
+   |
+help: call `.iter_mut()` first
+   |
+LL |         self.contents.iter_mut().for_each(|v| *v += 1)
+   |                       +++++++++++
+
+error[E0599]: no method named `filter` found for reference `&Vec<u32>` in the current scope
+  --> $DIR/collect-without-into-iter-call.rs:44:14
+   |
+LL |     contents.filter(|v| *v % 2 == 1).count()
+   |              ^^^^^^ `&Vec<u32>` is not an iterator
+   |
+help: call `.into_iter()` first
+   |
+LL |     contents.into_iter().filter(|v| *v % 2 == 1).count()
+   |              ++++++++++++
+
+error[E0599]: no method named `filter` found for struct `Vec<u32>` in the current scope
+  --> $DIR/collect-without-into-iter-call.rs:50:17
+   |
+LL |     (*contents).filter(|v| *v % 2 == 1).count()
+   |                 ^^^^^^ `Vec<u32>` is not an iterator
+   |
+help: call `.iter()` first
+   |
+LL |     (*contents).iter().filter(|v| *v % 2 == 1).count()
+   |                 +++++++
+
+error: aborting due to 7 previous errors
 
 For more information about this error, try `rustc --explain E0599`.