Auto merge of rust-lang#12693 - GuillaumeGomez:run-on-self-needless_pass_by_ref_mut, r=Manishearth

Emit the `needless_pass_by_ref_mut` lint on `self` arguments as well

Fixes rust-lang/rust-clippy#12589.
Fixes rust-lang/rust-clippy#9591.

The first commit fixes a bug I uncovered while working on this: sometimes, the mutable borrow "event" happens before the alias one, which makes some argument detected as not used mutably even if they are. The fix was simply to fill the map with the aliases afterwards.

The second commit removes the restriction to not run `self` argument for the `needless_pass_by_ref_mut` lint.

changelog: emit the `needless_pass_by_ref_mut` lint on `self` arguments as well

r? `@Manishearth`

Author: bors

clippy_lints/src/needless_pass_by_ref_mut.rs

@@ -83,7 +83,9 @@ fn should_skip<'tcx>(
     }
 
     if is_self(arg) {
-        return true;
+        // Interestingly enough, `self` arguments make `is_from_proc_macro` return `true`, hence why
+        // we return early here.
+        return false;
     }
 
     if let PatKind::Binding(.., name, _) = arg.pat.kind {
@@ -185,7 +187,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 +219,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 +311,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 {

tests/ui/needless_pass_by_ref_mut.rs

@@ -44,18 +44,13 @@ fn non_mut_ref(_: &Vec<u32>) {}
 struct Bar;
 
 impl Bar {
-    // Should not warn on `&mut self`.
     fn bar(&mut self) {}
+    //~^ ERROR: this argument is a mutable reference, but not used mutably
 
     fn mushroom(&self, vec: &mut Vec<i32>) -> usize {
         //~^ ERROR: this argument is a mutable reference, but not used mutably
         vec.len()
     }
-
-    fn badger(&mut self, vec: &mut Vec<i32>) -> usize {
-        //~^ ERROR: this argument is a mutable reference, but not used mutably
-        vec.len()
-    }
 }
 
 trait Babar {
@@ -307,6 +302,41 @@ fn filter_copy<T: Copy>(predicate: &mut impl FnMut(T) -> bool) -> impl FnMut(&T)
     move |&item| predicate(item)
 }
 
+trait MutSelfTrait {
+    // Should not warn since it's a trait method.
+    fn mut_self(&mut self);
+}
+
+struct MutSelf {
+    a: u32,
+}
+
+impl MutSelf {
+    fn bar(&mut self) {}
+    //~^ ERROR: this argument is a mutable reference, but not used mutably
+    async fn foo(&mut self, u: &mut i32, v: &mut u32) {
+        //~^ ERROR: this argument is a mutable reference, but not used mutably
+        //~| ERROR: this argument is a mutable reference, but not used mutably
+        async {
+            *u += 1;
+        }
+        .await;
+    }
+    async fn foo2(&mut self, u: &mut i32, v: &mut u32) {
+        //~^ ERROR: this argument is a mutable reference, but not used mutably
+        async {
+            self.a += 1;
+            *u += 1;
+        }
+        .await;
+    }
+}
+
+impl MutSelfTrait for MutSelf {
+    // Should not warn since it's a trait method.
+    fn mut_self(&mut self) {}
+}
+
 // `is_from_proc_macro` stress tests
 fn _empty_tup(x: &mut (())) {}
 fn _single_tup(x: &mut ((i32,))) {}

tests/ui/needless_pass_by_ref_mut.stderr

@@ -14,190 +14,206 @@ LL | fn foo6(s: &mut Vec<u32>) {
    |            ^^^^^^^^^^^^^ help: consider changing to: `&Vec<u32>`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:50:29
+  --> tests/ui/needless_pass_by_ref_mut.rs:47:12
    |
-LL |     fn mushroom(&self, vec: &mut Vec<i32>) -> usize {
-   |                             ^^^^^^^^^^^^^ help: consider changing to: `&Vec<i32>`
+LL |     fn bar(&mut self) {}
+   |            ^^^^^^^^^ help: consider changing to: `&self`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:55:31
+  --> tests/ui/needless_pass_by_ref_mut.rs:50:29
    |
-LL |     fn badger(&mut self, vec: &mut Vec<i32>) -> usize {
-   |                               ^^^^^^^^^^^^^ help: consider changing to: `&Vec<i32>`
+LL |     fn mushroom(&self, vec: &mut Vec<i32>) -> usize {
+   |                             ^^^^^^^^^^^^^ help: consider changing to: `&Vec<i32>`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:132:16
+  --> tests/ui/needless_pass_by_ref_mut.rs:127:16
    |
 LL | async fn a1(x: &mut i32) {
    |                ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:136:16
+  --> tests/ui/needless_pass_by_ref_mut.rs:131:16
    |
 LL | async fn a2(x: &mut i32, y: String) {
    |                ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:140:16
+  --> tests/ui/needless_pass_by_ref_mut.rs:135:16
    |
 LL | async fn a3(x: &mut i32, y: String, z: String) {
    |                ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:144:16
+  --> tests/ui/needless_pass_by_ref_mut.rs:139:16
    |
 LL | async fn a4(x: &mut i32, y: i32) {
    |                ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:148:24
+  --> tests/ui/needless_pass_by_ref_mut.rs:143:24
    |
 LL | async fn a5(x: i32, y: &mut i32) {
    |                        ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:152:24
+  --> tests/ui/needless_pass_by_ref_mut.rs:147:24
    |
 LL | async fn a6(x: i32, y: &mut i32) {
    |                        ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:156:32
+  --> tests/ui/needless_pass_by_ref_mut.rs:151:32
    |
 LL | async fn a7(x: i32, y: i32, z: &mut i32) {
    |                                ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:160:24
+  --> tests/ui/needless_pass_by_ref_mut.rs:155:24
    |
 LL | async fn a8(x: i32, a: &mut i32, y: i32, z: &mut i32) {
    |                        ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:160:45
+  --> tests/ui/needless_pass_by_ref_mut.rs:155:45
    |
 LL | async fn a8(x: i32, a: &mut i32, y: i32, z: &mut i32) {
    |                                             ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:194:16
+  --> tests/ui/needless_pass_by_ref_mut.rs:189:16
    |
 LL | fn cfg_warn(s: &mut u32) {}
    |                ^^^^^^^^ help: consider changing to: `&u32`
    |
    = note: this is cfg-gated and may require further changes
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:200:20
+  --> tests/ui/needless_pass_by_ref_mut.rs:195:20
    |
 LL |     fn cfg_warn(s: &mut u32) {}
    |                    ^^^^^^^^ help: consider changing to: `&u32`
    |
    = note: this is cfg-gated and may require further changes
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:214:39
+  --> tests/ui/needless_pass_by_ref_mut.rs:209:39
    |
 LL | async fn inner_async2(x: &mut i32, y: &mut u32) {
    |                                       ^^^^^^^^ help: consider changing to: `&u32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:222:26
+  --> tests/ui/needless_pass_by_ref_mut.rs:217:26
    |
 LL | async fn inner_async3(x: &mut i32, y: &mut u32) {
    |                          ^^^^^^^^ help: consider changing to: `&i32`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:241:34
+  --> tests/ui/needless_pass_by_ref_mut.rs:236:34
    |
 LL | pub async fn call_in_closure1(n: &mut str) {
    |                                  ^^^^^^^^ help: consider changing to: `&str`
    |
    = warning: changing this function will impact semver compatibility
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:253: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
-  --> tests/ui/needless_pass_by_ref_mut.rs:260:20
+  --> tests/ui/needless_pass_by_ref_mut.rs:255: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
-  --> tests/ui/needless_pass_by_ref_mut.rs:271:26
+  --> tests/ui/needless_pass_by_ref_mut.rs:266:26
    |
 LL | pub async fn closure4(n: &mut 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
-  --> tests/ui/needless_pass_by_ref_mut.rs:311:18
+  --> tests/ui/needless_pass_by_ref_mut.rs:315:12
+   |
+LL |     fn bar(&mut self) {}
+   |            ^^^^^^^^^ help: consider changing to: `&self`
+
+error: this argument is a mutable reference, but not used mutably
+  --> tests/ui/needless_pass_by_ref_mut.rs:317:18
+   |
+LL |     async fn foo(&mut self, u: &mut i32, v: &mut u32) {
+   |                  ^^^^^^^^^ help: consider changing to: `&self`
+
+error: this argument is a mutable reference, but not used mutably
+  --> tests/ui/needless_pass_by_ref_mut.rs:317:45
+   |
+LL |     async fn foo(&mut self, u: &mut i32, v: &mut u32) {
+   |                                             ^^^^^^^^ help: consider changing to: `&u32`
+
+error: this argument is a mutable reference, but not used mutably
+  --> tests/ui/needless_pass_by_ref_mut.rs:325:46
+   |
+LL |     async fn foo2(&mut self, u: &mut i32, v: &mut u32) {
+   |                                              ^^^^^^^^ help: consider changing to: `&u32`
+
+error: this argument is a mutable reference, but not used mutably
+  --> tests/ui/needless_pass_by_ref_mut.rs:341:18
    |
 LL | fn _empty_tup(x: &mut (())) {}
    |                  ^^^^^^^^^ help: consider changing to: `&()`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:312:19
+  --> tests/ui/needless_pass_by_ref_mut.rs:342:19
    |
 LL | fn _single_tup(x: &mut ((i32,))) {}
    |                   ^^^^^^^^^^^^^ help: consider changing to: `&(i32,)`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:313:18
+  --> tests/ui/needless_pass_by_ref_mut.rs:343:18
    |
 LL | fn _multi_tup(x: &mut ((i32, u32))) {}
    |                  ^^^^^^^^^^^^^^^^^ help: consider changing to: `&(i32, u32)`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:314:11
+  --> tests/ui/needless_pass_by_ref_mut.rs:344:11
    |
 LL | fn _fn(x: &mut (fn())) {}
    |           ^^^^^^^^^^^ help: consider changing to: `&fn()`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:316:23
+  --> tests/ui/needless_pass_by_ref_mut.rs:346:23
    |
 LL | fn _extern_rust_fn(x: &mut extern "Rust" fn()) {}
    |                       ^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&extern "Rust" fn()`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:317:20
+  --> tests/ui/needless_pass_by_ref_mut.rs:347:20
    |
 LL | fn _extern_c_fn(x: &mut extern "C" fn()) {}
    |                    ^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&extern "C" fn()`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:318:18
+  --> tests/ui/needless_pass_by_ref_mut.rs:348:18
    |
 LL | fn _unsafe_fn(x: &mut unsafe fn()) {}
    |                  ^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe fn()`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:319:25
+  --> tests/ui/needless_pass_by_ref_mut.rs:349:25
    |
 LL | fn _unsafe_extern_fn(x: &mut unsafe extern "C" fn()) {}
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe extern "C" fn()`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:320:20
+  --> tests/ui/needless_pass_by_ref_mut.rs:350:20
    |
 LL | fn _fn_with_arg(x: &mut unsafe extern "C" fn(i32)) {}
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe extern "C" fn(i32)`
 
 error: this argument is a mutable reference, but not used mutably
-  --> tests/ui/needless_pass_by_ref_mut.rs:321:20
+  --> tests/ui/needless_pass_by_ref_mut.rs:351:20
    |
 LL | fn _fn_with_ret(x: &mut unsafe extern "C" fn() -> (i32)) {}
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider changing to: `&unsafe extern "C" fn() -> (i32)`
 
-error: aborting due to 31 previous errors
+error: aborting due to 34 previous errors
 

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() {}