Repository navigation
[Tracking issue] Rearrange location of some of the existing lints #6680
Description
Currently, the lint rules grouped as "methods" reside in clippy_lints/src/methods as there are a number of rules belonging to the "methods" group.
However, although the other groups such as "loops", "types", "transmute" and "misc" also have many relavent rules, they reside not in clippy_lints/src/<group_name> but in clippy_lints/src.
It would be great to have these groups located under <group_name> directory just like "methods". So I open this issue to do this refactoring and to keep track of it.
-
loops (done by @nahuakang & @Y-Nak Refactor: organize loops file into loops module (Delegated) #6824) -
types (done by @Y-Nak Tracking Issue for refactoringtypes.rs#6724) -
transmute (done by @magurotuna Refactor: arrange transmute lints #6716) -
functions (done by @Y-Nak Organize functions into functions module #6990) -
methods (also needs to be refactored because currently most of rule implementation are included inclippy_lints/src/methods/mod.rs. Would be good if the implementations went toclippy_lints/src/methods/<lint_name>.rs.) (done by @TaKO8Ki [Tracking Issue] Refactor lints in methods module #6886) - misc (instead of creating a new directory named
misc, split the lints one by one, and move them into where they fit most. ) - misc_early (same as
misc) (in progress by @TaKO8Ki Refactor: arrange lints in misc_early module #7166, [Tracking Issue] Refactor lints in misc_early module #7208) - matches (in progress by @Y-Nak)
How to rearrange is like the following, proposed by @flip1995 on zulip
I would keep the declare_clippy_lint! in the mod.rs Best case would be that mod.rs only contains the lint definitions and the LintPass impl. Utility functions of the module should probably go in methods/utils.rs and then reexported for the module in mod.rs.
proposed by @camsteffen:
For each lint, create a new module at clippy_lints/src/<group_name>/<lint_name>.rs. Within that module, add a function fn check(cx, ..) (with any needed arguments) and move the implementation there.
Activity
nahuakang commentedon Feb 6, 2021
@magurotuna @camsteffen Shall we kick this off sometime this month? I could focus on
loopssince I just came from it (and there are only 18 lints). But I think formethodswe'd need to split carefully since it has 50+ lints 🤔flip1995 commentedon Feb 6, 2021
Also feel free to split this not only in multiple commits, but also in multiple PRs.
camsteffen commentedon Feb 6, 2021
@nahuakang Sure! But frankly I don't want to promise anything from me at this point. For organization, I would recommend we comment here on what group you're currently working on. But you don't have to do an entire group.
phansch commentedon Feb 7, 2021
If
group_namerefers to lint groups likerestrictionorpedantic, it's worth noting that these change quite often compared to lint names. So, if someone changes the lint group of a lint they will have to move it to the correct directory as well, adding some extra work. Not sure if that's fine?If it just refers to unrelated group names, ignore what I said =)
nahuakang commentedon Feb 7, 2021
@phansch The
group_namewould bemethodsorloopsso that new contributors have an easier time finding lints or understanding the structure of the repo 😁 You are right that it shouldn't be the lint group categories.magurotuna commentedon Feb 7, 2021
I'll work on "transmute" group :)
Y-Nak commentedon Feb 9, 2021
I'll work on
types!flip1995 commentedon Feb 14, 2021
@Y-Nak @nahuakang @magurotuna Do you want to cross-review your PRs to make sure that you all used the same "style" for the restructuring, so the Clippy code is consistent? That would be a big help for me reviewing those PRs because I don't have to compare them to eachother.
nahuakang commentedon Feb 14, 2021
Can do. Mine is ready for review. @Y-Nak @magurotuna is either of yours ready for a first comparison? :)
flip1995 commentedon Feb 14, 2021
After a quick scan of the PRs it seems that all are ready for review. 👍
magurotuna commentedon Feb 14, 2021
Yes, mine is ready too. I'll have a look at the PRs of @nahuakang and @Y-Nak when I have time :)
Y-Nak commentedon Feb 14, 2021
@flip1995 Sure, I'll review them.
@nahuakang @magurotuna mine is also ready, please see #6724 before reviewing my PR.
11 remaining items
magurotuna commentedon Mar 24, 2021
Agreed. I don't see any specific reason why the lints that are currently categorized as "misc" are there. "misc" is so ambiguous that it makes people think a new lint that seems trivial to them should go into "misc". What kind of lints seem trivial to people differs from person to person IMO. Thus it makes more sense to me too, to separate the lints in "misc" and declare individual lint passes for them.
flip1995 commentedon Mar 24, 2021
I think for
miscsome of the lints there would also fit in already existing modules.miscandmisc_earlyare both artifacts from the early days of Clippy. I agree that it might be good to split up and get rid of those modules completely.magurotuna commentedon Mar 24, 2021
Cool! I updated the description of this issue, adding a
misc_earlycheckbox and how to refactormiscandmisc_early.I would like to work on
misc:)Auto merge of #6990 - Y-Nak:refactor-functions, r=giraffate
Y-Nak commentedon Mar 31, 2021
@magurotuna Could you update the TODO list? Refactoring
functionshas been done in #6990.magurotuna commentedon Mar 31, 2021
@Y-Nak Great job! Updated.
Y-Nak commentedon Apr 16, 2021
Oh, we missed
matches.rs. I'll work on it.@magurotuna could you add it to the TODO list, please?
magurotuna commentedon Apr 17, 2021
@Y-Nak Sure, added!
Anyway, recently I've been mostly working on another project so I don't have enough time to refactor
miscdespite of my previous post. I'll withdraw frommiscwork for the moment. If anyone is interested in it, please feel free to take it over :DTaKO8Ki commentedon Apr 22, 2021
Can I also work on
misc_earlygroup?flip1995 commentedon Apr 22, 2021
Sure! If you have questions on where lints should go, I think the best place to discuss this is on Zulip.
Auto merge of #7166 - TaKO8Ki:refactor_misc_early_module, r=llogiq
Auto merge of #8400 - Jarcho:split_matches, r=Manishearth
extract `duplicate_underscore_argument`, and move it into `functions` (…
ada4a commentedon Sep 7, 2025
I think
matchescan be crossed out as well, thanks to #8400move `toplevel_ref_args` out of `misc` (#15627)