add lint for recreation of an entire struct

This lint makes Clippy warn about situations where an owned
struct is essentially recreated by moving all its fields into a
new instance of the struct.

Clippy will suggest to simply use the original struct. The lint
is not machine-applicable because the source struct may have been
partially moved.

Author: phi-gamma

CHANGELOG.md

@@ -5692,6 +5692,7 @@ Released 2018-09-13
 [`redundant_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names
 [`redundant_guards`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_guards
 [`redundant_locals`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_locals
+[`redundant_owned_struct_recreation`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_owned_struct_recreation
 [`redundant_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern
 [`redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching
 [`redundant_pub_crate`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pub_crate

clippy_lints/src/declared_lints.rs

@@ -619,6 +619,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::redundant_else::REDUNDANT_ELSE_INFO,
     crate::redundant_field_names::REDUNDANT_FIELD_NAMES_INFO,
     crate::redundant_locals::REDUNDANT_LOCALS_INFO,
+    crate::redundant_owned_struct_recreation::REDUNDANT_OWNED_STRUCT_RECREATION_INFO,
     crate::redundant_pub_crate::REDUNDANT_PUB_CRATE_INFO,
     crate::redundant_slicing::DEREF_BY_SLICING_INFO,
     crate::redundant_slicing::REDUNDANT_SLICING_INFO,

clippy_lints/src/lib.rs

@@ -298,6 +298,7 @@ mod redundant_closure_call;
 mod redundant_else;
 mod redundant_field_names;
 mod redundant_locals;
+mod redundant_owned_struct_recreation;
 mod redundant_pub_crate;
 mod redundant_slicing;
 mod redundant_static_lifetimes;
@@ -706,6 +707,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static 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(|_| Box::new(non_octal_unix_permissions::NonOctalUnixPermissions));
+    store.register_late_pass(|_| Box::new(redundant_owned_struct_recreation::RedundantOwnedStructRecreation));
     store.register_early_pass(|| Box::new(unnecessary_self_imports::UnnecessarySelfImports));
     store.register_late_pass(move |_| Box::new(approx_const::ApproxConstant::new(msrv())));
     let format_args = format_args_storage.clone();

clippy_lints/src/redundant_owned_struct_recreation.rs

@@ -0,0 +1,115 @@
+#![allow(unused)]
+use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
+use clippy_utils::source::snippet;
+use rustc_data_structures::fx::FxHashMap;
+use rustc_errors::Applicability;
+
+use rustc_hir::{self as hir, ExprKind, Path, PathSegment, QPath};
+
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::declare_lint_pass;
+use std::fmt::{self, Write as _};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for struct literals that recreate a struct we already have an owned
+    /// version of.
+    ///
+    /// ### Why is this bad?
+    /// This is essentially a no-op as all members are being moved into the new
+    /// struct and the old one becomes inaccessible.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// fn redundant () {
+    ///     struct Foo { a: i32, b: String }
+    ///
+    ///     let foo = Foo { a: 42, b: "Hello, Foo!".into() };
+    ///
+    ///     let bar = Foo { a: foo.a, b: foo.b };
+    ///
+    ///     println!("redundant: bar = Foo {{ a: {}, b: \"{}\" }}", bar.a, bar.b);
+    /// }
+    /// ```
+    ///
+    /// The struct literal in the assignment to ``bar`` could be replaced with
+    /// ``let bar = foo``.
+    ///
+    /// Empty structs are ignored by the lint.
+    #[clippy::version = "pre 1.29.0"]
+    pub REDUNDANT_OWNED_STRUCT_RECREATION,
+    //pedantic,
+    correctness,
+    "recreation of owned structs from identical parts"
+}
+
+declare_lint_pass!(RedundantOwnedStructRecreation => [REDUNDANT_OWNED_STRUCT_RECREATION]);
+
+impl<'tcx> LateLintPass<'tcx> for RedundantOwnedStructRecreation {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
+        if !expr.span.from_expansion()
+            && let ExprKind::Struct(qpath, fields, base) = expr.kind
+        {
+            let ty = cx.typeck_results().expr_ty(expr);
+
+            // Conditions that must be satisfied to trigger the lint:
+            // - source of the assignment must be a struct
+            // - type of struct in expr must match
+            // - names of assignee struct fields (lhs) must match the names of the assigned on (rhs)
+            // - all fields musts be assigned (no Default)
+
+            if base.is_some() {
+                // one or more fields assigned from `..Default`
+                return;
+            }
+
+            let matching = fields.iter().filter(|f| !f.is_shorthand).try_fold(
+                None,
+                |seen_path, f| -> Result<Option<&'tcx Path<'tcx>>, ()> {
+                    // fields are assigned from expression
+                    if let ExprKind::Field(expr, ident) = f.expr.kind
+                        // expression type matches
+                        && ty == cx.typeck_results().expr_ty(expr)
+                        // field name matches
+                        && f.ident == ident
+                        && let ExprKind::Path(QPath::Resolved(None, path)) = expr.kind
+                    {
+                        match seen_path {
+                            None => {
+                                // first field assignment
+                                Ok(Some(path))
+                            },
+                            Some(p) if p.res == path.res => {
+                                // subsequent field assignment, same origin struct as before
+                                Ok(seen_path)
+                            },
+                            Some(_) => {
+                                // subsequent field assignment, different origin struct as before
+                                Err(())
+                            },
+                        }
+                    } else {
+                        // source of field assignment not an expression
+                        Err(())
+                    }
+                },
+            );
+
+            let Ok(Some(path)) = matching else { return };
+
+            let sugg = format!("{}", snippet(cx, path.span, ".."),);
+
+            span_lint_and_sugg(
+                cx,
+                REDUNDANT_OWNED_STRUCT_RECREATION,
+                expr.span,
+                "recreation of owned struct from identical parts",
+                "try replacing it with",
+                sugg,
+                // the lint may be incomplete due to partial moves
+                // of struct fields
+                Applicability::MaybeIncorrect,
+            );
+        }
+    }
+}

tests/ui/redundant_owned_struct_recreation.rs

@@ -0,0 +1,68 @@
+//@no-rustfix
+#![warn(clippy::redundant_owned_struct_recreation)]
+#![allow(dead_code, unused_variables)]
+
+fn recreate() {
+    struct Recreated<'s> {
+        a: i32,
+        b: &'s str,
+    }
+
+    fn recreate_immediate(r: Recreated) -> Recreated {
+        Recreated { a: r.a, b: r.b }
+    }
+
+    fn recreate_through_binding(r: Recreated) -> Recreated {
+        let b = r.b;
+        Recreated { a: r.a, b }
+    }
+
+    let b = String::from("Answer");
+    let r = recreate_immediate(Recreated { a: 42, b: &b });
+    let r = recreate_through_binding(Recreated { a: 42, b: &b });
+}
+
+struct One;
+struct Two;
+struct Three {
+    one: One,
+    two: Two,
+}
+
+fn recreate_immediate(xy: Three) -> Three {
+    Three {
+        one: xy.one,
+        two: xy.two,
+    }
+}
+
+fn recreate_through_binding(xy: Three) -> Three {
+    let two = xy.two;
+    // Here ``xy`` is subject to a partial move so we cannot
+    // emit a machine applicable fixup suggestion.
+    Three { one: xy.one, two }
+}
+
+impl Three {
+    fn recreate_through_member(self) -> Self {
+        Self {
+            one: self.one,
+            two: self.two,
+        }
+    }
+}
+
+fn ignore_struct_with_default() {
+    #[derive(Default)]
+    struct WithDefault {
+        a: usize,
+        b: u8,
+    }
+
+    let one = WithDefault { a: 1337usize, b: 42u8 };
+    // This *must not* trigger the lint.
+    let two = WithDefault {
+        a: one.a,
+        ..Default::default()
+    };
+}

tests/ui/redundant_owned_struct_recreation.stderr

@@ -0,0 +1,41 @@
+error: recreation of owned struct from identical parts
+  --> tests/ui/redundant_owned_struct_recreation.rs:12:9
+   |
+LL |         Recreated { a: r.a, b: r.b }
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing it with: `r`
+   |
+   = note: `-D clippy::redundant-owned-struct-recreation` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::redundant_owned_struct_recreation)]`
+
+error: recreation of owned struct from identical parts
+  --> tests/ui/redundant_owned_struct_recreation.rs:17:9
+   |
+LL |         Recreated { a: r.a, b }
+   |         ^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing it with: `r`
+
+error: recreation of owned struct from identical parts
+  --> tests/ui/redundant_owned_struct_recreation.rs:33:5
+   |
+LL | /     Three {
+LL | |         one: xy.one,
+LL | |         two: xy.two,
+LL | |     }
+   | |_____^ help: try replacing it with: `xy`
+
+error: recreation of owned struct from identical parts
+  --> tests/ui/redundant_owned_struct_recreation.rs:43:5
+   |
+LL |     Three { one: xy.one, two }
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing it with: `xy`
+
+error: recreation of owned struct from identical parts
+  --> tests/ui/redundant_owned_struct_recreation.rs:48:9
+   |
+LL | /         Self {
+LL | |             one: self.one,
+LL | |             two: self.two,
+LL | |         }
+   | |_________^ help: try replacing it with: `self`
+
+error: aborting due to 5 previous errors
+