about summary refs log tree commit diff
path: root/clippy_lints/src/inconsistent_struct_constructor.rs
diff options
context:
space:
mode:
authorPhilipp Krones <hello@philkrones.com>2025-01-09 18:00:37 +0100
committerPhilipp Krones <hello@philkrones.com>2025-01-09 18:00:37 +0100
commitb5bf09e57afa50a2ecc9bfc07bad4ef64d9df447 (patch)
treecf262aea51c147a0348a7112966198ea63fc0303 /clippy_lints/src/inconsistent_struct_constructor.rs
parent11f38ade90a2e1f9cee925260ffbe8bc0e3ad761 (diff)
parent894e87cd5160a2198940a35dc105ce6e46d9763e (diff)
downloadrust-b5bf09e57afa50a2ecc9bfc07bad4ef64d9df447.tar.gz
rust-b5bf09e57afa50a2ecc9bfc07bad4ef64d9df447.zip
Merge remote-tracking branch 'upstream/master' into rustup
Diffstat (limited to 'clippy_lints/src/inconsistent_struct_constructor.rs')
-rw-r--r--clippy_lints/src/inconsistent_struct_constructor.rs127
1 files changed, 90 insertions, 37 deletions
diff --git a/clippy_lints/src/inconsistent_struct_constructor.rs b/clippy_lints/src/inconsistent_struct_constructor.rs
index 4fcd2abb769..39ff3c13bcc 100644
--- a/clippy_lints/src/inconsistent_struct_constructor.rs
+++ b/clippy_lints/src/inconsistent_struct_constructor.rs
@@ -1,19 +1,21 @@
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_config::Conf;
+use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::fulfill_or_allowed;
 use clippy_utils::source::snippet;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::Applicability;
-use rustc_hir::{self as hir, ExprKind, StructTailExpr};
+use rustc_hir::{self as hir, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_session::declare_lint_pass;
+use rustc_middle::ty::TyCtxt;
+use rustc_session::impl_lint_pass;
+use rustc_span::Span;
 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,37 @@ 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 {
+    lint_inconsistent_struct_field_initializers: bool,
+}
+
+impl InconsistentStructConstructor {
+    pub fn new(conf: &'static Conf) -> Self {
+        Self {
+            lint_inconsistent_struct_field_initializers: conf.lint_inconsistent_struct_field_initializers,
+        }
+    }
+}
+
+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(_, fields, _) = expr.kind else {
+            return;
+        };
+        let all_fields_are_shorthand = fields.iter().all(|f| f.is_shorthand);
+        let applicability = if all_fields_are_shorthand {
+            Applicability::MachineApplicable
+        } else if self.lint_inconsistent_struct_field_initializers {
+            Applicability::MaybeIncorrect
+        } 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,36 +108,24 @@ 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 fields_snippet = String::new();
-            let (last_ident, idents) = ordered_fields.split_last().unwrap();
-            for ident in idents {
-                let _: fmt::Result = write!(fields_snippet, "{ident}, ");
-            }
-            fields_snippet.push_str(&last_ident.to_string());
-
-            let base_snippet = if let StructTailExpr::Base(base) = base {
-                format!(", ..{}", snippet(cx, base.span, ".."))
-            } else {
-                String::new()
-            };
-
-            let sugg = format!(
-                "{} {{ {fields_snippet}{base_snippet} }}",
-                snippet(cx, qpath.span(), ".."),
-            );
+            let span = field_with_attrs_span(cx.tcx, fields.first().unwrap())
+                .with_hi(field_with_attrs_span(cx.tcx, fields.last().unwrap()).hi());
 
             if !fulfill_or_allowed(cx, INCONSISTENT_STRUCT_CONSTRUCTOR, Some(ty_hir_id)) {
-                span_lint_and_sugg(
+                span_lint_and_then(
                     cx,
                     INCONSISTENT_STRUCT_CONSTRUCTOR,
-                    expr.span,
+                    span,
                     "struct constructor field order is inconsistent with struct definition field order",
-                    "try",
-                    sugg,
-                    Applicability::MachineApplicable,
+                    |diag| {
+                        let msg = if all_fields_are_shorthand {
+                            "try"
+                        } else {
+                            "if the field evaluation order doesn't matter, try"
+                        };
+                        let sugg = suggestion(cx, fields, &def_order_map);
+                        diag.span_suggestion(span, msg, sugg, applicability);
+                    },
                 );
             }
         }
@@ -135,3 +146,45 @@ fn is_consistent_order<'tcx>(fields: &'tcx [hir::ExprField<'tcx>], def_order_map
 
     true
 }
+
+fn suggestion<'tcx>(
+    cx: &LateContext<'_>,
+    fields: &'tcx [hir::ExprField<'tcx>],
+    def_order_map: &FxHashMap<Symbol, usize>,
+) -> String {
+    let ws = fields
+        .windows(2)
+        .map(|w| {
+            let w0_span = field_with_attrs_span(cx.tcx, &w[0]);
+            let w1_span = field_with_attrs_span(cx.tcx, &w[1]);
+            let span = w0_span.between(w1_span);
+            snippet(cx, span, " ")
+        })
+        .collect::<Vec<_>>();
+
+    let mut fields = fields.to_vec();
+    fields.sort_unstable_by_key(|field| def_order_map[&field.ident.name]);
+    let field_snippets = fields
+        .iter()
+        .map(|field| snippet(cx, field_with_attrs_span(cx.tcx, field), ".."))
+        .collect::<Vec<_>>();
+
+    assert_eq!(field_snippets.len(), ws.len() + 1);
+
+    let mut sugg = String::new();
+    for i in 0..field_snippets.len() {
+        sugg += &field_snippets[i];
+        if i < ws.len() {
+            sugg += &ws[i];
+        }
+    }
+    sugg
+}
+
+fn field_with_attrs_span(tcx: TyCtxt<'_>, field: &hir::ExprField<'_>) -> Span {
+    if let Some(attr) = tcx.hir().attrs(field.hir_id).first() {
+        field.span.with_lo(attr.span.lo())
+    } else {
+        field.span
+    }
+}