Skip to content

Commit 03f8899

Browse files
Merge #3350
3350: Don't emit `new_without_default_derive` if an impl of Default exists regardless of generics r=oli-obk a=pengowen123 Fixes #2226 Co-authored-by: Owen Sanchez <[email protected]>
2 parents 122da1d + 50b9e7a commit 03f8899

File tree

3 files changed

+43
-5
lines changed

3 files changed

+43
-5
lines changed

clippy_lints/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
391391
reg.register_early_lint_pass(box int_plus_one::IntPlusOne);
392392
reg.register_late_lint_pass(box overflow_check_conditional::OverflowCheckConditional);
393393
reg.register_late_lint_pass(box unused_label::UnusedLabel);
394-
reg.register_late_lint_pass(box new_without_default::NewWithoutDefault);
394+
reg.register_late_lint_pass(box new_without_default::NewWithoutDefault::default());
395395
reg.register_late_lint_pass(box blacklisted_name::BlackListedName::new(conf.blacklisted_names.clone()));
396396
reg.register_late_lint_pass(box functions::Functions::new(conf.too_many_arguments_threshold));
397397
reg.register_early_lint_pass(box doc::Doc::new(conf.doc_valid_idents.clone()));

clippy_lints/src/new_without_default.rs

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use crate::rustc::hir::def_id::DefId;
1212
use crate::rustc::hir;
1313
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass, in_external_macro, LintContext};
14+
use crate::rustc::util::nodemap::NodeSet;
1415
use crate::rustc::{declare_tool_lint, lint_array};
1516
use if_chain::if_chain;
1617
use crate::rustc::ty::{self, Ty};
@@ -91,8 +92,10 @@ declare_clippy_lint! {
9192
"`fn new() -> Self` without `#[derive]`able `Default` implementation"
9293
}
9394

94-
#[derive(Copy, Clone)]
95-
pub struct NewWithoutDefault;
95+
#[derive(Clone, Default)]
96+
pub struct NewWithoutDefault {
97+
impling_types: Option<NodeSet>,
98+
}
9699

97100
impl LintPass for NewWithoutDefault {
98101
fn get_lints(&self) -> LintArray {
@@ -130,13 +133,39 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault {
130133
return;
131134
}
132135
if sig.decl.inputs.is_empty() && name == "new" && cx.access_levels.is_reachable(id) {
136+
let self_did = cx.tcx.hir.local_def_id(cx.tcx.hir.get_parent(id));
133137
let self_ty = cx.tcx
134-
.type_of(cx.tcx.hir.local_def_id(cx.tcx.hir.get_parent(id)));
138+
.type_of(self_did);
135139
if_chain! {
136140
if same_tys(cx, self_ty, return_ty(cx, id));
137141
if let Some(default_trait_id) = get_trait_def_id(cx, &paths::DEFAULT_TRAIT);
138-
if !implements_trait(cx, self_ty, default_trait_id, &[]);
139142
then {
143+
if self.impling_types.is_none() {
144+
let mut impls = NodeSet();
145+
cx.tcx.for_each_impl(default_trait_id, |d| {
146+
if let Some(ty_def) = cx.tcx.type_of(d).ty_adt_def() {
147+
if let Some(node_id) = cx.tcx.hir.as_local_node_id(ty_def.did) {
148+
impls.insert(node_id);
149+
}
150+
}
151+
});
152+
self.impling_types = Some(impls);
153+
}
154+
155+
// Check if a Default implementation exists for the Self type, regardless of
156+
// generics
157+
if_chain! {
158+
if let Some(ref impling_types) = self.impling_types;
159+
if let Some(self_def) = cx.tcx.type_of(self_did).ty_adt_def();
160+
if self_def.did.is_local();
161+
then {
162+
let self_id = cx.tcx.hir.local_def_id_to_node_id(self_def.did.to_local());
163+
if impling_types.contains(&self_id) {
164+
return;
165+
}
166+
}
167+
}
168+
140169
if let Some(sp) = can_derive_default(self_ty, cx, default_trait_id) {
141170
span_lint_and_then(
142171
cx,

tests/ui/new_without_default.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,4 +107,13 @@ impl IgnoreUnsafeNew {
107107
pub unsafe fn new() -> Self { IgnoreUnsafeNew }
108108
}
109109

110+
#[derive(Default)]
111+
pub struct OptionRefWrapper<'a, T: 'a>(Option<&'a T>);
112+
113+
impl<'a, T: 'a> OptionRefWrapper<'a, T> {
114+
pub fn new() -> Self {
115+
OptionRefWrapper(None)
116+
}
117+
}
118+
110119
fn main() {}

0 commit comments

Comments
 (0)