Rollup merge of #157917 - onehr:fix-for-loop-missing-in-suggestion-103561, r=folkertdev

Don't suggest adding `in` to a `for` loop that already has one

Closes #103561.

When a `for` loop is missing its `in`, the parser suggested inserting one based only on the token following the pattern. For a malformed binding such as `for i i in 0..10` it therefore suggested `for i in i in 0..10`, which is itself invalid.

The suggestion is now only emitted when the loop header does not already contain an `in` before the body. The `for x EXPR` / `for x of EXPR` / `for x = EXPR` suggestions are unchanged.

Author: JonathanBrouwer

compiler/rustc_parse/src/errors.rs

@@ -689,7 +689,7 @@ pub(crate) struct MissingInInForLoop {
     #[primary_span]
     pub span: Span,
     #[subdiagnostic]
-    pub sub: MissingInInForLoopSub,
+    pub sub: Option<MissingInInForLoopSub>,
 }
 
 #[derive(Subdiagnostic)]

compiler/rustc_parse/src/parser/expr.rs

@@ -3086,18 +3086,41 @@ impl<'a> Parser<'a> {
     }
 
     fn error_missing_in_for_loop(&mut self) {
-        let (span, sub): (_, fn(_) -> _) = if self.token.is_ident_named(sym::of) {
+        let (span, sub) = if self.token.is_ident_named(sym::of) {
             // Possibly using JS syntax (#75311).
             let span = self.token.span;
             self.bump();
-            (span, errors::MissingInInForLoopSub::InNotOf)
+            (span, Some(errors::MissingInInForLoopSub::InNotOf(span)))
         } else if self.eat(exp!(Eq)) {
-            (self.prev_token.span, errors::MissingInInForLoopSub::InNotEq)
+            let span = self.prev_token.span;
+            (span, Some(errors::MissingInInForLoopSub::InNotEq(span)))
         } else {
-            (self.prev_token.span.between(self.token.span), errors::MissingInInForLoopSub::AddIn)
+            let span = self.prev_token.span.between(self.token.span);
+            let sub = (!self.for_loop_head_has_in())
+                .then_some(errors::MissingInInForLoopSub::AddIn(span));
+            (span, sub)
         };
 
-        self.dcx().emit_err(errors::MissingInInForLoop { span, sub: sub(span) });
+        self.dcx().emit_err(errors::MissingInInForLoop { span, sub });
+    }
+
+    /// Whether the `for` loop header already contains an `in` before its body.
+    /// If it does, the binding is malformed (e.g. `for i i in 0..10`) rather
+    /// than missing `in`, so suggesting another `in` would just be invalid too.
+    fn for_loop_head_has_in(&self) -> bool {
+        let mut dist = 0;
+        loop {
+            let (is_in, is_end) = self.look_ahead(dist, |t| {
+                (t.is_keyword(kw::In), matches!(t.kind, token::OpenBrace | token::Eof))
+            });
+            if is_in {
+                return true;
+            }
+            if is_end {
+                return false;
+            }
+            dist += 1;
+        }
     }
 
     /// Parses a `while` or `while let` expression (`while` token already eaten).

tests/ui/suggestions/for-loop-no-spurious-in-suggestion.rs

@@ -0,0 +1,9 @@
+// Regression test for https://github.com/rust-lang/rust/issues/103561: when the
+// `for` loop header already contains an `in`, a missing `in` is not the problem,
+// so we must not suggest inserting another one (which would not compile).
+
+fn main() {
+    for i i in 0..10 {}
+    //~^ ERROR missing `in` in `for` loop
+    //~| ERROR expected `{`, found keyword `in`
+}

tests/ui/suggestions/for-loop-no-spurious-in-suggestion.stderr

@@ -0,0 +1,14 @@
+error: missing `in` in `for` loop
+  --> $DIR/for-loop-no-spurious-in-suggestion.rs:6:10
+   |
+LL |     for i i in 0..10 {}
+   |          ^
+
+error: expected `{`, found keyword `in`
+  --> $DIR/for-loop-no-spurious-in-suggestion.rs:6:13
+   |
+LL |     for i i in 0..10 {}
+   |             ^^ expected `{`
+
+error: aborting due to 2 previous errors
+