Skip to content

Emit the needless_pass_by_ref_mut lint on self arguments as well #12693

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 18, 2024
Merged

Emit the needless_pass_by_ref_mut lint on self arguments as well #12693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f8b1e3
needless_pass_by_ref_mut: Fix corner case in async functions
GuillaumeGomez Apr 18, 2024
0999867
needless_pass_by_ref_mut: emit the lint on `self` as well
GuillaumeGomez Apr 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions clippy_lints/src/needless_pass_by_ref_mut.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
42 changes: 36 additions & 6 deletions tests/ui/needless_pass_by_ref_mut.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,))) {}
Expand Down
98 changes: 57 additions & 41 deletions tests/ui/needless_pass_by_ref_mut.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

24 changes: 24 additions & 0 deletions tests/ui/needless_pass_by_ref_mut2.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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() {}
Loading