[Tracking issue] Rearrange location of some of the existing lints #6680

magurotuna · 2021-02-05T17:22:39Z

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 refactoring types.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 in clippy_lints/src/methods/mod.rs. Would be good if the implementations went to clippy_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.

CC @nahuakang @camsteffen

The text was updated successfully, but these errors were encountered:

nahuakang · 2021-02-06T19:30:19Z

@magurotuna @camsteffen Shall we kick this off sometime this month? I could focus on loops since I just came from it (and there are only 18 lints). But I think for methods we'd need to split carefully since it has 50+ lints 🤔

flip1995 · 2021-02-06T21:15:13Z

Also feel free to split this not only in multiple commits, but also in multiple PRs.

camsteffen · 2021-02-06T22:19:32Z

@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 · 2021-02-07T09:12:07Z

clippy_lints/src/<group_name>/<lint_name>.rs

If group_name refers to lint groups like restriction or pedantic, 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 · 2021-02-07T09:21:35Z

@phansch The group_name would be methods or loops so 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 · 2021-02-07T13:47:34Z

I'll work on "transmute" group :)

Y-Nak · 2021-02-09T15:08:09Z

I'll work on types!

flip1995 · 2021-02-14T14:35:26Z

@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 · 2021-02-14T14:38:22Z

Can do. Mine is ready for review. @Y-Nak @magurotuna is either of yours ready for a first comparison? :)

flip1995 · 2021-02-14T14:49:44Z

After a quick scan of the PRs it seems that all are ready for review. 👍

magurotuna · 2021-02-14T14:54:29Z

Yes, mine is ready too. I'll have a look at the PRs of @nahuakang and @Y-Nak when I have time :)

Y-Nak · 2021-02-14T15:04:47Z

@flip1995 Sure, I'll review them.
@nahuakang @magurotuna mine is also ready, please see #6724 before reviewing my PR.

Y-Nak · 2021-02-18T02:20:32Z

functions.rs can also be refactored, I think.

magurotuna · 2021-02-18T12:02:51Z

@Y-Nak Good catch! I updated the PR description.

TaKO8Ki · 2021-02-25T17:30:34Z

Can I work on methods group?

magurotuna · 2021-02-26T11:34:17Z

@TaKO8Ki
I think you may go ahead! Updated PR description.

Refactor: arrange transmute lints This PR arranges `transmute` lints so that they can be accessed more easily. Basically, I followed the instruction described in #6680 as to how to do the refactoring. - `declare_clippy_lint!` and `impl LintPass` is placed in `transmute/mod.rs` - Uitlity functions is placed in `transmute/utils.rs` - Each lint function about `transmute` is moved into its own module, like `transmute/useless_transmute.rs` For ease of review, I refactored step by step, keeping each commit small. For instance, all I did in 2451781 was to move `useless_transmute` into its own module. --- changelog: Refactor `transmute.rs` file into `transmute` module.

camsteffen · 2021-03-03T17:04:55Z

Some good teamwork happening in here!

Refactor: arrange lints in `methods` module This PR arranges methods lints so that they can be accessed more easily. Basically, I refactored them following the instruction described in #6680. changelog: Move lints in methods module into their own modules.

TaKO8Ki · 2021-03-12T03:03:45Z

@magurotuna
I decided to split into a new PR because #6826 became too large. So, could you add a link to my tracing issue #6886 to this issue?

magurotuna · 2021-03-12T10:27:09Z

@TaKO8Ki Sure thing! Done it.

…nsch Refactor lints in methods module This PR refactors methods lints other than the lints I refactored in #6826 and moves some functions to methods/utils.rs. Basically, I follow the instruction described in #6680. **For ease of review, I refactored step by step, keeping each commit small.** closes #6886 cc: `@phansch,` `@flip1995,` `@Y-Nak` changelog: Move lints in methods module to their own modules and some function to methods/utils.rs.

Y-Nak · 2021-03-24T11:55:06Z

I'm working on functions.

Btw, it would be better to discuss how to refactor misc. All lints in misc seem not relevant to each other at all, so I think it makes sense to declare lint passes for each lint.

magurotuna · 2021-03-24T12:49:59Z

Btw, it would be better to discuss how to refactor misc. All lints in misc seem not relevant to each other at all, so I think it makes sense to declare lint passes for each lint.

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 · 2021-03-24T12:50:38Z

I think for misc some of the lints there would also fit in already existing modules. misc and misc_early are 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 · 2021-03-24T13:26:45Z

Cool! I updated the description of this issue, adding a misc_early checkbox and how to refactor misc and misc_early.
I would like to work on misc :)

Organize functions into functions module Ref: #6680 Rearrange lints in `functions`. changelog: none

Y-Nak · 2021-03-31T14:09:54Z

@magurotuna Could you update the TODO list? Refactoring functions has been done in #6990.

magurotuna · 2021-03-31T23:23:07Z

@Y-Nak Great job! Updated.

Y-Nak · 2021-04-16T05:37:09Z

Oh, we missed matches.rs. I'll work on it.
@magurotuna could you add it to the TODO list, please?

magurotuna · 2021-04-17T12:44:38Z

@Y-Nak Sure, added!

Anyway, recently I've been mostly working on another project so I don't have enough time to refactor misc despite of my previous post. I'll withdraw from misc work for the moment. If anyone is interested in it, please feel free to take it over :D

TaKO8Ki · 2021-04-22T08:03:38Z

Can I also work on misc_early group?

flip1995 · 2021-04-22T08:06:19Z

Sure! If you have questions on where lints should go, I think the best place to discuss this is on Zulip.

Refactor: arrange lints in misc_early module This PR arranges misc_early lints so that they can be accessed more easily. Basically, I refactored them following the instruction described in #6680. cc: `@Y-Nak,` `@flip1995,` `@magurotuna` changelog: Move lints in misc_early module into their own modules.

Split matches Part of #6680 changelog: None

flip1995 added C-an-interesting-project Category: Interesting projects, that usually are more involved design/code wise. C-tracking-issue Category: Tracking Issue E-medium Call for participation: Medium difficulty level problem and requires some initial experience. labels Feb 6, 2021

nahuakang mentioned this issue Feb 7, 2021

Refactor: organize loops file into loops module #6693

Closed

magurotuna mentioned this issue Feb 11, 2021

Refactor: arrange transmute lints #6716

Merged

Y-Nak mentioned this issue Feb 12, 2021

Tracking Issue for refactoring types.rs #6724

Closed

4 tasks

anall mentioned this issue Feb 12, 2021

added new lint owned_to_owned #6730

Merged

Y-Nak mentioned this issue Feb 25, 2021

New lint: iter_count #6791

Merged

TaKO8Ki mentioned this issue Mar 2, 2021

Refactor: arrange lints in methods module #6826

Merged

TaKO8Ki mentioned this issue Mar 11, 2021

[Tracking Issue] Refactor lints in methods module #6886

Closed

8 tasks

TaKO8Ki mentioned this issue Mar 13, 2021

Refactor lints in methods module #6896

Merged

Y-Nak mentioned this issue Mar 27, 2021

Organize functions into functions module #6990

Merged

bors added a commit that referenced this issue Mar 30, 2021

Auto merge of #6990 - Y-Nak:refactor-functions, r=giraffate

1ddeaa6

Organize functions into functions module Ref: #6680 Rearrange lints in `functions`. changelog: none

TaKO8Ki mentioned this issue May 4, 2021

Refactor: arrange lints in misc_early module #7166

Merged

TaKO8Ki mentioned this issue May 11, 2021

[Tracking Issue] Refactor lints in misc_early module #7208

Open

8 tasks

Jarcho mentioned this issue Feb 6, 2022

Split matches #8400

Merged

bors added a commit that referenced this issue Feb 7, 2022

Auto merge of #8400 - Jarcho:split_matches, r=Manishearth

2590701

Split matches Part of #6680 changelog: None

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Tracking issue] Rearrange location of some of the existing lints #6680

[Tracking issue] Rearrange location of some of the existing lints #6680

magurotuna commented Feb 5, 2021 •

edited

Loading

nahuakang commented Feb 6, 2021

flip1995 commented Feb 6, 2021

camsteffen commented Feb 6, 2021 •

edited

Loading

phansch commented Feb 7, 2021

nahuakang commented Feb 7, 2021 •

edited

Loading

magurotuna commented Feb 7, 2021

Y-Nak commented Feb 9, 2021

flip1995 commented Feb 14, 2021

nahuakang commented Feb 14, 2021

flip1995 commented Feb 14, 2021

magurotuna commented Feb 14, 2021

Y-Nak commented Feb 14, 2021

Y-Nak commented Feb 18, 2021 •

edited

Loading

magurotuna commented Feb 18, 2021

TaKO8Ki commented Feb 25, 2021 •

edited

Loading

magurotuna commented Feb 26, 2021

camsteffen commented Mar 3, 2021

TaKO8Ki commented Mar 12, 2021

magurotuna commented Mar 12, 2021

Y-Nak commented Mar 24, 2021 •

edited

Loading

magurotuna commented Mar 24, 2021

flip1995 commented Mar 24, 2021

magurotuna commented Mar 24, 2021

Y-Nak commented Mar 31, 2021

magurotuna commented Mar 31, 2021

Y-Nak commented Apr 16, 2021 •

edited

Loading

magurotuna commented Apr 17, 2021

TaKO8Ki commented Apr 22, 2021

flip1995 commented Apr 22, 2021

[Tracking issue] Rearrange location of some of the existing lints #6680

[Tracking issue] Rearrange location of some of the existing lints #6680

Comments

magurotuna commented Feb 5, 2021 • edited Loading

nahuakang commented Feb 6, 2021

flip1995 commented Feb 6, 2021

camsteffen commented Feb 6, 2021 • edited Loading

phansch commented Feb 7, 2021

nahuakang commented Feb 7, 2021 • edited Loading

magurotuna commented Feb 7, 2021

Y-Nak commented Feb 9, 2021

flip1995 commented Feb 14, 2021

nahuakang commented Feb 14, 2021

flip1995 commented Feb 14, 2021

magurotuna commented Feb 14, 2021

Y-Nak commented Feb 14, 2021

Y-Nak commented Feb 18, 2021 • edited Loading

magurotuna commented Feb 18, 2021

TaKO8Ki commented Feb 25, 2021 • edited Loading

magurotuna commented Feb 26, 2021

camsteffen commented Mar 3, 2021

TaKO8Ki commented Mar 12, 2021

magurotuna commented Mar 12, 2021

Y-Nak commented Mar 24, 2021 • edited Loading

magurotuna commented Mar 24, 2021

flip1995 commented Mar 24, 2021

magurotuna commented Mar 24, 2021

Y-Nak commented Mar 31, 2021

magurotuna commented Mar 31, 2021

Y-Nak commented Apr 16, 2021 • edited Loading

magurotuna commented Apr 17, 2021

TaKO8Ki commented Apr 22, 2021

flip1995 commented Apr 22, 2021

magurotuna commented Feb 5, 2021 •

edited

Loading

camsteffen commented Feb 6, 2021 •

edited

Loading

nahuakang commented Feb 7, 2021 •

edited

Loading

Y-Nak commented Feb 18, 2021 •

edited

Loading

TaKO8Ki commented Feb 25, 2021 •

edited

Loading

Y-Nak commented Mar 24, 2021 •

edited

Loading

Y-Nak commented Apr 16, 2021 •

edited

Loading