Only add ..Default::default() to suggestion if needed

Author: Henri Lunnikivi

clippy_lints/src/field_reassign_with_default.rs

@@ -4,6 +4,7 @@ use rustc_hir::def::Res;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::{Block, Expr, ExprKind, PatKind, QPath, Stmt, StmtKind};
 use rustc_span::symbol::{Ident, Symbol};
+use rustc_middle::ty::{Adt, TyS};
 
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -44,11 +45,11 @@ impl LateLintPass<'_> for FieldReassignWithDefault {
         // find all binding statements like `let mut _ = T::default()` where `T::default()` is the
         // `default` method of the `Default` trait. and store statement index in current block being
         // checked and the name of the bound variable
-        let binding_statements_using_default: Vec<(usize, Symbol)> = enumerate_bindings_using_default(cx, block);
+        let binding_statements_using_default: Vec<(usize, Symbol, &TyS<'_>)> = enumerate_bindings_using_default(cx, block);
 
         // start from the `let mut _ = _::default();` and look at all the following
         // statements, see if they re-assign the fields of the binding
-        for (stmt_idx, binding_name) in binding_statements_using_default {
+        for (stmt_idx, binding_name, binding_type) in binding_statements_using_default {
             // last statement of block cannot trigger the lint
             if stmt_idx == block.stmts.len() - 1 {
                 break;
@@ -71,9 +72,9 @@ impl LateLintPass<'_> for FieldReassignWithDefault {
                     if let Some((field_ident, assign_rhs)) = field_reassigned_by_stmt(consequtive_statement, &binding_name) {
                         // extract and store the assigned value for help message
                         let value_snippet = snippet(cx, assign_rhs.span, "..");
-                        if !assigned_fields.contains_key(&field_ident.name) {
-                            assigned_fields.insert(field_ident.name, value_snippet);
-                        }
+
+                        // always re-insert set value, this way the latest value is stored for output snippet
+                        assigned_fields.insert(field_ident.name, value_snippet);
 
                         // also set first instance of error for help message
                         if first_assign.is_none() {
@@ -88,8 +89,13 @@ impl LateLintPass<'_> for FieldReassignWithDefault {
             if !assigned_fields.is_empty() {
                 // take the original assignment as span
                 let stmt = &block.stmts[stmt_idx];
+
                 if let StmtKind::Local(preceding_local) = &stmt.kind {
                     if let Some(ty) = &preceding_local.ty {
+
+                        // if all fields of the struct are not assigned, add `.. Default::default()` to the suggestion.
+                        let ext_with_default = !fields_of_type(&binding_type).iter().all(|field| assigned_fields.contains_key(&field.name));
+
                         let ty_snippet = snippet(cx, ty.span, "_");
 
                         let field_list = assigned_fields
@@ -98,7 +104,11 @@ impl LateLintPass<'_> for FieldReassignWithDefault {
                             .collect::<Vec<String>>()
                             .join(", ");
 
-                        let sugg = format!("{} {{ {}, ..Default::default() }}", ty_snippet, field_list);
+                        let sugg = if ext_with_default {
+                            format!("{} {{ {}, ..Default::default() }}", ty_snippet, field_list)
+                        } else {
+                            format!("{} {{ {} }}", ty_snippet, field_list)
+                        };
 
                         // span lint once per statement that binds default
                         span_lint_and_note(
@@ -109,14 +119,16 @@ impl LateLintPass<'_> for FieldReassignWithDefault {
                             Some(preceding_local.span),
                             &format!("consider initializing the variable with `{}`", sugg),
                         );
+
                     }
                 }
             }
         }
     }
 }
 
-fn enumerate_bindings_using_default(cx: &LateContext<'_>, block: &Block<'_>) -> Vec<(usize, Symbol)> {
+/// Returns the indices, identifiers and types of bindings set as `Default::default()`.
+fn enumerate_bindings_using_default<'cx, 'hir>(cx: &LateContext<'cx>, block: &Block<'hir>) -> Vec<(usize, Symbol, &'cx TyS<'cx>)> {
     block
         .stmts
         .iter()
@@ -135,7 +147,9 @@ fn enumerate_bindings_using_default(cx: &LateContext<'_>, block: &Block<'_>) ->
                 // right hand side of assignment is `Default::default`
                 if match_def_path(cx, def_id, &paths::DEFAULT_TRAIT_METHOD);
                 then {
-                    Some((idx, ident.name))
+                    // Get the type of the pattern
+                    let ty = cx.typeck_results().pat_ty(local.pat);
+                    Some((idx, ident.name, ty))
                 }
                 else {
                     None
@@ -173,3 +187,15 @@ fn field_reassigned_by_stmt<'hir>(this: &Stmt<'hir>, binding_name: &Symbol) -> O
         }
     }
 }
+
+/// Returns the vec of fields for a struct and an empty vec for non-struct ADTs.
+fn fields_of_type<'a>(ty: &'a TyS<'_>) -> Vec<Ident> {
+    if let Adt(adt, _) = ty.kind() {
+        if adt.is_struct() {
+            // unwrap is safe, because this is a struct and structs have only one variant
+            let variant = &adt.variants.get(0usize.into()).unwrap();
+            return variant.fields.iter().map(|f| f.ident).collect();
+        }
+    }
+    vec![]
+}

tests/ui/field_reassign_with_default.stderr

@@ -17,7 +17,7 @@ error: field assignment outside of initializer for an instance created with Defa
 LL |     a.i = 42;
    |     ^^^^^^^^^
    |
-note: consider initializing the variable with `A { i: 42, j: 43, ..Default::default() }`
+note: consider initializing the variable with `A { i: 42, j: 43 }`
   --> $DIR/field_reassign_with_default.rs:56:5
    |
 LL |     let mut a: A = Default::default();
@@ -29,7 +29,7 @@ error: field assignment outside of initializer for an instance created with Defa
 LL |     a.i = 42;
    |     ^^^^^^^^^
    |
-note: consider initializing the variable with `A { i: 42, j: 43, ..Default::default() }`
+note: consider initializing the variable with `A { i: 42, j: 44 }`
   --> $DIR/field_reassign_with_default.rs:61:5
    |
 LL |     let mut a: A = Default::default();