-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fix ptr_arg
suggests changes when it's actually better not to bother
#15105
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
base: master
Are you sure you want to change the base?
Conversation
fc15f8a
to
a78eeb3
Compare
This change only makes an exception for An exception is not made in the following scenarios:
|
I would ignore all references, not just mutable ones. An underscore named parameter is a good indicator that the signature matters for some reason (e.g. for function pointers). |
For example, it matters for methods that are not otherwise possible to call, or the user may want to clone the value. I've applied the necessary changes. I've also updated the parameter names in some earlier tests so that they remain effective. |
7fe341c
to
dbb0910
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Can you just squash the two commits please.
dbb0910
to
76038d5
Compare
changelog: fix false positive: [`ptr_arg`] no longer triggers with underscore binding to `&mut` argument
76038d5
to
75c330b
Compare
Sure, there you go, @Jarcho |
No longer suggests
&[i32]
or&mut [i32]
instead of&Vec<i32>
or&mut Vec<i32>
(also:Path
andPathBuf
, etc.) for the parameter type when the parameter name starts with an underscore (or, if that does not start with one, then a locallet
binding in the function body, pointing to the same value, does) – (Fixes #13489, fixes #13728)changelog: fix false positive: [ptr_arg
] no longer triggers with underscore binding to&mut
argumentchangelog: fix false positive: [
ptr_arg
] no longer triggers with underscore binding to&T
or&mut T
argumentEdit: This change has been extended to all references, not just mutable ones. See discussion below.