hir_typeck: suggest removing redundant .iter() calls

This diagnostic intercepts cases where `.iter()` is called on a type
that already implements `Iterator`

Instead of confusing trait fallback suggestions, it explicitly guides
the user to remove the `.iter()` call. It implements precise span
computation to support `cargo fix` while carefully preserving all inline
comments and horizontal whitespace during the AST string replacement.

Author: vishnupoddar12

compiler/rustc_hir_typeck/src/method/suggest.rs

@@ -34,8 +34,8 @@ use rustc_middle::ty::print::{
 use rustc_middle::ty::{self, GenericArgKind, IsSuggestable, Ty, TyCtxt, TypeVisitableExt};
 use rustc_span::def_id::DefIdSet;
 use rustc_span::{
-    DUMMY_SP, ErrorGuaranteed, ExpnKind, FileName, Ident, MacroKind, Span, Symbol, edit_distance,
-    kw, sym,
+    DUMMY_SP, ErrorGuaranteed, ExpnKind, FileName, Ident, MacroKind, Pos, Span, Symbol,
+    edit_distance, kw, sym,
 };
 use rustc_trait_selection::error_reporting::traits::DefIdOrName;
 use rustc_trait_selection::infer::InferCtxtExt;
@@ -97,6 +97,119 @@ impl TraitBoundDuplicateTracker {
 }
 
 impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
+    /// Returns the byte offset of the last `.` in `s` that lies outside any
+    /// comment: line comments (`//`) or block comments (`/* ... */`, including nested).
+    fn find_last_real_dot(s: &str) -> Option<usize> {
+        enum State {
+            Code,
+            LineComment,
+            BlockComment { depth: u32 },
+        }
+
+        let mut state = State::Code;
+        let mut last_dot = None;
+        let mut chars = s.char_indices().peekable();
+
+        while let Some((i, c)) = chars.next() {
+            let peeked = chars.peek().map(|&(_, c)| c);
+
+            state = match state {
+                State::Code => match (c, peeked) {
+                    ('/', Some('/')) => {
+                        chars.next();
+                        State::LineComment
+                    }
+                    ('/', Some('*')) => {
+                        chars.next();
+                        State::BlockComment { depth: 1 }
+                    }
+                    ('.', _) => {
+                        last_dot = Some(i);
+                        State::Code
+                    }
+                    _ => State::Code,
+                },
+                State::LineComment => match c {
+                    '\n' => State::Code,
+                    _ => State::LineComment,
+                },
+                State::BlockComment { depth } => match (c, peeked) {
+                    ('/', Some('*')) => {
+                        chars.next();
+                        State::BlockComment { depth: depth + 1 }
+                    }
+                    ('*', Some('/')) => {
+                        chars.next();
+                        if depth == 1 {
+                            State::Code
+                        } else {
+                            State::BlockComment { depth: depth - 1 }
+                        }
+                    }
+                    _ => State::BlockComment { depth },
+                },
+            };
+        }
+
+        last_dot
+    }
+
+    /// Computes the exact span to delete and its applicability for removing a
+    /// redundant `.iter()` call.
+    /// Returns `None` if the suggestion cannot be safely derived from the source map.
+    fn compute_iter_removal_suggestion(
+        &self,
+        call_span: Span,
+        span: Span,
+    ) -> Option<(Span, Applicability)> {
+        if span.lo() < call_span.lo() || span.hi() > call_span.hi() {
+            return None;
+        }
+
+        let snippet = self.tcx.sess.source_map().span_to_snippet(call_span).ok()?;
+        let method_offset = (span.lo() - call_span.lo()).to_usize();
+        let dot_offset = Self::find_last_real_dot(&snippet[..method_offset])?;
+        let text_before_dot = &snippet[..dot_offset];
+
+        let contains_comment = |s: &str| s.contains("//") || s.contains("/*") || s.contains("*/");
+
+        // Inspect the text immediately preceding the dot, ignoring horizontal whitespace.
+        let stripped_horizontal_ws = text_before_dot.trim_end_matches([' ', '\t']);
+        let is_on_new_line = stripped_horizontal_ws.ends_with(['\n', '\r']);
+        let has_preceding_comment = contains_comment(stripped_horizontal_ws);
+
+        // Determine where to start the replacement span based on surrounding formatting.
+        let replacement_offset = match (is_on_new_line, has_preceding_comment) {
+            // The dot is on its own line with no preceding comment: collapse the whole indentation.
+            (true, false) => stripped_horizontal_ws.trim_end().len(),
+            // A comment sits between the expression and the dot on the same line:
+            // preserve the comment by starting the replacement exactly at the dot.
+            (false, true) => dot_offset,
+            // `(false, false)`: standard inline call (e.g. `foo.iter()`).
+            // `(true, true)`: comment on the previous line, followed by a newline and indented dot.
+            // In both cases, only strip the trailing horizontal whitespace.
+            _ => stripped_horizontal_ws.len(),
+        };
+
+        let replacement_span = call_span.with_lo(
+            call_span.lo()
+                + rustc_span::BytePos(
+                    u32::try_from(replacement_offset)
+                        .expect("replacement offset exceeded maximum span length"),
+                ),
+        );
+
+        // A comment in the section being removed risks deleting user documentation,
+        // so we downgrade applicability to signal that human review is warranted.
+        let applicability = if contains_comment(&snippet[replacement_offset..]) {
+            Applicability::MaybeIncorrect
+        } else {
+            Applicability::MachineApplicable
+        };
+
+        Some((replacement_span, applicability))
+    }
+
     fn is_slice_ty(&self, ty: Ty<'tcx>, span: Span) -> bool {
         self.autoderef(span, ty)
             .silence_errors()
@@ -758,6 +871,50 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
     }
 
+    /// Suggests removing a redundant `.iter()` call if the receiver already
+    /// implements the `Iterator` trait.
+    /// Returns `true` if a suggestion or note was emitted, signaling that
+    /// further diagnostic fallback should be suppressed.
+    fn suggest_removing_iter_call(
+        &self,
+        err: &mut Diag<'_>,
+        rcvr_expr: &hir::Expr<'_>,
+        rcvr_ty: Ty<'tcx>,
+        span: Span,
+        sugg_span: Span,
+    ) -> bool {
+        // Ensure the receiver and the suggestion are in the same macro context.
+        if !rcvr_expr.span.eq_ctxt(sugg_span) {
+            return false;
+        }
+
+        // Guard against constructing an invalid span across macro edges.
+        if rcvr_expr.span.hi() > sugg_span.hi() {
+            return false;
+        }
+
+        // The span encompassing the entire `.iter()` call.
+        let call_span = span.with_lo(rcvr_expr.span.hi()).with_hi(sugg_span.hi());
+        if call_span.from_expansion() {
+            return false;
+        }
+
+        let ty_str = self.tcx.short_string(rcvr_ty, err.long_ty_path());
+        err.note(format!("`{ty_str}` already implements `Iterator`"));
+        match self.compute_iter_removal_suggestion(call_span, span) {
+            Some((replacement_span, applicability)) => {
+                err.span_suggestion_verbose(
+                    replacement_span,
+                    format!("consider removing the `.iter()` call"),
+                    "",
+                    applicability,
+                );
+                true
+            }
+            None => false,
+        }
+    }
+
     fn suggest_static_method_candidates(
         &self,
         err: &mut Diag<'_>,
@@ -828,6 +985,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         source: SelfSource<'tcx>,
         unsatisfied_predicates: &UnsatisfiedPredicates<'tcx>,
         static_candidates: &[CandidateSource],
+        sugg_span: Span,
     ) -> Result<(bool, bool, bool, bool, SortedMap<Span, Vec<String>>), ()> {
         let mut restrict_type_params = false;
         let mut suggested_derive = false;
@@ -836,6 +994,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let mut bound_spans: SortedMap<Span, Vec<String>> = Default::default();
         let tcx = self.tcx;
 
+        if item_ident.name == sym::iter
+            && let SelfSource::MethodCall(rcvr_expr) = source
+            && let Some(iterator_trait) = self.tcx.get_diagnostic_item(sym::Iterator)
+            && self.predicate_must_hold_modulo_regions(&Obligation::new(
+                self.tcx,
+                self.misc(span),
+                self.param_env,
+                ty::TraitRef::new(self.tcx, iterator_trait, [rcvr_ty]),
+            ))
+        {
+            if self.suggest_removing_iter_call(err, rcvr_expr, rcvr_ty, span, sugg_span) {
+                return Err(());
+            }
+        }
+
         if item_ident.name == sym::count && self.is_slice_ty(rcvr_ty, span) {
             let msg = "consider using `len` instead";
             if let SelfSource::MethodCall(_expr) = source {
@@ -1268,6 +1441,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             source,
             unsatisfied_predicates,
             &static_candidates,
+            sugg_span,
         )
         else {
             return err.emit();

tests/ui/iterators/iter-on-already-iterator-competing-suggestions.fixed

@@ -0,0 +1,19 @@
+//@ run-rustfix
+#![allow(dead_code, unused_imports)]
+// Regression test for short-circuiting redundant iter() diagnostics
+// to prevent competing suggestions.
+
+mod out_of_scope {
+    pub trait MyIterExt {
+        fn iter(&self) {}
+    }
+    impl<T: Iterator> MyIterExt for T {}
+}
+
+fn out_of_scope_trait(iter: impl Iterator<Item = i32>) {
+    // Out-of-scope trait that actually has `.iter()`.
+    let _ = iter;
+    //~^ ERROR no method named `iter` found
+}
+
+fn main() {}

tests/ui/iterators/iter-on-already-iterator-competing-suggestions.rs

@@ -0,0 +1,19 @@
+//@ run-rustfix
+#![allow(dead_code, unused_imports)]
+// Regression test for short-circuiting redundant iter() diagnostics
+// to prevent competing suggestions.
+
+mod out_of_scope {
+    pub trait MyIterExt {
+        fn iter(&self) {}
+    }
+    impl<T: Iterator> MyIterExt for T {}
+}
+
+fn out_of_scope_trait(iter: impl Iterator<Item = i32>) {
+    // Out-of-scope trait that actually has `.iter()`.
+    let _ = iter.iter();
+    //~^ ERROR no method named `iter` found
+}
+
+fn main() {}

tests/ui/iterators/iter-on-already-iterator-competing-suggestions.stderr

@@ -0,0 +1,16 @@
+error[E0599]: no method named `iter` found for type parameter `impl Iterator<Item = i32>` in the current scope
+  --> $DIR/iter-on-already-iterator-competing-suggestions.rs:15:18
+   |
+LL |     let _ = iter.iter();
+   |                  ^^^^
+   |
+   = note: `impl Iterator<Item = i32>` already implements `Iterator`
+help: consider removing the `.iter()` call
+   |
+LL -     let _ = iter.iter();
+LL +     let _ = iter;
+   |
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0599`.

tests/ui/iterators/iter-on-already-iterator.fixed

@@ -0,0 +1,89 @@
+//@ run-rustfix
+//! Exhaustive tests for the targeted diagnostic that suggests removing redundant `.iter()` calls.
+//! This verifies that the span surgery handles comments and whitespace correctly
+//! during `cargo fix`.
+#![allow(unused)]
+
+#[rustfmt::skip]
+fn main() {
+    let receiver = vec![1, 2, 3].into_iter();
+
+    // Simple inline call.
+    let _ = receiver;
+    //~^ ERROR no method named `iter`
+
+    let receiver = vec![1, 2, 3].into_iter();
+    // Call with extra horizontal whitespace between receiver and dot.
+    let _ = receiver;
+    //~^ ERROR no method named `iter`
+
+    let receiver = vec![1, 2, 3].into_iter();
+    // Call with a newline and indentation before the dot.
+    let _ = receiver;
+        //~^ ERROR no method named `iter`
+
+    let receiver = vec![1, 2, 3].into_iter();
+    // Call with a block comment between the receiver and the dot.
+    let _ = receiver /* info */ ;
+    //~^ ERROR no method named `iter`
+
+    let receiver = vec![1, 2, 3].into_iter();
+    // Call with a line comment on the preceding line.
+    let _ = receiver // info
+;
+        //~^ ERROR no method named `iter`
+
+    let receiver = vec![1, 2, 3].into_iter();
+    // Downgrade applicability if a comment is present within the deleted span.
+    // Because it's `MaybeIncorrect`, `rustfix` will only show the suggestion
+    // but not apply it.
+    let _ = receiver /* wait, keep this! */;
+        //~^ ERROR no method named `iter`
+
+    let receiver = vec![1, 2, 3].into_iter();
+    // A comment containing a dot shouldn't trick the span parser.
+    let _ = receiver;
+    //~^ ERROR no method named `iter`
+
+    let receiver = vec![1, 2, 3].into_iter();
+    // A comment containing a dot shouldn't trick the span parser (dot afterwards).
+    let _ = receiver /* . */ ;
+    //~^ ERROR no method named `iter`
+
+    let receiver = vec![1, 2, 3].into_iter();
+    // Chained method call after `.iter()`: only `.iter()` is removed,
+    let _ = receiver.map(|x| x);
+    //~^ ERROR no method named `iter`
+
+    // Multi-segment path receiver.
+    let _ = std::iter::empty::<i32>();
+    //~^ ERROR no method named `iter`
+}
+
+// Concrete user-defined struct implementing Iterator directly.
+struct MyIter;
+
+impl Iterator for MyIter {
+    type Item = i32;
+    fn next(&mut self) -> Option<Self::Item> {
+        None
+    }
+}
+
+fn concrete_iterator() {
+    let it = MyIter;
+    let _ = it;
+    //~^ ERROR no method named `iter`
+}
+
+// `Box<dyn Iterator>` -- verify the trait check fires through trait objects.
+fn dyn_iterator(it: Box<dyn Iterator<Item = i32>>) {
+    let _ = it;
+    //~^ ERROR no method named `iter`
+}
+
+// Generic context with an explicit bound.
+fn generic_iterator<I: Iterator<Item = i32>>(it: I) {
+    let _ = it;
+    //~^ ERROR no method named `iter`
+}