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.

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

Jump to bottom

Add unnecessary_ref_mut lint #12399

Closed
wants to merge 1 commit into from
Closed

Add unnecessary_ref_mut lint #12399

wants to merge 1 commit into from

Conversation

not-elm
Copy link

@not-elm not-elm commented Mar 2, 2024

Related Issues: #12361

Added lint to detect whether a mutable reference is actually needed.

changelog: Add(unnecessary_ref_mut)

@rustbot
Copy link
Collaborator

rustbot commented Mar 2, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Manishearth (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 2, 2024
@samueltardieu
Copy link
Contributor

As-is, this will erroneously flag https://github.com/smol-rs/async-io/blob/master/src/lib.rs#L481 (*when = next; later requires that ref mut is used).

@Manishearth
Copy link
Member

Yeah I think we should be careful about this lint.

I don't think this makes sense as a restriction lint. I'm also a bit worried it's low-value.

@not-elm
Copy link
Author

not-elm commented Mar 5, 2024

Yeah I think we should be careful about this lint.

I don't think this makes sense as a restriction lint. I'm also a bit worried it's low-value.

Sorry, I'm still learning English, so I couldn't read the text accurately.
Do you mean that I should correct the category of lint?
In that case, would 'nursery' or 'pedantic' ?

@Manishearth
Copy link
Member

Yes, probably pedantic but as I mentioned I'm not sure if we should be adding this in the first place.

@bors
Copy link
Collaborator

bors commented Mar 5, 2024

☔ The latest upstream changes (presumably #12077) made this pull request unmergeable. Please resolve the merge conflicts.

ADD: unnessary_ref_mut lint
33c3f92 
ADD: add test case

ADD: unnecessary ref mut lint
add test cases and logic.

FIX: Added case when passed as function argument

FIX: add rebound variable  to be inspected

FIX: rebind

FIX: rebind

- If bound to static, it is considered mut
- Fixed to get Ident inside Some

FIX: cargo clippy

Fixed conflict resolution failed.
@not-elm not-elm force-pushed the feature/unnecessary_ref_mut branch from 454e64d to 33c3f92 Compare March 6, 2024 14:46
@not-elm
Copy link
Author

not-elm commented Mar 6, 2024

Oh... I understand. Thank you.
Sorry, ignore this PR.

@not-elm not-elm closed this by deleting the head repository May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Manishearth Manishearth

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@not-elm @rustbot @samueltardieu @Manishearth @bors