needless_pass_by_ref_mut: Fix corner case in async functions

4 files changed, 84 insertions, 6 deletions

diff --git a/clippy_lints/src/needless_pass_by_ref_mut.rs b/clippy_lints/src/needless_pass_by_ref_mut.rs
index 30d3e86dc4e..c3ceb994f60 100644
--- a/clippy_lints/src/needless_pass_by_ref_mut.rs
+++ b/clippy_lints/src/needless_pass_by_ref_mut.rs
@@ -185,7 +185,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> {
         }
         // Collect variables mutably used and spans which will need dereferencings from the
         // function body.
-        let MutablyUsedVariablesCtxt { mutably_used_vars, .. } = {
+        let mutably_used_vars = {
             let mut ctx = MutablyUsedVariablesCtxt {
                 mutably_used_vars: HirIdSet::default(),
                 prev_bind: None,
@@ -217,7 +217,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> {
                     check_closures(&mut ctx, cx, &infcx, &mut checked_closures, async_closures);
                 }
             }
-            ctx
+            ctx.generate_mutably_used_ids_from_aliases()
         };
         for ((&input, &_), arg) in it {
             // Only take `&mut` arguments.
@@ -309,12 +309,22 @@ struct MutablyUsedVariablesCtxt<'tcx> {
 }
 
 impl<'tcx> MutablyUsedVariablesCtxt<'tcx> {
-    fn add_mutably_used_var(&mut self, mut used_id: HirId) {
-        while let Some(id) = self.aliases.get(&used_id) {
+    fn add_mutably_used_var(&mut self, used_id: HirId) {
+        self.mutably_used_vars.insert(used_id);
+    }
+
+    // Because the alias may come after the mutable use of a variable, we need to fill the map at
+    // the end.
+    fn generate_mutably_used_ids_from_aliases(mut self) -> HirIdSet {
+        let all_ids = self.mutably_used_vars.iter().copied().collect::<Vec<_>>();
+        for mut used_id in all_ids {
+            while let Some(id) = self.aliases.get(&used_id) {
+                self.mutably_used_vars.insert(used_id);
+                used_id = *id;
+            }
             self.mutably_used_vars.insert(used_id);
-            used_id = *id;
         }
-        self.mutably_used_vars.insert(used_id);
+        self.mutably_used_vars
     }
 
     fn would_be_alias_cycle(&self, alias: HirId, mut target: HirId) -> bool {
diff --git a/tests/ui/needless_pass_by_ref_mut2.fixed b/tests/ui/needless_pass_by_ref_mut2.fixed
new file mode 100644
index 00000000000..3c2576213cd
--- /dev/null
+++ b/tests/ui/needless_pass_by_ref_mut2.fixed
@@ -0,0 +1,24 @@
+// If both `inner_async3` and `inner_async4` are present, aliases are declared after
+// they're used in `inner_async4` for some reasons... This test ensures that no
+// only `v` is marked as not used mutably in `inner_async4`.
+
+#![allow(clippy::redundant_closure_call)]
+#![warn(clippy::needless_pass_by_ref_mut)]
+
+pub async fn inner_async3(x: &i32, y: &mut u32) {
+    //~^ ERROR: this argument is a mutable reference, but not used mutably
+    async {
+        *y += 1;
+    }
+    .await;
+}
+
+pub async fn inner_async4(u: &mut i32, v: &u32) {
+    //~^ ERROR: this argument is a mutable reference, but not used mutably
+    async {
+        *u += 1;
+    }
+    .await;
+}
+
+fn main() {}
diff --git a/tests/ui/needless_pass_by_ref_mut2.rs b/tests/ui/needless_pass_by_ref_mut2.rs
new file mode 100644
index 00000000000..34b0b564deb
--- /dev/null
+++ b/tests/ui/needless_pass_by_ref_mut2.rs
@@ -0,0 +1,24 @@
+// If both `inner_async3` and `inner_async4` are present, aliases are declared after
+// they're used in `inner_async4` for some reasons... This test ensures that no
+// only `v` is marked as not used mutably in `inner_async4`.
+
+#![allow(clippy::redundant_closure_call)]
+#![warn(clippy::needless_pass_by_ref_mut)]
+
+pub async fn inner_async3(x: &mut i32, y: &mut u32) {
+    //~^ ERROR: this argument is a mutable reference, but not used mutably
+    async {
+        *y += 1;
+    }
+    .await;
+}
+
+pub async fn inner_async4(u: &mut i32, v: &mut u32) {
+    //~^ ERROR: this argument is a mutable reference, but not used mutably
+    async {
+        *u += 1;
+    }
+    .await;
+}
+
+fn main() {}
diff --git a/tests/ui/needless_pass_by_ref_mut2.stderr b/tests/ui/needless_pass_by_ref_mut2.stderr
new file mode 100644
index 00000000000..c8753603225
--- /dev/null
+++ b/tests/ui/needless_pass_by_ref_mut2.stderr
@@ -0,0 +1,20 @@
+error: this argument is a mutable reference, but not used mutably
+  --> tests/ui/needless_pass_by_ref_mut2.rs:8:30
+   |
+LL | pub async fn inner_async3(x: &mut i32, y: &mut u32) {
+   |                              ^^^^^^^^ help: consider changing to: `&i32`
+   |
+   = warning: changing this function will impact semver compatibility
+   = note: `-D clippy::needless-pass-by-ref-mut` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::needless_pass_by_ref_mut)]`
+
+error: this argument is a mutable reference, but not used mutably
+  --> tests/ui/needless_pass_by_ref_mut2.rs:16:43
+   |
+LL | pub async fn inner_async4(u: &mut i32, v: &mut u32) {
+   |                                           ^^^^^^^^ help: consider changing to: `&u32`
+   |
+   = warning: changing this function will impact semver compatibility
+
+error: aborting due to 2 previous errors
+