arbitrary_source_item_ordering: Make alphabetic ordering in module item groups optional

CHANGELOG.md

@@ -6372,6 +6372,7 @@ Released 2018-09-13
 [`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
+[`module-items-ordered-within-groupings`]: https://doc.rust-lang.org/clippy/lint_configuration.html#module-items-ordered-within-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

book/src/lint_configuration.md

@@ -744,6 +744,19 @@ The named groupings of different source item kinds within modules.
 * [`arbitrary_source_item_ordering`](https://rust-lang.github.io/rust-clippy/master/index.html#arbitrary_source_item_ordering)
 
 
+## `module-items-ordered-within-groupings`
+Whether the items within module groups should be ordered alphabetically or not.
+
+This option can be configured to "all", "none", or a list of specific grouping names that should be checked
+(e.g. only "enums").
+
+**Default Value:** `"none"`
+
+---
+**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`
 

clippy_config/src/conf.rs

@@ -3,14 +3,17 @@ use crate::types::{
     DisallowedPath, DisallowedPathWithoutReplacement, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour,
     Rename, SourceItemOrdering, SourceItemOrderingCategory, SourceItemOrderingModuleItemGroupings,
     SourceItemOrderingModuleItemKind, SourceItemOrderingTraitAssocItemKind, SourceItemOrderingTraitAssocItemKinds,
+    SourceItemOrderingWithinModuleItemGroupings,
 };
 use clippy_utils::msrvs::Msrv;
+use itertools::Itertools;
 use rustc_errors::Applicability;
 use rustc_session::Session;
 use rustc_span::edit_distance::edit_distance;
 use rustc_span::{BytePos, Pos, SourceFile, Span, SyntaxContext};
 use serde::de::{IgnoredAny, IntoDeserializer, MapAccess, Visitor};
 use serde::{Deserialize, Deserializer, Serialize};
+use std::collections::HashMap;
 use std::fmt::{Debug, Display, Formatter};
 use std::ops::Range;
 use std::path::PathBuf;
@@ -79,6 +82,7 @@ const DEFAULT_SOURCE_ITEM_ORDERING: &[SourceItemOrderingCategory] = {
 #[derive(Default)]
 struct TryConf {
     conf: Conf,
+    value_spans: HashMap<String, Range<usize>>,
     errors: Vec<ConfError>,
     warnings: Vec<ConfError>,
 }
@@ -87,6 +91,7 @@ impl TryConf {
     fn from_toml_error(file: &SourceFile, error: &toml::de::Error) -> Self {
         Self {
             conf: Conf::default(),
+            value_spans: HashMap::default(),
             errors: vec![ConfError::from_toml(file, error)],
             warnings: vec![],
         }
@@ -210,6 +215,7 @@ macro_rules! define_Conf {
             }
 
             fn visit_map<V>(self, mut map: V) -> Result<Self::Value, V::Error> where V: MapAccess<'de> {
+                let mut value_spans = HashMap::new();
                 let mut errors = Vec::new();
                 let mut warnings = Vec::new();
                 $(let mut $name = None;)*
@@ -232,6 +238,7 @@ macro_rules! define_Conf {
                                     }
                                     None => {
                                         $name = Some(value);
+                                        value_spans.insert(name.get_ref().as_str().to_string(), value_span);
                                         // $new_conf is the same as one of the defined `$name`s, so
                                         // this variable is defined in line 2 of this function.
                                         $(match $new_conf {
@@ -250,7 +257,7 @@ macro_rules! define_Conf {
                     }
                 }
                 let conf = Conf { $($name: $name.unwrap_or_else(defaults::$name),)* };
-                Ok(TryConf { conf, errors, warnings })
+                Ok(TryConf { conf, value_spans, errors, warnings })
             }
         }
 
@@ -596,6 +603,13 @@ define_Conf! {
     /// 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(),
+    /// Whether the items within module groups should be ordered alphabetically or not.
+    ///
+    /// This option can be configured to "all", "none", or a list of specific grouping names that should be checked
+    /// (e.g. only "enums").
+    #[lints(arbitrary_source_item_ordering)]
+    module_items_ordered_within_groupings: SourceItemOrderingWithinModuleItemGroupings =
+        SourceItemOrderingWithinModuleItemGroupings::None,
     /// The minimum rust version that the project supports. Defaults to the `rust-version` field in `Cargo.toml`
     #[default_text = "current version"]
     #[lints(
@@ -815,6 +829,36 @@ fn deserialize(file: &SourceFile) -> TryConf {
                 &mut conf.conf.allow_renamed_params_for,
                 DEFAULT_ALLOWED_TRAITS_WITH_RENAMED_PARAMS,
             );
+
+            // Confirms that the user has not accidentally configured ordering requirements for groups that
+            // aren't configured.
+            if let SourceItemOrderingWithinModuleItemGroupings::Custom(groupings) =
+                &conf.conf.module_items_ordered_within_groupings
+            {
+                for grouping in groupings {
+                    if !conf.conf.module_item_order_groupings.is_grouping(grouping) {
+                        // Since this isn't fixable by rustfix, don't emit a `Suggestion`. This just adds some useful
+                        // info for the user instead.
+
+                        let names = conf.conf.module_item_order_groupings.grouping_names();
+                        let suggestion = suggest_candidate(grouping, names.iter().map(String::as_str))
+                            .map(|s| format!(" perhaps you meant `{s}`?"))
+                            .unwrap_or_default();
+                        let names = names.iter().map(|s| format!("`{s}`")).join(", ");
+                        let message = format!(
+                            "unknown ordering group: `{grouping}` was not specified in `module-items-ordered-within-groupings`,{suggestion} expected one of: {names}"
+                        );
+
+                        let span = conf
+                            .value_spans
+                            .get("module_item_order_groupings")
+                            .cloned()
+                            .unwrap_or_default();
+                        conf.errors.push(ConfError::spanned(file, message, None, span));
+                    }
+                }
+            }
+
             // TODO: THIS SHOULD BE TESTED, this comment will be gone soon
             if conf.conf.allowed_idents_below_min_chars.iter().any(|e| e == "..") {
                 conf.conf
@@ -860,6 +904,7 @@ impl Conf {
 
         let TryConf {
             mut conf,
+            value_spans: _,
             errors,
             warnings,
         } = match path {
@@ -950,17 +995,10 @@ impl serde::de::Error for FieldError {
             }
         }
 
-        let suggestion = expected
-            .iter()
-            .filter_map(|expected| {
-                let dist = edit_distance(field, expected, 4)?;
-                Some((dist, expected))
-            })
-            .min_by_key(|&(dist, _)| dist)
-            .map(|(_, suggestion)| Suggestion {
-                message: "perhaps you meant",
-                suggestion,
-            });
+        let suggestion = suggest_candidate(field, expected).map(|suggestion| Suggestion {
+            message: "perhaps you meant",
+            suggestion,
+        });
 
         Self { error: msg, suggestion }
     }
@@ -998,6 +1036,22 @@ fn calculate_dimensions(fields: &[&str]) -> (usize, Vec<usize>) {
     (rows, column_widths)
 }
 
+/// Given a user-provided value that couldn't be matched to a known option, finds the most likely
+/// candidate among candidates that the user might have meant.
+fn suggest_candidate<'a, I>(value: &str, candidates: I) -> Option<&'a str>
+where
+    I: IntoIterator<Item = &'a str>,
+{
+    candidates
+        .into_iter()
+        .filter_map(|expected| {
+            let dist = edit_distance(value, expected, 4)?;
+            Some((dist, expected))
+        })
+        .min_by_key(|&(dist, _)| dist)
+        .map(|(_, suggestion)| suggestion)
+}
+
 #[cfg(test)]
 mod tests {
     use serde::de::IgnoredAny;

clippy_config/src/types.rs

@@ -305,6 +305,7 @@ impl SourceItemOrderingModuleItemKind {
 pub struct SourceItemOrderingModuleItemGroupings {
     groups: Vec<(String, Vec<SourceItemOrderingModuleItemKind>)>,
     lut: HashMap<SourceItemOrderingModuleItemKind, usize>,
+    back_lut: HashMap<SourceItemOrderingModuleItemKind, String>,
 }
 
 impl SourceItemOrderingModuleItemGroupings {
@@ -320,6 +321,30 @@ impl SourceItemOrderingModuleItemGroupings {
         lut
     }
 
+    fn build_back_lut(
+        groups: &[(String, Vec<SourceItemOrderingModuleItemKind>)],
+    ) -> HashMap<SourceItemOrderingModuleItemKind, String> {
+        let mut lut = HashMap::new();
+        for (group_name, items) in groups {
+            for item in items {
+                lut.insert(item.clone(), group_name.clone());
+            }
+        }
+        lut
+    }
+
+    pub fn grouping_name_of(&self, item: &SourceItemOrderingModuleItemKind) -> Option<&String> {
+        self.back_lut.get(item)
+    }
+
+    pub fn grouping_names(&self) -> Vec<String> {
+        self.groups.iter().map(|(name, _)| name.clone()).collect()
+    }
+
+    pub fn is_grouping(&self, grouping: &str) -> bool {
+        self.groups.iter().any(|(g, _)| g == grouping)
+    }
+
     pub fn module_level_order_of(&self, item: &SourceItemOrderingModuleItemKind) -> Option<usize> {
         self.lut.get(item).copied()
     }
@@ -330,7 +355,8 @@ impl From<&[(&str, &[SourceItemOrderingModuleItemKind])]> for SourceItemOrdering
         let groups: Vec<(String, Vec<SourceItemOrderingModuleItemKind>)> =
             value.iter().map(|item| (item.0.to_string(), item.1.to_vec())).collect();
         let lut = Self::build_lut(&groups);
-        Self { groups, lut }
+        let back_lut = Self::build_back_lut(&groups);
+        Self { groups, lut, back_lut }
     }
 }
 
@@ -348,6 +374,7 @@ impl<'de> Deserialize<'de> for SourceItemOrderingModuleItemGroupings {
         let groups = Vec::<(String, Vec<SourceItemOrderingModuleItemKind>)>::deserialize(deserializer)?;
         let items_total: usize = groups.iter().map(|(_, v)| v.len()).sum();
         let lut = Self::build_lut(&groups);
+        let back_lut = Self::build_back_lut(&groups);
 
         let mut expected_items = SourceItemOrderingModuleItemKind::all_variants();
         for item in lut.keys() {
@@ -370,7 +397,7 @@ impl<'de> Deserialize<'de> for SourceItemOrderingModuleItemGroupings {
                 ));
             }
 
-            Ok(Self { groups, lut })
+            Ok(Self { groups, lut, back_lut })
         } else if items_total != all_items.len() {
             Err(de::Error::custom(format!(
                 "Some module item kinds were configured more than once, or were missing, in the source ordering configuration. \
@@ -482,6 +509,83 @@ impl Serialize for SourceItemOrderingTraitAssocItemKinds {
     }
 }
 
+/// Describes which specific groupings should have their items ordered
+/// alphabetically.
+///
+/// This is separate from defining and enforcing groupings. For example,
+/// defining enums are grouped before structs still allows for an enum B to be
+/// placed before an enum A. Only when enforcing ordering within the grouping,
+/// will it be checked if A is placed before B.
+#[derive(Clone, Debug)]
+pub enum SourceItemOrderingWithinModuleItemGroupings {
+    /// All groupings should have their items ordered.
+    All,
+
+    /// None of the groupings should have their order checked.
+    None,
+
+    /// Only the specified groupings should have their order checked.
+    Custom(Vec<String>),
+}
+
+impl SourceItemOrderingWithinModuleItemGroupings {
+    pub fn ordered_within(&self, grouping_name: &String) -> bool {
+        match self {
+            SourceItemOrderingWithinModuleItemGroupings::All => true,
+            SourceItemOrderingWithinModuleItemGroupings::None => false,
+            SourceItemOrderingWithinModuleItemGroupings::Custom(groups) => groups.contains(grouping_name),
+        }
+    }
+}
+
+/// Helper struct for deserializing the [`SourceItemOrderingWithinModuleItemGroupings`].
+#[derive(Deserialize)]
+#[serde(untagged)]
+enum StringOrVecOfString {
+    String(String),
+    Vec(Vec<String>),
+}
+
+impl<'de> Deserialize<'de> for SourceItemOrderingWithinModuleItemGroupings {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let description = "The available options for configuring an ordering within module item groups are: \
+                    \"all\", \"none\", or a list of module item group names \
+                    (as configured with the `module-item-order-groupings` configuration option).";
+
+        match StringOrVecOfString::deserialize(deserializer) {
+            Ok(StringOrVecOfString::String(preset)) if preset == "all" => {
+                Ok(SourceItemOrderingWithinModuleItemGroupings::All)
+            },
+            Ok(StringOrVecOfString::String(preset)) if preset == "none" => {
+                Ok(SourceItemOrderingWithinModuleItemGroupings::None)
+            },
+            Ok(StringOrVecOfString::String(preset)) => Err(de::Error::custom(format!(
+                "Unknown configuration option: {preset}.\n{description}"
+            ))),
+            Ok(StringOrVecOfString::Vec(groupings)) => {
+                Ok(SourceItemOrderingWithinModuleItemGroupings::Custom(groupings))
+            },
+            Err(e) => Err(de::Error::custom(format!("{e}\n{description}"))),
+        }
+    }
+}
+
+impl Serialize for SourceItemOrderingWithinModuleItemGroupings {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: ser::Serializer,
+    {
+        match self {
+            SourceItemOrderingWithinModuleItemGroupings::All => serializer.serialize_str("all"),
+            SourceItemOrderingWithinModuleItemGroupings::None => serializer.serialize_str("none"),
+            SourceItemOrderingWithinModuleItemGroupings::Custom(vec) => vec.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 {