From 9428138562bad89fc2d7d011fe276cd198e6155a Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Tue, 25 Apr 2023 20:44:09 +0200 Subject: [PATCH 1/8] adds lint to detect construction of unit struct using `default` Using `default` to construct a unit struct increases code complexity and adds a function call. This can be avoided by simply removing the call to `default` and simply construct by name. --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + .../src/default_constructed_unit_struct.rs | 66 +++++++++++++++++ clippy_lints/src/lib.rs | 2 + tests/ui/default_constructed_unit_struct.rs | 72 +++++++++++++++++++ .../ui/default_constructed_unit_struct.stderr | 28 ++++++++ 6 files changed, 170 insertions(+) create mode 100644 clippy_lints/src/default_constructed_unit_struct.rs create mode 100644 tests/ui/default_constructed_unit_struct.rs create mode 100644 tests/ui/default_constructed_unit_struct.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 23f4f97ee07..735ac59758a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4582,6 +4582,7 @@ Released 2018-09-13 [`debug_assert_with_mut_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#debug_assert_with_mut_call [`decimal_literal_representation`]: https://rust-lang.github.io/rust-clippy/master/index.html#decimal_literal_representation [`declare_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const +[`default_constructed_unit_struct`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_constructed_unit_struct [`default_instead_of_iter_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_instead_of_iter_empty [`default_numeric_fallback`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_numeric_fallback [`default_trait_access`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_trait_access diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 4aebd0b7d01..bf30f5b52f7 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -105,6 +105,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::dbg_macro::DBG_MACRO_INFO, crate::default::DEFAULT_TRAIT_ACCESS_INFO, crate::default::FIELD_REASSIGN_WITH_DEFAULT_INFO, + crate::default_constructed_unit_struct::DEFAULT_CONSTRUCTED_UNIT_STRUCT_INFO, crate::default_instead_of_iter_empty::DEFAULT_INSTEAD_OF_ITER_EMPTY_INFO, crate::default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK_INFO, crate::default_union_representation::DEFAULT_UNION_REPRESENTATION_INFO, diff --git a/clippy_lints/src/default_constructed_unit_struct.rs b/clippy_lints/src/default_constructed_unit_struct.rs new file mode 100644 index 00000000000..a04f7b9d9a9 --- /dev/null +++ b/clippy_lints/src/default_constructed_unit_struct.rs @@ -0,0 +1,66 @@ +use clippy_utils::{diagnostics::span_lint_and_sugg, is_from_proc_macro, match_def_path, paths}; +use hir::{def::Res, ExprKind}; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// Check for construction on unit struct using `default`. + /// + /// ### Why is this bad? + /// This adds code complexity and an unnecessary function call. + /// + /// ### Example + /// ```rust + /// #[derive(Default)] + /// struct S { + /// _marker: PhantomData + /// } + /// + /// let _: S = S { + /// _marker: PhantomData::default() + /// }; + /// ``` + /// Use instead: + /// ```rust + /// let _: S = Something { + /// _marker: PhantomData + /// } + /// ``` + #[clippy::version = "1.71.0"] + pub DEFAULT_CONSTRUCTED_UNIT_STRUCT, + complexity, + "unit structs can be contructed without calling `default`" +} +declare_lint_pass!(DefaultConstructedUnitStruct => [DEFAULT_CONSTRUCTED_UNIT_STRUCT]); + +impl LateLintPass<'_> for DefaultConstructedUnitStruct { + fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) { + if_chain!( + // make sure we have a call to `Default::default` + if let hir::ExprKind::Call(fn_expr, &[]) = expr.kind; + if let ExprKind::Path(ref qpath) = fn_expr.kind; + if let Res::Def(_, def_id) = cx.qpath_res(qpath, fn_expr.hir_id); + if match_def_path(cx, def_id, &paths::DEFAULT_TRAIT_METHOD); + // make sure we have a struct with no fields (unit struct) + if let ty::Adt(def, ..) = cx.typeck_results().expr_ty(expr).kind(); + if def.is_struct() && def.is_payloadfree() + && !def.non_enum_variant().is_field_list_non_exhaustive() + && !is_from_proc_macro(cx, expr); + then { + span_lint_and_sugg( + cx, + DEFAULT_CONSTRUCTED_UNIT_STRUCT, + qpath.last_segment_span(), + "Use of `default` to create a unit struct.", + "remove this call to `default`", + String::new(), + Applicability::MachineApplicable, + ) + } + ); + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 48dbecc9f6a..9af0b17e27b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -94,6 +94,7 @@ mod crate_in_macro_def; mod create_dir; mod dbg_macro; mod default; +mod default_constructed_unit_struct; mod default_instead_of_iter_empty; mod default_numeric_fallback; mod default_union_representation; @@ -970,6 +971,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(manual_slice_size_calculation::ManualSliceSizeCalculation)); store.register_early_pass(|| Box::new(suspicious_doc_comments::SuspiciousDocComments)); store.register_late_pass(|_| Box::new(items_after_test_module::ItemsAfterTestModule)); + store.register_late_pass(|_| Box::new(default_constructed_unit_struct::DefaultConstructedUnitStruct)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/tests/ui/default_constructed_unit_struct.rs b/tests/ui/default_constructed_unit_struct.rs new file mode 100644 index 00000000000..b64da9b863a --- /dev/null +++ b/tests/ui/default_constructed_unit_struct.rs @@ -0,0 +1,72 @@ +#![allow(unused)] +#![warn(clippy::default_constructed_unit_struct)] +use std::marker::PhantomData; + +#[derive(Default)] +struct UnitStruct; + +#[derive(Default)] +struct TupleStruct(usize); + +// no lint for derived impl +#[derive(Default)] +struct NormalStruct { + inner: PhantomData, +} + +struct NonDefaultStruct; + +impl NonDefaultStruct { + fn default() -> Self { + Self + } +} + +#[derive(Default)] +enum SomeEnum { + #[default] + Unit, + Tuple(UnitStruct), + Struct { + inner: usize, + }, +} + +impl NormalStruct { + fn new() -> Self { + // should lint + Self { + inner: PhantomData::default(), + } + } +} + +#[derive(Default)] +struct GenericStruct { + t: T, +} + +impl GenericStruct { + fn new() -> Self { + // should not lint + Self { t: T::default() } + } +} + +#[derive(Default)] +#[non_exhaustive] +struct NonExhaustiveStruct; + +fn main() { + // should lint + let _ = PhantomData::::default(); + let _: PhantomData = PhantomData::default(); + let _ = UnitStruct::default(); + + // should not lint + let _ = TupleStruct::default(); + let _ = NormalStruct::default(); + let _ = NonExhaustiveStruct::default(); + let _ = SomeEnum::default(); + let _ = NonDefaultStruct::default(); +} diff --git a/tests/ui/default_constructed_unit_struct.stderr b/tests/ui/default_constructed_unit_struct.stderr new file mode 100644 index 00000000000..13439414f4a --- /dev/null +++ b/tests/ui/default_constructed_unit_struct.stderr @@ -0,0 +1,28 @@ +error: Use of `default` to create a unit struct. + --> $DIR/default_constructed_unit_struct.rs:39:33 + | +LL | inner: PhantomData::default(), + | ^^^^^^^ help: remove this call to `default` + | + = note: `-D clippy::default-constructed-unit-struct` implied by `-D warnings` + +error: Use of `default` to create a unit struct. + --> $DIR/default_constructed_unit_struct.rs:62:35 + | +LL | let _ = PhantomData::::default(); + | ^^^^^^^ help: remove this call to `default` + +error: Use of `default` to create a unit struct. + --> $DIR/default_constructed_unit_struct.rs:63:44 + | +LL | let _: PhantomData = PhantomData::default(); + | ^^^^^^^ help: remove this call to `default` + +error: Use of `default` to create a unit struct. + --> $DIR/default_constructed_unit_struct.rs:64:25 + | +LL | let _ = UnitStruct::default(); + | ^^^^^^^ help: remove this call to `default` + +error: aborting due to 4 previous errors + From 032bc11fd482cc99fd3f85f053855b3e3e6ad18a Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Sat, 29 Apr 2023 21:11:34 +0200 Subject: [PATCH 2/8] fix diagnostic message style Co-authored-by: Ruby Lazuli --- clippy_lints/src/default_constructed_unit_struct.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/default_constructed_unit_struct.rs b/clippy_lints/src/default_constructed_unit_struct.rs index a04f7b9d9a9..d518c8e7c56 100644 --- a/clippy_lints/src/default_constructed_unit_struct.rs +++ b/clippy_lints/src/default_constructed_unit_struct.rs @@ -55,7 +55,7 @@ impl LateLintPass<'_> for DefaultConstructedUnitStruct { cx, DEFAULT_CONSTRUCTED_UNIT_STRUCT, qpath.last_segment_span(), - "Use of `default` to create a unit struct.", + "use of `default` to create a unit struct", "remove this call to `default`", String::new(), Applicability::MachineApplicable, From 220a9db64215df07f730cd01322a0c8b658629cd Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Mon, 1 May 2023 20:12:34 +0200 Subject: [PATCH 3/8] fixed span and corrected test output --- .../src/default_constructed_unit_struct.rs | 2 +- .../ui/default_constructed_unit_struct.stderr | 24 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/default_constructed_unit_struct.rs b/clippy_lints/src/default_constructed_unit_struct.rs index d518c8e7c56..d261cdd44a5 100644 --- a/clippy_lints/src/default_constructed_unit_struct.rs +++ b/clippy_lints/src/default_constructed_unit_struct.rs @@ -54,7 +54,7 @@ impl LateLintPass<'_> for DefaultConstructedUnitStruct { span_lint_and_sugg( cx, DEFAULT_CONSTRUCTED_UNIT_STRUCT, - qpath.last_segment_span(), + expr.span.with_lo(qpath.qself_span().hi()), "use of `default` to create a unit struct", "remove this call to `default`", String::new(), diff --git a/tests/ui/default_constructed_unit_struct.stderr b/tests/ui/default_constructed_unit_struct.stderr index 13439414f4a..952d4019644 100644 --- a/tests/ui/default_constructed_unit_struct.stderr +++ b/tests/ui/default_constructed_unit_struct.stderr @@ -1,28 +1,28 @@ -error: Use of `default` to create a unit struct. - --> $DIR/default_constructed_unit_struct.rs:39:33 +error: use of `default` to create a unit struct + --> $DIR/default_constructed_unit_struct.rs:39:31 | LL | inner: PhantomData::default(), - | ^^^^^^^ help: remove this call to `default` + | ^^^^^^^^^^^ help: remove this call to `default` | = note: `-D clippy::default-constructed-unit-struct` implied by `-D warnings` -error: Use of `default` to create a unit struct. - --> $DIR/default_constructed_unit_struct.rs:62:35 +error: use of `default` to create a unit struct + --> $DIR/default_constructed_unit_struct.rs:62:33 | LL | let _ = PhantomData::::default(); - | ^^^^^^^ help: remove this call to `default` + | ^^^^^^^^^^^ help: remove this call to `default` -error: Use of `default` to create a unit struct. - --> $DIR/default_constructed_unit_struct.rs:63:44 +error: use of `default` to create a unit struct + --> $DIR/default_constructed_unit_struct.rs:63:42 | LL | let _: PhantomData = PhantomData::default(); - | ^^^^^^^ help: remove this call to `default` + | ^^^^^^^^^^^ help: remove this call to `default` -error: Use of `default` to create a unit struct. - --> $DIR/default_constructed_unit_struct.rs:64:25 +error: use of `default` to create a unit struct + --> $DIR/default_constructed_unit_struct.rs:64:23 | LL | let _ = UnitStruct::default(); - | ^^^^^^^ help: remove this call to `default` + | ^^^^^^^^^^^ help: remove this call to `default` error: aborting due to 4 previous errors From 4e049036313759df55c4c68ad87e1cd7ad5cf214 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Wed, 3 May 2023 19:25:25 +0200 Subject: [PATCH 4/8] rename to plural form --- CHANGELOG.md | 2 +- clippy_lints/src/declared_lints.rs | 2 +- ...t_struct.rs => default_constructed_unit_structs.rs} | 8 ++++---- clippy_lints/src/lib.rs | 4 ++-- ...t_struct.rs => default_constructed_unit_structs.rs} | 2 +- ....stderr => default_constructed_unit_structs.stderr} | 10 +++++----- 6 files changed, 14 insertions(+), 14 deletions(-) rename clippy_lints/src/{default_constructed_unit_struct.rs => default_constructed_unit_structs.rs} (90%) rename tests/ui/{default_constructed_unit_struct.rs => default_constructed_unit_structs.rs} (96%) rename tests/ui/{default_constructed_unit_struct.stderr => default_constructed_unit_structs.stderr} (73%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 735ac59758a..31ad00536c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4582,7 +4582,7 @@ Released 2018-09-13 [`debug_assert_with_mut_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#debug_assert_with_mut_call [`decimal_literal_representation`]: https://rust-lang.github.io/rust-clippy/master/index.html#decimal_literal_representation [`declare_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const -[`default_constructed_unit_struct`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_constructed_unit_struct +[`default_constructed_unit_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_constructed_unit_structs [`default_instead_of_iter_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_instead_of_iter_empty [`default_numeric_fallback`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_numeric_fallback [`default_trait_access`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_trait_access diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index bf30f5b52f7..d3f5cb6e5f4 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -105,7 +105,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::dbg_macro::DBG_MACRO_INFO, crate::default::DEFAULT_TRAIT_ACCESS_INFO, crate::default::FIELD_REASSIGN_WITH_DEFAULT_INFO, - crate::default_constructed_unit_struct::DEFAULT_CONSTRUCTED_UNIT_STRUCT_INFO, + crate::default_constructed_unit_structs::DEFAULT_CONSTRUCTED_UNIT_STRUCTS_INFO, crate::default_instead_of_iter_empty::DEFAULT_INSTEAD_OF_ITER_EMPTY_INFO, crate::default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK_INFO, crate::default_union_representation::DEFAULT_UNION_REPRESENTATION_INFO, diff --git a/clippy_lints/src/default_constructed_unit_struct.rs b/clippy_lints/src/default_constructed_unit_structs.rs similarity index 90% rename from clippy_lints/src/default_constructed_unit_struct.rs rename to clippy_lints/src/default_constructed_unit_structs.rs index d261cdd44a5..a79923e9715 100644 --- a/clippy_lints/src/default_constructed_unit_struct.rs +++ b/clippy_lints/src/default_constructed_unit_structs.rs @@ -31,13 +31,13 @@ declare_clippy_lint! { /// } /// ``` #[clippy::version = "1.71.0"] - pub DEFAULT_CONSTRUCTED_UNIT_STRUCT, + pub DEFAULT_CONSTRUCTED_UNIT_STRUCTS, complexity, "unit structs can be contructed without calling `default`" } -declare_lint_pass!(DefaultConstructedUnitStruct => [DEFAULT_CONSTRUCTED_UNIT_STRUCT]); +declare_lint_pass!(DefaultConstructedUnitStructs => [DEFAULT_CONSTRUCTED_UNIT_STRUCTS]); -impl LateLintPass<'_> for DefaultConstructedUnitStruct { +impl LateLintPass<'_> for DefaultConstructedUnitStructs { fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) { if_chain!( // make sure we have a call to `Default::default` @@ -53,7 +53,7 @@ impl LateLintPass<'_> for DefaultConstructedUnitStruct { then { span_lint_and_sugg( cx, - DEFAULT_CONSTRUCTED_UNIT_STRUCT, + DEFAULT_CONSTRUCTED_UNIT_STRUCTS, expr.span.with_lo(qpath.qself_span().hi()), "use of `default` to create a unit struct", "remove this call to `default`", diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 9af0b17e27b..657a3d1f431 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -94,7 +94,7 @@ mod crate_in_macro_def; mod create_dir; mod dbg_macro; mod default; -mod default_constructed_unit_struct; +mod default_constructed_unit_structs; mod default_instead_of_iter_empty; mod default_numeric_fallback; mod default_union_representation; @@ -971,7 +971,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(manual_slice_size_calculation::ManualSliceSizeCalculation)); store.register_early_pass(|| Box::new(suspicious_doc_comments::SuspiciousDocComments)); store.register_late_pass(|_| Box::new(items_after_test_module::ItemsAfterTestModule)); - store.register_late_pass(|_| Box::new(default_constructed_unit_struct::DefaultConstructedUnitStruct)); + store.register_late_pass(|_| Box::new(default_constructed_unit_structs::DefaultConstructedUnitStructs)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/tests/ui/default_constructed_unit_struct.rs b/tests/ui/default_constructed_unit_structs.rs similarity index 96% rename from tests/ui/default_constructed_unit_struct.rs rename to tests/ui/default_constructed_unit_structs.rs index b64da9b863a..505be09a50e 100644 --- a/tests/ui/default_constructed_unit_struct.rs +++ b/tests/ui/default_constructed_unit_structs.rs @@ -1,5 +1,5 @@ #![allow(unused)] -#![warn(clippy::default_constructed_unit_struct)] +#![warn(clippy::default_constructed_unit_structs)] use std::marker::PhantomData; #[derive(Default)] diff --git a/tests/ui/default_constructed_unit_struct.stderr b/tests/ui/default_constructed_unit_structs.stderr similarity index 73% rename from tests/ui/default_constructed_unit_struct.stderr rename to tests/ui/default_constructed_unit_structs.stderr index 952d4019644..23ffa278288 100644 --- a/tests/ui/default_constructed_unit_struct.stderr +++ b/tests/ui/default_constructed_unit_structs.stderr @@ -1,25 +1,25 @@ error: use of `default` to create a unit struct - --> $DIR/default_constructed_unit_struct.rs:39:31 + --> $DIR/default_constructed_unit_structs.rs:39:31 | LL | inner: PhantomData::default(), | ^^^^^^^^^^^ help: remove this call to `default` | - = note: `-D clippy::default-constructed-unit-struct` implied by `-D warnings` + = note: `-D clippy::default-constructed-unit-structs` implied by `-D warnings` error: use of `default` to create a unit struct - --> $DIR/default_constructed_unit_struct.rs:62:33 + --> $DIR/default_constructed_unit_structs.rs:62:33 | LL | let _ = PhantomData::::default(); | ^^^^^^^^^^^ help: remove this call to `default` error: use of `default` to create a unit struct - --> $DIR/default_constructed_unit_struct.rs:63:42 + --> $DIR/default_constructed_unit_structs.rs:63:42 | LL | let _: PhantomData = PhantomData::default(); | ^^^^^^^^^^^ help: remove this call to `default` error: use of `default` to create a unit struct - --> $DIR/default_constructed_unit_struct.rs:64:23 + --> $DIR/default_constructed_unit_structs.rs:64:23 | LL | let _ = UnitStruct::default(); | ^^^^^^^^^^^ help: remove this call to `default` From 8701009860273828cb6853e1dba79a82e1271619 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Wed, 3 May 2023 20:06:29 +0200 Subject: [PATCH 5/8] add more test cases --- .../src/default_constructed_unit_structs.rs | 8 ++-- tests/ui/default_constructed_unit_structs.rs | 45 +++++++++++++++++++ .../default_constructed_unit_structs.stderr | 20 ++++++--- 3 files changed, 62 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/default_constructed_unit_structs.rs b/clippy_lints/src/default_constructed_unit_structs.rs index a79923e9715..dab76a2c41f 100644 --- a/clippy_lints/src/default_constructed_unit_structs.rs +++ b/clippy_lints/src/default_constructed_unit_structs.rs @@ -42,14 +42,14 @@ impl LateLintPass<'_> for DefaultConstructedUnitStructs { if_chain!( // make sure we have a call to `Default::default` if let hir::ExprKind::Call(fn_expr, &[]) = expr.kind; - if let ExprKind::Path(ref qpath) = fn_expr.kind; + if let ExprKind::Path(ref qpath@ hir::QPath::TypeRelative(_,_)) = fn_expr.kind; if let Res::Def(_, def_id) = cx.qpath_res(qpath, fn_expr.hir_id); if match_def_path(cx, def_id, &paths::DEFAULT_TRAIT_METHOD); // make sure we have a struct with no fields (unit struct) if let ty::Adt(def, ..) = cx.typeck_results().expr_ty(expr).kind(); - if def.is_struct() && def.is_payloadfree() - && !def.non_enum_variant().is_field_list_non_exhaustive() - && !is_from_proc_macro(cx, expr); + if def.is_struct(); + if let var @ ty::VariantDef { ctor: Some((hir::def::CtorKind::Const, _)), .. } = def.non_enum_variant(); + if !var.is_field_list_non_exhaustive() && !is_from_proc_macro(cx, expr); then { span_lint_and_sugg( cx, diff --git a/tests/ui/default_constructed_unit_structs.rs b/tests/ui/default_constructed_unit_structs.rs index 505be09a50e..276fc40aa18 100644 --- a/tests/ui/default_constructed_unit_structs.rs +++ b/tests/ui/default_constructed_unit_structs.rs @@ -5,9 +5,23 @@ use std::marker::PhantomData; #[derive(Default)] struct UnitStruct; +impl UnitStruct { + fn new() -> Self { + //should lint + Self::default() + } +} + #[derive(Default)] struct TupleStruct(usize); +impl TupleStruct { + fn new() -> Self { + // should not lint + Self(Default::default()) + } +} + // no lint for derived impl #[derive(Default)] struct NormalStruct { @@ -39,6 +53,13 @@ impl NormalStruct { inner: PhantomData::default(), } } + + fn new2() -> Self { + // should not lint + Self { + inner: Default::default(), + } + } } #[derive(Default)] @@ -51,8 +72,29 @@ impl GenericStruct { // should not lint Self { t: T::default() } } + + fn new2() -> Self { + // should not lint + Self { t: Default::default() } + } } +struct FakeDefault; +impl FakeDefault { + fn default() -> Self { + Self + } +} + +impl Default for FakeDefault { + fn default() -> Self { + Self + } +} + +#[derive(Default)] +struct EmptyStruct {} + #[derive(Default)] #[non_exhaustive] struct NonExhaustiveStruct; @@ -69,4 +111,7 @@ fn main() { let _ = NonExhaustiveStruct::default(); let _ = SomeEnum::default(); let _ = NonDefaultStruct::default(); + let _ = EmptyStruct::default(); + let _ = FakeDefault::default(); + let _ = ::default(); } diff --git a/tests/ui/default_constructed_unit_structs.stderr b/tests/ui/default_constructed_unit_structs.stderr index 23ffa278288..fa39ef4cda1 100644 --- a/tests/ui/default_constructed_unit_structs.stderr +++ b/tests/ui/default_constructed_unit_structs.stderr @@ -1,28 +1,34 @@ error: use of `default` to create a unit struct - --> $DIR/default_constructed_unit_structs.rs:39:31 + --> $DIR/default_constructed_unit_structs.rs:11:13 | -LL | inner: PhantomData::default(), - | ^^^^^^^^^^^ help: remove this call to `default` +LL | Self::default() + | ^^^^^^^^^^^ help: remove this call to `default` | = note: `-D clippy::default-constructed-unit-structs` implied by `-D warnings` error: use of `default` to create a unit struct - --> $DIR/default_constructed_unit_structs.rs:62:33 + --> $DIR/default_constructed_unit_structs.rs:53:31 + | +LL | inner: PhantomData::default(), + | ^^^^^^^^^^^ help: remove this call to `default` + +error: use of `default` to create a unit struct + --> $DIR/default_constructed_unit_structs.rs:104:33 | LL | let _ = PhantomData::::default(); | ^^^^^^^^^^^ help: remove this call to `default` error: use of `default` to create a unit struct - --> $DIR/default_constructed_unit_structs.rs:63:42 + --> $DIR/default_constructed_unit_structs.rs:105:42 | LL | let _: PhantomData = PhantomData::default(); | ^^^^^^^^^^^ help: remove this call to `default` error: use of `default` to create a unit struct - --> $DIR/default_constructed_unit_structs.rs:64:23 + --> $DIR/default_constructed_unit_structs.rs:106:23 | LL | let _ = UnitStruct::default(); | ^^^^^^^^^^^ help: remove this call to `default` -error: aborting due to 4 previous errors +error: aborting due to 5 previous errors From 4ed7fd1ecc94eb82d99fe18192c156f367029731 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Wed, 3 May 2023 20:07:02 +0200 Subject: [PATCH 6/8] fix failing tests --- tests/ui/box_default.fixed | 1 + tests/ui/box_default.rs | 1 + tests/ui/box_default.stderr | 32 ++++++++++++++++---------------- tests/ui/from_over_into.fixed | 2 +- tests/ui/from_over_into.rs | 2 +- tests/ui/from_over_into.stderr | 2 +- tests/ui/use_self_trait.fixed | 4 ++-- tests/ui/use_self_trait.rs | 4 ++-- tests/ui/use_self_trait.stderr | 2 +- 9 files changed, 26 insertions(+), 24 deletions(-) diff --git a/tests/ui/box_default.fixed b/tests/ui/box_default.fixed index 6afce208769..e6331290420 100644 --- a/tests/ui/box_default.fixed +++ b/tests/ui/box_default.fixed @@ -1,5 +1,6 @@ //@run-rustfix #![warn(clippy::box_default)] +#![allow(clippy::default_constructed_unit_structs)] #[derive(Default)] struct ImplementsDefault; diff --git a/tests/ui/box_default.rs b/tests/ui/box_default.rs index 09365618e63..34a05a29c5a 100644 --- a/tests/ui/box_default.rs +++ b/tests/ui/box_default.rs @@ -1,5 +1,6 @@ //@run-rustfix #![warn(clippy::box_default)] +#![allow(clippy::default_constructed_unit_structs)] #[derive(Default)] struct ImplementsDefault; diff --git a/tests/ui/box_default.stderr b/tests/ui/box_default.stderr index 78e17b9f035..c9834863601 100644 --- a/tests/ui/box_default.stderr +++ b/tests/ui/box_default.stderr @@ -1,5 +1,5 @@ error: `Box::new(_)` of default value - --> $DIR/box_default.rs:22:32 + --> $DIR/box_default.rs:23:32 | LL | let _string: Box = Box::new(Default::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` @@ -7,91 +7,91 @@ LL | let _string: Box = Box::new(Default::default()); = note: `-D clippy::box-default` implied by `-D warnings` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:23:17 + --> $DIR/box_default.rs:24:17 | LL | let _byte = Box::new(u8::default()); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:24:16 + --> $DIR/box_default.rs:25:16 | LL | let _vec = Box::new(Vec::::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::>::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:25:17 + --> $DIR/box_default.rs:26:17 | LL | let _impl = Box::new(ImplementsDefault::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:26:18 + --> $DIR/box_default.rs:27:18 | LL | let _impl2 = Box::new(::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:27:42 + --> $DIR/box_default.rs:28:42 | LL | let _impl3: Box = Box::new(Default::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:29:28 + --> $DIR/box_default.rs:30:28 | LL | let _in_macro = outer!(Box::new(String::new())); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:30:34 + --> $DIR/box_default.rs:31:34 | LL | let _string_default = outer!(Box::new(String::from(""))); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:31:46 + --> $DIR/box_default.rs:32:46 | LL | let _vec2: Box> = Box::new(vec![]); | ^^^^^^^^^^^^^^^^ help: try: `Box::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:32:33 + --> $DIR/box_default.rs:33:33 | LL | let _vec3: Box> = Box::new(Vec::from([])); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:33:25 + --> $DIR/box_default.rs:34:25 | LL | let _vec4: Box<_> = Box::new(Vec::from([false; 0])); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::>::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:35:16 + --> $DIR/box_default.rs:36:16 | LL | call_ty_fn(Box::new(u8::default())); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:40:5 + --> $DIR/box_default.rs:41:5 | LL | Box::new(bool::default()) | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:57:28 + --> $DIR/box_default.rs:58:28 | LL | let _: Box = Box::new(ImplementsDefault::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:66:17 + --> $DIR/box_default.rs:67:17 | LL | let _ = Box::new(WeirdPathed::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:78:18 + --> $DIR/box_default.rs:79:18 | LL | Some(Box::new(Foo::default())) | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::::default()` diff --git a/tests/ui/from_over_into.fixed b/tests/ui/from_over_into.fixed index fc6d937060d..d18f9387565 100644 --- a/tests/ui/from_over_into.fixed +++ b/tests/ui/from_over_into.fixed @@ -32,7 +32,7 @@ struct SelfKeywords; impl From for SelfKeywords { fn from(val: X) -> Self { - let _ = X::default(); + let _ = X; let _ = X::FOO; let _: X = val; diff --git a/tests/ui/from_over_into.rs b/tests/ui/from_over_into.rs index fe1ebee35f1..de8ff0b06bd 100644 --- a/tests/ui/from_over_into.rs +++ b/tests/ui/from_over_into.rs @@ -32,7 +32,7 @@ struct SelfKeywords; impl Into for X { fn into(self) -> SelfKeywords { - let _ = Self::default(); + let _ = Self; let _ = Self::FOO; let _: Self = self; diff --git a/tests/ui/from_over_into.stderr b/tests/ui/from_over_into.stderr index 990b905c1b1..6039f86fe67 100644 --- a/tests/ui/from_over_into.stderr +++ b/tests/ui/from_over_into.stderr @@ -35,7 +35,7 @@ help: replace the `Into` implementation with `From` | LL ~ impl From for SelfKeywords { LL ~ fn from(val: X) -> Self { -LL ~ let _ = X::default(); +LL ~ let _ = X; LL ~ let _ = X::FOO; LL ~ let _: X = val; | diff --git a/tests/ui/use_self_trait.fixed b/tests/ui/use_self_trait.fixed index 4623aeeb0eb..20138a29fd1 100644 --- a/tests/ui/use_self_trait.fixed +++ b/tests/ui/use_self_trait.fixed @@ -33,7 +33,7 @@ impl SelfTrait for Bad { fn nested(_p1: Box, _p2: (&u8, &Self)) {} fn vals(_: Self) -> Self { - Self::default() + Self } } @@ -70,7 +70,7 @@ impl SelfTrait for Good { fn nested(_p1: Box, _p2: (&u8, &Self)) {} fn vals(_: Self) -> Self { - Self::default() + Self } } diff --git a/tests/ui/use_self_trait.rs b/tests/ui/use_self_trait.rs index d7d76dd9623..bf697b01a42 100644 --- a/tests/ui/use_self_trait.rs +++ b/tests/ui/use_self_trait.rs @@ -33,7 +33,7 @@ impl SelfTrait for Bad { fn nested(_p1: Box, _p2: (&u8, &Bad)) {} fn vals(_: Bad) -> Bad { - Bad::default() + Bad } } @@ -70,7 +70,7 @@ impl SelfTrait for Good { fn nested(_p1: Box, _p2: (&u8, &Self)) {} fn vals(_: Self) -> Self { - Self::default() + Self } } diff --git a/tests/ui/use_self_trait.stderr b/tests/ui/use_self_trait.stderr index 090729b9c3d..6257f802dd8 100644 --- a/tests/ui/use_self_trait.stderr +++ b/tests/ui/use_self_trait.stderr @@ -63,7 +63,7 @@ LL | fn vals(_: Bad) -> Bad { error: unnecessary structure name repetition --> $DIR/use_self_trait.rs:36:9 | -LL | Bad::default() +LL | Bad | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition From 160371550fbb7783200f6184109c02babf06efb3 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Wed, 3 May 2023 21:05:50 +0200 Subject: [PATCH 7/8] add `rustfix` annotation --- .../ui/default_constructed_unit_structs.fixed | 119 ++++++++++++++++++ tests/ui/default_constructed_unit_structs.rs | 2 + .../default_constructed_unit_structs.stderr | 10 +- 3 files changed, 126 insertions(+), 5 deletions(-) create mode 100644 tests/ui/default_constructed_unit_structs.fixed diff --git a/tests/ui/default_constructed_unit_structs.fixed b/tests/ui/default_constructed_unit_structs.fixed new file mode 100644 index 00000000000..4c2d1ea48e1 --- /dev/null +++ b/tests/ui/default_constructed_unit_structs.fixed @@ -0,0 +1,119 @@ +//@run-rustfix + +#![allow(unused)] +#![warn(clippy::default_constructed_unit_structs)] +use std::marker::PhantomData; + +#[derive(Default)] +struct UnitStruct; + +impl UnitStruct { + fn new() -> Self { + //should lint + Self + } +} + +#[derive(Default)] +struct TupleStruct(usize); + +impl TupleStruct { + fn new() -> Self { + // should not lint + Self(Default::default()) + } +} + +// no lint for derived impl +#[derive(Default)] +struct NormalStruct { + inner: PhantomData, +} + +struct NonDefaultStruct; + +impl NonDefaultStruct { + fn default() -> Self { + Self + } +} + +#[derive(Default)] +enum SomeEnum { + #[default] + Unit, + Tuple(UnitStruct), + Struct { + inner: usize, + }, +} + +impl NormalStruct { + fn new() -> Self { + // should lint + Self { + inner: PhantomData, + } + } + + fn new2() -> Self { + // should not lint + Self { + inner: Default::default(), + } + } +} + +#[derive(Default)] +struct GenericStruct { + t: T, +} + +impl GenericStruct { + fn new() -> Self { + // should not lint + Self { t: T::default() } + } + + fn new2() -> Self { + // should not lint + Self { t: Default::default() } + } +} + +struct FakeDefault; +impl FakeDefault { + fn default() -> Self { + Self + } +} + +impl Default for FakeDefault { + fn default() -> Self { + Self + } +} + +#[derive(Default)] +struct EmptyStruct {} + +#[derive(Default)] +#[non_exhaustive] +struct NonExhaustiveStruct; + +fn main() { + // should lint + let _ = PhantomData::; + let _: PhantomData = PhantomData; + let _ = UnitStruct; + + // should not lint + let _ = TupleStruct::default(); + let _ = NormalStruct::default(); + let _ = NonExhaustiveStruct::default(); + let _ = SomeEnum::default(); + let _ = NonDefaultStruct::default(); + let _ = EmptyStruct::default(); + let _ = FakeDefault::default(); + let _ = ::default(); +} diff --git a/tests/ui/default_constructed_unit_structs.rs b/tests/ui/default_constructed_unit_structs.rs index 276fc40aa18..850793dd5de 100644 --- a/tests/ui/default_constructed_unit_structs.rs +++ b/tests/ui/default_constructed_unit_structs.rs @@ -1,3 +1,5 @@ +//@run-rustfix + #![allow(unused)] #![warn(clippy::default_constructed_unit_structs)] use std::marker::PhantomData; diff --git a/tests/ui/default_constructed_unit_structs.stderr b/tests/ui/default_constructed_unit_structs.stderr index fa39ef4cda1..4058943d087 100644 --- a/tests/ui/default_constructed_unit_structs.stderr +++ b/tests/ui/default_constructed_unit_structs.stderr @@ -1,5 +1,5 @@ error: use of `default` to create a unit struct - --> $DIR/default_constructed_unit_structs.rs:11:13 + --> $DIR/default_constructed_unit_structs.rs:13:13 | LL | Self::default() | ^^^^^^^^^^^ help: remove this call to `default` @@ -7,25 +7,25 @@ LL | Self::default() = note: `-D clippy::default-constructed-unit-structs` implied by `-D warnings` error: use of `default` to create a unit struct - --> $DIR/default_constructed_unit_structs.rs:53:31 + --> $DIR/default_constructed_unit_structs.rs:55:31 | LL | inner: PhantomData::default(), | ^^^^^^^^^^^ help: remove this call to `default` error: use of `default` to create a unit struct - --> $DIR/default_constructed_unit_structs.rs:104:33 + --> $DIR/default_constructed_unit_structs.rs:106:33 | LL | let _ = PhantomData::::default(); | ^^^^^^^^^^^ help: remove this call to `default` error: use of `default` to create a unit struct - --> $DIR/default_constructed_unit_structs.rs:105:42 + --> $DIR/default_constructed_unit_structs.rs:107:42 | LL | let _: PhantomData = PhantomData::default(); | ^^^^^^^^^^^ help: remove this call to `default` error: use of `default` to create a unit struct - --> $DIR/default_constructed_unit_structs.rs:106:23 + --> $DIR/default_constructed_unit_structs.rs:108:23 | LL | let _ = UnitStruct::default(); | ^^^^^^^^^^^ help: remove this call to `default` From 48ae5a071bc32e5fb69669248ab103ecba7509ab Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Wed, 3 May 2023 21:43:10 +0200 Subject: [PATCH 8/8] fix lint docs --- clippy_lints/src/default_constructed_unit_structs.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/default_constructed_unit_structs.rs b/clippy_lints/src/default_constructed_unit_structs.rs index dab76a2c41f..e529d81a7e9 100644 --- a/clippy_lints/src/default_constructed_unit_structs.rs +++ b/clippy_lints/src/default_constructed_unit_structs.rs @@ -15,6 +15,7 @@ declare_clippy_lint! { /// /// ### Example /// ```rust + /// # use std::marker::PhantomData; /// #[derive(Default)] /// struct S { /// _marker: PhantomData @@ -26,9 +27,14 @@ declare_clippy_lint! { /// ``` /// Use instead: /// ```rust - /// let _: S = Something { - /// _marker: PhantomData + /// # use std::marker::PhantomData; + /// struct S { + /// _marker: PhantomData /// } + /// + /// let _: S = S { + /// _marker: PhantomData + /// }; /// ``` #[clippy::version = "1.71.0"] pub DEFAULT_CONSTRUCTED_UNIT_STRUCTS,