fix(new_without_default): if new has #[cfg], copy that onto impl Default

Author: ada4a

clippy_lints/src/new_without_default.rs

@@ -74,10 +74,8 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault {
             .filter_by_name_unhygienic(sym::new)
         {
             if let AssocKind::Fn { has_self: false, .. } = assoc_item.kind
-                && let impl_item = cx
-                    .tcx
-                    .hir_node_by_def_id(assoc_item.def_id.expect_local())
-                    .expect_impl_item()
+                && let assoc_item_hir_id = cx.tcx.local_def_id_to_hir_id(assoc_item.def_id.expect_local())
+                && let impl_item = cx.tcx.hir_node(assoc_item_hir_id).expect_impl_item()
                 && !impl_item.span.in_external_macro(cx.sess().source_map())
                 && let hir::ImplItemKind::Fn(ref sig, _) = impl_item.kind
                 && let id = impl_item.owner_id
@@ -120,6 +118,26 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault {
                 }
 
                 let mut app = Applicability::MachineApplicable;
+                let attrs_sugg = {
+                    let mut sugg = String::new();
+                    for attr in cx.tcx.hir_attrs(assoc_item_hir_id) {
+                        if !attr.has_name(sym::cfg_trace) {
+                            // This might be some other attribute that the `impl Default` ought to inherit.
+                            // But it could also be one of the many attributes that:
+                            // - can't be put on an impl block -- like `#[inline]`
+                            // - we can't even build a suggestion for, since `Attribute::span` may panic.
+                            //
+                            // Because of all that, remain on the safer side -- don't inherit this attr, and just
+                            // reduce the applicability
+                            app = Applicability::MaybeIncorrect;
+                            continue;
+                        }
+
+                        sugg.push_str(&snippet_with_applicability(cx.sess(), attr.span(), "_", &mut app));
+                        sugg.push('\n');
+                    }
+                    sugg
+                };
                 let generics_sugg = snippet_with_applicability(cx, generics.span, "", &mut app);
                 let where_clause_sugg = if generics.has_where_clause_predicates {
                     format!(
@@ -143,6 +161,7 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault {
                             item.span,
                             "try adding this",
                             &create_new_without_default_suggest_msg(
+                                &attrs_sugg,
                                 &self_type_snip,
                                 &generics_sugg,
                                 &where_clause_sugg,
@@ -157,13 +176,14 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault {
 }
 
 fn create_new_without_default_suggest_msg(
+    attrs_sugg: &str,
     self_type_snip: &str,
     generics_sugg: &str,
     where_clause_sugg: &str,
 ) -> String {
     #[rustfmt::skip]
     format!(
-"impl{generics_sugg} Default for {self_type_snip}{where_clause_sugg} {{
+"{attrs_sugg}impl{generics_sugg} Default for {self_type_snip}{where_clause_sugg} {{
     fn default() -> Self {{
         Self::new()
     }}

tests/ui/new_without_default.fixed

@@ -322,6 +322,74 @@ where
     }
 }
 
+// From issue #14552, but with `#[cfg]`s that are actually `true` in the uitest context
+
+pub struct NewWithCfg;
+#[cfg(not(test))]
+impl Default for NewWithCfg {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl NewWithCfg {
+    #[cfg(not(test))]
+    pub fn new() -> Self {
+        //~^ new_without_default
+        unimplemented!()
+    }
+}
+
+pub struct NewWith2Cfgs;
+#[cfg(not(test))]
+#[cfg(panic = "unwind")]
+impl Default for NewWith2Cfgs {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl NewWith2Cfgs {
+    #[cfg(not(test))]
+    #[cfg(panic = "unwind")]
+    pub fn new() -> Self {
+        //~^ new_without_default
+        unimplemented!()
+    }
+}
+
+pub struct NewWithExtraneous;
+impl Default for NewWithExtraneous {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl NewWithExtraneous {
+    #[inline]
+    pub fn new() -> Self {
+        //~^ new_without_default
+        unimplemented!()
+    }
+}
+
+pub struct NewWithCfgAndExtraneous;
+#[cfg(not(test))]
+impl Default for NewWithCfgAndExtraneous {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl NewWithCfgAndExtraneous {
+    #[cfg(not(test))]
+    #[inline]
+    pub fn new() -> Self {
+        //~^ new_without_default
+        unimplemented!()
+    }
+}
+
 mod issue15778 {
     pub struct Foo(Vec<i32>);
 

tests/ui/new_without_default.rs

@@ -265,6 +265,46 @@ where
     }
 }
 
+// From issue #14552, but with `#[cfg]`s that are actually `true` in the uitest context
+
+pub struct NewWithCfg;
+impl NewWithCfg {
+    #[cfg(not(test))]
+    pub fn new() -> Self {
+        //~^ new_without_default
+        unimplemented!()
+    }
+}
+
+pub struct NewWith2Cfgs;
+impl NewWith2Cfgs {
+    #[cfg(not(test))]
+    #[cfg(panic = "unwind")]
+    pub fn new() -> Self {
+        //~^ new_without_default
+        unimplemented!()
+    }
+}
+
+pub struct NewWithExtraneous;
+impl NewWithExtraneous {
+    #[inline]
+    pub fn new() -> Self {
+        //~^ new_without_default
+        unimplemented!()
+    }
+}
+
+pub struct NewWithCfgAndExtraneous;
+impl NewWithCfgAndExtraneous {
+    #[cfg(not(test))]
+    #[inline]
+    pub fn new() -> Self {
+        //~^ new_without_default
+        unimplemented!()
+    }
+}
+
 mod issue15778 {
     pub struct Foo(Vec<i32>);
 

tests/ui/new_without_default.stderr

@@ -174,5 +174,84 @@ LL +     }
 LL + }
    |
 
-error: aborting due to 9 previous errors
+error: you should consider adding a `Default` implementation for `NewWithCfg`
+  --> tests/ui/new_without_default.rs:273:5
+   |
+LL | /     pub fn new() -> Self {
+LL | |
+LL | |         unimplemented!()
+LL | |     }
+   | |_____^
+   |
+help: try adding this
+   |
+LL + #[cfg(not(test))]
+LL + impl Default for NewWithCfg {
+LL +     fn default() -> Self {
+LL +         Self::new()
+LL +     }
+LL + }
+LL | impl NewWithCfg {
+   |
+
+error: you should consider adding a `Default` implementation for `NewWith2Cfgs`
+  --> tests/ui/new_without_default.rs:283:5
+   |
+LL | /     pub fn new() -> Self {
+LL | |
+LL | |         unimplemented!()
+LL | |     }
+   | |_____^
+   |
+help: try adding this
+   |
+LL + #[cfg(not(test))]
+LL + #[cfg(panic = "unwind")]
+LL + impl Default for NewWith2Cfgs {
+LL +     fn default() -> Self {
+LL +         Self::new()
+LL +     }
+LL + }
+LL | impl NewWith2Cfgs {
+   |
+
+error: you should consider adding a `Default` implementation for `NewWithExtraneous`
+  --> tests/ui/new_without_default.rs:292:5
+   |
+LL | /     pub fn new() -> Self {
+LL | |
+LL | |         unimplemented!()
+LL | |     }
+   | |_____^
+   |
+help: try adding this
+   |
+LL + impl Default for NewWithExtraneous {
+LL +     fn default() -> Self {
+LL +         Self::new()
+LL +     }
+LL + }
+   |
+
+error: you should consider adding a `Default` implementation for `NewWithCfgAndExtraneous`
+  --> tests/ui/new_without_default.rs:302:5
+   |
+LL | /     pub fn new() -> Self {
+LL | |
+LL | |         unimplemented!()
+LL | |     }
+   | |_____^
+   |
+help: try adding this
+   |
+LL + #[cfg(not(test))]
+LL + impl Default for NewWithCfgAndExtraneous {
+LL +     fn default() -> Self {
+LL +         Self::new()
+LL +     }
+LL + }
+LL | impl NewWithCfgAndExtraneous {
+   |
+
+error: aborting due to 13 previous errors