Skip to content

Conversation

MCJOHN974
Copy link

@MCJOHN974 MCJOHN974 commented Apr 16, 2025

Description

Make [unnecessary_mut_passed] auto applicable and automatically fixable via cargo clippy --fix

changelog: [`unnecessary_mut_passed`]: is auto applicable now

Why it is necessary

There is no case where auto-applying this lint can harm, and it is always nice to have lint applied automatically.

fixes #14617

Checklist

  • Followed [lint naming conventions][lint_naming]
  • Edited UI tests (including committed .stderr file) and make them passing
  • cargo test passes locally
  • Executed cargo dev update_lints
  • Added lint documentation
  • Run cargo dev fmt

Disclaimer

It's my first PR in clippy, so sorry if my PR deviates from any guidelines or traditions, and thanks you for the feedback!

@rustbot
Copy link
Collaborator

rustbot commented Apr 16, 2025

r? @Alexendoo

rustbot has assigned @Alexendoo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 16, 2025
Comment on lines +91 to +92
let snippet = snippet(cx, argument.span, "..");
let trimmed_snippet = &snippet[5..];
Copy link
Member

@samueltardieu samueltardieu Apr 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursory review:

It would probably be best if you use span_lint_and_then(), with .span_suggestion() (or .span_suggestion_verbose(), whether this gives a prettier result or not), and propose to rewrite &mut as &, instead of reconstructing the snippet. You can proceed as follows:

  • The span to replace is from the start of argument to the start of the AddrOf argument.
  • You should also ensure that the context of argument is the same one as the context of the AddrOf argument, to ensure that the &mut has not been built by a macro.
  • Add tests where &mut comes from a macro for example (build a macro refmut and call refmut!(variable)), and where the expression comes from a macro (build a macro identity and call &mut identity!(var)).

Also, in the PR message (changelog) and title, you should use the lint name in lower case.

@MCJOHN974
Copy link
Author

Also, are the tests I edited enough, or clippy have some tests to check if lint actually is machine-applicable and is applied correctly ?

@MCJOHN974 MCJOHN974 changed the title [feat]: make UNNECESSARY_MUT_PASSED lint auto applicable [feat]: make unnecessary_mut_passed lint auto applicable Apr 16, 2025
@Alexendoo
Copy link
Member

You can remove this

from the existing test file, that will create .fixed file when you cargo uibless it

@rustbot
Copy link
Collaborator

rustbot commented May 26, 2025

☔ The latest upstream changes (possibly 32a3744) made this pull request unmergeable. Please resolve the merge conflicts.

@Jarcho
Copy link
Contributor

Jarcho commented Sep 18, 2025

Ping @MCJOHN974 from triage. Do you plan to return to working on this?

@ada4a
Copy link
Contributor

ada4a commented Oct 3, 2025

Hi @MCJOHN974, it turns out I've implemented the exact same thing in #15438, because I learned about this issue from #8943 instead of #14617 and thus didn't know it was supposed to be left to first-time contributors -- sorry about that..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cargo clippy --fix don't remove redundant mut

6 participants