Fix mutaby used async function argument in closure for `needless_pass_by_ref_mut`

3 files changed, 118 insertions, 21 deletions

diff --git a/clippy_lints/src/needless_pass_by_ref_mut.rs b/clippy_lints/src/needless_pass_by_ref_mut.rs
index 7b00eabf97b..bea44ed92ef 100644
--- a/clippy_lints/src/needless_pass_by_ref_mut.rs
+++ b/clippy_lints/src/needless_pass_by_ref_mut.rs
@@ -4,12 +4,13 @@ use clippy_utils::source::snippet;
 use clippy_utils::{get_parent_node, inherits_cfg, is_from_proc_macro, is_self};
 use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
 use rustc_errors::Applicability;
-use rustc_hir::intravisit::{walk_qpath, FnKind, Visitor};
+use rustc_hir::intravisit::{walk_fn, walk_qpath, FnKind, Visitor};
 use rustc_hir::{
-    Body, Closure, Expr, ExprKind, FnDecl, HirId, HirIdMap, HirIdSet, Impl, ItemKind, Mutability, Node, PatKind, QPath,
+    Body, BodyId, Closure, Expr, ExprKind, FnDecl, HirId, HirIdMap, HirIdSet, Impl, ItemKind, Mutability, Node,
+    PatKind, QPath,
 };
 use rustc_hir_typeck::expr_use_visitor as euv;
-use rustc_infer::infer::TyCtxtInferExt;
+use rustc_infer::infer::{InferCtxt, TyCtxtInferExt};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::hir::map::associated_body;
 use rustc_middle::hir::nested_filter::OnlyBodies;
@@ -95,6 +96,30 @@ fn should_skip<'tcx>(
     is_from_proc_macro(cx, &input)
 }
 
+fn check_closures<'tcx>(
+    ctx: &mut MutablyUsedVariablesCtxt<'tcx>,
+    cx: &LateContext<'tcx>,
+    infcx: &InferCtxt<'tcx>,
+    checked_closures: &mut FxHashSet<LocalDefId>,
+    closures: FxHashSet<LocalDefId>,
+) {
+    let hir = cx.tcx.hir();
+    for closure in closures {
+        if !checked_closures.insert(closure) {
+            continue;
+        }
+        ctx.prev_bind = None;
+        ctx.prev_move_to_closure.clear();
+        if let Some(body) = hir
+            .find_by_def_id(closure)
+            .and_then(associated_body)
+            .map(|(_, body_id)| hir.body(body_id))
+        {
+            euv::ExprUseVisitor::new(ctx, infcx, closure, cx.param_env, cx.typeck_results()).consume_body(body);
+        }
+    }
+}
+
 impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> {
     fn check_fn(
         &mut self,
@@ -161,25 +186,20 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> {
             euv::ExprUseVisitor::new(&mut ctx, &infcx, fn_def_id, cx.param_env, cx.typeck_results()).consume_body(body);
             if is_async {
                 let mut checked_closures = FxHashSet::default();
+
+                // We retrieve all the closures declared in the async function because they will
+                // not be found by `euv::Delegate`.
+                let mut closures_retriever = ClosuresRetriever {
+                    cx,
+                    closures: FxHashSet::default(),
+                };
+                walk_fn(&mut closures_retriever, kind, decl, body.id(), fn_def_id);
+                check_closures(&mut ctx, cx, &infcx, &mut checked_closures, closures_retriever.closures);
+
                 while !ctx.async_closures.is_empty() {
                     let closures = ctx.async_closures.clone();
                     ctx.async_closures.clear();
-                    let hir = cx.tcx.hir();
-                    for closure in closures {
-                        if !checked_closures.insert(closure) {
-                            continue;
-                        }
-                        ctx.prev_bind = None;
-                        ctx.prev_move_to_closure.clear();
-                        if let Some(body) = hir
-                            .find_by_def_id(closure)
-                            .and_then(associated_body)
-                            .map(|(_, body_id)| hir.body(body_id))
-                        {
-                            euv::ExprUseVisitor::new(&mut ctx, &infcx, closure, cx.param_env, cx.typeck_results())
-                                .consume_body(body);
-                        }
-                    }
+                    check_closures(&mut ctx, cx, &infcx, &mut checked_closures, closures);
                 }
             }
             ctx
@@ -439,3 +459,30 @@ impl<'tcx> Visitor<'tcx> for FnNeedsMutVisitor<'_, 'tcx> {
         }
     }
 }
+
+struct ClosuresRetriever<'a, 'tcx> {
+    cx: &'a LateContext<'tcx>,
+    closures: FxHashSet<LocalDefId>,
+}
+
+impl<'a, 'tcx> Visitor<'tcx> for ClosuresRetriever<'a, 'tcx> {
+    type NestedFilter = OnlyBodies;
+
+    fn nested_visit_map(&mut self) -> Self::Map {
+        self.cx.tcx.hir()
+    }
+
+    fn visit_fn(
+        &mut self,
+        kind: FnKind<'tcx>,
+        decl: &'tcx FnDecl<'tcx>,
+        body_id: BodyId,
+        _span: Span,
+        fn_def_id: LocalDefId,
+    ) {
+        if matches!(kind, FnKind::Closure) {
+            self.closures.insert(fn_def_id);
+        }
+        walk_fn(self, kind, decl, body_id, fn_def_id);
+    }
+}
diff --git a/tests/ui/needless_pass_by_ref_mut.rs b/tests/ui/needless_pass_by_ref_mut.rs
index e1e5e8fd220..da2a72caacb 100644
--- a/tests/ui/needless_pass_by_ref_mut.rs
+++ b/tests/ui/needless_pass_by_ref_mut.rs
@@ -1,4 +1,4 @@
-#![allow(clippy::if_same_then_else, clippy::no_effect)]
+#![allow(clippy::if_same_then_else, clippy::no_effect, clippy::redundant_closure_call)]
 #![feature(lint_reasons)]
 //@no-rustfix
 use std::ptr::NonNull;
@@ -231,6 +231,32 @@ async fn async_vec2(b: &mut Vec<bool>) {
     b.push(true);
 }
 
+// Should not warn.
+pub async fn closure(n: &mut usize) -> impl '_ + FnMut() {
+    || {
+        *n += 1;
+    }
+}
+
+// Should warn.
+pub fn closure2(n: &mut usize) -> impl '_ + FnMut() -> usize {
+    //~^ ERROR: this argument is a mutable reference, but not used mutably
+    || *n + 1
+}
+
+// Should not warn.
+pub async fn closure3(n: &mut usize) {
+    (|| *n += 1)();
+}
+
+// Should warn.
+pub async fn closure4(n: &mut usize) {
+    //~^ ERROR: this argument is a mutable reference, but not used mutably
+    (|| {
+        let _x = *n + 1;
+    })();
+}
+
 fn main() {
     let mut u = 0;
     let mut v = vec![0];
diff --git a/tests/ui/needless_pass_by_ref_mut.stderr b/tests/ui/needless_pass_by_ref_mut.stderr
index df3df045776..0fb9458d794 100644
--- a/tests/ui/needless_pass_by_ref_mut.stderr
+++ b/tests/ui/needless_pass_by_ref_mut.stderr
@@ -107,5 +107,29 @@ error: this argument is a mutable reference, but not used mutably
 LL | async fn inner_async3(x: &mut i32, y: &mut u32) {
    |                          ^^^^^^^^ help: consider changing to: `&i32`
 
-error: aborting due to 17 previous errors
+error: this argument is a mutable reference, but not used mutably
+  --> $DIR/needless_pass_by_ref_mut.rs:235:25
+   |
+LL | pub async fn closure(n: &mut usize) -> impl '_ + FnMut() {
+   |                         ^^^^^^^^^^ help: consider changing to: `&usize`
+   |
+   = warning: changing this function will impact semver compatibility
+
+error: this argument is a mutable reference, but not used mutably
+  --> $DIR/needless_pass_by_ref_mut.rs:242:20
+   |
+LL | pub fn closure2(n: &mut usize) -> impl '_ + FnMut() -> usize {
+   |                    ^^^^^^^^^^ help: consider changing to: `&usize`
+   |
+   = warning: changing this function will impact semver compatibility
+
+error: this argument is a mutable reference, but not used mutably
+  --> $DIR/needless_pass_by_ref_mut.rs:253:26
+   |
+LL | pub async fn closure4(n: &mut usize) {
+   |                          ^^^^^^^^^^ help: consider changing to: `&usize`
+   |
+   = warning: changing this function will impact semver compatibility
+
+error: aborting due to 20 previous errors