new lint: source_item_ordering

[decryphe]

changelog: [source_item_ordering]: Introduced a new restriction lint that checks the ordering of items in Modules, Enums, Structs, Impls and Traits.

From the written documentation:

Why restrict this?
Keeping a consistent ordering throughout the codebase helps with working as a team, and possibly improves maintainability of the codebase. The idea is that by defining a consistent and enforceable rule for how source files are structured, less time will be wasted during reviews on a topic that is (under most circumstances) not relevant to the logic implemented in the code. Sometimes this will be referred to as "bike-shedding".

Keep in mind, that ordering source code alphabetically can lead to reduced performance in cases where the most commonly used enum variant isn't the first entry anymore, and similar optimizations that can reduce branch misses, cache locality and such. Either don't use this lint if that's relevant, or disable the lint in modules or items specifically where it matters. Other solutions can be to use profile guided optimization (PGO), or other advanced optimization methods.

I tried to build it as configurable as possible, as such a highly opinionated lint should be adjustable to personal opinions.

I'm open to any input and will be available both here and on the zulip for communication. In the meantime I'll be testing this lint against my own code-bases, which I've (manually) kept ordered with the default config, to see how well it works in practice.

And lastly, a big thanks to the community for making clippy the best linter there is!

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @llogiq (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[llogiq]

The big open question is around macro-generated code. Otherwise, apart from a few small doc improvements, this looks good.

[llogiq]

This should be under a # Known problems header.

We also might want to add that the lint doesn't supply suggestions because it's quite hard to produce them in a correct way.

[decryphe]

Moved it down to the (now renamed) chapter "Known Problems". Is there a standard way to write this?

Ctrl+F "# Known" yields different ways to write this: "Known problems", "Known Problems", "Known issues"

[llogiq]

I'd like to see some macro-generated items. I'm open to debate whether the lint should apply to them.

[decryphe]

I've updated the uitests to contain some derive macros. What other things should be tested?
I also added a uitest that passes the ordering lint and produces no output.

[decryphe]

Also, note that I've added ordering for trait impls on the module level.
I will add at least a test where I reconfigure item placement of trait impls into its own section, such that it's certainly possible to split trait defs and trait impls. The default groups all CamelCase items, including impls, but I can see why somebody would like to split this category up.

[decryphe]

Note: I've added a bunch of test cases, including modified configurations.

[decryphe]

Turns out that it can't reasonably be applied to macro-generated items, there's a ton of things in a real codebase that generates items. Using the in_external_macro() function from clippy_utils to filter items yields a decent behaviour in a real code-base.
Also, turns out we do adhere surprisingly well to our own convention, even though we do so manually. Some typos and forgotten things, but nothing majorly wrong in our source file ordering. :-)

I believe this is now in a reviewable state and should work as advertised.

[llogiq]

@rustbot author

[bors]

☔ The latest upstream changes (presumably #13322) made this pull request unmergeable. Please resolve the merge conflicts.

[decryphe]

I'd like to discuss whether or not I should implement another loop before generating lint output that determines the actual location where a snippet should be placed.
Considering the kind-of-weird looking output asking for placement of items before derive macro entries, it's probably worthwhile to do now.

[bors]

☔ The latest upstream changes (presumably #13334) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #13395) made this pull request unmergeable. Please resolve the merge conflicts.