From 3b4d3984342ae4917b1652ebb9fbbabd215d65dc Mon Sep 17 00:00:00 2001 From: decryphe <12104091+decryphe@users.noreply.github.com> Date: Thu, 29 Aug 2024 17:58:40 +0200 Subject: [PATCH 1/9] new lint: `source_item_ordering` --- CHANGELOG.md | 4 + book/src/lint_configuration.md | 30 ++ clippy_config/src/conf.rs | 41 +- clippy_config/src/types.rs | 301 +++++++++++++ .../src/arbitrary_source_item_ordering.rs | 421 ++++++++++++++++++ clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + .../toml_unknown_key/conf_unknown_key.stderr | 9 + tests/ui/arbitrary_source_item_ordering.rs | 140 ++++++ .../ui/arbitrary_source_item_ordering.stderr | 184 ++++++++ 10 files changed, 1132 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/arbitrary_source_item_ordering.rs create mode 100644 tests/ui/arbitrary_source_item_ordering.rs create mode 100644 tests/ui/arbitrary_source_item_ordering.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index b7ac0ddfd32e..1595eeef2577 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5296,6 +5296,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 @@ -6152,6 +6153,7 @@ Released 2018-09-13 [`disallowed-types`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-types [`doc-valid-idents`]: https://doc.rust-lang.org/clippy/lint_configuration.html#doc-valid-idents [`enable-raw-pointer-heuristic-for-send`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enable-raw-pointer-heuristic-for-send +[`enable-source-item-ordering-for`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enable-source-item-ordering-for [`enforce-iter-loop-reborrow`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enforce-iter-loop-reborrow [`enforced-import-renames`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enforced-import-renames [`enum-variant-name-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enum-variant-name-threshold @@ -6169,6 +6171,7 @@ 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 @@ -6182,6 +6185,7 @@ Released 2018-09-13 [`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 diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 43b551ae2161..ca8951b0736b 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -473,6 +473,16 @@ Whether to apply the raw pointer heuristic to determine if a type is `Send`. * [`non_send_fields_in_send_ty`](https://rust-lang.github.io/rust-clippy/master/index.html#non_send_fields_in_send_ty) +## `enable-source-item-ordering-for` +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) + + ## `enforce-iter-loop-reborrow` Whether to recommend using implicit into iter for reborrowed values. @@ -666,6 +676,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", ["mod", "foreign_mod"]], ["use", ["use"]], ["macros", ["macro"]], ["global_asm", ["global_asm"]], ["UPPER_SNAKE_CASE", ["static", "const"]], ["PascalCase", ["ty_alias", "opaque_ty", "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` @@ -862,6 +882,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 diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 4757c0b1339a..097c817c091f 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -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, SourceItemOrderingCategory, + SourceItemOrderingEnableFor, SourceItemOrderingModuleItemGroupings, SourceItemOrderingModuleItemKind, + SourceItemOrderingTraitAssocItemKind, SourceItemOrderingTraitAssocItemKinds, +}; use rustc_errors::Applicability; use rustc_session::Session; use rustc_span::edit_distance::edit_distance; @@ -46,6 +50,32 @@ 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", &[Mod, ForeignMod]), + ("use", &[Use]), + ("macros", &[Macro]), + ("global_asm", &[GlobalAsm]), + ("UPPER_SNAKE_CASE", &[Static, Const]), + ( + "PascalCase", + &[TyAlias, OpaqueTy, 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_ENABLE_SOURCE_ITEM_ORDERING_FOR: &[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)] @@ -459,6 +489,9 @@ define_Conf! { /// Whether to apply the raw pointer heuristic to determine if a type is `Send`. #[lints(non_send_fields_in_send_ty)] enable_raw_pointer_heuristic_for_send: bool = true, + /// Which kind of elements should be ordered internally, possible values being `enum`, `impl`, `module`, `struct`, `trait`. + #[lints(arbitrary_source_item_ordering)] + enable_source_item_ordering_for: SourceItemOrderingEnableFor = DEFAULT_ENABLE_SOURCE_ITEM_ORDERING_FOR.into(), /// Whether to recommend using implicit into iter for reborrowed values. /// /// #### Example @@ -530,6 +563,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( @@ -637,6 +673,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"] diff --git a/clippy_config/src/types.rs b/clippy_config/src/types.rs index bab63911182d..f05b0a390d32 100644 --- a/clippy_config/src/types.rs +++ b/clippy_config/src/types.rs @@ -1,5 +1,6 @@ use serde::de::{self, Deserializer, Visitor}; use serde::{Deserialize, Serialize, ser}; +use std::collections::HashMap; use std::fmt; #[derive(Debug, Deserialize)] @@ -102,6 +103,306 @@ impl<'de> Deserialize<'de> for MacroMatcher { } } +/// Represents the item categories that can be ordered by the source ordering lint. +#[derive(Clone, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum SourceItemOrderingCategory { + Enum, + Impl, + Module, + Struct, + Trait, +} + +/// Represents which item categories are enabled for ordering. +/// +/// The [`Deserialize`] implementation checks that there are no duplicates in +/// the user configuration. +pub struct SourceItemOrderingEnableFor(Vec); + +impl SourceItemOrderingEnableFor { + pub fn contains(&self, category: &SourceItemOrderingCategory) -> bool { + self.0.contains(category) + } +} + +impl From for SourceItemOrderingEnableFor +where + T: Into>, +{ + fn from(value: T) -> Self { + Self(value.into()) + } +} + +impl core::fmt::Debug for SourceItemOrderingEnableFor { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +impl<'de> Deserialize<'de> for SourceItemOrderingEnableFor { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let items = Vec::::deserialize(deserializer)?; + let mut items_set = std::collections::HashSet::new(); + + for item in &items { + if items_set.contains(item) { + return Err(de::Error::custom(format!( + "The category \"{item:?}\" was enabled more than once in the source ordering configuration." + ))); + } + items_set.insert(item); + } + + Ok(Self(items)) + } +} + +impl Serialize for SourceItemOrderingEnableFor { + fn serialize(&self, serializer: S) -> Result + where + S: ser::Serializer, + { + self.0.serialize(serializer) + } +} + +/// Represents the items that can occur within a module. +#[derive(Clone, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum SourceItemOrderingModuleItemKind { + ExternCrate, + Mod, + ForeignMod, + Use, + Macro, + GlobalAsm, + Static, + Const, + TyAlias, + OpaqueTy, + Enum, + Struct, + Union, + Trait, + TraitAlias, + Impl, + Fn, +} + +impl SourceItemOrderingModuleItemKind { + pub fn all_variants() -> Vec { + #[allow(clippy::enum_glob_use)] // Very local glob use for legibility. + use SourceItemOrderingModuleItemKind::*; + vec![ + ExternCrate, + Mod, + ForeignMod, + Use, + Macro, + GlobalAsm, + Static, + Const, + TyAlias, + OpaqueTy, + Enum, + Struct, + Union, + Trait, + TraitAlias, + Impl, + Fn, + ] + } +} + +/// Represents the configured ordering of items within a module. +/// +/// The [`Deserialize`] implementation checks that no item kinds have been +/// omitted and that there are no duplicates in the user configuration. +#[derive(Clone)] +pub struct SourceItemOrderingModuleItemGroupings { + groups: Vec<(String, Vec)>, + lut: HashMap, +} + +impl SourceItemOrderingModuleItemGroupings { + fn build_lut( + groups: &[(String, Vec)], + ) -> HashMap { + let mut lut = HashMap::new(); + for (group_index, (_, items)) in groups.iter().enumerate() { + for item in items { + lut.insert(item.clone(), group_index); + } + } + lut + } + + pub fn module_level_order_of(&self, item: &SourceItemOrderingModuleItemKind) -> Option { + self.lut.get(item).copied() + } +} + +impl From<&[(&str, &[SourceItemOrderingModuleItemKind])]> for SourceItemOrderingModuleItemGroupings { + fn from(value: &[(&str, &[SourceItemOrderingModuleItemKind])]) -> Self { + let groups: Vec<(String, Vec)> = + value.iter().map(|item| (item.0.to_string(), item.1.to_vec())).collect(); + let lut = Self::build_lut(&groups); + Self { groups, lut } + } +} + +impl core::fmt::Debug for SourceItemOrderingModuleItemGroupings { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.groups.fmt(f) + } +} + +impl<'de> Deserialize<'de> for SourceItemOrderingModuleItemGroupings { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let groups = Vec::<(String, Vec)>::deserialize(deserializer)?; + let items_total: usize = groups.iter().map(|(_, v)| v.len()).sum(); + let lut = Self::build_lut(&groups); + + let mut expected_items = SourceItemOrderingModuleItemKind::all_variants(); + for item in lut.keys() { + expected_items.retain(|i| i != item); + } + + let all_items = SourceItemOrderingModuleItemKind::all_variants(); + if expected_items.is_empty() && items_total == all_items.len() { + let Some(use_group_index) = lut.get(&SourceItemOrderingModuleItemKind::Use) else { + return Err(de::Error::custom("Error in internal LUT.")); + }; + let Some((_, use_group_items)) = groups.get(*use_group_index) else { + return Err(de::Error::custom("Error in internal LUT.")); + }; + if use_group_items.len() > 1 { + return Err(de::Error::custom( + "The group containing the \"use\" item kind may not contain any other item kinds. \ + The \"use\" items will (generally) be sorted by rustfmt already. \ + Therefore it makes no sense to implement linting rules that may conflict with rustfmt.", + )); + } + + Ok(Self { groups, lut }) + } else if items_total != all_items.len() { + Err(de::Error::custom( + "Some module item kinds were configured more than once in the source ordering configuration.", + )) + } else { + Err(de::Error::custom(format!( + "Not all module item kinds were part of the configured source ordering rule. \ + All item kinds must be provided in the config, otherwise the required source ordering would remain ambiguous.\ + The module item kinds are: {all_items:?}" + ))) + } + } +} + +impl Serialize for SourceItemOrderingModuleItemGroupings { + fn serialize(&self, serializer: S) -> Result + where + S: ser::Serializer, + { + self.groups.serialize(serializer) + } +} + +/// Represents all kinds of trait associated items. +#[derive(Clone, Debug, Deserialize, PartialEq, PartialOrd, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum SourceItemOrderingTraitAssocItemKind { + Const, + Fn, + Type, +} + +impl SourceItemOrderingTraitAssocItemKind { + pub fn all_variants() -> Vec { + #[allow(clippy::enum_glob_use)] // Very local glob use for legibility. + use SourceItemOrderingTraitAssocItemKind::*; + vec![Const, Fn, Type] + } +} + +/// Represents the order in which associated trait items should be ordered. +/// +/// The reason to wrap a `Vec` in a newtype is to be able to implement +/// [`Deserialize`]. Implementing `Deserialize` allows for implementing checks +/// on configuration completeness at the time of loading the clippy config, +/// letting the user know if there's any issues with the config (e.g. not +/// listing all item kinds that should be sorted). +#[derive(Clone)] +pub struct SourceItemOrderingTraitAssocItemKinds(Vec); + +impl SourceItemOrderingTraitAssocItemKinds { + pub fn index_of(&self, item: &SourceItemOrderingTraitAssocItemKind) -> Option { + self.0.iter().position(|i| i == item) + } +} + +impl From for SourceItemOrderingTraitAssocItemKinds +where + T: Into>, +{ + fn from(value: T) -> Self { + Self(value.into()) + } +} + +impl core::fmt::Debug for SourceItemOrderingTraitAssocItemKinds { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +impl<'de> Deserialize<'de> for SourceItemOrderingTraitAssocItemKinds { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let items = Vec::::deserialize(deserializer)?; + + let mut expected_items = SourceItemOrderingTraitAssocItemKind::all_variants(); + for item in &items { + expected_items.retain(|i| i != item); + } + + let all_items = SourceItemOrderingTraitAssocItemKind::all_variants(); + if expected_items.is_empty() && items.len() == all_items.len() { + Ok(Self(items)) + } else if items.len() != all_items.len() { + Err(de::Error::custom( + "Some associated item kinds were configured more than once in the source ordering configuration.", + )) + } else { + Err(de::Error::custom(format!( + "Not all trait associated item kinds were part of the configured source ordering rule. \ + All item kinds must be provided in the config, otherwise the required source ordering would remain ambiguous.\ + The trait associated item kinds are: {all_items:?}" + ))) + } + } +} + +impl Serialize for SourceItemOrderingTraitAssocItemKinds { + fn serialize(&self, serializer: S) -> Result + where + S: ser::Serializer, + { + self.0.serialize(serializer) + } +} + // these impls are never actually called but are used by the various config options that default to // empty lists macro_rules! unimplemented_serialize { diff --git a/clippy_lints/src/arbitrary_source_item_ordering.rs b/clippy_lints/src/arbitrary_source_item_ordering.rs new file mode 100644 index 000000000000..34da4759bef1 --- /dev/null +++ b/clippy_lints/src/arbitrary_source_item_ordering.rs @@ -0,0 +1,421 @@ +use clippy_config::types::{ + SourceItemOrderingCategory, SourceItemOrderingModuleItemGroupings, SourceItemOrderingModuleItemKind, + SourceItemOrderingTraitAssocItemKind, SourceItemOrderingTraitAssocItemKinds, +}; +use clippy_config::Conf; +use clippy_utils::diagnostics::span_lint_and_note; +use rustc_hir::{ + AssocItemKind, FieldDef, HirId, ImplItemRef, IsAuto, Item, ItemKind, Mod, TraitItemRef, UseKind, Variant, + VariantData, +}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::impl_lint_pass; + +declare_clippy_lint! { + /// ### What it does + /// + /// Confirms that items are sorted in source files as per configuration. + /// + /// ### 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. + /// + /// ### Default Ordering and Configuration + /// + /// As there is no generally applicable rule, and each project may have + /// different requirements, the lint can be configured with high + /// granularity. The configuration is split into two stages: + /// + /// 1. Which item kinds that should have an internal order enforced. + /// 2. Individual ordering rules per item kind. + /// + /// The item kinds that can be linted are: + /// - Module (with customized groupings, alphabetical within) + /// - Trait (with customized order of associated items, alphabetical within) + /// - Enum, Impl, Struct (purely alphabetical) + /// + /// #### Module Item Order + /// + /// Due to the large variation of items within modules, the ordering can be + /// configured on a very granular level. Item kinds can be grouped together + /// arbitrarily, items within groups will be ordered alphabetically. The + /// following table shows the default groupings: + /// + /// | Group | Item Kinds | + /// |--------------------|----------------------| + /// | `modules` | "mod", "foreign_mod" | + /// | `use` | "use" | + /// | `macros` | "macro" | + /// | `global_asm` | "global_asm" | + /// | `UPPER_SNAKE_CASE` | "static", "const" | + /// | `PascalCase` | "ty_alias", "opaque_ty", "enum", "struct", "union", "trait", "trait_alias", "impl" | + /// | `lower_snake_case` | "fn" | + /// + /// All item kinds must be accounted for to create an enforceable linting + /// rule set. + /// + /// ### Known Issues and Limitations + /// + /// #### Lints on a Contains basis + /// + /// The lint can be disabled only on a "contains" basis, but not per element + /// within a "container", e.g. the lint works per-module, per-struct, + /// per-enum, etc. but not for "don't order this particular enum variant". + /// + /// #### Module documentation + /// + /// Module level rustdoc comments are not part of the resulting syntax tree + /// and as such cannot be linted from within `check_mod`. Instead, the + /// `rustdoc::missing_documentation` lint may be used. + /// + /// #### Module Tests + /// + /// This lint does not implement detection of module tests (or other feature + /// dependent elements for that matter). To lint the location of mod tests, + /// the lint `items_after_test_module` can be used instead. + /// + /// ### Example + /// + /// ```no_run + /// trait TraitUnordered { + /// const A: bool; + /// const C: bool; + /// const B: bool; + /// + /// type SomeType; + /// + /// fn a(); + /// fn c(); + /// fn b(); + /// } + /// ``` + /// + /// Use instead: + /// ```no_run + /// trait TraitOrdered { + /// const A: bool; + /// const B: bool; + /// const C: bool; + /// + /// type SomeType; + /// + /// fn a(); + /// fn b(); + /// fn c(); + /// } + /// ``` + /// + #[clippy::version = "1.81.0"] + pub ARBITRARY_SOURCE_ITEM_ORDERING, + restriction, + "arbitrary source item ordering" +} + +impl_lint_pass!(ArbitrarySourceItemOrdering => [ARBITRARY_SOURCE_ITEM_ORDERING]); + +#[derive(Debug)] +#[allow(clippy::struct_excessive_bools)] // Bools are cached feature flags. +pub struct ArbitrarySourceItemOrdering { + assoc_types_order: SourceItemOrderingTraitAssocItemKinds, + enable_ordering_for_enum: bool, + enable_ordering_for_impl: bool, + enable_ordering_for_module: bool, + enable_ordering_for_struct: bool, + enable_ordering_for_trait: bool, + module_item_order_groupings: SourceItemOrderingModuleItemGroupings, +} + +impl ArbitrarySourceItemOrdering { + pub fn new(conf: &'static Conf) -> Self { + #[allow(clippy::enum_glob_use)] // Very local glob use for legibility. + use SourceItemOrderingCategory::*; + Self { + assoc_types_order: conf.trait_assoc_item_kinds_order.clone(), + enable_ordering_for_enum: conf.enable_source_item_ordering_for.contains(&Enum), + enable_ordering_for_impl: conf.enable_source_item_ordering_for.contains(&Impl), + enable_ordering_for_module: conf.enable_source_item_ordering_for.contains(&Module), + enable_ordering_for_struct: conf.enable_source_item_ordering_for.contains(&Struct), + enable_ordering_for_trait: conf.enable_source_item_ordering_for.contains(&Trait), + module_item_order_groupings: conf.module_item_order_groupings.clone(), + } + } + + /// Produces a linting warning for incorrectly ordered impl items. + fn lint_impl_item(&self, cx: &T, item: &ImplItemRef, before_item: &ImplItemRef) { + span_lint_and_note( + cx, + ARBITRARY_SOURCE_ITEM_ORDERING, + item.span, + format!( + "incorrect ordering of impl items (defined order: {:?})", + self.assoc_types_order + ), + Some(before_item.span), + format!("should be placed before `{}`", before_item.ident.as_str(),), + ); + } + + /// Produces a linting warning for incorrectly ordered item members. + fn lint_member_name( + cx: &T, + ident: &rustc_span::symbol::Ident, + before_ident: &rustc_span::symbol::Ident, + ) { + span_lint_and_note( + cx, + ARBITRARY_SOURCE_ITEM_ORDERING, + ident.span, + "incorrect ordering of items (must be alphabetically ordered)", + Some(before_ident.span), + format!("should be placed before `{}`", before_ident.as_str(),), + ); + } + + /// Produces a linting warning for incorrectly ordered trait items. + fn lint_trait_item(&self, cx: &T, item: &TraitItemRef, before_item: &TraitItemRef) { + span_lint_and_note( + cx, + ARBITRARY_SOURCE_ITEM_ORDERING, + item.span, + format!( + "incorrect ordering of trait items (defined order: {:?})", + self.assoc_types_order + ), + Some(before_item.span), + format!("should be placed before `{}`", before_item.ident.as_str(),), + ); + } +} + +impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { + match &item.kind { + ItemKind::Enum(enum_def, _generics) if self.enable_ordering_for_enum => { + let mut cur_v: Option<&Variant<'_>> = None; + for variant in enum_def.variants { + if let Some(cur_v) = cur_v { + if cur_v.ident.name.as_str() > variant.ident.name.as_str() { + Self::lint_member_name(cx, &variant.ident, &cur_v.ident); + } + } + cur_v = Some(variant); + } + }, + ItemKind::Struct(VariantData::Struct { fields, .. }, _generics) if self.enable_ordering_for_struct => { + let mut cur_f: Option<&FieldDef<'_>> = None; + for field in *fields { + if let Some(cur_f) = cur_f { + if cur_f.ident.name.as_str() > field.ident.name.as_str() { + Self::lint_member_name(cx, &field.ident, &cur_f.ident); + } + } + cur_f = Some(field); + } + }, + ItemKind::Trait(is_auto, _safety, _generics, _generic_bounds, item_ref) + if self.enable_ordering_for_trait && *is_auto == IsAuto::No => + { + let mut cur_t: Option<&TraitItemRef> = None; + + for item in *item_ref { + if let Some(cur_t) = cur_t { + let cur_t_kind = convert_assoc_item_kind(cur_t.kind); + let cur_t_kind_index = self.assoc_types_order.index_of(&cur_t_kind); + let item_kind = convert_assoc_item_kind(item.kind); + let item_kind_index = self.assoc_types_order.index_of(&item_kind); + + if cur_t_kind == item_kind && cur_t.ident.name.as_str() > item.ident.name.as_str() { + Self::lint_member_name(cx, &item.ident, &cur_t.ident); + } else if cur_t_kind_index > item_kind_index { + // let type1 = item + self.lint_trait_item(cx, item, cur_t); + } + } + cur_t = Some(item); + } + }, + ItemKind::Impl(trait_impl) if self.enable_ordering_for_impl => { + let mut cur_t: Option<&ImplItemRef> = None; + + for item in trait_impl.items { + if let Some(cur_t) = cur_t { + let cur_t_kind = convert_assoc_item_kind(cur_t.kind); + let cur_t_kind_index = self.assoc_types_order.index_of(&cur_t_kind); + let item_kind = convert_assoc_item_kind(item.kind); + let item_kind_index = self.assoc_types_order.index_of(&item_kind); + + if cur_t_kind == item_kind && cur_t.ident.name.as_str() > item.ident.name.as_str() { + Self::lint_member_name(cx, &item.ident, &cur_t.ident); + } else if cur_t_kind_index > item_kind_index { + // let type1 = item + self.lint_impl_item(cx, item, cur_t); + } + } + cur_t = Some(item); + } + }, + _ => {}, // Catch-all for `ItemKinds` that don't have fields. + } + } + + fn check_mod(&mut self, cx: &LateContext<'tcx>, module: &'tcx Mod<'tcx>, _: HirId) { + struct CurItem<'a> { + item: &'a Item<'a>, + order: usize, + name: String, + } + let mut cur_t: Option> = None; + + if !self.enable_ordering_for_module { + return; + } + + let items = module.item_ids.iter().map(|&id| cx.tcx.hir().item(id)); + + // Iterates over the items within a module. + // + // As of 2023-05-09, the Rust compiler will hold the entries in the same + // order as they appear in the source code, which is convenient for us, + // as no sorting by source map/line of code has to be applied. + // + for item in items { + // The following exceptions (skipping with `continue;`) may not be + // complete, edge cases have not been explored further than what + // appears in the existing code base. + if item.ident.name == rustc_span::symbol::kw::Empty { + if let ItemKind::Impl(_) = item.kind { + // Filters attribute parameters. + // TODO: This could be of relevance to order. + continue; + } else if let ItemKind::ForeignMod { .. } = item.kind { + continue; + } else if let ItemKind::GlobalAsm(_) = item.kind { + continue; + } else if let ItemKind::Use(path, use_kind) = item.kind { + if path.segments.is_empty() { + // Use statements that contain braces get caught here. + // They will still be linted internally. + continue; + } else if path.segments.len() >= 2 + && (path.segments[0].ident.name == rustc_span::sym::std + || path.segments[0].ident.name == rustc_span::sym::core) + && path.segments[1].ident.name == rustc_span::sym::prelude + { + // Filters the autogenerated prelude use statement. + // e.g. `use std::prelude::rustc_2021` + } else if use_kind == UseKind::Glob { + // Filters glob kinds of uses. + // e.g. `use std::sync::*` + } else { + // This can be used for debugging. + // println!("Unknown autogenerated use statement: {:?}", item); + } + continue; + } + } + + if item.ident.name.as_str().starts_with('_') { + // Filters out unnamed macro-like impls for various derives, + // e.g. serde::Serialize or num_derive::FromPrimitive. + continue; + } + + if item.ident.name == rustc_span::sym::std && item.span.is_dummy() { + if let ItemKind::ExternCrate(None) = item.kind { + // Filters the auto-included Rust standard library. + continue; + } + println!("Unknown item: {item:?}"); + } + + let item_kind = convert_module_item_kind(&item.kind); + let module_level_order = self + .module_item_order_groupings + .module_level_order_of(&item_kind) + .unwrap_or_default(); + + if let Some(cur_t) = cur_t.as_ref() { + use std::cmp::Ordering; // Better legibility. + match module_level_order.cmp(&cur_t.order) { + Ordering::Less => { + Self::lint_member_name(cx, &item.ident, &cur_t.item.ident); + }, + Ordering::Equal if item_kind == SourceItemOrderingModuleItemKind::Use => { + // Skip ordering use statements, as these should be ordered by rustfmt. + }, + Ordering::Equal if cur_t.name.as_str() > item.ident.name.as_str() => { + Self::lint_member_name(cx, &item.ident, &cur_t.item.ident); + }, + Ordering::Equal | Ordering::Greater => { + // Nothing to do in this case, they're already in the right order. + }, + } + } + + // Makes a note of the current item for comparison with the next. + cur_t = Some(CurItem { + order: module_level_order, + item, + name: item.ident.name.as_str().to_owned(), + }); + } + } +} + +/// Converts a [`rustc_hir::AssocItemKind`] to a +/// [`SourceItemOrderingTraitAssocItemKind`]. +/// +/// This is implemented here because `rustc_hir` is not a dependency of +/// `clippy_config`. +fn convert_assoc_item_kind(value: AssocItemKind) -> SourceItemOrderingTraitAssocItemKind { + #[allow(clippy::enum_glob_use)] // Very local glob use for legibility. + use SourceItemOrderingTraitAssocItemKind::*; + match value { + AssocItemKind::Const { .. } => Const, + AssocItemKind::Type { .. } => Type, + AssocItemKind::Fn { .. } => Fn, + } +} + +/// Converts a [`rustc_hir::ItemKind`] to a +/// [`SourceItemOrderingModuleItemKind`]. +/// +/// This is implemented here because `rustc_hir` is not a dependency of +/// `clippy_config`. +fn convert_module_item_kind(value: &ItemKind<'_>) -> SourceItemOrderingModuleItemKind { + #[allow(clippy::enum_glob_use)] // Very local glob use for legibility. + use SourceItemOrderingModuleItemKind::*; + match value { + ItemKind::ExternCrate(..) => ExternCrate, + ItemKind::Use(..) => Use, + ItemKind::Static(..) => Static, + ItemKind::Const(..) => Const, + ItemKind::Fn(..) => Fn, + ItemKind::Macro(..) => Macro, + ItemKind::Mod(..) => Mod, + ItemKind::ForeignMod { .. } => ForeignMod, + ItemKind::GlobalAsm(..) => GlobalAsm, + ItemKind::TyAlias(..) => TyAlias, + ItemKind::OpaqueTy(..) => OpaqueTy, + ItemKind::Enum(..) => Enum, + ItemKind::Struct(..) => Struct, + ItemKind::Union(..) => Union, + ItemKind::Trait(..) => Trait, + ItemKind::TraitAlias(..) => TraitAlias, + ItemKind::Impl(..) => Impl, + } +} diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 0f9dbec8a755..f64b37052c4e 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -34,6 +34,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::absolute_paths::ABSOLUTE_PATHS_INFO, crate::almost_complete_range::ALMOST_COMPLETE_RANGE_INFO, crate::approx_const::APPROX_CONSTANT_INFO, + crate::arbitrary_source_item_ordering::ARBITRARY_SOURCE_ITEM_ORDERING_INFO, crate::arc_with_non_send_sync::ARC_WITH_NON_SEND_SYNC_INFO, crate::as_conversions::AS_CONVERSIONS_INFO, crate::asm_syntax::INLINE_ASM_X86_ATT_SYNTAX_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index b2229b53afd2..93b0a54f95d1 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -75,6 +75,7 @@ pub mod deprecated_lints; mod absolute_paths; mod almost_complete_range; mod approx_const; +mod arbitrary_source_item_ordering; mod arc_with_non_send_sync; mod as_conversions; mod asm_syntax; @@ -949,5 +950,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(non_zero_suggestions::NonZeroSuggestions)); store.register_late_pass(move |_| Box::new(unused_trait_names::UnusedTraitNames::new(conf))); store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp)); + store.register_late_pass(move |_| Box::new(arbitrary_source_item_ordering::ArbitrarySourceItemOrdering::new(conf))); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 5cf9c0fb2710..614ddda785bd 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -37,6 +37,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect disallowed-types doc-valid-idents enable-raw-pointer-heuristic-for-send + enable-source-item-ordering-for enforce-iter-loop-reborrow enforced-import-renames enum-variant-name-threshold @@ -54,6 +55,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect max-trait-bounds min-ident-chars-threshold missing-docs-in-crate-items + module-item-order-groupings msrv pass-by-value-size-limit pub-underscore-fields-behavior @@ -68,6 +70,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect too-large-for-stack too-many-arguments-threshold too-many-lines-threshold + trait-assoc-item-kinds-order trivial-copy-size-limit type-complexity-threshold unnecessary-box-size @@ -121,6 +124,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect disallowed-types doc-valid-idents enable-raw-pointer-heuristic-for-send + enable-source-item-ordering-for enforce-iter-loop-reborrow enforced-import-renames enum-variant-name-threshold @@ -138,6 +142,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect max-trait-bounds min-ident-chars-threshold missing-docs-in-crate-items + module-item-order-groupings msrv pass-by-value-size-limit pub-underscore-fields-behavior @@ -152,6 +157,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect too-large-for-stack too-many-arguments-threshold too-many-lines-threshold + trait-assoc-item-kinds-order trivial-copy-size-limit type-complexity-threshold unnecessary-box-size @@ -205,6 +211,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni disallowed-types doc-valid-idents enable-raw-pointer-heuristic-for-send + enable-source-item-ordering-for enforce-iter-loop-reborrow enforced-import-renames enum-variant-name-threshold @@ -222,6 +229,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni max-trait-bounds min-ident-chars-threshold missing-docs-in-crate-items + module-item-order-groupings msrv pass-by-value-size-limit pub-underscore-fields-behavior @@ -236,6 +244,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni too-large-for-stack too-many-arguments-threshold too-many-lines-threshold + trait-assoc-item-kinds-order trivial-copy-size-limit type-complexity-threshold unnecessary-box-size diff --git a/tests/ui/arbitrary_source_item_ordering.rs b/tests/ui/arbitrary_source_item_ordering.rs new file mode 100644 index 000000000000..65a0fbb402df --- /dev/null +++ b/tests/ui/arbitrary_source_item_ordering.rs @@ -0,0 +1,140 @@ +#![allow(dead_code)] +#![warn(clippy::arbitrary_source_item_ordering)] + +/// This module gets linted before clippy gives up. +mod i_am_just_right { + const BEFORE: i8 = 0; + + const AFTER: i8 = 0; +} + +// Use statements should not be linted internally - this is normally auto-sorted using rustfmt. +use std::rc::Rc; +use std::sync::{Arc, Barrier, RwLock}; + +const SNAKE_CASE: &str = "zzzzzzzz"; + +use std::rc::Weak; + +enum EnumOrdered { + A, + B, + C, +} + +enum EnumUnordered { + A, + C, + B, +} + +#[allow(clippy::arbitrary_source_item_ordering)] +enum EnumUnorderedAllowed { + A, + C, + B, +} + +struct StructOrdered { + a: bool, + b: bool, + c: bool, +} + +struct StructUnordered { + a: bool, + c: bool, + b: bool, + d: bool, +} + +struct StructUnorderedGeneric { + _1: std::marker::PhantomData, + a: bool, + c: bool, + b: bool, + d: bool, +} + +trait TraitOrdered { + const A: bool; + const B: bool; + const C: bool; + + type SomeType; + + fn a(); + fn b(); + fn c(); +} + +trait TraitUnordered { + const A: bool; + const C: bool; + const B: bool; + + type SomeType; + + fn a(); + fn c(); + fn b(); +} + +trait TraitUnorderedItemKinds { + type SomeType; + + const A: bool; + const B: bool; + const C: bool; + + fn a(); + fn b(); + fn c(); +} + +impl TraitUnordered for StructUnordered { + const A: bool = false; + const C: bool = false; + const B: bool = false; + + type SomeType = (); + + fn a() {} + fn c() {} + fn b() {} +} + +impl TraitUnorderedItemKinds for StructUnordered { + type SomeType = (); + + const A: bool = false; + const B: bool = false; + const C: bool = false; + + fn a() {} + fn b() {} + fn c() {} +} + +const ZIS_SHOULD_BE_REALLY_EARLY: () = (); + +fn main() { + // test code goes here +} + +/// Note that the linting pass is stopped before recursing into this module. +mod this_is_in_the_wrong_position { + const C: i8 = 0; + const A: i8 = 1; +} + +struct ZisShouldBeBeforeZeMainFn; + +const ZIS_SHOULD_BE_EVEN_EARLIER: () = (); + +#[cfg(test)] +mod test { + const B: i8 = 1; + + const A: i8 = 0; +} diff --git a/tests/ui/arbitrary_source_item_ordering.stderr b/tests/ui/arbitrary_source_item_ordering.stderr new file mode 100644 index 000000000000..95519a7adba7 --- /dev/null +++ b/tests/ui/arbitrary_source_item_ordering.stderr @@ -0,0 +1,184 @@ +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering.rs:17:14 + | +LL | use std::rc::Weak; + | ^^^^ + | +note: should be placed before `SNAKE_CASE` + --> tests/ui/arbitrary_source_item_ordering.rs:15:7 + | +LL | const SNAKE_CASE: &str = "zzzzzzzz"; + | ^^^^^^^^^^ + = note: `-D clippy::arbitrary-source-item-ordering` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::arbitrary_source_item_ordering)]` + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering.rs:119:7 + | +LL | const ZIS_SHOULD_BE_REALLY_EARLY: () = (); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: should be placed before `TraitUnorderedItemKinds` + --> tests/ui/arbitrary_source_item_ordering.rs:83:7 + | +LL | trait TraitUnorderedItemKinds { + | ^^^^^^^^^^^^^^^^^^^^^^^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering.rs:126:5 + | +LL | mod this_is_in_the_wrong_position { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: should be placed before `main` + --> tests/ui/arbitrary_source_item_ordering.rs:121:4 + | +LL | fn main() { + | ^^^^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering.rs:133:7 + | +LL | const ZIS_SHOULD_BE_EVEN_EARLIER: () = (); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: should be placed before `ZisShouldBeBeforeZeMainFn` + --> tests/ui/arbitrary_source_item_ordering.rs:131:8 + | +LL | struct ZisShouldBeBeforeZeMainFn; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering.rs:8:11 + | +LL | const AFTER: i8 = 0; + | ^^^^^ + | +note: should be placed before `BEFORE` + --> tests/ui/arbitrary_source_item_ordering.rs:6:11 + | +LL | const BEFORE: i8 = 0; + | ^^^^^^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering.rs:28:5 + | +LL | B, + | ^ + | +note: should be placed before `C` + --> tests/ui/arbitrary_source_item_ordering.rs:27:5 + | +LL | C, + | ^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering.rs:47:5 + | +LL | b: bool, + | ^ + | +note: should be placed before `c` + --> tests/ui/arbitrary_source_item_ordering.rs:46:5 + | +LL | c: bool, + | ^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering.rs:55:5 + | +LL | b: bool, + | ^ + | +note: should be placed before `c` + --> tests/ui/arbitrary_source_item_ordering.rs:54:5 + | +LL | c: bool, + | ^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering.rs:74:11 + | +LL | const B: bool; + | ^ + | +note: should be placed before `C` + --> tests/ui/arbitrary_source_item_ordering.rs:73:11 + | +LL | const C: bool; + | ^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering.rs:80:8 + | +LL | fn b(); + | ^ + | +note: should be placed before `c` + --> tests/ui/arbitrary_source_item_ordering.rs:79:8 + | +LL | fn c(); + | ^ + +error: incorrect ordering of trait items (defined order: [Const, Type, Fn]) + --> tests/ui/arbitrary_source_item_ordering.rs:86:5 + | +LL | const A: bool; + | ^^^^^^^^^^^^^^ + | +note: should be placed before `SomeType` + --> tests/ui/arbitrary_source_item_ordering.rs:84:5 + | +LL | type SomeType; + | ^^^^^^^^^^^^^^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering.rs:98:11 + | +LL | const B: bool = false; + | ^ + | +note: should be placed before `C` + --> tests/ui/arbitrary_source_item_ordering.rs:97:11 + | +LL | const C: bool = false; + | ^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering.rs:104:8 + | +LL | fn b() {} + | ^ + | +note: should be placed before `c` + --> tests/ui/arbitrary_source_item_ordering.rs:103:8 + | +LL | fn c() {} + | ^ + +error: incorrect ordering of impl items (defined order: [Const, Type, Fn]) + --> tests/ui/arbitrary_source_item_ordering.rs:110:5 + | +LL | const A: bool = false; + | ^^^^^^^^^^^^^^^^^^^^^^ + | +note: should be placed before `SomeType` + --> tests/ui/arbitrary_source_item_ordering.rs:108:5 + | +LL | type SomeType = (); + | ^^^^^^^^^^^^^^^^^^^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering.rs:128:11 + | +LL | const A: i8 = 1; + | ^ + | +note: should be placed before `C` + --> tests/ui/arbitrary_source_item_ordering.rs:127:11 + | +LL | const C: i8 = 0; + | ^ + +error: aborting due to 15 previous errors + From 5f0054080dadc2ff3d8fb76a4005677af7694ccd Mon Sep 17 00:00:00 2001 From: decryphe <12104091+decryphe@users.noreply.github.com> Date: Thu, 12 Sep 2024 18:27:06 +0200 Subject: [PATCH 2/9] fixup! new lint: `source_item_ordering` --- CHANGELOG.md | 2 +- book/src/lint_configuration.md | 20 +++++++++---------- clippy_config/src/conf.rs | 12 +++++------ clippy_config/src/types.rs | 12 +++++------ .../src/arbitrary_source_item_ordering.rs | 10 +++++----- .../toml_unknown_key/conf_unknown_key.stderr | 6 +++--- 6 files changed, 31 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1595eeef2577..f0c398f985f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6153,7 +6153,6 @@ Released 2018-09-13 [`disallowed-types`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-types [`doc-valid-idents`]: https://doc.rust-lang.org/clippy/lint_configuration.html#doc-valid-idents [`enable-raw-pointer-heuristic-for-send`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enable-raw-pointer-heuristic-for-send -[`enable-source-item-ordering-for`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enable-source-item-ordering-for [`enforce-iter-loop-reborrow`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enforce-iter-loop-reborrow [`enforced-import-renames`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enforced-import-renames [`enum-variant-name-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enum-variant-name-threshold @@ -6178,6 +6177,7 @@ Released 2018-09-13 [`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 diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index ca8951b0736b..8f6d44f29011 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -473,16 +473,6 @@ Whether to apply the raw pointer heuristic to determine if a type is `Send`. * [`non_send_fields_in_send_ty`](https://rust-lang.github.io/rust-clippy/master/index.html#non_send_fields_in_send_ty) -## `enable-source-item-ordering-for` -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) - - ## `enforce-iter-loop-reborrow` Whether to recommend using implicit into iter for reborrowed values. @@ -803,6 +793,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 diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 097c817c091f..50261dabd530 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -1,8 +1,8 @@ use crate::ClippyConfiguration; use crate::msrvs::Msrv; use crate::types::{ - DisallowedPath, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour, Rename, SourceItemOrderingCategory, - SourceItemOrderingEnableFor, SourceItemOrderingModuleItemGroupings, SourceItemOrderingModuleItemKind, + DisallowedPath, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour, Rename, SourceItemOrdering, + SourceItemOrderingCategory, SourceItemOrderingModuleItemGroupings, SourceItemOrderingModuleItemKind, SourceItemOrderingTraitAssocItemKind, SourceItemOrderingTraitAssocItemKinds, }; use rustc_errors::Applicability; @@ -71,7 +71,7 @@ const DEFAULT_TRAIT_ASSOC_ITEM_KINDS_ORDER: &[SourceItemOrderingTraitAssocItemKi use SourceItemOrderingTraitAssocItemKind::*; &[Const, Type, Fn] }; -const DEFAULT_ENABLE_SOURCE_ITEM_ORDERING_FOR: &[SourceItemOrderingCategory] = { +const DEFAULT_SOURCE_ITEM_ORDERING: &[SourceItemOrderingCategory] = { #[allow(clippy::enum_glob_use)] // Very local glob use for legibility. use SourceItemOrderingCategory::*; &[Enum, Impl, Module, Struct, Trait] @@ -489,9 +489,6 @@ define_Conf! { /// Whether to apply the raw pointer heuristic to determine if a type is `Send`. #[lints(non_send_fields_in_send_ty)] enable_raw_pointer_heuristic_for_send: bool = true, - /// Which kind of elements should be ordered internally, possible values being `enum`, `impl`, `module`, `struct`, `trait`. - #[lints(arbitrary_source_item_ordering)] - enable_source_item_ordering_for: SourceItemOrderingEnableFor = DEFAULT_ENABLE_SOURCE_ITEM_ORDERING_FOR.into(), /// Whether to recommend using implicit into iter for reborrowed values. /// /// #### Example @@ -644,6 +641,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, diff --git a/clippy_config/src/types.rs b/clippy_config/src/types.rs index f05b0a390d32..950296337fd2 100644 --- a/clippy_config/src/types.rs +++ b/clippy_config/src/types.rs @@ -118,15 +118,15 @@ pub enum SourceItemOrderingCategory { /// /// The [`Deserialize`] implementation checks that there are no duplicates in /// the user configuration. -pub struct SourceItemOrderingEnableFor(Vec); +pub struct SourceItemOrdering(Vec); -impl SourceItemOrderingEnableFor { +impl SourceItemOrdering { pub fn contains(&self, category: &SourceItemOrderingCategory) -> bool { self.0.contains(category) } } -impl From for SourceItemOrderingEnableFor +impl From for SourceItemOrdering where T: Into>, { @@ -135,13 +135,13 @@ where } } -impl core::fmt::Debug for SourceItemOrderingEnableFor { +impl core::fmt::Debug for SourceItemOrdering { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { self.0.fmt(f) } } -impl<'de> Deserialize<'de> for SourceItemOrderingEnableFor { +impl<'de> Deserialize<'de> for SourceItemOrdering { fn deserialize(deserializer: D) -> Result where D: Deserializer<'de>, @@ -162,7 +162,7 @@ impl<'de> Deserialize<'de> for SourceItemOrderingEnableFor { } } -impl Serialize for SourceItemOrderingEnableFor { +impl Serialize for SourceItemOrdering { fn serialize(&self, serializer: S) -> Result where S: ser::Serializer, diff --git a/clippy_lints/src/arbitrary_source_item_ordering.rs b/clippy_lints/src/arbitrary_source_item_ordering.rs index 34da4759bef1..e4fc5f2eee2b 100644 --- a/clippy_lints/src/arbitrary_source_item_ordering.rs +++ b/clippy_lints/src/arbitrary_source_item_ordering.rs @@ -145,11 +145,11 @@ impl ArbitrarySourceItemOrdering { use SourceItemOrderingCategory::*; Self { assoc_types_order: conf.trait_assoc_item_kinds_order.clone(), - enable_ordering_for_enum: conf.enable_source_item_ordering_for.contains(&Enum), - enable_ordering_for_impl: conf.enable_source_item_ordering_for.contains(&Impl), - enable_ordering_for_module: conf.enable_source_item_ordering_for.contains(&Module), - enable_ordering_for_struct: conf.enable_source_item_ordering_for.contains(&Struct), - enable_ordering_for_trait: conf.enable_source_item_ordering_for.contains(&Trait), + enable_ordering_for_enum: conf.source_item_ordering.contains(&Enum), + enable_ordering_for_impl: conf.source_item_ordering.contains(&Impl), + enable_ordering_for_module: conf.source_item_ordering.contains(&Module), + enable_ordering_for_struct: conf.source_item_ordering.contains(&Struct), + enable_ordering_for_trait: conf.source_item_ordering.contains(&Trait), module_item_order_groupings: conf.module_item_order_groupings.clone(), } } diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 614ddda785bd..6fa583fc0417 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -37,7 +37,6 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect disallowed-types doc-valid-idents enable-raw-pointer-heuristic-for-send - enable-source-item-ordering-for enforce-iter-loop-reborrow enforced-import-renames enum-variant-name-threshold @@ -62,6 +61,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect semicolon-inside-block-ignore-singleline semicolon-outside-block-ignore-multiline single-char-binding-names-threshold + source-item-ordering stack-size-threshold standard-macro-braces struct-field-name-threshold @@ -124,7 +124,6 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect disallowed-types doc-valid-idents enable-raw-pointer-heuristic-for-send - enable-source-item-ordering-for enforce-iter-loop-reborrow enforced-import-renames enum-variant-name-threshold @@ -149,6 +148,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect semicolon-inside-block-ignore-singleline semicolon-outside-block-ignore-multiline single-char-binding-names-threshold + source-item-ordering stack-size-threshold standard-macro-braces struct-field-name-threshold @@ -211,7 +211,6 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni disallowed-types doc-valid-idents enable-raw-pointer-heuristic-for-send - enable-source-item-ordering-for enforce-iter-loop-reborrow enforced-import-renames enum-variant-name-threshold @@ -236,6 +235,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni semicolon-inside-block-ignore-singleline semicolon-outside-block-ignore-multiline single-char-binding-names-threshold + source-item-ordering stack-size-threshold standard-macro-braces struct-field-name-threshold From d3535c4b153d8b0dacd91dc5badd3308f5535808 Mon Sep 17 00:00:00 2001 From: decryphe <12104091+decryphe@users.noreply.github.com> Date: Fri, 27 Sep 2024 16:29:34 +0200 Subject: [PATCH 3/9] fixup! new lint: `source_item_ordering` --- clippy_config/src/conf.rs | 7 +++---- clippy_lints/src/arbitrary_source_item_ordering.rs | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 50261dabd530..108f11ccc4c9 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -59,10 +59,9 @@ const DEFAULT_MODULE_ITEM_ORDERING_GROUPS: &[(&str, &[SourceItemOrderingModuleIt ("macros", &[Macro]), ("global_asm", &[GlobalAsm]), ("UPPER_SNAKE_CASE", &[Static, Const]), - ( - "PascalCase", - &[TyAlias, OpaqueTy, Enum, Struct, Union, Trait, TraitAlias, Impl], - ), + ("PascalCase", &[ + TyAlias, OpaqueTy, Enum, Struct, Union, Trait, TraitAlias, Impl, + ]), ("lower_snake_case", &[Fn]), ] }; diff --git a/clippy_lints/src/arbitrary_source_item_ordering.rs b/clippy_lints/src/arbitrary_source_item_ordering.rs index e4fc5f2eee2b..4afa4a1a3d8a 100644 --- a/clippy_lints/src/arbitrary_source_item_ordering.rs +++ b/clippy_lints/src/arbitrary_source_item_ordering.rs @@ -1,8 +1,8 @@ +use clippy_config::Conf; use clippy_config::types::{ SourceItemOrderingCategory, SourceItemOrderingModuleItemGroupings, SourceItemOrderingModuleItemKind, SourceItemOrderingTraitAssocItemKind, SourceItemOrderingTraitAssocItemKinds, }; -use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_note; use rustc_hir::{ AssocItemKind, FieldDef, HirId, ImplItemRef, IsAuto, Item, ItemKind, Mod, TraitItemRef, UseKind, Variant, @@ -24,7 +24,7 @@ declare_clippy_lint! { /// 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". + /// "bikeshedding". /// /// Keep in mind, that ordering source code alphabetically can lead to /// reduced performance in cases where the most commonly used enum variant From 69bb5b71125838e0c98dddbc98c51fc11b3cc5f1 Mon Sep 17 00:00:00 2001 From: decryphe <12104091+decryphe@users.noreply.github.com> Date: Fri, 27 Sep 2024 16:42:22 +0200 Subject: [PATCH 4/9] fixup! new lint: `source_item_ordering` --- .../src/arbitrary_source_item_ordering.rs | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/arbitrary_source_item_ordering.rs b/clippy_lints/src/arbitrary_source_item_ordering.rs index 4afa4a1a3d8a..a08e196f017b 100644 --- a/clippy_lints/src/arbitrary_source_item_ordering.rs +++ b/clippy_lints/src/arbitrary_source_item_ordering.rs @@ -26,14 +26,6 @@ declare_clippy_lint! { /// implemented in the code. Sometimes this will be referred to as /// "bikeshedding". /// - /// 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. - /// /// ### Default Ordering and Configuration /// /// As there is no generally applicable rule, and each project may have @@ -68,7 +60,19 @@ declare_clippy_lint! { /// All item kinds must be accounted for to create an enforceable linting /// rule set. /// - /// ### Known Issues and Limitations + /// ### Known Problems + /// + /// #### Performance Impact + /// + /// 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), post-link optimization (e.g. using BOLT for LLVM), + /// or other advanced optimization methods. A good starting point to dig + /// into optimization is [cargo-pgo]. /// /// #### Lints on a Contains basis /// @@ -119,6 +123,8 @@ declare_clippy_lint! { /// } /// ``` /// + /// [cargo-pgo]: https://github.com/Kobzol/cargo-pgo/blob/main/README.md + /// #[clippy::version = "1.81.0"] pub ARBITRARY_SOURCE_ITEM_ORDERING, restriction, From e7a7bf53bc732cf45c3fa3a94e2a1176adf78d08 Mon Sep 17 00:00:00 2001 From: decryphe <12104091+decryphe@users.noreply.github.com> Date: Tue, 1 Oct 2024 10:38:52 +0200 Subject: [PATCH 5/9] fixup! new lint: `source_item_ordering` --- .../src/arbitrary_source_item_ordering.rs | 103 ++++++- .../ui/arbitrary_source_item_ordering.stderr | 184 ------------- .../ordering_good.rs | 178 ++++++++++++ .../ordering_mixed.rs} | 45 +++- .../ordering_mixed.stderr | 253 ++++++++++++++++++ 5 files changed, 565 insertions(+), 198 deletions(-) delete mode 100644 tests/ui/arbitrary_source_item_ordering.stderr create mode 100644 tests/ui/arbitrary_source_item_ordering/ordering_good.rs rename tests/ui/{arbitrary_source_item_ordering.rs => arbitrary_source_item_ordering/ordering_mixed.rs} (73%) create mode 100644 tests/ui/arbitrary_source_item_ordering/ordering_mixed.stderr diff --git a/clippy_lints/src/arbitrary_source_item_ordering.rs b/clippy_lints/src/arbitrary_source_item_ordering.rs index a08e196f017b..b7a916bc2afd 100644 --- a/clippy_lints/src/arbitrary_source_item_ordering.rs +++ b/clippy_lints/src/arbitrary_source_item_ordering.rs @@ -5,8 +5,8 @@ use clippy_config::types::{ }; use clippy_utils::diagnostics::span_lint_and_note; use rustc_hir::{ - AssocItemKind, FieldDef, HirId, ImplItemRef, IsAuto, Item, ItemKind, Mod, TraitItemRef, UseKind, Variant, - VariantData, + AssocItemKind, FieldDef, HirId, ImplItemRef, IsAuto, Item, ItemKind, Mod, QPath, TraitItemRef, TyKind, UseKind, + Variant, VariantData, }; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::impl_lint_pass; @@ -72,7 +72,7 @@ declare_clippy_lint! { /// where it matters. Other solutions can be to use profile guided /// optimization (PGO), post-link optimization (e.g. using BOLT for LLVM), /// or other advanced optimization methods. A good starting point to dig - /// into optimization is [cargo-pgo]. + /// into optimization is [cargo-pgo][cargo-pgo]. /// /// #### Lints on a Contains basis /// @@ -191,6 +191,35 @@ impl ArbitrarySourceItemOrdering { ); } + fn lint_member_item(cx: &T, item: &Item<'_>, before_item: &Item<'_>) { + let span = if item.ident.as_str().is_empty() { + &item.span + } else { + &item.ident.span + }; + + let (before_span, note) = if before_item.ident.as_str().is_empty() { + ( + &before_item.span, + "should be placed before the following item".to_owned(), + ) + } else { + ( + &before_item.ident.span, + format!("should be placed before `{}`", before_item.ident.as_str(),), + ) + }; + + span_lint_and_note( + cx, + ARBITRARY_SOURCE_ITEM_ORDERING, + *span, + "incorrect ordering of items (must be alphabetically ordered)", + Some(*before_span), + note, + ); + } + /// Produces a linting warning for incorrectly ordered trait items. fn lint_trait_item(&self, cx: &T, item: &TraitItemRef, before_item: &TraitItemRef) { span_lint_and_note( @@ -247,7 +276,6 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering { if cur_t_kind == item_kind && cur_t.ident.name.as_str() > item.ident.name.as_str() { Self::lint_member_name(cx, &item.ident, &cur_t.ident); } else if cur_t_kind_index > item_kind_index { - // let type1 = item self.lint_trait_item(cx, item, cur_t); } } @@ -267,7 +295,6 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering { if cur_t_kind == item_kind && cur_t.ident.name.as_str() > item.ident.name.as_str() { Self::lint_member_name(cx, &item.ident, &cur_t.ident); } else if cur_t_kind_index > item_kind_index { - // let type1 = item self.lint_impl_item(cx, item, cur_t); } } @@ -304,9 +331,10 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering { // appears in the existing code base. if item.ident.name == rustc_span::symbol::kw::Empty { if let ItemKind::Impl(_) = item.kind { - // Filters attribute parameters. - // TODO: This could be of relevance to order. - continue; + // Sorting trait impls for unnamed types makes no sense. + if get_item_name(item).is_empty() { + continue; + } } else if let ItemKind::ForeignMod { .. } = item.kind { continue; } else if let ItemKind::GlobalAsm(_) = item.kind { @@ -358,13 +386,13 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering { use std::cmp::Ordering; // Better legibility. match module_level_order.cmp(&cur_t.order) { Ordering::Less => { - Self::lint_member_name(cx, &item.ident, &cur_t.item.ident); + Self::lint_member_item(cx, item, cur_t.item); }, Ordering::Equal if item_kind == SourceItemOrderingModuleItemKind::Use => { // Skip ordering use statements, as these should be ordered by rustfmt. }, - Ordering::Equal if cur_t.name.as_str() > item.ident.name.as_str() => { - Self::lint_member_name(cx, &item.ident, &cur_t.item.ident); + Ordering::Equal if cur_t.name > get_item_name(item) => { + Self::lint_member_item(cx, item, cur_t.item); }, Ordering::Equal | Ordering::Greater => { // Nothing to do in this case, they're already in the right order. @@ -376,7 +404,7 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering { cur_t = Some(CurItem { order: module_level_order, item, - name: item.ident.name.as_str().to_owned(), + name: get_item_name(item), }); } } @@ -425,3 +453,54 @@ fn convert_module_item_kind(value: &ItemKind<'_>) -> SourceItemOrderingModuleIte ItemKind::Impl(..) => Impl, } } + +/// Gets the item name for sorting purposes, which in the general case is +/// `item.ident.name`. +/// +/// For trait impls, the name used for sorting will be the written path of +/// `item.self_ty` plus the written path of `item.of_trait`, joined with +/// exclamation marks. Exclamation marks are used because they are the first +/// printable ASCII character. +/// +/// Trait impls generated using a derive-macro will have their path rewritten, +/// such that for example `Default` is `$crate::default::Default`, and +/// `std::clone::Clone` is `$crate::clone::Clone`. This behaviour is described +/// further in the [Rust Reference, Paths Chapter][rust_ref]. +/// +/// [rust_ref]: https://doc.rust-lang.org/reference/paths.html#crate-1 +fn get_item_name(item: &Item<'_>) -> String { + match item.kind { + ItemKind::Impl(im) => { + if let TyKind::Path(path) = im.self_ty.kind { + match path { + QPath::Resolved(_, path) => { + let segs = path.segments.iter(); + let mut segs: Vec = segs.map(|s| s.ident.name.as_str().to_owned()).collect(); + + if let Some(of_trait) = im.of_trait { + let mut trait_segs: Vec = of_trait + .path + .segments + .iter() + .map(|s| s.ident.name.as_str().to_owned()) + .collect(); + segs.append(&mut trait_segs); + } + + segs.push(String::new()); + segs.join("!!") + }, + QPath::TypeRelative(_, _path_seg) => { + // This case doesn't exist in the clippy tests codebase. + String::new() + }, + QPath::LangItem(_, _) => String::new(), + } + } else { + // Impls for anything that isn't a named type can be skipped. + String::new() + } + }, + _ => item.ident.name.as_str().to_owned(), + } +} diff --git a/tests/ui/arbitrary_source_item_ordering.stderr b/tests/ui/arbitrary_source_item_ordering.stderr deleted file mode 100644 index 95519a7adba7..000000000000 --- a/tests/ui/arbitrary_source_item_ordering.stderr +++ /dev/null @@ -1,184 +0,0 @@ -error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering.rs:17:14 - | -LL | use std::rc::Weak; - | ^^^^ - | -note: should be placed before `SNAKE_CASE` - --> tests/ui/arbitrary_source_item_ordering.rs:15:7 - | -LL | const SNAKE_CASE: &str = "zzzzzzzz"; - | ^^^^^^^^^^ - = note: `-D clippy::arbitrary-source-item-ordering` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::arbitrary_source_item_ordering)]` - -error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering.rs:119:7 - | -LL | const ZIS_SHOULD_BE_REALLY_EARLY: () = (); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -note: should be placed before `TraitUnorderedItemKinds` - --> tests/ui/arbitrary_source_item_ordering.rs:83:7 - | -LL | trait TraitUnorderedItemKinds { - | ^^^^^^^^^^^^^^^^^^^^^^^ - -error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering.rs:126:5 - | -LL | mod this_is_in_the_wrong_position { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -note: should be placed before `main` - --> tests/ui/arbitrary_source_item_ordering.rs:121:4 - | -LL | fn main() { - | ^^^^ - -error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering.rs:133:7 - | -LL | const ZIS_SHOULD_BE_EVEN_EARLIER: () = (); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -note: should be placed before `ZisShouldBeBeforeZeMainFn` - --> tests/ui/arbitrary_source_item_ordering.rs:131:8 - | -LL | struct ZisShouldBeBeforeZeMainFn; - | ^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering.rs:8:11 - | -LL | const AFTER: i8 = 0; - | ^^^^^ - | -note: should be placed before `BEFORE` - --> tests/ui/arbitrary_source_item_ordering.rs:6:11 - | -LL | const BEFORE: i8 = 0; - | ^^^^^^ - -error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering.rs:28:5 - | -LL | B, - | ^ - | -note: should be placed before `C` - --> tests/ui/arbitrary_source_item_ordering.rs:27:5 - | -LL | C, - | ^ - -error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering.rs:47:5 - | -LL | b: bool, - | ^ - | -note: should be placed before `c` - --> tests/ui/arbitrary_source_item_ordering.rs:46:5 - | -LL | c: bool, - | ^ - -error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering.rs:55:5 - | -LL | b: bool, - | ^ - | -note: should be placed before `c` - --> tests/ui/arbitrary_source_item_ordering.rs:54:5 - | -LL | c: bool, - | ^ - -error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering.rs:74:11 - | -LL | const B: bool; - | ^ - | -note: should be placed before `C` - --> tests/ui/arbitrary_source_item_ordering.rs:73:11 - | -LL | const C: bool; - | ^ - -error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering.rs:80:8 - | -LL | fn b(); - | ^ - | -note: should be placed before `c` - --> tests/ui/arbitrary_source_item_ordering.rs:79:8 - | -LL | fn c(); - | ^ - -error: incorrect ordering of trait items (defined order: [Const, Type, Fn]) - --> tests/ui/arbitrary_source_item_ordering.rs:86:5 - | -LL | const A: bool; - | ^^^^^^^^^^^^^^ - | -note: should be placed before `SomeType` - --> tests/ui/arbitrary_source_item_ordering.rs:84:5 - | -LL | type SomeType; - | ^^^^^^^^^^^^^^ - -error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering.rs:98:11 - | -LL | const B: bool = false; - | ^ - | -note: should be placed before `C` - --> tests/ui/arbitrary_source_item_ordering.rs:97:11 - | -LL | const C: bool = false; - | ^ - -error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering.rs:104:8 - | -LL | fn b() {} - | ^ - | -note: should be placed before `c` - --> tests/ui/arbitrary_source_item_ordering.rs:103:8 - | -LL | fn c() {} - | ^ - -error: incorrect ordering of impl items (defined order: [Const, Type, Fn]) - --> tests/ui/arbitrary_source_item_ordering.rs:110:5 - | -LL | const A: bool = false; - | ^^^^^^^^^^^^^^^^^^^^^^ - | -note: should be placed before `SomeType` - --> tests/ui/arbitrary_source_item_ordering.rs:108:5 - | -LL | type SomeType = (); - | ^^^^^^^^^^^^^^^^^^^ - -error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering.rs:128:11 - | -LL | const A: i8 = 1; - | ^ - | -note: should be placed before `C` - --> tests/ui/arbitrary_source_item_ordering.rs:127:11 - | -LL | const C: i8 = 0; - | ^ - -error: aborting due to 15 previous errors - diff --git a/tests/ui/arbitrary_source_item_ordering/ordering_good.rs b/tests/ui/arbitrary_source_item_ordering/ordering_good.rs new file mode 100644 index 000000000000..e11d1c2a38b8 --- /dev/null +++ b/tests/ui/arbitrary_source_item_ordering/ordering_good.rs @@ -0,0 +1,178 @@ +#![allow(dead_code)] +#![warn(clippy::arbitrary_source_item_ordering)] + +/// This module gets linted before clippy gives up. +mod i_am_just_right { + const AFTER: i8 = 0; + + const BEFORE: i8 = 0; +} + +/// Since the upper module passes linting, the lint now recurses into this module. +mod this_is_in_the_wrong_position { + const A: i8 = 1; + const C: i8 = 0; +} + +// Use statements should not be linted internally - this is normally auto-sorted using rustfmt. +use std::rc::{Rc, Weak}; +use std::sync::{Arc, Barrier, RwLock}; + +const SNAKE_CASE: &str = "zzzzzzzz"; + +const ZIS_SHOULD_BE_EVEN_EARLIER: () = (); + +const ZIS_SHOULD_BE_REALLY_EARLY: () = (); + +trait BasicEmptyTrait {} + +trait CloneSelf { + fn clone_self(&self) -> Self; +} + +enum EnumOrdered { + A, + B, + C, +} + +enum EnumUnordered { + A, + B, + C, +} + +#[allow(clippy::arbitrary_source_item_ordering)] +enum EnumUnorderedAllowed { + A, + B, + C, +} + +struct StructOrdered { + a: bool, + b: bool, + c: bool, +} + +impl BasicEmptyTrait for StructOrdered {} + +impl CloneSelf for StructOrdered { + fn clone_self(&self) -> Self { + Self { + a: true, + b: true, + c: true, + } + } +} + +impl Default for StructOrdered { + fn default() -> Self { + Self { + a: true, + b: true, + c: true, + } + } +} + +impl std::clone::Clone for StructOrdered { + fn clone(&self) -> Self { + Self { + a: true, + b: true, + c: true, + } + } +} + +#[derive(Clone, Default)] +struct StructUnordered { + a: bool, + b: bool, + c: bool, + d: bool, +} + +impl TraitUnordered for StructUnordered { + const A: bool = false; + const B: bool = false; + const C: bool = false; + + type SomeType = (); + + fn a() {} + fn b() {} + fn c() {} +} + +impl TraitUnorderedItemKinds for StructUnordered { + const A: bool = false; + const B: bool = false; + const C: bool = false; + + type SomeType = (); + + fn a() {} + fn b() {} + fn c() {} +} + +struct StructUnorderedGeneric { + _1: std::marker::PhantomData, + a: bool, + b: bool, + c: bool, + d: bool, +} + +trait TraitOrdered { + const A: bool; + const B: bool; + const C: bool; + + type SomeType; + + fn a(); + fn b(); + fn c(); +} + +trait TraitUnordered { + const A: bool; + const B: bool; + const C: bool; + + type SomeType; + + fn a(); + fn b(); + fn c(); +} + +trait TraitUnorderedItemKinds { + const A: bool; + const B: bool; + const C: bool; + + type SomeType; + + fn a(); + fn b(); + fn c(); +} + +#[derive(std::clone::Clone, Default)] +struct ZisShouldBeBeforeZeMainFn; + +fn main() { + // test code goes here +} + +#[cfg(test)] +mod test { + const B: i8 = 1; + + const A: i8 = 0; +} diff --git a/tests/ui/arbitrary_source_item_ordering.rs b/tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs similarity index 73% rename from tests/ui/arbitrary_source_item_ordering.rs rename to tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs index 65a0fbb402df..953f065d0fe0 100644 --- a/tests/ui/arbitrary_source_item_ordering.rs +++ b/tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs @@ -16,6 +16,12 @@ const SNAKE_CASE: &str = "zzzzzzzz"; use std::rc::Weak; +trait BasicEmptyTrait {} + +trait CloneSelf { + fn clone_self(&self) -> Self; +} + enum EnumOrdered { A, B, @@ -41,6 +47,37 @@ struct StructOrdered { c: bool, } +impl Default for StructOrdered { + fn default() -> Self { + Self { + a: true, + b: true, + c: true, + } + } +} + +impl CloneSelf for StructOrdered { + fn clone_self(&self) -> Self { + Self { + a: true, + b: true, + c: true, + } + } +} + +impl std::clone::Clone for StructOrdered { + fn clone(&self) -> Self { + Self { + a: true, + b: true, + c: true, + } + } +} + +#[derive(Default, Clone)] struct StructUnordered { a: bool, c: bool, @@ -92,6 +129,8 @@ trait TraitUnorderedItemKinds { fn c(); } +const ZIS_SHOULD_BE_REALLY_EARLY: () = (); + impl TraitUnordered for StructUnordered { const A: bool = false; const C: bool = false; @@ -104,6 +143,9 @@ impl TraitUnordered for StructUnordered { fn b() {} } +// Trait impls should be located just after the type they implement it for. +impl BasicEmptyTrait for StructOrdered {} + impl TraitUnorderedItemKinds for StructUnordered { type SomeType = (); @@ -116,8 +158,6 @@ impl TraitUnorderedItemKinds for StructUnordered { fn c() {} } -const ZIS_SHOULD_BE_REALLY_EARLY: () = (); - fn main() { // test code goes here } @@ -128,6 +168,7 @@ mod this_is_in_the_wrong_position { const A: i8 = 1; } +#[derive(Default, std::clone::Clone)] struct ZisShouldBeBeforeZeMainFn; const ZIS_SHOULD_BE_EVEN_EARLIER: () = (); diff --git a/tests/ui/arbitrary_source_item_ordering/ordering_mixed.stderr b/tests/ui/arbitrary_source_item_ordering/ordering_mixed.stderr new file mode 100644 index 000000000000..23c1c1e3829b --- /dev/null +++ b/tests/ui/arbitrary_source_item_ordering/ordering_mixed.stderr @@ -0,0 +1,253 @@ +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:17:14 + | +LL | use std::rc::Weak; + | ^^^^ + | +note: should be placed before `SNAKE_CASE` + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:15:7 + | +LL | const SNAKE_CASE: &str = "zzzzzzzz"; + | ^^^^^^^^^^ + = note: `-D clippy::arbitrary-source-item-ordering` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::arbitrary_source_item_ordering)]` + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:60:1 + | +LL | / impl CloneSelf for StructOrdered { +LL | | fn clone_self(&self) -> Self { +LL | | Self { +LL | | a: true, +... | +LL | | } +LL | | } + | |_^ + | +note: should be placed before the following item + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:50:1 + | +LL | / impl Default for StructOrdered { +LL | | fn default() -> Self { +LL | | Self { +LL | | a: true, +... | +LL | | } +LL | | } + | |_^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:80:19 + | +LL | #[derive(Default, Clone)] + | ^^^^^ + | +note: should be placed before the following item + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:80:10 + | +LL | #[derive(Default, Clone)] + | ^^^^^^^ + = note: this error originates in the derive macro `Clone` which comes from the expansion of the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:132:7 + | +LL | const ZIS_SHOULD_BE_REALLY_EARLY: () = (); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: should be placed before `TraitUnorderedItemKinds` + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:120:7 + | +LL | trait TraitUnorderedItemKinds { + | ^^^^^^^^^^^^^^^^^^^^^^^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:147:1 + | +LL | impl BasicEmptyTrait for StructOrdered {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: should be placed before the following item + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:134:1 + | +LL | / impl TraitUnordered for StructUnordered { +LL | | const A: bool = false; +LL | | const C: bool = false; +LL | | const B: bool = false; +... | +LL | | fn b() {} +LL | | } + | |_^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:166:5 + | +LL | mod this_is_in_the_wrong_position { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: should be placed before `main` + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:161:4 + | +LL | fn main() { + | ^^^^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:171:19 + | +LL | #[derive(Default, std::clone::Clone)] + | ^^^^^^^^^^^^^^^^^ + | +note: should be placed before the following item + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:171:10 + | +LL | #[derive(Default, std::clone::Clone)] + | ^^^^^^^ + = note: this error originates in the derive macro `std::clone::Clone` which comes from the expansion of the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:174:7 + | +LL | const ZIS_SHOULD_BE_EVEN_EARLIER: () = (); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: should be placed before the following item + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:171:19 + | +LL | #[derive(Default, std::clone::Clone)] + | ^^^^^^^^^^^^^^^^^ + = note: this error originates in the derive macro `std::clone::Clone` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:8:11 + | +LL | const AFTER: i8 = 0; + | ^^^^^ + | +note: should be placed before `BEFORE` + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:6:11 + | +LL | const BEFORE: i8 = 0; + | ^^^^^^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:34:5 + | +LL | B, + | ^ + | +note: should be placed before `C` + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:33:5 + | +LL | C, + | ^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:84:5 + | +LL | b: bool, + | ^ + | +note: should be placed before `c` + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:83:5 + | +LL | c: bool, + | ^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:92:5 + | +LL | b: bool, + | ^ + | +note: should be placed before `c` + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:91:5 + | +LL | c: bool, + | ^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:111:11 + | +LL | const B: bool; + | ^ + | +note: should be placed before `C` + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:110:11 + | +LL | const C: bool; + | ^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:117:8 + | +LL | fn b(); + | ^ + | +note: should be placed before `c` + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:116:8 + | +LL | fn c(); + | ^ + +error: incorrect ordering of trait items (defined order: [Const, Type, Fn]) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:123:5 + | +LL | const A: bool; + | ^^^^^^^^^^^^^^ + | +note: should be placed before `SomeType` + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:121:5 + | +LL | type SomeType; + | ^^^^^^^^^^^^^^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:137:11 + | +LL | const B: bool = false; + | ^ + | +note: should be placed before `C` + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:136:11 + | +LL | const C: bool = false; + | ^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:143:8 + | +LL | fn b() {} + | ^ + | +note: should be placed before `c` + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:142:8 + | +LL | fn c() {} + | ^ + +error: incorrect ordering of impl items (defined order: [Const, Type, Fn]) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:152:5 + | +LL | const A: bool = false; + | ^^^^^^^^^^^^^^^^^^^^^^ + | +note: should be placed before `SomeType` + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:150:5 + | +LL | type SomeType = (); + | ^^^^^^^^^^^^^^^^^^^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:168:11 + | +LL | const A: i8 = 1; + | ^ + | +note: should be placed before `C` + --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:167:11 + | +LL | const C: i8 = 0; + | ^ + +error: aborting due to 19 previous errors + From 7ac8f2cdf15ac3d50652c1bd8a8baccfa94e1e91 Mon Sep 17 00:00:00 2001 From: decryphe <12104091+decryphe@users.noreply.github.com> Date: Fri, 11 Oct 2024 18:24:38 +0200 Subject: [PATCH 6/9] fixup! new lint: `source_item_ordering` --- clippy_config/src/types.rs | 18 +- .../bad_conf_1/clippy.toml | 1 + .../bad_conf_2/clippy.toml | 1 + .../default/clippy.toml | 0 .../ordering_good.bad_conf_1.stderr | 8 + .../ordering_good.bad_conf_2.stderr | 8 + .../ordering_good.rs | 6 + .../ordering_good_var_1.rs | 174 ++++++++++++++++++ .../ordering_mixed.default.stderr} | 76 ++++---- .../ordering_mixed.rs | 4 + .../ordering_mixed_var_1.rs | 174 ++++++++++++++++++ .../ordering_mixed_var_1.var_1.stderr | 100 ++++++++++ .../var_1/clippy.toml | 1 + 13 files changed, 525 insertions(+), 46 deletions(-) create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/bad_conf_1/clippy.toml create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/bad_conf_2/clippy.toml create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/default/clippy.toml create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/ordering_good.bad_conf_1.stderr create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/ordering_good.bad_conf_2.stderr rename tests/{ui => ui-toml}/arbitrary_source_item_ordering/ordering_good.rs (87%) create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/ordering_good_var_1.rs rename tests/{ui/arbitrary_source_item_ordering/ordering_mixed.stderr => ui-toml/arbitrary_source_item_ordering/ordering_mixed.default.stderr} (65%) rename tests/{ui => ui-toml}/arbitrary_source_item_ordering/ordering_mixed.rs (94%) create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.var_1.stderr create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/var_1/clippy.toml diff --git a/clippy_config/src/types.rs b/clippy_config/src/types.rs index 950296337fd2..81c503f68bad 100644 --- a/clippy_config/src/types.rs +++ b/clippy_config/src/types.rs @@ -295,13 +295,14 @@ impl<'de> Deserialize<'de> for SourceItemOrderingModuleItemGroupings { Ok(Self { groups, lut }) } else if items_total != all_items.len() { - Err(de::Error::custom( - "Some module item kinds were configured more than once in the source ordering configuration.", - )) + Err(de::Error::custom(format!( + "Some module item kinds were configured more than once, or were missing, in the source ordering configuration. \ + The module item kinds are: {all_items:?}" + ))) } else { Err(de::Error::custom(format!( "Not all module item kinds were part of the configured source ordering rule. \ - All item kinds must be provided in the config, otherwise the required source ordering would remain ambiguous.\ + All item kinds must be provided in the config, otherwise the required source ordering would remain ambiguous. \ The module item kinds are: {all_items:?}" ))) } @@ -381,13 +382,14 @@ impl<'de> Deserialize<'de> for SourceItemOrderingTraitAssocItemKinds { if expected_items.is_empty() && items.len() == all_items.len() { Ok(Self(items)) } else if items.len() != all_items.len() { - Err(de::Error::custom( - "Some associated item kinds were configured more than once in the source ordering configuration.", - )) + Err(de::Error::custom(format!( + "Some trait associated item kinds were configured more than once, or were missing, in the source ordering configuration. \ + The trait associated item kinds are: {all_items:?}", + ))) } else { Err(de::Error::custom(format!( "Not all trait associated item kinds were part of the configured source ordering rule. \ - All item kinds must be provided in the config, otherwise the required source ordering would remain ambiguous.\ + All item kinds must be provided in the config, otherwise the required source ordering would remain ambiguous. \ The trait associated item kinds are: {all_items:?}" ))) } diff --git a/tests/ui-toml/arbitrary_source_item_ordering/bad_conf_1/clippy.toml b/tests/ui-toml/arbitrary_source_item_ordering/bad_conf_1/clippy.toml new file mode 100644 index 000000000000..fbdb0c9c37d0 --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/bad_conf_1/clippy.toml @@ -0,0 +1 @@ +trait-assoc-item-kinds-order = ["fn", "type", "const", "type"] diff --git a/tests/ui-toml/arbitrary_source_item_ordering/bad_conf_2/clippy.toml b/tests/ui-toml/arbitrary_source_item_ordering/bad_conf_2/clippy.toml new file mode 100644 index 000000000000..720655e5cce1 --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/bad_conf_2/clippy.toml @@ -0,0 +1 @@ +trait-assoc-item-kinds-order = ["const", "type"] diff --git a/tests/ui-toml/arbitrary_source_item_ordering/default/clippy.toml b/tests/ui-toml/arbitrary_source_item_ordering/default/clippy.toml new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/ui-toml/arbitrary_source_item_ordering/ordering_good.bad_conf_1.stderr b/tests/ui-toml/arbitrary_source_item_ordering/ordering_good.bad_conf_1.stderr new file mode 100644 index 000000000000..e441c7c1241c --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_good.bad_conf_1.stderr @@ -0,0 +1,8 @@ +error: error reading Clippy's configuration file: Some trait associated item kinds were configured more than once, or were missing, in the source ordering configuration. The trait associated item kinds are: [Const, Fn, Type] + --> $DIR/tests/ui-toml/arbitrary_source_item_ordering/bad_conf_1/clippy.toml:1:32 + | +LL | trait-assoc-item-kinds-order = ["fn", "type", "const", "type"] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui-toml/arbitrary_source_item_ordering/ordering_good.bad_conf_2.stderr b/tests/ui-toml/arbitrary_source_item_ordering/ordering_good.bad_conf_2.stderr new file mode 100644 index 000000000000..183f0b033191 --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_good.bad_conf_2.stderr @@ -0,0 +1,8 @@ +error: error reading Clippy's configuration file: Some trait associated item kinds were configured more than once, or were missing, in the source ordering configuration. The trait associated item kinds are: [Const, Fn, Type] + --> $DIR/tests/ui-toml/arbitrary_source_item_ordering/bad_conf_2/clippy.toml:1:32 + | +LL | trait-assoc-item-kinds-order = ["const", "type"] + | ^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui/arbitrary_source_item_ordering/ordering_good.rs b/tests/ui-toml/arbitrary_source_item_ordering/ordering_good.rs similarity index 87% rename from tests/ui/arbitrary_source_item_ordering/ordering_good.rs rename to tests/ui-toml/arbitrary_source_item_ordering/ordering_good.rs index e11d1c2a38b8..4632e77fc852 100644 --- a/tests/ui/arbitrary_source_item_ordering/ordering_good.rs +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_good.rs @@ -1,3 +1,9 @@ +//@aux-build:../../ui/auxiliary/proc_macros.rs +//@revisions: default bad_conf_1 bad_conf_2 +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/arbitrary_source_item_ordering/default +//@[bad_conf_1] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/arbitrary_source_item_ordering/bad_conf_1 +//@[bad_conf_2] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/arbitrary_source_item_ordering/bad_conf_2 + #![allow(dead_code)] #![warn(clippy::arbitrary_source_item_ordering)] diff --git a/tests/ui-toml/arbitrary_source_item_ordering/ordering_good_var_1.rs b/tests/ui-toml/arbitrary_source_item_ordering/ordering_good_var_1.rs new file mode 100644 index 000000000000..0fccbd4790bc --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_good_var_1.rs @@ -0,0 +1,174 @@ +//@aux-build:../../ui/auxiliary/proc_macros.rs +//@revisions: var_1 +//@[var_1] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/arbitrary_source_item_ordering/var_1 + +#![allow(dead_code)] +#![warn(clippy::arbitrary_source_item_ordering)] + +/// This module gets linted before clippy gives up. +mod i_am_just_right { + const AFTER: i8 = 0; + + const BEFORE: i8 = 0; +} + +/// Since the upper module passes linting, the lint now recurses into this module. +mod this_is_in_the_wrong_position { + const A: i8 = 1; + const C: i8 = 0; +} + +// Use statements should not be linted internally - this is normally auto-sorted using rustfmt. +use std::rc::{Rc, Weak}; +use std::sync::{Arc, Barrier, RwLock}; + +const SNAKE_CASE: &str = "zzzzzzzz"; + +const ZIS_SHOULD_BE_EVEN_EARLIER: () = (); + +const ZIS_SHOULD_BE_REALLY_EARLY: () = (); + +trait BasicEmptyTrait {} + +trait CloneSelf { + fn clone_self(&self) -> Self; +} + +enum EnumOrdered { + A, + B, + C, +} + +enum EnumUnordered { + A, + B, + C, +} + +#[allow(clippy::arbitrary_source_item_ordering)] +enum EnumUnorderedAllowed { + A, + B, + C, +} + +struct StructOrdered { + a: bool, + b: bool, + c: bool, +} + +impl BasicEmptyTrait for StructOrdered {} + +impl CloneSelf for StructOrdered { + fn clone_self(&self) -> Self { + Self { + a: true, + b: true, + c: true, + } + } +} + +impl Default for StructOrdered { + fn default() -> Self { + Self { + a: true, + b: true, + c: true, + } + } +} + +impl std::clone::Clone for StructOrdered { + fn clone(&self) -> Self { + Self { + a: true, + b: true, + c: true, + } + } +} + +#[derive(Clone, Default)] +struct StructUnordered { + a: bool, + b: bool, + c: bool, + d: bool, +} + +impl TraitUnordered for StructUnordered { + fn a() {} + fn b() {} + fn c() {} + + type SomeType = (); + + const A: bool = false; + const B: bool = false; + const C: bool = false; +} + +impl TraitUnorderedItemKinds for StructUnordered { + fn a() {} + + type SomeType = (); + + const A: bool = false; +} + +struct StructUnorderedGeneric { + _1: std::marker::PhantomData, + a: bool, + b: bool, + c: bool, + d: bool, +} + +trait TraitOrdered { + fn a(); + fn b(); + fn c(); + + type SomeType; + + const A: bool; + const B: bool; + const C: bool; +} + +trait TraitUnordered { + fn a(); + fn b(); + fn c(); + + type SomeType; + + const A: bool; + const B: bool; + const C: bool; +} + +trait TraitUnorderedItemKinds { + fn a(); + + type SomeType; + + const A: bool; +} + +#[derive(std::clone::Clone, Default)] +struct ZisShouldBeBeforeZeMainFn; + +fn main() { + // test code goes here +} + +#[cfg(test)] +mod test { + const B: i8 = 1; + + const A: i8 = 0; +} diff --git a/tests/ui/arbitrary_source_item_ordering/ordering_mixed.stderr b/tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.default.stderr similarity index 65% rename from tests/ui/arbitrary_source_item_ordering/ordering_mixed.stderr rename to tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.default.stderr index 23c1c1e3829b..c9ec22de9c1d 100644 --- a/tests/ui/arbitrary_source_item_ordering/ordering_mixed.stderr +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.default.stderr @@ -1,11 +1,11 @@ error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:17:14 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:21:14 | LL | use std::rc::Weak; | ^^^^ | note: should be placed before `SNAKE_CASE` - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:15:7 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:19:7 | LL | const SNAKE_CASE: &str = "zzzzzzzz"; | ^^^^^^^^^^ @@ -13,7 +13,7 @@ LL | const SNAKE_CASE: &str = "zzzzzzzz"; = help: to override `-D warnings` add `#[allow(clippy::arbitrary_source_item_ordering)]` error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:60:1 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:64:1 | LL | / impl CloneSelf for StructOrdered { LL | | fn clone_self(&self) -> Self { @@ -25,7 +25,7 @@ LL | | } | |_^ | note: should be placed before the following item - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:50:1 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:54:1 | LL | / impl Default for StructOrdered { LL | | fn default() -> Self { @@ -37,38 +37,38 @@ LL | | } | |_^ error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:80:19 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:84:19 | LL | #[derive(Default, Clone)] | ^^^^^ | note: should be placed before the following item - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:80:10 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:84:10 | LL | #[derive(Default, Clone)] | ^^^^^^^ = note: this error originates in the derive macro `Clone` which comes from the expansion of the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info) error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:132:7 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:136:7 | LL | const ZIS_SHOULD_BE_REALLY_EARLY: () = (); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: should be placed before `TraitUnorderedItemKinds` - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:120:7 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:124:7 | LL | trait TraitUnorderedItemKinds { | ^^^^^^^^^^^^^^^^^^^^^^^ error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:147:1 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:151:1 | LL | impl BasicEmptyTrait for StructOrdered {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: should be placed before the following item - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:134:1 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:138:1 | LL | / impl TraitUnordered for StructUnordered { LL | | const A: bool = false; @@ -80,171 +80,171 @@ LL | | } | |_^ error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:166:5 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:170:5 | LL | mod this_is_in_the_wrong_position { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: should be placed before `main` - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:161:4 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:165:4 | LL | fn main() { | ^^^^ error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:171:19 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:175:19 | LL | #[derive(Default, std::clone::Clone)] | ^^^^^^^^^^^^^^^^^ | note: should be placed before the following item - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:171:10 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:175:10 | LL | #[derive(Default, std::clone::Clone)] | ^^^^^^^ = note: this error originates in the derive macro `std::clone::Clone` which comes from the expansion of the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info) error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:174:7 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:178:7 | LL | const ZIS_SHOULD_BE_EVEN_EARLIER: () = (); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: should be placed before the following item - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:171:19 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:175:19 | LL | #[derive(Default, std::clone::Clone)] | ^^^^^^^^^^^^^^^^^ = note: this error originates in the derive macro `std::clone::Clone` (in Nightly builds, run with -Z macro-backtrace for more info) error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:8:11 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:12:11 | LL | const AFTER: i8 = 0; | ^^^^^ | note: should be placed before `BEFORE` - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:6:11 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:10:11 | LL | const BEFORE: i8 = 0; | ^^^^^^ error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:34:5 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:38:5 | LL | B, | ^ | note: should be placed before `C` - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:33:5 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:37:5 | LL | C, | ^ error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:84:5 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:88:5 | LL | b: bool, | ^ | note: should be placed before `c` - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:83:5 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:87:5 | LL | c: bool, | ^ error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:92:5 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:96:5 | LL | b: bool, | ^ | note: should be placed before `c` - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:91:5 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:95:5 | LL | c: bool, | ^ error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:111:11 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:115:11 | LL | const B: bool; | ^ | note: should be placed before `C` - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:110:11 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:114:11 | LL | const C: bool; | ^ error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:117:8 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:121:8 | LL | fn b(); | ^ | note: should be placed before `c` - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:116:8 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:120:8 | LL | fn c(); | ^ error: incorrect ordering of trait items (defined order: [Const, Type, Fn]) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:123:5 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:127:5 | LL | const A: bool; | ^^^^^^^^^^^^^^ | note: should be placed before `SomeType` - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:121:5 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:125:5 | LL | type SomeType; | ^^^^^^^^^^^^^^ error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:137:11 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:141:11 | LL | const B: bool = false; | ^ | note: should be placed before `C` - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:136:11 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:140:11 | LL | const C: bool = false; | ^ error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:143:8 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:147:8 | LL | fn b() {} | ^ | note: should be placed before `c` - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:142:8 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:146:8 | LL | fn c() {} | ^ error: incorrect ordering of impl items (defined order: [Const, Type, Fn]) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:152:5 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:156:5 | LL | const A: bool = false; | ^^^^^^^^^^^^^^^^^^^^^^ | note: should be placed before `SomeType` - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:150:5 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:154:5 | LL | type SomeType = (); | ^^^^^^^^^^^^^^^^^^^ error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:168:11 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:172:11 | LL | const A: i8 = 1; | ^ | note: should be placed before `C` - --> tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs:167:11 + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:171:11 | LL | const C: i8 = 0; | ^ diff --git a/tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs b/tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs similarity index 94% rename from tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs rename to tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs index 953f065d0fe0..2765bf935b7c 100644 --- a/tests/ui/arbitrary_source_item_ordering/ordering_mixed.rs +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs @@ -1,3 +1,7 @@ +//@aux-build:../../ui/auxiliary/proc_macros.rs +//@revisions: default +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/arbitrary_source_item_ordering/default + #![allow(dead_code)] #![warn(clippy::arbitrary_source_item_ordering)] diff --git a/tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs b/tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs new file mode 100644 index 000000000000..44902336573c --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs @@ -0,0 +1,174 @@ +//@aux-build:../../ui/auxiliary/proc_macros.rs +//@revisions: var_1 +//@[var_1] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/arbitrary_source_item_ordering/var_1 + +#![allow(dead_code)] +#![warn(clippy::arbitrary_source_item_ordering)] + +/// This module gets linted before clippy gives up. +mod i_am_just_right { + const AFTER: i8 = 0; + + const BEFORE: i8 = 0; +} + +/// Since the upper module passes linting, the lint now recurses into this module. +mod this_is_in_the_wrong_position { + const A: i8 = 1; + const C: i8 = 0; +} + +// Use statements should not be linted internally - this is normally auto-sorted using rustfmt. +use std::rc::{Rc, Weak}; +use std::sync::{Arc, Barrier, RwLock}; + +const SNAKE_CASE: &str = "zzzzzzzz"; + +const ZIS_SHOULD_BE_EVEN_EARLIER: () = (); + +const ZIS_SHOULD_BE_REALLY_EARLY: () = (); + +trait BasicEmptyTrait {} + +trait CloneSelf { + fn clone_self(&self) -> Self; +} + +enum EnumOrdered { + A, + B, + C, +} + +enum EnumUnordered { + A, + B, + C, +} + +#[allow(clippy::arbitrary_source_item_ordering)] +enum EnumUnorderedAllowed { + A, + B, + C, +} + +struct StructOrdered { + a: bool, + b: bool, + c: bool, +} + +impl BasicEmptyTrait for StructOrdered {} + +impl CloneSelf for StructOrdered { + fn clone_self(&self) -> Self { + Self { + a: true, + b: true, + c: true, + } + } +} + +impl Default for StructOrdered { + fn default() -> Self { + Self { + a: true, + b: true, + c: true, + } + } +} + +impl std::clone::Clone for StructOrdered { + fn clone(&self) -> Self { + Self { + a: true, + b: true, + c: true, + } + } +} + +#[derive(Clone, Default)] +struct StructUnordered { + a: bool, + b: bool, + c: bool, + d: bool, +} + +impl TraitUnordered for StructUnordered { + fn a() {} + fn c() {} + fn b() {} + + type SomeType = (); + + const A: bool = false; + const C: bool = false; + const B: bool = false; +} + +impl TraitUnorderedItemKinds for StructUnordered { + const A: bool = false; + + type SomeType = (); + + fn a() {} +} + +struct StructUnorderedGeneric { + _1: std::marker::PhantomData, + a: bool, + b: bool, + c: bool, + d: bool, +} + +trait TraitOrdered { + fn a(); + fn b(); + fn c(); + + type SomeType; + + const A: bool; + const B: bool; + const C: bool; +} + +trait TraitUnordered { + fn a(); + fn c(); + fn b(); + + type SomeType; + + const A: bool; + const C: bool; + const B: bool; +} + +trait TraitUnorderedItemKinds { + const A: bool; + + type SomeType; + + fn a(); +} + +#[derive(std::clone::Clone, Default)] +struct ZisShouldBeBeforeZeMainFn; + +fn main() { + // test code goes here +} + +#[cfg(test)] +mod test { + const B: i8 = 1; + + const A: i8 = 0; +} diff --git a/tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.var_1.stderr b/tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.var_1.stderr new file mode 100644 index 000000000000..f31f7f68c17e --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.var_1.stderr @@ -0,0 +1,100 @@ +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs:105:8 + | +LL | fn b() {} + | ^ + | +note: should be placed before `c` + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs:104:8 + | +LL | fn c() {} + | ^ + = note: `-D clippy::arbitrary-source-item-ordering` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::arbitrary_source_item_ordering)]` + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs:111:11 + | +LL | const B: bool = false; + | ^ + | +note: should be placed before `C` + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs:110:11 + | +LL | const C: bool = false; + | ^ + +error: incorrect ordering of impl items (defined order: [Fn, Type, Const]) + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs:117:5 + | +LL | type SomeType = (); + | ^^^^^^^^^^^^^^^^^^^ + | +note: should be placed before `A` + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs:115:5 + | +LL | const A: bool = false; + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: incorrect ordering of impl items (defined order: [Fn, Type, Const]) + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs:119:5 + | +LL | fn a() {} + | ^^^^^^^^^ + | +note: should be placed before `SomeType` + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs:117:5 + | +LL | type SomeType = (); + | ^^^^^^^^^^^^^^^^^^^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs:145:8 + | +LL | fn b(); + | ^ + | +note: should be placed before `c` + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs:144:8 + | +LL | fn c(); + | ^ + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs:151:11 + | +LL | const B: bool; + | ^ + | +note: should be placed before `C` + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs:150:11 + | +LL | const C: bool; + | ^ + +error: incorrect ordering of trait items (defined order: [Fn, Type, Const]) + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs:157:5 + | +LL | type SomeType; + | ^^^^^^^^^^^^^^ + | +note: should be placed before `A` + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs:155:5 + | +LL | const A: bool; + | ^^^^^^^^^^^^^^ + +error: incorrect ordering of trait items (defined order: [Fn, Type, Const]) + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs:159:5 + | +LL | fn a(); + | ^^^^^^^ + | +note: should be placed before `SomeType` + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed_var_1.rs:157:5 + | +LL | type SomeType; + | ^^^^^^^^^^^^^^ + +error: aborting due to 8 previous errors + diff --git a/tests/ui-toml/arbitrary_source_item_ordering/var_1/clippy.toml b/tests/ui-toml/arbitrary_source_item_ordering/var_1/clippy.toml new file mode 100644 index 000000000000..bede035e3885 --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/var_1/clippy.toml @@ -0,0 +1 @@ +trait-assoc-item-kinds-order = ["fn", "type", "const"] From a56cab430633a373245d2d0a9ad9a43a80a345e7 Mon Sep 17 00:00:00 2001 From: decryphe <12104091+decryphe@users.noreply.github.com> Date: Fri, 11 Oct 2024 18:39:48 +0200 Subject: [PATCH 7/9] fixup! new lint: `source_item_ordering` --- .../arbitrary_source_item_ordering/bad_conf_3/clippy.toml | 1 + .../default_exp/clippy.toml | 2 ++ .../ordering_good.bad_conf_3.stderr | 8 ++++++++ .../arbitrary_source_item_ordering/ordering_good.rs | 4 +++- 4 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/bad_conf_3/clippy.toml create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/default_exp/clippy.toml create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/ordering_good.bad_conf_3.stderr diff --git a/tests/ui-toml/arbitrary_source_item_ordering/bad_conf_3/clippy.toml b/tests/ui-toml/arbitrary_source_item_ordering/bad_conf_3/clippy.toml new file mode 100644 index 000000000000..0dd5407be10b --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/bad_conf_3/clippy.toml @@ -0,0 +1 @@ +source-item-ordering = ["enum", "impl", "module", "struct", "trait", "struct"] diff --git a/tests/ui-toml/arbitrary_source_item_ordering/default_exp/clippy.toml b/tests/ui-toml/arbitrary_source_item_ordering/default_exp/clippy.toml new file mode 100644 index 000000000000..a1b81fde00e1 --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/default_exp/clippy.toml @@ -0,0 +1,2 @@ +trait-assoc-item-kinds-order = ["const", "type", "fn"] +source-item-ordering = ["enum", "impl", "module", "struct", "trait"] diff --git a/tests/ui-toml/arbitrary_source_item_ordering/ordering_good.bad_conf_3.stderr b/tests/ui-toml/arbitrary_source_item_ordering/ordering_good.bad_conf_3.stderr new file mode 100644 index 000000000000..abf58dbd1101 --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_good.bad_conf_3.stderr @@ -0,0 +1,8 @@ +error: error reading Clippy's configuration file: The category "Struct" was enabled more than once in the source ordering configuration. + --> $DIR/tests/ui-toml/arbitrary_source_item_ordering/bad_conf_3/clippy.toml:1:24 + | +LL | source-item-ordering = ["enum", "impl", "module", "struct", "trait", "struct"] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui-toml/arbitrary_source_item_ordering/ordering_good.rs b/tests/ui-toml/arbitrary_source_item_ordering/ordering_good.rs index 4632e77fc852..e9bcc30c15e0 100644 --- a/tests/ui-toml/arbitrary_source_item_ordering/ordering_good.rs +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_good.rs @@ -1,8 +1,10 @@ //@aux-build:../../ui/auxiliary/proc_macros.rs -//@revisions: default bad_conf_1 bad_conf_2 +//@revisions: default default_exp bad_conf_1 bad_conf_2 bad_conf_3 //@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/arbitrary_source_item_ordering/default +//@[default_exp] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/arbitrary_source_item_ordering/default_exp //@[bad_conf_1] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/arbitrary_source_item_ordering/bad_conf_1 //@[bad_conf_2] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/arbitrary_source_item_ordering/bad_conf_2 +//@[bad_conf_3] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/arbitrary_source_item_ordering/bad_conf_3 #![allow(dead_code)] #![warn(clippy::arbitrary_source_item_ordering)] From 72fccdfd15b2fda2a224203e01bf843e5dd9ab24 Mon Sep 17 00:00:00 2001 From: decryphe <12104091+decryphe@users.noreply.github.com> Date: Fri, 11 Oct 2024 18:53:30 +0200 Subject: [PATCH 8/9] fixup! new lint: `source_item_ordering` --- book/src/lint_configuration.md | 2 +- clippy_config/src/conf.rs | 2 +- .../default_exp/clippy.toml | 10 +++ .../only_enum/clippy.toml | 1 + .../only_impl/clippy.toml | 1 + .../only_trait/clippy.toml | 1 + .../ordering_only_enum.only_enum.stderr | 16 +++++ .../ordering_only_enum.rs | 43 ++++++++++++ .../ordering_only_impl.only_impl.stderr | 40 +++++++++++ .../ordering_only_impl.rs | 67 +++++++++++++++++++ .../ordering_only_trait.only_trait.stderr | 40 +++++++++++ .../ordering_only_trait.rs | 48 +++++++++++++ 12 files changed, 269 insertions(+), 2 deletions(-) create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/only_enum/clippy.toml create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/only_impl/clippy.toml create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/only_trait/clippy.toml create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/ordering_only_enum.only_enum.stderr create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/ordering_only_enum.rs create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/ordering_only_impl.only_impl.stderr create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/ordering_only_impl.rs create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/ordering_only_trait.only_trait.stderr create mode 100644 tests/ui-toml/arbitrary_source_item_ordering/ordering_only_trait.rs diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 8f6d44f29011..56c6b1c1ce21 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -669,7 +669,7 @@ crate. For example, `pub(crate)` items. ## `module-item-order-groupings` The named groupings of different source item kinds within modules. -**Default Value:** `[["modules", ["mod", "foreign_mod"]], ["use", ["use"]], ["macros", ["macro"]], ["global_asm", ["global_asm"]], ["UPPER_SNAKE_CASE", ["static", "const"]], ["PascalCase", ["ty_alias", "opaque_ty", "enum", "struct", "union", "trait", "trait_alias", "impl"]], ["lower_snake_case", ["fn"]]]` +**Default Value:** `[["modules", ["extern_crate", "mod", "foreign_mod"]], ["use", ["use"]], ["macros", ["macro"]], ["global_asm", ["global_asm"]], ["UPPER_SNAKE_CASE", ["static", "const"]], ["PascalCase", ["ty_alias", "opaque_ty", "enum", "struct", "union", "trait", "trait_alias", "impl"]], ["lower_snake_case", ["fn"]]]` --- **Affected lints:** diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 108f11ccc4c9..dbc7a54b5ebe 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -54,7 +54,7 @@ const DEFAULT_MODULE_ITEM_ORDERING_GROUPS: &[(&str, &[SourceItemOrderingModuleIt #[allow(clippy::enum_glob_use)] // Very local glob use for legibility. use SourceItemOrderingModuleItemKind::*; &[ - ("modules", &[Mod, ForeignMod]), + ("modules", &[ExternCrate, Mod, ForeignMod]), ("use", &[Use]), ("macros", &[Macro]), ("global_asm", &[GlobalAsm]), diff --git a/tests/ui-toml/arbitrary_source_item_ordering/default_exp/clippy.toml b/tests/ui-toml/arbitrary_source_item_ordering/default_exp/clippy.toml index a1b81fde00e1..c5465ee03f67 100644 --- a/tests/ui-toml/arbitrary_source_item_ordering/default_exp/clippy.toml +++ b/tests/ui-toml/arbitrary_source_item_ordering/default_exp/clippy.toml @@ -1,2 +1,12 @@ trait-assoc-item-kinds-order = ["const", "type", "fn"] source-item-ordering = ["enum", "impl", "module", "struct", "trait"] +module-item-order-groupings = [ + ["modules", ["extern_crate", "mod", "foreign_mod"]], + ["use", ["use"]], + ["macros", ["macro"]], + ["global_asm", ["global_asm"]], + ["UPPER_SNAKE_CASE", ["static", "const"]], + ["PascalCase", ["ty_alias", "opaque_ty", "enum", "struct", "union", "trait", "trait_alias", "impl"]], + ["lower_snake_case", ["fn"]] +] + diff --git a/tests/ui-toml/arbitrary_source_item_ordering/only_enum/clippy.toml b/tests/ui-toml/arbitrary_source_item_ordering/only_enum/clippy.toml new file mode 100644 index 000000000000..2144bdc9a0ce --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/only_enum/clippy.toml @@ -0,0 +1 @@ +source-item-ordering = ["enum"] diff --git a/tests/ui-toml/arbitrary_source_item_ordering/only_impl/clippy.toml b/tests/ui-toml/arbitrary_source_item_ordering/only_impl/clippy.toml new file mode 100644 index 000000000000..54b6727fabf7 --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/only_impl/clippy.toml @@ -0,0 +1 @@ +source-item-ordering = ["impl"] diff --git a/tests/ui-toml/arbitrary_source_item_ordering/only_trait/clippy.toml b/tests/ui-toml/arbitrary_source_item_ordering/only_trait/clippy.toml new file mode 100644 index 000000000000..b551611c35e4 --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/only_trait/clippy.toml @@ -0,0 +1 @@ +source-item-ordering = ["trait"] diff --git a/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_enum.only_enum.stderr b/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_enum.only_enum.stderr new file mode 100644 index 000000000000..57069fd8feed --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_enum.only_enum.stderr @@ -0,0 +1,16 @@ +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_only_enum.rs:22:5 + | +LL | A, + | ^ + | +note: should be placed before `B` + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_only_enum.rs:21:5 + | +LL | B, + | ^ + = note: `-D clippy::arbitrary-source-item-ordering` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::arbitrary_source_item_ordering)]` + +error: aborting due to 1 previous error + diff --git a/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_enum.rs b/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_enum.rs new file mode 100644 index 000000000000..e8002c4a1633 --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_enum.rs @@ -0,0 +1,43 @@ +//@aux-build:../../ui/auxiliary/proc_macros.rs +//@revisions: only_enum +//@[only_enum] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/arbitrary_source_item_ordering/only_enum + +#![allow(dead_code)] +#![warn(clippy::arbitrary_source_item_ordering)] + +fn main() {} + +struct StructUnordered { + b: bool, + a: bool, +} + +enum EnumOrdered { + A, + B, +} + +enum EnumUnordered { + B, + A, +} + +trait TraitUnordered { + const B: bool; + const A: bool; + + type SomeType; + + fn b(); + fn a(); +} + +trait TraitUnorderedItemKinds { + type SomeType; + + const A: bool; + + fn a(); +} + +const ZIS_SHOULD_BE_AT_THE_TOP: () = (); diff --git a/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_impl.only_impl.stderr b/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_impl.only_impl.stderr new file mode 100644 index 000000000000..40348ecbdae1 --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_impl.only_impl.stderr @@ -0,0 +1,40 @@ +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_only_impl.rs:43:8 + | +LL | fn a() {} + | ^ + | +note: should be placed before `b` + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_only_impl.rs:42:8 + | +LL | fn b() {} + | ^ + = note: `-D clippy::arbitrary-source-item-ordering` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::arbitrary_source_item_ordering)]` + +error: incorrect ordering of impl items (defined order: [Const, Type, Fn]) + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_only_impl.rs:45:5 + | +LL | type SomeType = i8; + | ^^^^^^^^^^^^^^^^^^^ + | +note: should be placed before `a` + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_only_impl.rs:43:5 + | +LL | fn a() {} + | ^^^^^^^^^ + +error: incorrect ordering of impl items (defined order: [Const, Type, Fn]) + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_only_impl.rs:47:5 + | +LL | const A: bool = true; + | ^^^^^^^^^^^^^^^^^^^^^ + | +note: should be placed before `SomeType` + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_only_impl.rs:45:5 + | +LL | type SomeType = i8; + | ^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors + diff --git a/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_impl.rs b/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_impl.rs new file mode 100644 index 000000000000..bd969c865b5a --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_impl.rs @@ -0,0 +1,67 @@ +//@aux-build:../../ui/auxiliary/proc_macros.rs +//@revisions: only_impl +//@[only_impl] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/arbitrary_source_item_ordering/only_impl + +#![allow(dead_code)] +#![warn(clippy::arbitrary_source_item_ordering)] + +fn main() {} + +struct StructUnordered { + b: bool, + a: bool, +} + +struct BasicStruct {} + +trait BasicTrait { + const A: bool; + + type SomeType; + + fn b(); + fn a(); +} + +enum EnumUnordered { + B, + A, +} + +trait TraitUnordered { + const B: bool; + const A: bool; + + type SomeType; + + fn b(); + fn a(); +} + +impl BasicTrait for StructUnordered { + fn b() {} + fn a() {} + + type SomeType = i8; + + const A: bool = true; +} + +trait TraitUnorderedItemKinds { + type SomeType; + + const A: bool; + + fn a(); +} + +const ZIS_SHOULD_BE_AT_THE_TOP: () = (); + +impl BasicTrait for BasicStruct { + const A: bool = true; + + type SomeType = i8; + + fn a() {} + fn b() {} +} diff --git a/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_trait.only_trait.stderr b/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_trait.only_trait.stderr new file mode 100644 index 000000000000..9b86ebd48e3d --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_trait.only_trait.stderr @@ -0,0 +1,40 @@ +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_only_trait.rs:32:11 + | +LL | const A: bool; + | ^ + | +note: should be placed before `B` + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_only_trait.rs:31:11 + | +LL | const B: bool; + | ^ + = note: `-D clippy::arbitrary-source-item-ordering` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::arbitrary_source_item_ordering)]` + +error: incorrect ordering of items (must be alphabetically ordered) + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_only_trait.rs:37:8 + | +LL | fn a(); + | ^ + | +note: should be placed before `b` + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_only_trait.rs:36:8 + | +LL | fn b(); + | ^ + +error: incorrect ordering of trait items (defined order: [Const, Type, Fn]) + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_only_trait.rs:43:5 + | +LL | const A: bool; + | ^^^^^^^^^^^^^^ + | +note: should be placed before `SomeType` + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_only_trait.rs:41:5 + | +LL | type SomeType; + | ^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors + diff --git a/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_trait.rs b/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_trait.rs new file mode 100644 index 000000000000..979a52ecb100 --- /dev/null +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_only_trait.rs @@ -0,0 +1,48 @@ +//@aux-build:../../ui/auxiliary/proc_macros.rs +//@revisions: only_trait +//@[only_trait] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/arbitrary_source_item_ordering/only_trait + +#![allow(dead_code)] +#![warn(clippy::arbitrary_source_item_ordering)] + +fn main() {} + +struct StructUnordered { + b: bool, + a: bool, +} + +trait TraitOrdered { + const A: bool; + const B: bool; + + type SomeType; + + fn a(); + fn b(); +} + +enum EnumUnordered { + B, + A, +} + +trait TraitUnordered { + const B: bool; + const A: bool; + + type SomeType; + + fn b(); + fn a(); +} + +trait TraitUnorderedItemKinds { + type SomeType; + + const A: bool; + + fn a(); +} + +const ZIS_SHOULD_BE_AT_THE_TOP: () = (); From 1239c308dcdb4f23332e24ddd73f087b8070061b Mon Sep 17 00:00:00 2001 From: decryphe <12104091+decryphe@users.noreply.github.com> Date: Mon, 14 Oct 2024 17:20:34 +0200 Subject: [PATCH 9/9] fixup! new lint: `source_item_ordering` --- .../src/arbitrary_source_item_ordering.rs | 40 +++++++++++++++---- .../ordering_mixed.default.stderr | 37 +++-------------- 2 files changed, 38 insertions(+), 39 deletions(-) diff --git a/clippy_lints/src/arbitrary_source_item_ordering.rs b/clippy_lints/src/arbitrary_source_item_ordering.rs index b7a916bc2afd..bbd7ba47031f 100644 --- a/clippy_lints/src/arbitrary_source_item_ordering.rs +++ b/clippy_lints/src/arbitrary_source_item_ordering.rs @@ -8,7 +8,8 @@ use rustc_hir::{ AssocItemKind, FieldDef, HirId, ImplItemRef, IsAuto, Item, ItemKind, Mod, QPath, TraitItemRef, TyKind, UseKind, Variant, VariantData, }; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; use rustc_session::impl_lint_pass; declare_clippy_lint! { @@ -161,7 +162,7 @@ impl ArbitrarySourceItemOrdering { } /// Produces a linting warning for incorrectly ordered impl items. - fn lint_impl_item(&self, cx: &T, item: &ImplItemRef, before_item: &ImplItemRef) { + fn lint_impl_item(&self, cx: &T, item: &ImplItemRef, before_item: &ImplItemRef) { span_lint_and_note( cx, ARBITRARY_SOURCE_ITEM_ORDERING, @@ -176,7 +177,7 @@ impl ArbitrarySourceItemOrdering { } /// Produces a linting warning for incorrectly ordered item members. - fn lint_member_name( + fn lint_member_name( cx: &T, ident: &rustc_span::symbol::Ident, before_ident: &rustc_span::symbol::Ident, @@ -191,7 +192,7 @@ impl ArbitrarySourceItemOrdering { ); } - fn lint_member_item(cx: &T, item: &Item<'_>, before_item: &Item<'_>) { + fn lint_member_item(cx: &T, item: &Item<'_>, before_item: &Item<'_>) { let span = if item.ident.as_str().is_empty() { &item.span } else { @@ -210,6 +211,11 @@ impl ArbitrarySourceItemOrdering { ) }; + // This catches false positives where generated code gets linted. + if span == before_span { + return; + } + span_lint_and_note( cx, ARBITRARY_SOURCE_ITEM_ORDERING, @@ -221,7 +227,7 @@ impl ArbitrarySourceItemOrdering { } /// Produces a linting warning for incorrectly ordered trait items. - fn lint_trait_item(&self, cx: &T, item: &TraitItemRef, before_item: &TraitItemRef) { + fn lint_trait_item(&self, cx: &T, item: &TraitItemRef, before_item: &TraitItemRef) { span_lint_and_note( cx, ARBITRARY_SOURCE_ITEM_ORDERING, @@ -242,8 +248,12 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering { ItemKind::Enum(enum_def, _generics) if self.enable_ordering_for_enum => { let mut cur_v: Option<&Variant<'_>> = None; for variant in enum_def.variants { + if in_external_macro(cx.sess(), variant.span) { + continue; + } + if let Some(cur_v) = cur_v { - if cur_v.ident.name.as_str() > variant.ident.name.as_str() { + if cur_v.ident.name.as_str() > variant.ident.name.as_str() && cur_v.span != variant.span { Self::lint_member_name(cx, &variant.ident, &cur_v.ident); } } @@ -253,8 +263,12 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering { ItemKind::Struct(VariantData::Struct { fields, .. }, _generics) if self.enable_ordering_for_struct => { let mut cur_f: Option<&FieldDef<'_>> = None; for field in *fields { + if in_external_macro(cx.sess(), field.span) { + continue; + } + if let Some(cur_f) = cur_f { - if cur_f.ident.name.as_str() > field.ident.name.as_str() { + if cur_f.ident.name.as_str() > field.ident.name.as_str() && cur_f.span != field.span { Self::lint_member_name(cx, &field.ident, &cur_f.ident); } } @@ -267,6 +281,10 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering { let mut cur_t: Option<&TraitItemRef> = None; for item in *item_ref { + if in_external_macro(cx.sess(), item.span) { + continue; + } + if let Some(cur_t) = cur_t { let cur_t_kind = convert_assoc_item_kind(cur_t.kind); let cur_t_kind_index = self.assoc_types_order.index_of(&cur_t_kind); @@ -286,6 +304,10 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering { let mut cur_t: Option<&ImplItemRef> = None; for item in trait_impl.items { + if in_external_macro(cx.sess(), item.span) { + continue; + } + if let Some(cur_t) = cur_t { let cur_t_kind = convert_assoc_item_kind(cur_t.kind); let cur_t_kind_index = self.assoc_types_order.index_of(&cur_t_kind); @@ -326,6 +348,10 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering { // as no sorting by source map/line of code has to be applied. // for item in items { + if in_external_macro(cx.sess(), item.span) { + continue; + } + // The following exceptions (skipping with `continue;`) may not be // complete, edge cases have not been explored further than what // appears in the existing code base. diff --git a/tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.default.stderr b/tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.default.stderr index c9ec22de9c1d..062bf25ea624 100644 --- a/tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.default.stderr +++ b/tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.default.stderr @@ -36,19 +36,6 @@ LL | | } LL | | } | |_^ -error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:84:19 - | -LL | #[derive(Default, Clone)] - | ^^^^^ - | -note: should be placed before the following item - --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:84:10 - | -LL | #[derive(Default, Clone)] - | ^^^^^^^ - = note: this error originates in the derive macro `Clone` which comes from the expansion of the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info) - error: incorrect ordering of items (must be alphabetically ordered) --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:136:7 | @@ -91,31 +78,17 @@ note: should be placed before `main` LL | fn main() { | ^^^^ -error: incorrect ordering of items (must be alphabetically ordered) - --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:175:19 - | -LL | #[derive(Default, std::clone::Clone)] - | ^^^^^^^^^^^^^^^^^ - | -note: should be placed before the following item - --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:175:10 - | -LL | #[derive(Default, std::clone::Clone)] - | ^^^^^^^ - = note: this error originates in the derive macro `std::clone::Clone` which comes from the expansion of the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info) - error: incorrect ordering of items (must be alphabetically ordered) --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:178:7 | LL | const ZIS_SHOULD_BE_EVEN_EARLIER: () = (); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | -note: should be placed before the following item - --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:175:19 +note: should be placed before `ZisShouldBeBeforeZeMainFn` + --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:176:8 | -LL | #[derive(Default, std::clone::Clone)] - | ^^^^^^^^^^^^^^^^^ - = note: this error originates in the derive macro `std::clone::Clone` (in Nightly builds, run with -Z macro-backtrace for more info) +LL | struct ZisShouldBeBeforeZeMainFn; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: incorrect ordering of items (must be alphabetically ordered) --> tests/ui-toml/arbitrary_source_item_ordering/ordering_mixed.rs:12:11 @@ -249,5 +222,5 @@ note: should be placed before `C` LL | const C: i8 = 0; | ^ -error: aborting due to 19 previous errors +error: aborting due to 17 previous errors