Skip to content

Commit

Permalink
Auto merge of #13376 - decryphe:source-ordering, r=llogiq
Browse files Browse the repository at this point in the history
new lint: `source_item_ordering`

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!
  • Loading branch information
bors committed Nov 2, 2024
2 parents e8b78e2 + f7ab2c9 commit 5c6fe68
Show file tree
Hide file tree
Showing 32 changed files with 2,257 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5331,6 +5331,7 @@ Released 2018-09-13
[`almost_complete_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_range
[`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped
[`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant
[`arbitrary_source_item_ordering`]: https://rust-lang.github.io/rust-clippy/master/index.html#arbitrary_source_item_ordering
[`arc_with_non_send_sync`]: https://rust-lang.github.io/rust-clippy/master/index.html#arc_with_non_send_sync
[`arithmetic_side_effects`]: https://rust-lang.github.io/rust-clippy/master/index.html#arithmetic_side_effects
[`as_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_conversions
Expand Down Expand Up @@ -6208,19 +6209,22 @@ Released 2018-09-13
[`max-trait-bounds`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max-trait-bounds
[`min-ident-chars-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#min-ident-chars-threshold
[`missing-docs-in-crate-items`]: https://doc.rust-lang.org/clippy/lint_configuration.html#missing-docs-in-crate-items
[`module-item-order-groupings`]: https://doc.rust-lang.org/clippy/lint_configuration.html#module-item-order-groupings
[`msrv`]: https://doc.rust-lang.org/clippy/lint_configuration.html#msrv
[`pass-by-value-size-limit`]: https://doc.rust-lang.org/clippy/lint_configuration.html#pass-by-value-size-limit
[`pub-underscore-fields-behavior`]: https://doc.rust-lang.org/clippy/lint_configuration.html#pub-underscore-fields-behavior
[`semicolon-inside-block-ignore-singleline`]: https://doc.rust-lang.org/clippy/lint_configuration.html#semicolon-inside-block-ignore-singleline
[`semicolon-outside-block-ignore-multiline`]: https://doc.rust-lang.org/clippy/lint_configuration.html#semicolon-outside-block-ignore-multiline
[`single-char-binding-names-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#single-char-binding-names-threshold
[`source-item-ordering`]: https://doc.rust-lang.org/clippy/lint_configuration.html#source-item-ordering
[`stack-size-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#stack-size-threshold
[`standard-macro-braces`]: https://doc.rust-lang.org/clippy/lint_configuration.html#standard-macro-braces
[`struct-field-name-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#struct-field-name-threshold
[`suppress-restriction-lint-in-const`]: https://doc.rust-lang.org/clippy/lint_configuration.html#suppress-restriction-lint-in-const
[`too-large-for-stack`]: https://doc.rust-lang.org/clippy/lint_configuration.html#too-large-for-stack
[`too-many-arguments-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#too-many-arguments-threshold
[`too-many-lines-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#too-many-lines-threshold
[`trait-assoc-item-kinds-order`]: https://doc.rust-lang.org/clippy/lint_configuration.html#trait-assoc-item-kinds-order
[`trivial-copy-size-limit`]: https://doc.rust-lang.org/clippy/lint_configuration.html#trivial-copy-size-limit
[`type-complexity-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#type-complexity-threshold
[`unnecessary-box-size`]: https://doc.rust-lang.org/clippy/lint_configuration.html#unnecessary-box-size
Expand Down
30 changes: 30 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,16 @@ crate. For example, `pub(crate)` items.
* [`missing_docs_in_private_items`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items)


## `module-item-order-groupings`
The named groupings of different source item kinds within modules.

**Default Value:** `[["modules", ["extern_crate", "mod", "foreign_mod"]], ["use", ["use"]], ["macros", ["macro"]], ["global_asm", ["global_asm"]], ["UPPER_SNAKE_CASE", ["static", "const"]], ["PascalCase", ["ty_alias", "enum", "struct", "union", "trait", "trait_alias", "impl"]], ["lower_snake_case", ["fn"]]]`

---
**Affected lints:**
* [`arbitrary_source_item_ordering`](https://rust-lang.github.io/rust-clippy/master/index.html#arbitrary_source_item_ordering)


## `msrv`
The minimum rust version that the project supports. Defaults to the `rust-version` field in `Cargo.toml`

Expand Down Expand Up @@ -784,6 +794,16 @@ The maximum number of single char bindings a scope may have
* [`many_single_char_names`](https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names)


## `source-item-ordering`
Which kind of elements should be ordered internally, possible values being `enum`, `impl`, `module`, `struct`, `trait`.

**Default Value:** `["enum", "impl", "module", "struct", "trait"]`

---
**Affected lints:**
* [`arbitrary_source_item_ordering`](https://rust-lang.github.io/rust-clippy/master/index.html#arbitrary_source_item_ordering)


## `stack-size-threshold`
The maximum allowed stack size for functions in bytes

Expand Down Expand Up @@ -863,6 +883,16 @@ The maximum number of lines a function or method can have
* [`too_many_lines`](https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines)


## `trait-assoc-item-kinds-order`
The order of associated items in traits.

**Default Value:** `["const", "type", "fn"]`

---
**Affected lints:**
* [`arbitrary_source_item_ordering`](https://rust-lang.github.io/rust-clippy/master/index.html#arbitrary_source_item_ordering)


## `trivial-copy-size-limit`
The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by
reference. By default there is no limit
Expand Down
38 changes: 37 additions & 1 deletion clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
use crate::ClippyConfiguration;
use crate::msrvs::Msrv;
use crate::types::{DisallowedPath, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour, Rename};
use crate::types::{
DisallowedPath, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour, Rename, SourceItemOrdering,
SourceItemOrderingCategory, SourceItemOrderingModuleItemGroupings, SourceItemOrderingModuleItemKind,
SourceItemOrderingTraitAssocItemKind, SourceItemOrderingTraitAssocItemKinds,
};
use rustc_errors::Applicability;
use rustc_session::Session;
use rustc_span::edit_distance::edit_distance;
Expand Down Expand Up @@ -47,6 +51,29 @@ const DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS: &[&str] = &["i", "j", "x", "y", "z
const DEFAULT_ALLOWED_PREFIXES: &[&str] = &["to", "as", "into", "from", "try_into", "try_from"];
const DEFAULT_ALLOWED_TRAITS_WITH_RENAMED_PARAMS: &[&str] =
&["core::convert::From", "core::convert::TryFrom", "core::str::FromStr"];
const DEFAULT_MODULE_ITEM_ORDERING_GROUPS: &[(&str, &[SourceItemOrderingModuleItemKind])] = {
#[allow(clippy::enum_glob_use)] // Very local glob use for legibility.
use SourceItemOrderingModuleItemKind::*;
&[
("modules", &[ExternCrate, Mod, ForeignMod]),
("use", &[Use]),
("macros", &[Macro]),
("global_asm", &[GlobalAsm]),
("UPPER_SNAKE_CASE", &[Static, Const]),
("PascalCase", &[TyAlias, Enum, Struct, Union, Trait, TraitAlias, Impl]),
("lower_snake_case", &[Fn]),
]
};
const DEFAULT_TRAIT_ASSOC_ITEM_KINDS_ORDER: &[SourceItemOrderingTraitAssocItemKind] = {
#[allow(clippy::enum_glob_use)] // Very local glob use for legibility.
use SourceItemOrderingTraitAssocItemKind::*;
&[Const, Type, Fn]
};
const DEFAULT_SOURCE_ITEM_ORDERING: &[SourceItemOrderingCategory] = {
#[allow(clippy::enum_glob_use)] // Very local glob use for legibility.
use SourceItemOrderingCategory::*;
&[Enum, Impl, Module, Struct, Trait]
};

/// Conf with parse errors
#[derive(Default)]
Expand Down Expand Up @@ -533,6 +560,9 @@ define_Conf! {
/// crate. For example, `pub(crate)` items.
#[lints(missing_docs_in_private_items)]
missing_docs_in_crate_items: bool = false,
/// The named groupings of different source item kinds within modules.
#[lints(arbitrary_source_item_ordering)]
module_item_order_groupings: SourceItemOrderingModuleItemGroupings = DEFAULT_MODULE_ITEM_ORDERING_GROUPS.into(),
/// The minimum rust version that the project supports. Defaults to the `rust-version` field in `Cargo.toml`
#[default_text = "current version"]
#[lints(
Expand Down Expand Up @@ -612,6 +642,9 @@ define_Conf! {
/// The maximum number of single char bindings a scope may have
#[lints(many_single_char_names)]
single_char_binding_names_threshold: u64 = 4,
/// Which kind of elements should be ordered internally, possible values being `enum`, `impl`, `module`, `struct`, `trait`.
#[lints(arbitrary_source_item_ordering)]
source_item_ordering: SourceItemOrdering = DEFAULT_SOURCE_ITEM_ORDERING.into(),
/// The maximum allowed stack size for functions in bytes
#[lints(large_stack_frames)]
stack_size_threshold: u64 = 512_000,
Expand Down Expand Up @@ -641,6 +674,9 @@ define_Conf! {
/// The maximum number of lines a function or method can have
#[lints(too_many_lines)]
too_many_lines_threshold: u64 = 100,
/// The order of associated items in traits.
#[lints(arbitrary_source_item_ordering)]
trait_assoc_item_kinds_order: SourceItemOrderingTraitAssocItemKinds = DEFAULT_TRAIT_ASSOC_ITEM_KINDS_ORDER.into(),
/// The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by
/// reference. By default there is no limit
#[default_text = "target_pointer_width * 2"]
Expand Down
Loading

0 comments on commit 5c6fe68

Please sign in to comment.