Auto merge of #6945 - ThibsG:BadSuggestionGenericsForReassignDefault,…

… r=Manishearth Fix suggestion with generics for `field_reassign_with_default` lint Fix bad suggestion where `::` is missing after type if generics are involved Fixes #6944 changelog: none
rust-lang · Mar 21, 2021 · f3de78e · f3de78e
2 parents 0bdaa77 + 3ddaabc
commit f3de78e
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 1 deletion.
diff --git a/clippy_lints/src/default.rs b/clippy_lints/src/default.rs
@@ -104,6 +104,7 @@ impl LateLintPass<'_> for Default {
         }
     }
 
+    #[allow(clippy::too_many_lines)]
     fn check_block<'tcx>(&mut self, cx: &LateContext<'tcx>, block: &Block<'tcx>) {
         // start from the `let mut _ = _::default();` and look at all the following
         // statements, see if they re-assign the fields of the binding
@@ -197,6 +198,24 @@ impl LateLintPass<'_> for Default {
                     .collect::<Vec<String>>()
                     .join(", ");
 
+                // give correct suggestion if generics are involved (see #6944)
+                let binding_type = if_chain! {
+                    if let ty::Adt(adt_def, substs) = binding_type.kind();
+                    if !substs.is_empty();
+                    let adt_def_ty_name = cx.tcx.item_name(adt_def.did);
+                    let generic_args = substs.iter().collect::<Vec<_>>();
+                    let tys_str = generic_args
+                        .iter()
+                        .map(ToString::to_string)
+                        .collect::<Vec<_>>()
+                        .join(", ");
+                    then {
+                        format!("{}::<{}>", adt_def_ty_name, &tys_str)
+                    } else {
+                        binding_type.to_string()
+                    }
+                };
+
                 let sugg = if ext_with_default {
                     if field_list.is_empty() {
                         format!("{}::default()", binding_type)

diff --git a/tests/ui/field_reassign_with_default.rs b/tests/ui/field_reassign_with_default.rs
@@ -136,6 +136,13 @@ fn main() {
 
     // Don't lint in external macros
     field_reassign_with_default!();
+
+    // be sure suggestion is correct with generics
+    let mut a: Wrapper<bool> = Default::default();
+    a.i = true;
+
+    let mut a: WrapperMulti<i32, i64> = Default::default();
+    a.i = 42;
 }
 
 mod m {
@@ -145,3 +152,14 @@ mod m {
         b: u64,
     }
 }
+
+#[derive(Default)]
+struct Wrapper<T> {
+    i: T,
+}
+
+#[derive(Default)]
+struct WrapperMulti<T, U> {
+    i: T,
+    j: U,
+}
diff --git a/tests/ui/field_reassign_with_default.stderr b/tests/ui/field_reassign_with_default.stderr
@@ -83,5 +83,29 @@ note: consider initializing the variable with `C { i: vec![1], ..Default::defaul
 LL |     let mut a: C = C::default();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 7 previous errors
+error: field assignment outside of initializer for an instance created with Default::default()
+  --> $DIR/field_reassign_with_default.rs:142:5
+   |
+LL |     a.i = true;
+   |     ^^^^^^^^^^^
+   |
+note: consider initializing the variable with `Wrapper::<bool> { i: true }` and removing relevant reassignments
+  --> $DIR/field_reassign_with_default.rs:141:5
+   |
+LL |     let mut a: Wrapper<bool> = Default::default();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: field assignment outside of initializer for an instance created with Default::default()
+  --> $DIR/field_reassign_with_default.rs:145:5
+   |
+LL |     a.i = 42;
+   |     ^^^^^^^^^
+   |
+note: consider initializing the variable with `WrapperMulti::<i32, i64> { i: 42, ..Default::default() }` and removing relevant reassignments
+  --> $DIR/field_reassign_with_default.rs:144:5
+   |
+LL |     let mut a: WrapperMulti<i32, i64> = Default::default();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 9 previous errors