Make "all fields are shorthand" requirement configurable

Author: smoelius

CHANGELOG.md

@@ -6249,6 +6249,7 @@ Released 2018-09-13
 [`excessive-nesting-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#excessive-nesting-threshold
 [`future-size-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#future-size-threshold
 [`ignore-interior-mutability`]: https://doc.rust-lang.org/clippy/lint_configuration.html#ignore-interior-mutability
+[`initializer-suggestions`]: https://doc.rust-lang.org/clippy/lint_configuration.html#initializer-suggestions
 [`large-error-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#large-error-threshold
 [`literal-representation-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#literal-representation-threshold
 [`matches-for-let-else`]: https://doc.rust-lang.org/clippy/lint_configuration.html#matches-for-let-else

book/src/lint_configuration.md

@@ -562,6 +562,36 @@ A list of paths to types that should be treated as if they do not contain interi
 * [`mutable_key_type`](https://rust-lang.github.io/rust-clippy/master/index.html#mutable_key_type)
 
 
+## `initializer-suggestions`
+Suggestion behavior when initializers are present. Options are:
+
+- "none": do not suggest
+- "maybe-incorrect": suggest, but do not apply suggestions with `--fix`
+- "machine-applicable": suggest and apply suggestions with `--fix`
+
+The following example [due to @ronnodas] shows why "maybe-incorrect" may be the right choice.
+Swapping the fields in the constructor produces incompilable code:
+
+```rust
+struct MyStruct {
+    vector: Vec<u32>,
+    length: usize
+}
+fn main() {
+    let vector = vec![1,2,3];
+    MyStruct { length: vector.len(), vector};
+}
+```
+
+[due to @ronnodas]: https://github.com/rust-lang/rust-clippy/issues/11846#issuecomment-1820747924
+
+**Default Value:** `"none"`
+
+---
+**Affected lints:**
+* [`inconsistent_struct_constructor`](https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_struct_constructor)
+
+
 ## `large-error-threshold`
 The maximum size of the `Err`-variant in a `Result` returned from a function
 

clippy_config/src/conf.rs

@@ -1,8 +1,8 @@
 use crate::ClippyConfiguration;
 use crate::types::{
-    DisallowedPath, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour, Rename, SourceItemOrdering,
-    SourceItemOrderingCategory, SourceItemOrderingModuleItemGroupings, SourceItemOrderingModuleItemKind,
-    SourceItemOrderingTraitAssocItemKind, SourceItemOrderingTraitAssocItemKinds,
+    DisallowedPath, InitializerSuggestionApplicability, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour,
+    Rename, SourceItemOrdering, SourceItemOrderingCategory, SourceItemOrderingModuleItemGroupings,
+    SourceItemOrderingModuleItemKind, SourceItemOrderingTraitAssocItemKind, SourceItemOrderingTraitAssocItemKinds,
 };
 use clippy_utils::msrvs::Msrv;
 use rustc_errors::Applicability;
@@ -526,6 +526,29 @@ define_Conf! {
     /// A list of paths to types that should be treated as if they do not contain interior mutability
     #[lints(borrow_interior_mutable_const, declare_interior_mutable_const, ifs_same_cond, mutable_key_type)]
     ignore_interior_mutability: Vec<String> = Vec::from(["bytes::Bytes".into()]),
+    /// Suggestion behavior when initializers are present. Options are:
+    ///
+    /// - "none": do not suggest
+    /// - "maybe-incorrect": suggest, but do not apply suggestions with `--fix`
+    /// - "machine-applicable": suggest and apply suggestions with `--fix`
+    ///
+    /// The following example [due to @ronnodas] shows why "maybe-incorrect" may be the right choice.
+    /// Swapping the fields in the constructor produces incompilable code:
+    ///
+    /// ```rust
+    /// struct MyStruct {
+    ///     vector: Vec<u32>,
+    ///     length: usize
+    /// }
+    /// fn main() {
+    ///     let vector = vec![1,2,3];
+    ///     MyStruct { length: vector.len(), vector};
+    /// }
+    /// ```
+    ///
+    /// [due to @ronnodas]: https://github.com/rust-lang/rust-clippy/issues/11846#issuecomment-1820747924
+    #[lints(inconsistent_struct_constructor)]
+    initializer_suggestions: InitializerSuggestionApplicability = InitializerSuggestionApplicability::None,
     /// The maximum size of the `Err`-variant in a `Result` returned from a function
     #[lints(result_large_err)]
     large_error_threshold: u64 = 128,

clippy_config/src/types.rs

@@ -1,4 +1,5 @@
 use clippy_utils::def_path_def_ids;
+use rustc_errors::Applicability;
 use rustc_hir::def_id::DefIdMap;
 use rustc_middle::ty::TyCtxt;
 use serde::de::{self, Deserializer, Visitor};
@@ -46,6 +47,24 @@ pub fn create_disallowed_map(
         .collect()
 }
 
+#[derive(Clone, Copy, Deserialize, Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub enum InitializerSuggestionApplicability {
+    None,
+    MaybeIncorrect,
+    MachineApplicable,
+}
+
+impl InitializerSuggestionApplicability {
+    pub fn to_applicability(self) -> Option<Applicability> {
+        match self {
+            InitializerSuggestionApplicability::None => None,
+            InitializerSuggestionApplicability::MaybeIncorrect => Some(Applicability::MaybeIncorrect),
+            InitializerSuggestionApplicability::MachineApplicable => Some(Applicability::MachineApplicable),
+        }
+    }
+}
+
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)]
 pub enum MatchLintBehaviour {
     AllTypes,

clippy_lints/src/inconsistent_struct_constructor.rs

@@ -1,19 +1,21 @@
+use clippy_config::Conf;
+use clippy_config::types::InitializerSuggestionApplicability;
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::fulfill_or_allowed;
-use clippy_utils::source::snippet;
+use clippy_utils::source::{snippet, snippet_opt};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::Applicability;
 use rustc_hir::{self as hir, ExprKind, StructTailExpr};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_session::declare_lint_pass;
+use rustc_session::impl_lint_pass;
 use rustc_span::symbol::Symbol;
 use std::fmt::{self, Write as _};
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Checks for struct constructors where all fields are shorthand and
-    /// the order of the field init shorthand in the constructor is inconsistent
-    /// with the order in the struct definition.
+    /// Checks for struct constructors where the order of the field
+    /// init in the constructor is inconsistent with the order in the
+    /// struct definition.
     ///
     /// ### Why is this bad?
     /// Since the order of fields in a constructor doesn't affect the
@@ -59,16 +61,36 @@ declare_clippy_lint! {
     #[clippy::version = "1.52.0"]
     pub INCONSISTENT_STRUCT_CONSTRUCTOR,
     pedantic,
-    "the order of the field init shorthand is inconsistent with the order in the struct definition"
+    "the order of the field init is inconsistent with the order in the struct definition"
 }
 
-declare_lint_pass!(InconsistentStructConstructor => [INCONSISTENT_STRUCT_CONSTRUCTOR]);
+pub struct InconsistentStructConstructor {
+    initializer_suggestions: InitializerSuggestionApplicability,
+}
+
+impl InconsistentStructConstructor {
+    pub fn new(conf: &'static Conf) -> Self {
+        Self {
+            initializer_suggestions: conf.initializer_suggestions,
+        }
+    }
+}
+
+impl_lint_pass!(InconsistentStructConstructor => [INCONSISTENT_STRUCT_CONSTRUCTOR]);
 
 impl<'tcx> LateLintPass<'tcx> for InconsistentStructConstructor {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
-        if let ExprKind::Struct(qpath, fields, base) = expr.kind
-            && fields.iter().all(|f| f.is_shorthand)
-            && !expr.span.from_expansion()
+        let ExprKind::Struct(qpath, fields, base) = expr.kind else {
+            return;
+        };
+        let applicability = if fields.iter().all(|f| f.is_shorthand) {
+            Applicability::MachineApplicable
+        } else if let Some(applicability) = self.initializer_suggestions.to_applicability() {
+            applicability
+        } else {
+            return;
+        };
+        if !expr.span.from_expansion()
             && let ty = cx.typeck_results().expr_ty(expr)
             && let Some(adt_def) = ty.ty_adt_def()
             && adt_def.is_struct()
@@ -85,15 +107,15 @@ impl<'tcx> LateLintPass<'tcx> for InconsistentStructConstructor {
                 return;
             }
 
-            let mut ordered_fields: Vec<_> = fields.iter().map(|f| f.ident.name).collect();
-            ordered_fields.sort_unstable_by_key(|id| def_order_map[id]);
+            let mut ordered_fields: Vec<_> = fields.to_vec();
+            ordered_fields.sort_unstable_by_key(|id| def_order_map[&id.ident.name]);
 
             let mut fields_snippet = String::new();
-            let (last_ident, idents) = ordered_fields.split_last().unwrap();
-            for ident in idents {
-                let _: fmt::Result = write!(fields_snippet, "{ident}, ");
+            let (last_field, fields) = ordered_fields.split_last().unwrap();
+            for field in fields {
+                let _: fmt::Result = write!(fields_snippet, "{}, ", snippet_opt(cx, field.span).unwrap());
             }
-            fields_snippet.push_str(&last_ident.to_string());
+            fields_snippet.push_str(&snippet_opt(cx, last_field.span).unwrap());
 
             let base_snippet = if let StructTailExpr::Base(base) = base {
                 format!(", ..{}", snippet(cx, base.span, ".."))
@@ -114,7 +136,7 @@ impl<'tcx> LateLintPass<'tcx> for InconsistentStructConstructor {
                     "struct constructor field order is inconsistent with struct definition field order",
                     "try",
                     sugg,
-                    Applicability::MachineApplicable,
+                    applicability,
                 );
             }
         }

clippy_lints/src/lib.rs

@@ -648,7 +648,11 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(|_| Box::new(implicit_return::ImplicitReturn));
     store.register_late_pass(move |_| Box::new(implicit_saturating_sub::ImplicitSaturatingSub::new(conf)));
     store.register_late_pass(|_| Box::new(default_numeric_fallback::DefaultNumericFallback));
-    store.register_late_pass(|_| Box::new(inconsistent_struct_constructor::InconsistentStructConstructor));
+    store.register_late_pass(move |_| {
+        Box::new(inconsistent_struct_constructor::InconsistentStructConstructor::new(
+            conf,
+        ))
+    });
     store.register_late_pass(|_| Box::new(non_octal_unix_permissions::NonOctalUnixPermissions));
     store.register_early_pass(|| Box::new(unnecessary_self_imports::UnnecessarySelfImports));
     store.register_late_pass(move |_| Box::new(approx_const::ApproxConstant::new(conf)));

tests/ui-toml/toml_inconsistent_struct_constructor/clippy.toml

@@ -0,0 +1 @@
+initializer-suggestions = "machine-applicable"

tests/ui-toml/toml_inconsistent_struct_constructor/conf_inconsistent_struct_constructor.fixed

@@ -0,0 +1,21 @@
+#![warn(clippy::inconsistent_struct_constructor)]
+#![allow(clippy::redundant_field_names)]
+#![allow(clippy::unnecessary_operation)]
+#![allow(clippy::no_effect)]
+
+#[derive(Default)]
+struct Foo {
+    x: i32,
+    y: i32,
+    z: i32,
+}
+
+fn main() {
+    let x = 1;
+    let y = 1;
+    let z = 1;
+
+    Foo { x, y, z: z };
+
+    Foo { x, z: z, ..Default::default() };
+}

tests/ui-toml/toml_inconsistent_struct_constructor/conf_inconsistent_struct_constructor.rs

@@ -0,0 +1,25 @@
+#![warn(clippy::inconsistent_struct_constructor)]
+#![allow(clippy::redundant_field_names)]
+#![allow(clippy::unnecessary_operation)]
+#![allow(clippy::no_effect)]
+
+#[derive(Default)]
+struct Foo {
+    x: i32,
+    y: i32,
+    z: i32,
+}
+
+fn main() {
+    let x = 1;
+    let y = 1;
+    let z = 1;
+
+    Foo { y, x, z: z };
+
+    Foo {
+        z: z,
+        x,
+        ..Default::default()
+    };
+}

tests/ui-toml/toml_inconsistent_struct_constructor/conf_inconsistent_struct_constructor.stderr

@@ -0,0 +1,21 @@
+error: struct constructor field order is inconsistent with struct definition field order
+  --> tests/ui-toml/toml_inconsistent_struct_constructor/conf_inconsistent_struct_constructor.rs:18:5
+   |
+LL |     Foo { y, x, z: z };
+   |     ^^^^^^^^^^^^^^^^^^ help: try: `Foo { x, y, z: z }`
+   |
+   = note: `-D clippy::inconsistent-struct-constructor` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::inconsistent_struct_constructor)]`
+
+error: struct constructor field order is inconsistent with struct definition field order
+  --> tests/ui-toml/toml_inconsistent_struct_constructor/conf_inconsistent_struct_constructor.rs:20:5
+   |
+LL | /     Foo {
+LL | |         z: z,
+LL | |         x,
+LL | |         ..Default::default()
+LL | |     };
+   | |_____^ help: try: `Foo { x, z: z, ..Default::default() }`
+
+error: aborting due to 2 previous errors
+