Skip to content

Commit ae2ebd6

Browse files
committed
fix(new_without_default): if new has #[cfg], copy that onto impl Default
1 parent f677434 commit ae2ebd6

File tree

4 files changed

+75
-6
lines changed

4 files changed

+75
-6
lines changed

clippy_lints/src/new_without_default.rs

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,8 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault {
7474
.filter_by_name_unhygienic(sym::new)
7575
{
7676
if let AssocKind::Fn { has_self: false, .. } = assoc_item.kind
77-
&& let impl_item = cx
78-
.tcx
79-
.hir_node_by_def_id(assoc_item.def_id.expect_local())
80-
.expect_impl_item()
77+
&& let assoc_item_hir_id = cx.tcx.local_def_id_to_hir_id(assoc_item.def_id.expect_local())
78+
&& let impl_item = cx.tcx.hir_node(assoc_item_hir_id).expect_impl_item()
8179
&& !impl_item.span.in_external_macro(cx.sess().source_map())
8280
&& let hir::ImplItemKind::Fn(ref sig, _) = impl_item.kind
8381
&& let id = impl_item.owner_id
@@ -120,6 +118,24 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault {
120118
}
121119

122120
let mut appl = Applicability::MachineApplicable;
121+
let attrs_sugg = match cx.tcx.hir_attrs(assoc_item_hir_id) {
122+
[] => String::new(),
123+
[attr] if attr.has_name(sym::cfg_trace) => {
124+
format!(
125+
"{}\n",
126+
snippet_with_applicability(cx.sess(), attr.span(), "_", &mut appl)
127+
)
128+
},
129+
_ => {
130+
// There might be some other attributes which the `impl Default` ought to inherit from `fn new`.
131+
// At the same time, there are many attributes that actually can't be put on an impl block --
132+
// like `#[inline]`, for example. And for some attributes we can't even build a suggestion,
133+
// since `Attribute::span` may panic. Because of all this, remain conservative: don't inherit
134+
// any attrs, and just reduce the applicability
135+
appl = Applicability::MaybeIncorrect;
136+
String::new()
137+
},
138+
};
123139
let generics_sugg = snippet_with_applicability(cx, generics.span, "", &mut appl);
124140
let where_clause_sugg = if generics.has_where_clause_predicates {
125141
format!(
@@ -143,6 +159,7 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault {
143159
item.span,
144160
"try adding this",
145161
&create_new_without_default_suggest_msg(
162+
&attrs_sugg,
146163
&self_type_snip,
147164
&generics_sugg,
148165
&where_clause_sugg,
@@ -157,13 +174,14 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault {
157174
}
158175

159176
fn create_new_without_default_suggest_msg(
177+
attrs_sugg: &str,
160178
self_type_snip: &str,
161179
generics_sugg: &str,
162180
where_clause_sugg: &str,
163181
) -> String {
164182
#[rustfmt::skip]
165183
format!(
166-
"impl{generics_sugg} Default for {self_type_snip}{where_clause_sugg} {{
184+
"{attrs_sugg}impl{generics_sugg} Default for {self_type_snip}{where_clause_sugg} {{
167185
fn default() -> Self {{
168186
Self::new()
169187
}}

tests/ui/new_without_default.fixed

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,25 @@ where
323323
}
324324
}
325325

326+
// From issue #14552, but with `#[cfg]` condition inverted,
327+
// otherwise Clippy wouldn't see the code to lint it
328+
329+
pub struct NewWithCfg;
330+
#[cfg(not(test))]
331+
impl Default for NewWithCfg {
332+
fn default() -> Self {
333+
Self::new()
334+
}
335+
}
336+
337+
impl NewWithCfg {
338+
#[cfg(not(test))]
339+
pub fn new() -> NewWithCfg {
340+
//~^ new_without_default
341+
unimplemented!()
342+
}
343+
}
344+
326345
mod issue15778 {
327346
pub struct Foo(Vec<i32>);
328347

tests/ui/new_without_default.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,18 @@ where
266266
}
267267
}
268268

269+
// From issue #14552, but with `#[cfg]` condition inverted,
270+
// otherwise Clippy wouldn't see the code to lint it
271+
272+
pub struct NewWithCfg;
273+
impl NewWithCfg {
274+
#[cfg(not(test))]
275+
pub fn new() -> NewWithCfg {
276+
//~^ new_without_default
277+
unimplemented!()
278+
}
279+
}
280+
269281
mod issue15778 {
270282
pub struct Foo(Vec<i32>);
271283

tests/ui/new_without_default.stderr

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,5 +174,25 @@ LL + }
174174
LL + }
175175
|
176176

177-
error: aborting due to 9 previous errors
177+
error: you should consider adding a `Default` implementation for `NewWithCfg`
178+
--> tests/ui/new_without_default.rs:275:5
179+
|
180+
LL | / pub fn new() -> NewWithCfg {
181+
LL | |
182+
LL | | unimplemented!()
183+
LL | | }
184+
| |_____^
185+
|
186+
help: try adding this
187+
|
188+
LL + #[cfg(not(test))]
189+
LL + impl Default for NewWithCfg {
190+
LL + fn default() -> Self {
191+
LL + Self::new()
192+
LL + }
193+
LL + }
194+
LL | impl NewWithCfg {
195+
|
196+
197+
error: aborting due to 10 previous errors
178198

0 commit comments

Comments
 (0)