New lint: explicit_struct_update

CHANGELOG.md

@@ -5786,6 +5786,7 @@ Released 2018-09-13
 [`explicit_deref_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_deref_methods
 [`explicit_into_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_into_iter_loop
 [`explicit_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_iter_loop
+[`explicit_struct_update`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_struct_update
 [`explicit_write`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_write
 [`extend_from_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#extend_from_slice
 [`extend_with_drain`]: https://rust-lang.github.io/rust-clippy/master/index.html#extend_with_drain

clippy_lints/src/declared_lints.rs

@@ -159,6 +159,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::exhaustive_items::EXHAUSTIVE_ENUMS_INFO,
     crate::exhaustive_items::EXHAUSTIVE_STRUCTS_INFO,
     crate::exit::EXIT_INFO,
+    crate::explicit_struct_update::EXPLICIT_STRUCT_UPDATE_INFO,
     crate::explicit_write::EXPLICIT_WRITE_INFO,
     crate::extra_unused_type_parameters::EXTRA_UNUSED_TYPE_PARAMETERS_INFO,
     crate::fallible_impl_from::FALLIBLE_IMPL_FROM_INFO,

clippy_lints/src/explicit_struct_update.rs

@@ -0,0 +1,147 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::{snippet, snippet_indent};
+use rustc_errors::Applicability;
+use rustc_hir::{self as hir, ExprKind, StructTailExpr};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::declare_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for struct initializations where two or more fields are being set to the value of the same field from another struct of the same type.
+    /// ### Why is this bad?
+    /// This can be done more concisely using struct update syntax.
+    /// ### Example
+    /// ```no_run
+    /// struct Foo {
+    ///     a: i32,
+    ///     b: i32,
+    ///     c: i32,
+    /// }
+    ///
+    /// let my_foo = Foo {
+    ///     a: 1,
+    ///     b: 2,
+    ///     c: 3,
+    /// };
+    ///
+    /// let my_new_foo = Foo {
+    ///     a: 5,
+    ///     b: my_foo.b,
+    ///     c: my_foo.c,
+    /// };
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// struct Foo {
+    ///     a: i32,
+    ///     b: i32,
+    ///     c: i32,
+    /// }
+    ///
+    /// let my_foo = Foo {
+    ///     a: 1,
+    ///     b: 2,
+    ///     c: 3,
+    /// };
+    ///
+    /// let my_new_foo = Foo {
+    ///     a: 5,
+    ///     ..my_foo
+    /// };
+    /// ```
+    #[clippy::version = "1.89.0"]
+    pub EXPLICIT_STRUCT_UPDATE,
+    complexity,
+    "explicit struct updates in struct instantiations"
+}
+declare_lint_pass!(ExplicitStructUpdate => [EXPLICIT_STRUCT_UPDATE]);
+
+impl<'tcx> LateLintPass<'tcx> for ExplicitStructUpdate {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
+        if hir::is_range_literal(expr) {
+            // range literals are lowered to structs, false positive
+            return;
+        }
+
+        let ExprKind::Struct(path, fields, StructTailExpr::None) = expr.kind else {
+            return;
+        };
+
+        // the type of the struct
+        let ty = cx.typeck_results().expr_ty(expr);
+
+        // collect the fields that are being initialized with the same field from another struct of the same
+        // type
+        let update_fields = fields.iter().try_fold(
+            Vec::new(),
+            |mut acc: Vec<(&rustc_hir::Expr<'_>, &rustc_hir::Expr<'_>)>, f| {
+                if let ExprKind::Field(base_expr, field_ident) = f.expr.kind {
+                    if let Some(last) = acc.last() {
+                        match (last.1.kind, base_expr.kind) {
+                            (
+                                ExprKind::Path(hir::QPath::Resolved(_, hir::Path { res: res_a, .. })),
+                                ExprKind::Path(hir::QPath::Resolved(_, hir::Path { res: res_b, .. })),
+                            ) if res_a != res_b => return None, /* if we detect instantiation from multiple bases, we */
+                            // don't want to lint
+                            _ => (),
+                        }
+                    }
+
+                    if cx.typeck_results().expr_ty(base_expr) == ty && f.ident == field_ident {
+                        // accumulate the expressions mapping to the actual field expression, and the expression of the
+                        // base struct, we do this so we can determine if the base struct is the same for all
+                        acc.push((f.expr, base_expr));
+                    }
+                }
+
+                Some(acc)
+            },
+        );
+
+        let (update_base, update_fields): (_, Vec<_>) = match update_fields {
+            // we only care about the field expressions at this point
+            Some(fields) if fields.len() > 1 => (fields[0].1, fields.iter().map(|x| x.0.hir_id).collect()),
+            // no lint if there's no fields or multiple bases
+            _ => return,
+        };
+
+        // the field assignments we are keeping
+        let non_update_fields_spans: Vec<_> = fields
+            .iter()
+            .filter_map(|f| {
+                if update_fields.contains(&f.expr.hir_id) {
+                    None
+                } else {
+                    Some(f.span)
+                }
+            })
+            .collect();
+
+        let struct_indent = snippet_indent(cx, expr.span).unwrap_or_default();
+        let field_indent = format!("{struct_indent}    ");
+        let struct_type = snippet(cx, path.span(), "");
+        let struct_fields = non_update_fields_spans
+            .iter()
+            .fold(String::new(), |mut acc, &field_span| {
+                acc.push_str(&field_indent);
+                acc.push_str(&snippet(cx, field_span, ""));
+                acc.push_str(",\n");
+                acc
+            });
+        let struct_update_snip = snippet(cx, update_base.span, "");
+
+        let sugg = format!("{struct_type} {{\n{struct_fields}{field_indent}..{struct_update_snip}\n{struct_indent}}}");
+
+        let msg = "you seem to be updating a struct field with the same field from another struct of the same type";
+
+        span_lint_and_sugg(
+            cx,
+            EXPLICIT_STRUCT_UPDATE,
+            expr.span,
+            msg,
+            "consider using struct update syntax instead",
+            sugg,
+            Applicability::MachineApplicable,
+        );
+    }
+}

clippy_lints/src/lib.rs

@@ -138,6 +138,7 @@ mod excessive_bools;
 mod excessive_nesting;
 mod exhaustive_items;
 mod exit;
+mod explicit_struct_update;
 mod explicit_write;
 mod extra_unused_type_parameters;
 mod fallible_impl_from;
@@ -830,5 +831,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
     store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf)));
     store.register_late_pass(|_| Box::new(infallible_try_from::InfallibleTryFrom));
     store.register_late_pass(|_| Box::new(coerce_container_to_any::CoerceContainerToAny));
+    store.register_late_pass(|_| Box::new(explicit_struct_update::ExplicitStructUpdate));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

clippy_lints/src/manual_clamp.rs

@@ -451,12 +451,7 @@ fn is_match_pattern<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Opt
             Some((*first_hir_id, *second_hir_id)),
         ) {
             return Some(ClampSuggestion {
-                params: InputMinMax {
-                    input: value,
-                    min: params.min,
-                    max: params.max,
-                    is_float: params.is_float,
-                },
+                params: InputMinMax { input: value, ..params },
                 span: expr.span,
                 make_assignment: None,
                 hir_with_ignore_attr: None,
@@ -510,9 +505,7 @@ fn is_two_if_pattern<'tcx>(cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>) ->
                 Some(ClampSuggestion {
                     params: InputMinMax {
                         input: maybe_input_first_path,
-                        min: input_min_max.min,
-                        max: input_min_max.max,
-                        is_float: input_min_max.is_float,
+                        ..input_min_max
                     },
                     span: first_expr.span.to(second_expr.span),
                     make_assignment: Some(maybe_input_first_path),

clippy_utils/src/source.rs

@@ -733,8 +733,7 @@ pub fn trim_span(sm: &SourceMap, span: Span) -> Span {
     SpanData {
         lo: data.lo + BytePos::from_usize(trim_start),
         hi: data.hi - BytePos::from_usize(trim_end),
-        ctxt: data.ctxt,
-        parent: data.parent,
+        ..data
     }
     .span()
 }

tests/ui/explicit_struct_update.fixed

@@ -0,0 +1,74 @@
+#![warn(clippy::explicit_struct_update)]
+
+struct A {
+    a: i32,
+    b: i32,
+    c: i32,
+    d: i32,
+}
+
+struct B;
+
+struct C {
+    a: i32,
+    b: i32,
+}
+
+fn main() {
+    // should not lint, no explicit struct update
+    let a = A { a: 1, b: 2, c: 3, d: 4 };
+
+    let b = A {
+        ..a
+    };
+    //~^^^^^^explicit_struct_update
+
+    let c = A {
+        c: 4,
+        d: 5,
+        ..a
+    };
+    //~^^^^^^explicit_struct_update
+
+    let d = A {
+        d: 5,
+        ..a
+    };
+    //~^^^^^^explicit_struct_update
+
+    // should not lint, only one field is updated
+    let e = A {
+        a: a.a,
+        b: 5,
+        c: 6,
+        d: 7,
+    };
+
+    // should not lint, we already have update syntax
+    let f = A { ..a };
+
+    // should not lint, we already have update syntax
+    let g = A { a: a.a, b: a.b, ..a };
+
+    // should not lint, multiple bases
+    let h = A {
+        a: a.a,
+        b: d.b,
+        c: d.c,
+        d: 5,
+    };
+
+    // should not lint, no fields
+    let i = B {};
+
+    // should not lint, no explicit struct update
+    let j = C { a: 1, b: 2 };
+
+    // should not lint, fields filled from different type
+    let k = A {
+        a: j.a,
+        b: j.b,
+        c: 3,
+        d: 4,
+    };
+}

tests/ui/explicit_struct_update.rs

@@ -0,0 +1,80 @@
+#![warn(clippy::explicit_struct_update)]
+
+struct A {
+    a: i32,
+    b: i32,
+    c: i32,
+    d: i32,
+}
+
+struct B;
+
+struct C {
+    a: i32,
+    b: i32,
+}
+
+fn main() {
+    // should not lint, no explicit struct update
+    let a = A { a: 1, b: 2, c: 3, d: 4 };
+
+    let b = A {
+        a: a.a,
+        b: a.b,
+        c: a.c,
+        d: a.d,
+    };
+    //~^^^^^^explicit_struct_update
+
+    let c = A {
+        a: a.a,
+        b: a.b,
+        c: 4,
+        d: 5,
+    };
+    //~^^^^^^explicit_struct_update
+
+    let d = A {
+        a: a.a,
+        b: a.b,
+        c: a.c,
+        d: 5,
+    };
+    //~^^^^^^explicit_struct_update
+
+    // should not lint, only one field is updated
+    let e = A {
+        a: a.a,
+        b: 5,
+        c: 6,
+        d: 7,
+    };
+
+    // should not lint, we already have update syntax
+    let f = A { ..a };
+
+    // should not lint, we already have update syntax
+    let g = A { a: a.a, b: a.b, ..a };
+
+    // should not lint, multiple bases
+    let h = A {
+        a: a.a,
+        b: d.b,
+        c: d.c,
+        d: 5,
+    };
+
+    // should not lint, no fields
+    let i = B {};
+
+    // should not lint, no explicit struct update
+    let j = C { a: 1, b: 2 };
+
+    // should not lint, fields filled from different type
+    let k = A {
+        a: j.a,
+        b: j.b,
+        c: 3,
+        d: 4,
+    };
+}