Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 30, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
93355f1
clean-up
ada4a Sep 19, 2025
7a04ada
fix(new_without_default): if `new` has `#[cfg]`, copy that onto `impl…
ada4a Sep 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
190 changes: 103 additions & 87 deletions clippy_lints/src/new_without_default.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_hir_and_then;
use clippy_utils::return_ty;
use clippy_utils::source::snippet;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::sugg::DiagExt;
use rustc_errors::Applicability;
use rustc_hir as hir;
Expand Down Expand Up @@ -58,116 +58,132 @@ impl_lint_pass!(NewWithoutDefault => [NEW_WITHOUT_DEFAULT]);

impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
if let hir::ItemKind::Impl(hir::Impl {
let hir::ItemKind::Impl(hir::Impl {
of_trait: None,
generics,
self_ty: impl_self_ty,
..
}) = item.kind
else {
return;
};

for assoc_item in cx
.tcx
.associated_items(item.owner_id.def_id)
.filter_by_name_unhygienic(sym::new)
{
for assoc_item in cx
.tcx
.associated_items(item.owner_id.def_id)
.filter_by_name_unhygienic(sym::new)
if let AssocKind::Fn { has_self: false, .. } = assoc_item.kind
&& 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
// can't be implemented for unsafe new
&& !sig.header.is_unsafe()
// shouldn't be implemented when it is hidden in docs
&& !cx.tcx.is_doc_hidden(impl_item.owner_id.def_id)
// when the result of `new()` depends on a parameter we should not require
// an impl of `Default`
&& impl_item.generics.params.is_empty()
&& sig.decl.inputs.is_empty()
&& cx.effective_visibilities.is_exported(impl_item.owner_id.def_id)
&& let self_ty = cx.tcx.type_of(item.owner_id).instantiate_identity()
&& self_ty == return_ty(cx, impl_item.owner_id)
&& let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
{
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();
if impl_item.span.in_external_macro(cx.sess().source_map()) {
return;
}
if let hir::ImplItemKind::Fn(ref sig, _) = impl_item.kind {
let id = impl_item.owner_id;
if sig.header.is_unsafe() {
// can't be implemented for unsafe new
return;
}
if cx.tcx.is_doc_hidden(impl_item.owner_id.def_id) {
// shouldn't be implemented when it is hidden in docs
return;
}
if !impl_item.generics.params.is_empty() {
// when the result of `new()` depends on a parameter we should not require
// an impl of `Default`
return;
}
if sig.decl.inputs.is_empty()
&& cx.effective_visibilities.is_exported(impl_item.owner_id.def_id)
&& let self_ty = cx.tcx.type_of(item.owner_id).instantiate_identity()
&& self_ty == return_ty(cx, impl_item.owner_id)
&& let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
if self.impling_types.is_none() {
let mut impls = HirIdSet::default();
for &d in cx.tcx.local_trait_impls(default_trait_id) {
let ty = cx.tcx.type_of(d).instantiate_identity();
if let Some(ty_def) = ty.ty_adt_def()
&& let Some(local_def_id) = ty_def.did().as_local()
{
if self.impling_types.is_none() {
let mut impls = HirIdSet::default();
for &d in cx.tcx.local_trait_impls(default_trait_id) {
let ty = cx.tcx.type_of(d).instantiate_identity();
if let Some(ty_def) = ty.ty_adt_def()
&& let Some(local_def_id) = ty_def.did().as_local()
{
impls.insert(cx.tcx.local_def_id_to_hir_id(local_def_id));
}
}
self.impling_types = Some(impls);
}
impls.insert(cx.tcx.local_def_id_to_hir_id(local_def_id));
}
}
self.impling_types = Some(impls);
}

// Check if a Default implementation exists for the Self type, regardless of
// generics
if let Some(ref impling_types) = self.impling_types
&& let self_def = cx.tcx.type_of(item.owner_id).instantiate_identity()
&& let Some(self_def) = self_def.ty_adt_def()
&& let Some(self_local_did) = self_def.did().as_local()
&& let self_id = cx.tcx.local_def_id_to_hir_id(self_local_did)
&& impling_types.contains(&self_id)
{
return;
}
// Check if a Default implementation exists for the Self type, regardless of
// generics
if let Some(ref impling_types) = self.impling_types
&& let self_def = cx.tcx.type_of(item.owner_id).instantiate_identity()
&& let Some(self_def) = self_def.ty_adt_def()
&& let Some(self_local_did) = self_def.did().as_local()
&& let self_id = cx.tcx.local_def_id_to_hir_id(self_local_did)
&& impling_types.contains(&self_id)
{
return;
}

let generics_sugg = snippet(cx, generics.span, "");
let where_clause_sugg = if generics.has_where_clause_predicates {
format!("\n{}\n", snippet(cx, generics.where_clause_span, ""))
} else {
String::new()
};
let self_ty_fmt = self_ty.to_string();
let self_type_snip = snippet(cx, impl_self_ty.span, &self_ty_fmt);
span_lint_hir_and_then(
cx,
NEW_WITHOUT_DEFAULT,
id.into(),
impl_item.span,
format!("you should consider adding a `Default` implementation for `{self_type_snip}`"),
|diag| {
diag.suggest_prepend_item(
cx,
item.span,
"try adding this",
&create_new_without_default_suggest_msg(
&self_type_snip,
&generics_sugg,
&where_clause_sugg,
),
Applicability::MachineApplicable,
);
},
);
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!(
"\n{}\n",
snippet_with_applicability(cx, generics.where_clause_span, "", &mut app)
)
} else {
String::new()
};
let self_ty_fmt = self_ty.to_string();
let self_type_snip = snippet_with_applicability(cx, impl_self_ty.span, &self_ty_fmt, &mut app);
span_lint_hir_and_then(
cx,
NEW_WITHOUT_DEFAULT,
id.into(),
impl_item.span,
format!("you should consider adding a `Default` implementation for `{self_type_snip}`"),
|diag| {
diag.suggest_prepend_item(
cx,
item.span,
"try adding this",
&create_new_without_default_suggest_msg(
&attrs_sugg,
&self_type_snip,
&generics_sugg,
&where_clause_sugg,
),
app,
);
},
);
}
}
}
}

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()
}}
Expand Down
69 changes: 68 additions & 1 deletion tests/ui/new_without_default.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#![allow(
dead_code,
clippy::missing_safety_doc,
clippy::extra_unused_lifetimes,
clippy::extra_unused_type_parameters,
Expand Down Expand Up @@ -323,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>);

Expand Down
41 changes: 40 additions & 1 deletion tests/ui/new_without_default.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#![allow(
dead_code,
clippy::missing_safety_doc,
clippy::extra_unused_lifetimes,
clippy::extra_unused_type_parameters,
Expand Down Expand Up @@ -266,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>);

Expand Down
Loading