Rollup merge of rust-lang#99696 - WaffleLapkin:uplift, r=fee1-dead

Uplift `clippy::for_loops_over_fallibles` lint into rustc

This PR, as the title suggests, uplifts [`clippy::for_loops_over_fallibles`] lint into rustc. This lint warns for code like this:
```rust
for _ in Some(1) {}
for _ in Ok::<_, ()>(1) {}
```
i.e. directly iterating over `Option` and `Result` using `for` loop.

There are a number of suggestions that this PR adds (on top of what clippy suggested):
1. If the argument (? is there a better name for that expression) of a `for` loop is a `.next()` call, then we can suggest removing it (or rather replacing with `.by_ref()` to allow iterator being used later)
   ```rust
    for _ in iter.next() {}
    // turns into
    for _ in iter.by_ref() {}
    ```
2. (otherwise) We can suggest using `while let`, this is useful for non-iterator, iterator-like things like [async] channels
   ```rust
   for _ in rx.recv() {}
   // turns into
   while let Some(_) = rx.recv() {}
   ```
3. If the argument type is `Result<impl IntoIterator, _>` and the body has a `Result<_, _>` type, we can suggest using `?`
   ```rust
   for _ in f() {}
   // turns into
   for _ in f()? {}
   ```
4. To preserve the original behavior and clear intent, we can suggest using `if let`
   ```rust
   for _ in f() {}
   // turns into
   if let Some(_) = f() {}
   ```
(P.S. `Some` and `Ok` are interchangeable depending on the type)

I still feel that the lint wording/look is somewhat off, so I'll be happy to hear suggestions (on how to improve suggestions :D)!

Resolves rust-lang#99272

[`clippy::for_loops_over_fallibles`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loops_over_fallibles

clippy_lints/src/lib.register_all.rs

@@ -109,7 +109,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(loops::EMPTY_LOOP),
     LintId::of(loops::EXPLICIT_COUNTER_LOOP),
     LintId::of(loops::FOR_KV_MAP),
-    LintId::of(loops::FOR_LOOPS_OVER_FALLIBLES),
     LintId::of(loops::ITER_NEXT_LOOP),
     LintId::of(loops::MANUAL_FIND),
     LintId::of(loops::MANUAL_FLATTEN),

clippy_lints/src/lib.register_lints.rs

@@ -227,7 +227,6 @@ store.register_lints(&[
     loops::EXPLICIT_INTO_ITER_LOOP,
     loops::EXPLICIT_ITER_LOOP,
     loops::FOR_KV_MAP,
-    loops::FOR_LOOPS_OVER_FALLIBLES,
     loops::ITER_NEXT_LOOP,
     loops::MANUAL_FIND,
     loops::MANUAL_FLATTEN,

clippy_lints/src/lib.register_suspicious.rs

@@ -21,7 +21,6 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec!
     LintId::of(formatting::SUSPICIOUS_ELSE_FORMATTING),
     LintId::of(formatting::SUSPICIOUS_UNARY_OP_FORMATTING),
     LintId::of(loops::EMPTY_LOOP),
-    LintId::of(loops::FOR_LOOPS_OVER_FALLIBLES),
     LintId::of(loops::MUT_RANGE_BOUND),
     LintId::of(methods::NO_EFFECT_REPLACE),
     LintId::of(methods::SUSPICIOUS_MAP),

clippy_lints/src/loops/iter_next_loop.rs

@@ -5,7 +5,7 @@ use rustc_hir::Expr;
 use rustc_lint::LateContext;
 use rustc_span::sym;
 
-pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>) -> bool {
+pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>) {
     if is_trait_method(cx, arg, sym::Iterator) {
         span_lint(
             cx,
@@ -14,8 +14,5 @@ pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>) -> bool {
             "you are iterating over `Iterator::next()` which is an Option; this will compile but is \
             probably not what you want",
         );
-        true
-    } else {
-        false
     }
 }

clippy_lints/src/loops/mod.rs

@@ -3,7 +3,6 @@ mod explicit_counter_loop;
 mod explicit_into_iter_loop;
 mod explicit_iter_loop;
 mod for_kv_map;
-mod for_loops_over_fallibles;
 mod iter_next_loop;
 mod manual_find;
 mod manual_flatten;
@@ -173,49 +172,6 @@ declare_clippy_lint! {
     "for-looping over `_.next()` which is probably not intended"
 }
 
-declare_clippy_lint! {
-    /// ### What it does
-    /// Checks for `for` loops over `Option` or `Result` values.
-    ///
-    /// ### Why is this bad?
-    /// Readability. This is more clearly expressed as an `if
-    /// let`.
-    ///
-    /// ### Example
-    /// ```rust
-    /// # let opt = Some(1);
-    /// # let res: Result<i32, std::io::Error> = Ok(1);
-    /// for x in opt {
-    ///     // ..
-    /// }
-    ///
-    /// for x in &res {
-    ///     // ..
-    /// }
-    ///
-    /// for x in res.iter() {
-    ///     // ..
-    /// }
-    /// ```
-    ///
-    /// Use instead:
-    /// ```rust
-    /// # let opt = Some(1);
-    /// # let res: Result<i32, std::io::Error> = Ok(1);
-    /// if let Some(x) = opt {
-    ///     // ..
-    /// }
-    ///
-    /// if let Ok(x) = res {
-    ///     // ..
-    /// }
-    /// ```
-    #[clippy::version = "1.45.0"]
-    pub FOR_LOOPS_OVER_FALLIBLES,
-    suspicious,
-    "for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`"
-}
-
 declare_clippy_lint! {
     /// ### What it does
     /// Detects `loop + match` combinations that are easier
@@ -648,7 +604,6 @@ declare_lint_pass!(Loops => [
     EXPLICIT_ITER_LOOP,
     EXPLICIT_INTO_ITER_LOOP,
     ITER_NEXT_LOOP,
-    FOR_LOOPS_OVER_FALLIBLES,
     WHILE_LET_LOOP,
     NEEDLESS_COLLECT,
     EXPLICIT_COUNTER_LOOP,
@@ -739,30 +694,22 @@ fn check_for_loop<'tcx>(
     manual_find::check(cx, pat, arg, body, span, expr);
 }
 
-fn check_for_loop_arg(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) {
-    let mut next_loop_linted = false; // whether or not ITER_NEXT_LOOP lint was used
-
+fn check_for_loop_arg(cx: &LateContext<'_>, _: &Pat<'_>, arg: &Expr<'_>) {
     if let ExprKind::MethodCall(method, self_arg, [], _) = arg.kind {
         let method_name = method.ident.as_str();
         // check for looping over x.iter() or x.iter_mut(), could use &x or &mut x
         match method_name {
             "iter" | "iter_mut" => {
                 explicit_iter_loop::check(cx, self_arg, arg, method_name);
-                for_loops_over_fallibles::check(cx, pat, self_arg, Some(method_name));
             },
             "into_iter" => {
                 explicit_iter_loop::check(cx, self_arg, arg, method_name);
                 explicit_into_iter_loop::check(cx, self_arg, arg);
-                for_loops_over_fallibles::check(cx, pat, self_arg, Some(method_name));
             },
             "next" => {
-                next_loop_linted = iter_next_loop::check(cx, arg);
+                iter_next_loop::check(cx, arg);
             },
             _ => {},
         }
     }
-
-    if !next_loop_linted {
-        for_loops_over_fallibles::check(cx, pat, arg, None);
-    }
 }

clippy_lints/src/renamed_lints.rs

@@ -11,8 +11,8 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[
     ("clippy::disallowed_method", "clippy::disallowed_methods"),
     ("clippy::disallowed_type", "clippy::disallowed_types"),
     ("clippy::eval_order_dependence", "clippy::mixed_read_write_in_expression"),
-    ("clippy::for_loop_over_option", "clippy::for_loops_over_fallibles"),
-    ("clippy::for_loop_over_result", "clippy::for_loops_over_fallibles"),
+    ("clippy::for_loop_over_option", "for_loops_over_fallibles"),
+    ("clippy::for_loop_over_result", "for_loops_over_fallibles"),
     ("clippy::identity_conversion", "clippy::useless_conversion"),
     ("clippy::if_let_some_result", "clippy::match_result_ok"),
     ("clippy::logic_bug", "clippy::overly_complex_bool_expr"),
@@ -31,6 +31,7 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[
     ("clippy::to_string_in_display", "clippy::recursive_format_impl"),
     ("clippy::zero_width_space", "clippy::invisible_characters"),
     ("clippy::drop_bounds", "drop_bounds"),
+    ("clippy::for_loops_over_fallibles", "for_loops_over_fallibles"),
     ("clippy::into_iter_on_array", "array_into_iter"),
     ("clippy::invalid_atomic_ordering", "invalid_atomic_ordering"),
     ("clippy::invalid_ref", "invalid_value"),

src/docs.rs

@@ -170,7 +170,6 @@ docs! {
     "fn_to_numeric_cast_any",
     "fn_to_numeric_cast_with_truncation",
     "for_kv_map",
-    "for_loops_over_fallibles",
     "forget_copy",
     "forget_non_drop",
     "forget_ref",

tests/ui/for_loop_unfixable.rs

@@ -8,6 +8,7 @@
     clippy::for_kv_map
 )]
 #[allow(clippy::linkedlist, clippy::unnecessary_mut_passed, clippy::similar_names)]
+#[allow(for_loops_over_fallibles)]
 fn main() {
     let vec = vec![1, 2, 3, 4];
 

tests/ui/for_loop_unfixable.stderr

@@ -1,5 +1,5 @@
 error: you are iterating over `Iterator::next()` which is an Option; this will compile but is probably not what you want
-  --> $DIR/for_loop_unfixable.rs:14:15
+  --> $DIR/for_loop_unfixable.rs:15:15
    |
 LL |     for _v in vec.iter().next() {}
    |               ^^^^^^^^^^^^^^^^^