Auto merge of rust-lang#8676 - Alexendoo:local-used-across-loop, r=xFrednet

Check for loops/closures in `local_used_after_expr`

Follow up to rust-lang#8646, catches when a local is used multiple times because it's in a loop or a closure

changelog: none

Author: bors

clippy_lints/src/eta_reduction.rs

@@ -3,7 +3,7 @@ use clippy_utils::higher::VecArgs;
 use clippy_utils::source::snippet_opt;
 use clippy_utils::ty::is_type_diagnostic_item;
 use clippy_utils::usage::local_used_after_expr;
-use clippy_utils::{get_enclosing_loop_or_closure, higher, path_to_local, path_to_local_id};
+use clippy_utils::{higher, path_to_local, path_to_local_id};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::def_id::DefId;
@@ -125,8 +125,7 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction {
                         if_chain! {
                             if let ty::Closure(_, substs) = callee_ty.peel_refs().kind();
                             if substs.as_closure().kind() == ClosureKind::FnMut;
-                            if get_enclosing_loop_or_closure(cx.tcx, expr).is_some()
-                                || path_to_local(callee).map_or(false, |l| local_used_after_expr(cx, l, callee));
+                            if path_to_local(callee).map_or(false, |l| local_used_after_expr(cx, l, expr));
 
                             then {
                                 // Mutable closure is used after current expr; we cannot consume it.

clippy_utils/src/usage.rs

@@ -3,7 +3,7 @@ use crate::visitors::{expr_visitor, expr_visitor_no_bodies};
 use rustc_hir as hir;
 use rustc_hir::intravisit::{self, Visitor};
 use rustc_hir::HirIdSet;
-use rustc_hir::{Expr, ExprKind, HirId};
+use rustc_hir::{Expr, ExprKind, HirId, Node};
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_lint::LateContext;
 use rustc_middle::hir::nested_filter;
@@ -169,6 +169,32 @@ pub fn contains_return_break_continue_macro(expression: &Expr<'_>) -> bool {
 
 pub fn local_used_after_expr(cx: &LateContext<'_>, local_id: HirId, after: &Expr<'_>) -> bool {
     let Some(block) = utils::get_enclosing_block(cx, local_id) else { return false };
+
+    // for _ in 1..3 {
+    //    local
+    // }
+    //
+    // let closure = || local;
+    // closure();
+    // closure();
+    let in_loop_or_closure = cx
+        .tcx
+        .hir()
+        .parent_iter(after.hir_id)
+        .take_while(|&(id, _)| id != block.hir_id)
+        .any(|(_, node)| {
+            matches!(
+                node,
+                Node::Expr(Expr {
+                    kind: ExprKind::Loop(..) | ExprKind::Closure(..),
+                    ..
+                })
+            )
+        });
+    if in_loop_or_closure {
+        return true;
+    }
+
     let mut used_after_expr = false;
     let mut past_expr = false;
     expr_visitor(cx, |expr| {
@@ -178,7 +204,10 @@ pub fn local_used_after_expr(cx: &LateContext<'_>, local_id: HirId, after: &Expr
 
         if expr.hir_id == after.hir_id {
             past_expr = true;
-        } else if past_expr && utils::path_to_local_id(expr, local_id) {
+            return false;
+        }
+
+        if past_expr && utils::path_to_local_id(expr, local_id) {
             used_after_expr = true;
         }
         !used_after_expr

tests/ui/eta.fixed

@@ -211,6 +211,10 @@ fn mutable_closure_in_loop() {
     let mut closure = |n| value += n;
     for _ in 0..5 {
         Some(1).map(&mut closure);
+
+        let mut value = 0;
+        let mut in_loop = |n| value += n;
+        Some(1).map(in_loop);
     }
 }
 

tests/ui/eta.rs

@@ -211,6 +211,10 @@ fn mutable_closure_in_loop() {
     let mut closure = |n| value += n;
     for _ in 0..5 {
         Some(1).map(|n| closure(n));
+
+        let mut value = 0;
+        let mut in_loop = |n| value += n;
+        Some(1).map(|n| in_loop(n));
     }
 }
 

tests/ui/eta.stderr

@@ -117,10 +117,16 @@ LL |         Some(1).map(|n| closure(n));
    |                     ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut closure`
 
 error: redundant closure
-  --> $DIR/eta.rs:232:21
+  --> $DIR/eta.rs:217:21
+   |
+LL |         Some(1).map(|n| in_loop(n));
+   |                     ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `in_loop`
+
+error: redundant closure
+  --> $DIR/eta.rs:236:21
    |
 LL |     map_str_to_path(|s| s.as_ref());
    |                     ^^^^^^^^^^^^^^ help: replace the closure with the method itself: `std::convert::AsRef::as_ref`
 
-error: aborting due to 20 previous errors
+error: aborting due to 21 previous errors
 

tests/ui/needless_option_as_deref.fixed

@@ -16,6 +16,20 @@ fn main() {
     let _ = Some(Box::new(1)).as_deref();
     let _ = Some(Box::new(1)).as_deref_mut();
 
+    let mut y = 0;
+    let mut x = Some(&mut y);
+    for _ in 0..3 {
+        let _ = x.as_deref_mut();
+    }
+
+    let mut y = 0;
+    let mut x = Some(&mut y);
+    let mut closure = || {
+        let _ = x.as_deref_mut();
+    };
+    closure();
+    closure();
+
     // #7846
     let mut i = 0;
     let mut opt_vec = vec![Some(&mut i)];

tests/ui/needless_option_as_deref.rs

@@ -16,6 +16,20 @@ fn main() {
     let _ = Some(Box::new(1)).as_deref();
     let _ = Some(Box::new(1)).as_deref_mut();
 
+    let mut y = 0;
+    let mut x = Some(&mut y);
+    for _ in 0..3 {
+        let _ = x.as_deref_mut();
+    }
+
+    let mut y = 0;
+    let mut x = Some(&mut y);
+    let mut closure = || {
+        let _ = x.as_deref_mut();
+    };
+    closure();
+    closure();
+
     // #7846
     let mut i = 0;
     let mut opt_vec = vec![Some(&mut i)];