diff --git a/clippy_lints/src/needless_pass_by_ref_mut.rs b/clippy_lints/src/needless_pass_by_ref_mut.rs index 30d3e86dc4ed..9e47c3ad0b7f 100644 --- a/clippy_lints/src/needless_pass_by_ref_mut.rs +++ b/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::>(); + 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_mut.rs b/tests/ui/needless_pass_by_ref_mut.rs index a92197fb0af5..3f5f55f40020 100644 --- a/tests/ui/needless_pass_by_ref_mut.rs +++ b/tests/ui/needless_pass_by_ref_mut.rs @@ -44,18 +44,13 @@ fn non_mut_ref(_: &Vec) {} 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) -> usize { //~^ ERROR: this argument is a mutable reference, but not used mutably vec.len() } - - fn badger(&mut self, vec: &mut Vec) -> usize { - //~^ ERROR: this argument is a mutable reference, but not used mutably - vec.len() - } } trait Babar { @@ -307,6 +302,41 @@ fn filter_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,))) {} diff --git a/tests/ui/needless_pass_by_ref_mut.stderr b/tests/ui/needless_pass_by_ref_mut.stderr index 89dad3e60b14..21ca393dcb63 100644 --- a/tests/ui/needless_pass_by_ref_mut.stderr +++ b/tests/ui/needless_pass_by_ref_mut.stderr @@ -14,73 +14,73 @@ LL | fn foo6(s: &mut Vec) { | ^^^^^^^^^^^^^ help: consider changing to: `&Vec` 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) -> usize { - | ^^^^^^^^^^^^^ help: consider changing to: `&Vec` +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) -> usize { - | ^^^^^^^^^^^^^ help: consider changing to: `&Vec` +LL | fn mushroom(&self, vec: &mut Vec) -> usize { + | ^^^^^^^^^^^^^ help: consider changing to: `&Vec` 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` @@ -88,7 +88,7 @@ LL | fn cfg_warn(s: &mut 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` @@ -96,19 +96,19 @@ LL | fn cfg_warn(s: &mut 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` @@ -116,15 +116,7 @@ LL | pub async fn call_in_closure1(n: &mut 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` @@ -132,7 +124,7 @@ LL | pub fn closure2(n: &mut usize) -> impl '_ + FnMut() -> 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` @@ -140,64 +132,88 @@ LL | pub async fn closure4(n: &mut 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 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 000000000000..3c2576213cd7 --- /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 000000000000..34b0b564deb8 --- /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 000000000000..c87536032256 --- /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 +