Skip to content
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.

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

Unecessary mut reference in parameter #8863

Closed
Tracked by #79
emilk opened this issue May 21, 2022 · 7 comments · Fixed by #10900
Closed
Tracked by #79

Unecessary mut reference in parameter #8863

emilk opened this issue May 21, 2022 · 7 comments · Fixed by #10900
Labels
A-lint Area: New lints

Comments

@emilk
Copy link

emilk commented May 21, 2022

What it does

It warns against using &mut in an parameter where & would suffice.

Lint Name

unnecessary-mut-ref

Category

suspicious

Advantage

Less mut means less fights with the borrow checker. It can also lead to more opportunities for parallelization.

This lint could also catch bugs where the user intended to mutate an argument but forgot too.

Drawbacks

A user may want to create an API where they reserve the right to mutate an argument, but does not do so yet.

We should not warn when implementing a trait.

Example

fn foo(&mut self, y: &mut i32) -> i32 {
    self.x + *y
}

Could be written as:

fn foo(&self, y: &i32) -> i32 {
    self.x + *y
}
@junderw
Copy link

junderw commented Jun 5, 2023

I wonder if this could be also made into a separate lint where ownership is passed when a &mut or & would suffice.

Basically, warning when your fn is taking more than it needs.

I like it!

@Centri3
Copy link
Member

Centri3 commented Jun 5, 2023

I wonder if this could be also made into a separate lint where ownership is passed when a &mut or & would suffice.

Basically, warning when your fn is taking more than it needs.

I like it!

Is this already covered by needless_pass_by_value ?

@junderw
Copy link

junderw commented Jun 5, 2023

Good point.

Maybe instead of unnecessary-mut-ref it should be proposed that this issue's lint is needless_pass_by_mut_ref

@junderw
Copy link

junderw commented Jun 5, 2023

Referencing the pass_by_value PR in case someone wants to look at it for inspiration: #1556

@emilk emilk changed the title Unecessary mut reference Unecessary mut reference in parameter Jun 6, 2023
@GuillaumeGomez
Copy link
Member

I'll take a look.

@GuillaumeGomez
Copy link
Member

I'm done with the implementation. I'll open a draft PR after the cleanup but I'll need people to test it so I can add more cases into the UI test.

@GuillaumeGomez
Copy link
Member

I opened the draft here. Please test it so we can uncover cases I didn't think about. Thanks in advance! :)

@bors bors closed this as completed in b46033e Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants