rust-clippy
rust-clippy copied to clipboard
rust-lang
add new lint that disallow renaming parameters in trait functions
fixes: #11443 fixes: #486
changelog: add new lint [renamed_function_params]
Note that the lint name is not final, because I have a bad reputation in naming things, and I don't trust myself.
I haven't looked at the implementation, so this might already be implemented: I would recommend that it allows removing leading underscores from the names. Here is an example, of what I mean:
trait Foo {
fn foo(_param: u32) {}
}
struct FooImpl;
impl Foo for FooImpl {
// vvvvv This should be allowed
fn foo(param: u32) {
println!("{param}");
}
}
Potentially, it could also allow adding an underscore, but here I'm uncertain if that's a good idea. Still, this would be the example for that case:
trait Bar {
fn bar(param: u32) {
println!("{param}");
}
}
struct BarImpl;
impl Bar for BarImpl {
// vvvvvv Maybe this could be allowed?
fn bar(_param: u32) {}
}
I haven't looked at the implementation, so this might already be implemented: I would recommend that it allows removing leading underscores from the names. Here is an example, of what I mean:
trait Foo { fn foo(_param: u32) {} } struct FooImpl; impl Foo for FooImpl { // vvvvv This should be allowed fn foo(param: u32) { println!("{param}"); } }Potentially, it could also allow adding an underscore, but here I'm uncertain if that's a good idea. Still, this would be the example for that case:
trait Bar { fn bar(param: u32) { println!("{param}"); } } struct BarImpl; impl Bar for BarImpl { // vvvvvv Maybe this could be allowed? fn bar(_param: u32) {} }
It should already be implemented :D in fact any name starts with underscore will be skipped, that includes names in trait def and trait impl
:umbrella: The latest upstream changes (presumably #11550) made this pull request unmergeable. Please resolve the merge conflicts.
Perhaps this was forgotten 😭 , even I couldn't remember what I did
Plus, seems like she's busy btw? so... umm @rustbot r? @rust-lang/clippy
some extra info: there are 2644 occurance of this lint after running lintcheck --recursive
On zulip it was mentioned that this lint should ideally have a configuration to ignore some lints, like From, TryFrom, and FromStr. Once that is done I believe we can move forward with this. There was also this suggestion:
It looks a bit similar to
disallowed_names. Perhaps the name could bedisallow_parameter_renamesordisallow_renames?
I like the current name, but can understand the reasoning :)
@xFrednet Finally got some time working on this~ I've added a configuration for it, but again... not confident about that config's name, which is ignored-traits for now, if there's better name plz let me know~
some time I really wish there's an AI that could tell me what should I name my variables/functions/lint/config lol
(Sorry for the delay with the review, my past few weeks have been busy)
no worries, me too XD
ping @xFrednet
In Clippy's realm, a rule takes flight, No renaming parameters, to keep code tight. Thanks for the effort, I merge with delight.
=^.^=
:pushpin: Commit 23581897ba77a905c00e644d5ed097706488042f has been approved by xFrednet
It is now in the queue for this repository.
:hourglass: Testing commit 23581897ba77a905c00e644d5ed097706488042f with merge d47bc8a4d63a8352cbdb9399c5c59e6643184773...
ahh, forgot the cargo collect-metadata again, third times a charm I guess~
:v: @J-ZhengLi, you can now approve this pull request!
If @xFrednet told you to "r=me" after making some further change, please make that change, then do @bors r=@xFrednet
:pushpin: Commit 46659acdbde160324bae653e704203221df3b980 has been approved by xFrednet
It is now in the queue for this repository.
:hourglass: Testing commit 46659acdbde160324bae653e704203221df3b980 with merge 7cfb9a0d6fee91ad636a091f6a425521259f776a...
:sunny: Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test Approved by: xFrednet Pushing 7cfb9a0d6fee91ad636a091f6a425521259f776a to master...