From 7c3908f86c61547e1a15873c156be5be8c012a06 Mon Sep 17 00:00:00 2001
From: y21 <30553356+y21@users.noreply.github.com>
Date: Sun, 4 Feb 2024 20:12:15 +0100
Subject: [PATCH] [`redundant_locals`]: take by-value closure captures into
 account

---
 clippy_lints/src/redundant_locals.rs | 30 ++++++++++++-
 tests/ui/redundant_locals.rs         | 36 +++++++++++++++
 tests/ui/redundant_locals.stderr     | 66 ++++++++++++++--------------
 3 files changed, 98 insertions(+), 34 deletions(-)

diff --git a/clippy_lints/src/redundant_locals.rs b/clippy_lints/src/redundant_locals.rs
index 2c511ee0bc0..b0040bfa582 100644
--- a/clippy_lints/src/redundant_locals.rs
+++ b/clippy_lints/src/redundant_locals.rs
@@ -2,10 +2,12 @@
 use clippy_utils::is_from_proc_macro;
 use clippy_utils::ty::needs_ordered_drop;
 use rustc_ast::Mutability;
-use rustc_hir::def::Res;
+use rustc_hir::def::{DefKind, Res};
 use rustc_hir::{BindingAnnotation, ByRef, ExprKind, HirId, Local, Node, Pat, PatKind, QPath};
+use rustc_hir_typeck::expr_use_visitor::PlaceBase;
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
+use rustc_middle::ty::UpvarCapture;
 use rustc_session::declare_lint_pass;
 use rustc_span::symbol::Ident;
 use rustc_span::DesugaringKind;
@@ -69,6 +71,7 @@ fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) {
             // the local is user-controlled
             && !in_external_macro(cx.sess(), local.span)
             && !is_from_proc_macro(cx, expr)
+            && !is_closure_capture(cx, local.hir_id, binding_id)
         {
             span_lint_and_help(
                 cx,
@@ -82,6 +85,31 @@ fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) {
     }
 }
 
+/// Checks if the enclosing body is a closure and if the given local is captured by value.
+///
+/// In those cases, the redefinition may be necessary to force a move:
+/// ```
+/// fn assert_static<T: 'static>(_: T) {}
+///
+/// let v = String::new();
+/// let closure = || {
+///   let v = v; // <- removing this redefinition makes `closure` no longer `'static`
+///   dbg!(&v);
+/// };
+/// assert_static(closure);
+/// ```
+fn is_closure_capture(cx: &LateContext<'_>, redefinition: HirId, root_variable: HirId) -> bool {
+    let body = cx.tcx.hir().enclosing_body_owner(redefinition);
+    if let DefKind::Closure = cx.tcx.def_kind(body) {
+        cx.tcx.closure_captures(body).iter().any(|c| {
+            matches!(c.info.capture_kind, UpvarCapture::ByValue)
+                && matches!(c.place.base, PlaceBase::Upvar(upvar) if upvar.var_path.hir_id == root_variable)
+        })
+    } else {
+        false
+    }
+}
+
 /// Find the annotation of a binding introduced by a pattern, or `None` if it's not introduced.
 fn find_binding(pat: &Pat<'_>, name: Ident) -> Option<BindingAnnotation> {
     let mut ret = None;
diff --git a/tests/ui/redundant_locals.rs b/tests/ui/redundant_locals.rs
index 182d067a5e9..40bd89bc595 100644
--- a/tests/ui/redundant_locals.rs
+++ b/tests/ui/redundant_locals.rs
@@ -1,6 +1,7 @@
 //@aux-build:proc_macros.rs
 #![allow(unused, clippy::no_effect, clippy::needless_pass_by_ref_mut)]
 #![warn(clippy::redundant_locals)]
+#![feature(async_closure)]
 
 extern crate proc_macros;
 use proc_macros::{external, with_span};
@@ -163,3 +164,38 @@ fn drop_compose() {
     let b = ComposeDrop { d: WithDrop(1) };
     let a = a;
 }
+
+fn issue12225() {
+    fn assert_static<T: 'static>(_: T) {}
+
+    let v1 = String::new();
+    let v2 = String::new();
+    let v3 = String::new();
+    let v4 = String::new();
+
+    assert_static(|| {
+        let v1 = v1;
+        dbg!(&v1);
+    });
+    assert_static(async {
+        let v2 = v2;
+        dbg!(&v2);
+    });
+    assert_static(|| async {
+        let v3 = v3;
+        dbg!(&v3);
+    });
+    assert_static(async || {
+        let v4 = v4;
+        dbg!(&v4);
+    });
+
+    fn foo(a: &str, b: &str) {}
+
+    let do_not_move = String::new();
+    let things_to_move = vec!["a".to_string(), "b".to_string()];
+    let futures = things_to_move.into_iter().map(|move_me| async {
+        let move_me = move_me;
+        foo(&do_not_move, &move_me)
+    });
+}
diff --git a/tests/ui/redundant_locals.stderr b/tests/ui/redundant_locals.stderr
index 30ab4aa2ea9..610d587ddad 100644
--- a/tests/ui/redundant_locals.stderr
+++ b/tests/ui/redundant_locals.stderr
@@ -1,11 +1,11 @@
 error: redundant redefinition of a binding `x`
-  --> $DIR/redundant_locals.rs:12:5
+  --> $DIR/redundant_locals.rs:13:5
    |
 LL |     let x = x;
    |     ^^^^^^^^^^
    |
 help: `x` is initially defined here
-  --> $DIR/redundant_locals.rs:11:9
+  --> $DIR/redundant_locals.rs:12:9
    |
 LL |     let x = 1;
    |         ^
@@ -13,41 +13,29 @@ LL |     let x = 1;
    = help: to override `-D warnings` add `#[allow(clippy::redundant_locals)]`
 
 error: redundant redefinition of a binding `x`
-  --> $DIR/redundant_locals.rs:17:5
+  --> $DIR/redundant_locals.rs:18:5
    |
 LL |     let mut x = x;
    |     ^^^^^^^^^^^^^^
    |
 help: `x` is initially defined here
-  --> $DIR/redundant_locals.rs:16:9
+  --> $DIR/redundant_locals.rs:17:9
    |
 LL |     let mut x = 1;
    |         ^^^^^
 
 error: redundant redefinition of a binding `x`
-  --> $DIR/redundant_locals.rs:47:5
+  --> $DIR/redundant_locals.rs:48:5
    |
 LL |     let x = x;
    |     ^^^^^^^^^^
    |
 help: `x` is initially defined here
-  --> $DIR/redundant_locals.rs:46:14
+  --> $DIR/redundant_locals.rs:47:14
    |
 LL | fn parameter(x: i32) {
    |              ^
 
-error: redundant redefinition of a binding `x`
-  --> $DIR/redundant_locals.rs:52:5
-   |
-LL |     let x = x;
-   |     ^^^^^^^^^^
-   |
-help: `x` is initially defined here
-  --> $DIR/redundant_locals.rs:51:9
-   |
-LL |     let x = 1;
-   |         ^
-
 error: redundant redefinition of a binding `x`
   --> $DIR/redundant_locals.rs:53:5
    |
@@ -57,7 +45,7 @@ LL |     let x = x;
 help: `x` is initially defined here
   --> $DIR/redundant_locals.rs:52:9
    |
-LL |     let x = x;
+LL |     let x = 1;
    |         ^
 
 error: redundant redefinition of a binding `x`
@@ -84,86 +72,98 @@ help: `x` is initially defined here
 LL |     let x = x;
    |         ^
 
+error: redundant redefinition of a binding `x`
+  --> $DIR/redundant_locals.rs:56:5
+   |
+LL |     let x = x;
+   |     ^^^^^^^^^^
+   |
+help: `x` is initially defined here
+  --> $DIR/redundant_locals.rs:55:9
+   |
+LL |     let x = x;
+   |         ^
+
 error: redundant redefinition of a binding `a`
-  --> $DIR/redundant_locals.rs:61:5
+  --> $DIR/redundant_locals.rs:62:5
    |
 LL |     let a = a;
    |     ^^^^^^^^^^
    |
 help: `a` is initially defined here
-  --> $DIR/redundant_locals.rs:59:9
+  --> $DIR/redundant_locals.rs:60:9
    |
 LL |     let a = 1;
    |         ^
 
 error: redundant redefinition of a binding `b`
-  --> $DIR/redundant_locals.rs:62:5
+  --> $DIR/redundant_locals.rs:63:5
    |
 LL |     let b = b;
    |     ^^^^^^^^^^
    |
 help: `b` is initially defined here
-  --> $DIR/redundant_locals.rs:60:9
+  --> $DIR/redundant_locals.rs:61:9
    |
 LL |     let b = 2;
    |         ^
 
 error: redundant redefinition of a binding `x`
-  --> $DIR/redundant_locals.rs:68:9
+  --> $DIR/redundant_locals.rs:69:9
    |
 LL |         let x = x;
    |         ^^^^^^^^^^
    |
 help: `x` is initially defined here
-  --> $DIR/redundant_locals.rs:67:13
+  --> $DIR/redundant_locals.rs:68:13
    |
 LL |         let x = 1;
    |             ^
 
 error: redundant redefinition of a binding `x`
-  --> $DIR/redundant_locals.rs:75:9
+  --> $DIR/redundant_locals.rs:76:9
    |
 LL |         let x = x;
    |         ^^^^^^^^^^
    |
 help: `x` is initially defined here
-  --> $DIR/redundant_locals.rs:74:13
+  --> $DIR/redundant_locals.rs:75:13
    |
 LL |         let x = 1;
    |             ^
 
 error: redundant redefinition of a binding `x`
-  --> $DIR/redundant_locals.rs:78:9
+  --> $DIR/redundant_locals.rs:79:9
    |
 LL |         let x = x;
    |         ^^^^^^^^^^
    |
 help: `x` is initially defined here
-  --> $DIR/redundant_locals.rs:77:6
+  --> $DIR/redundant_locals.rs:78:6
    |
 LL |     |x: i32| {
    |      ^
 
 error: redundant redefinition of a binding `x`
-  --> $DIR/redundant_locals.rs:97:9
+  --> $DIR/redundant_locals.rs:98:9
    |
 LL |         let x = x;
    |         ^^^^^^^^^^
    |
 help: `x` is initially defined here
-  --> $DIR/redundant_locals.rs:94:9
+  --> $DIR/redundant_locals.rs:95:9
    |
 LL |     let x = 1;
    |         ^
 
 error: redundant redefinition of a binding `a`
-  --> $DIR/redundant_locals.rs:152:5
+  --> $DIR/redundant_locals.rs:153:5
    |
 LL |     let a = a;
    |     ^^^^^^^^^^
    |
 help: `a` is initially defined here
-  --> $DIR/redundant_locals.rs:150:9
+  --> $DIR/redundant_locals.rs:151:9
    |
 LL |     let a = WithoutDrop(1);
    |         ^