From 7a06d7ec51706222753048c6741962f60b7f14c5 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Tue, 1 Aug 2023 19:59:34 +0200 Subject: [PATCH 1/3] [new_without_default]: lifetimes are included in output The comment was likely obsolete. --- tests/ui/new_without_default.rs | 1 - tests/ui/new_without_default.stderr | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/ui/new_without_default.rs b/tests/ui/new_without_default.rs index 7803418cb047..7dbfa002d3fb 100644 --- a/tests/ui/new_without_default.rs +++ b/tests/ui/new_without_default.rs @@ -84,7 +84,6 @@ impl<'c> LtKo<'c> { pub fn new() -> LtKo<'c> { unimplemented!() } - // FIXME: that suggestion is missing lifetimes } struct Private; diff --git a/tests/ui/new_without_default.stderr b/tests/ui/new_without_default.stderr index 583dd327d6a5..7641035d2a6a 100644 --- a/tests/ui/new_without_default.stderr +++ b/tests/ui/new_without_default.stderr @@ -51,7 +51,7 @@ LL + } | error: you should consider adding a `Default` implementation for `NewNotEqualToDerive` - --> $DIR/new_without_default.rs:177:5 + --> $DIR/new_without_default.rs:176:5 | LL | / pub fn new() -> Self { LL | | NewNotEqualToDerive { foo: 1 } @@ -68,7 +68,7 @@ LL + } | error: you should consider adding a `Default` implementation for `FooGenerics` - --> $DIR/new_without_default.rs:185:5 + --> $DIR/new_without_default.rs:184:5 | LL | / pub fn new() -> Self { LL | | Self(Default::default()) @@ -85,7 +85,7 @@ LL + } | error: you should consider adding a `Default` implementation for `BarGenerics` - --> $DIR/new_without_default.rs:192:5 + --> $DIR/new_without_default.rs:191:5 | LL | / pub fn new() -> Self { LL | | Self(Default::default()) @@ -102,7 +102,7 @@ LL + } | error: you should consider adding a `Default` implementation for `Foo` - --> $DIR/new_without_default.rs:203:9 + --> $DIR/new_without_default.rs:202:9 | LL | / pub fn new() -> Self { LL | | todo!() From 621e76d2528e0872926305686041f6a935028db8 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Mon, 31 Jul 2023 21:57:31 +0200 Subject: [PATCH 2/3] [new_without_default]: include `where` clauses in suggestion Fix #11267 --- clippy_lints/src/new_without_default.rs | 19 ++++++++++++++++--- tests/ui/new_without_default.rs | 18 ++++++++++++++++++ tests/ui/new_without_default.stderr | 22 +++++++++++++++++++++- 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/new_without_default.rs b/clippy_lints/src/new_without_default.rs index cf7cd671dcab..6900502e8e11 100644 --- a/clippy_lints/src/new_without_default.rs +++ b/clippy_lints/src/new_without_default.rs @@ -130,6 +130,11 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault { } 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( @@ -145,7 +150,11 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault { cx, item.span, "try adding this", - &create_new_without_default_suggest_msg(&self_type_snip, &generics_sugg), + &create_new_without_default_suggest_msg( + &self_type_snip, + &generics_sugg, + &where_clause_sugg + ), Applicability::MaybeIncorrect, ); }, @@ -159,10 +168,14 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault { } } -fn create_new_without_default_suggest_msg(self_type_snip: &str, generics_sugg: &str) -> String { +fn create_new_without_default_suggest_msg( + self_type_snip: &str, + generics_sugg: &str, + where_clause_sugg: &str, +) -> String { #[rustfmt::skip] format!( -"impl{generics_sugg} Default for {self_type_snip} {{ +"impl{generics_sugg} Default for {self_type_snip}{where_clause_sugg} {{ fn default() -> Self {{ Self::new() }} diff --git a/tests/ui/new_without_default.rs b/tests/ui/new_without_default.rs index 7dbfa002d3fb..bbd7a51d6b98 100644 --- a/tests/ui/new_without_default.rs +++ b/tests/ui/new_without_default.rs @@ -230,3 +230,21 @@ impl IgnoreLifetimeNew { Self } } + +// From issue #11267 + +pub struct MyStruct +where + K: std::hash::Hash + Eq + PartialEq, +{ + _kv: Option<(K, V)>, +} + +impl MyStruct +where + K: std::hash::Hash + Eq + PartialEq, +{ + pub fn new() -> Self { + Self { _kv: None } + } +} diff --git a/tests/ui/new_without_default.stderr b/tests/ui/new_without_default.stderr index 7641035d2a6a..61973abcde88 100644 --- a/tests/ui/new_without_default.stderr +++ b/tests/ui/new_without_default.stderr @@ -120,5 +120,25 @@ LL + LL ~ impl Foo { | -error: aborting due to 7 previous errors +error: you should consider adding a `Default` implementation for `MyStruct` + --> $DIR/new_without_default.rs:247:5 + | +LL | / pub fn new() -> Self { +LL | | Self { _kv: None } +LL | | } + | |_____^ + | +help: try adding this + | +LL + impl Default for MyStruct +LL + where +LL + K: std::hash::Hash + Eq + PartialEq, +LL + { +LL + fn default() -> Self { +LL + Self::new() +LL + } +LL + } + | + +error: aborting due to 8 previous errors From f9b22e7b84df2bfef723fbb456349d1d5948d611 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Tue, 1 Aug 2023 20:28:10 +0200 Subject: [PATCH 3/3] [new_without_default]: make the suggestion machine-applicable Now that generics and lifetimes are output as expected, the lint should be applicable. --- clippy_lints/src/new_without_default.rs | 2 +- tests/ui/new_without_default.fixed | 28 ++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/new_without_default.rs b/clippy_lints/src/new_without_default.rs index 6900502e8e11..f7f9dccfbceb 100644 --- a/clippy_lints/src/new_without_default.rs +++ b/clippy_lints/src/new_without_default.rs @@ -155,7 +155,7 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault { &generics_sugg, &where_clause_sugg ), - Applicability::MaybeIncorrect, + Applicability::MachineApplicable, ); }, ); diff --git a/tests/ui/new_without_default.fixed b/tests/ui/new_without_default.fixed index 0c4283ad0c12..56359a4cbc3c 100644 --- a/tests/ui/new_without_default.fixed +++ b/tests/ui/new_without_default.fixed @@ -102,7 +102,6 @@ impl<'c> LtKo<'c> { pub fn new() -> LtKo<'c> { unimplemented!() } - // FIXME: that suggestion is missing lifetimes } struct Private; @@ -273,3 +272,30 @@ impl IgnoreLifetimeNew { Self } } + +// From issue #11267 + +pub struct MyStruct +where + K: std::hash::Hash + Eq + PartialEq, +{ + _kv: Option<(K, V)>, +} + +impl Default for MyStruct +where + K: std::hash::Hash + Eq + PartialEq, + { + fn default() -> Self { + Self::new() + } +} + +impl MyStruct +where + K: std::hash::Hash + Eq + PartialEq, +{ + pub fn new() -> Self { + Self { _kv: None } + } +}