diff --git a/CHANGELOG.md b/CHANGELOG.md index d1e98a93940d..df064871c302 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_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 44e725be710e..8174c3b0564d 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_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_structs.rs b/clippy_lints/src/default_constructed_unit_structs.rs new file mode 100644 index 000000000000..e529d81a7e9f --- /dev/null +++ b/clippy_lints/src/default_constructed_unit_structs.rs @@ -0,0 +1,72 @@ +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 + /// # use std::marker::PhantomData; + /// #[derive(Default)] + /// struct S { + /// _marker: PhantomData + /// } + /// + /// let _: S = S { + /// _marker: PhantomData::default() + /// }; + /// ``` + /// Use instead: + /// ```rust + /// # use std::marker::PhantomData; + /// struct S { + /// _marker: PhantomData + /// } + /// + /// let _: S = S { + /// _marker: PhantomData + /// }; + /// ``` + #[clippy::version = "1.71.0"] + pub DEFAULT_CONSTRUCTED_UNIT_STRUCTS, + complexity, + "unit structs can be contructed without calling `default`" +} +declare_lint_pass!(DefaultConstructedUnitStructs => [DEFAULT_CONSTRUCTED_UNIT_STRUCTS]); + +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` + if let hir::ExprKind::Call(fn_expr, &[]) = 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(); + 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, + 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`", + String::new(), + Applicability::MachineApplicable, + ) + } + ); + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 48dbecc9f6aa..657a3d1f4310 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_structs; 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_structs::DefaultConstructedUnitStructs)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/tests/ui/box_default.fixed b/tests/ui/box_default.fixed index 6afce2087697..e6331290420b 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 09365618e633..34a05a29c5aa 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 78e17b9f0359..c98348636014 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/default_constructed_unit_structs.fixed b/tests/ui/default_constructed_unit_structs.fixed new file mode 100644 index 000000000000..4c2d1ea48e11 --- /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 new file mode 100644 index 000000000000..850793dd5de8 --- /dev/null +++ b/tests/ui/default_constructed_unit_structs.rs @@ -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::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 { + 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(), + } + } + + 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::::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(); + 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 new file mode 100644 index 000000000000..4058943d0872 --- /dev/null +++ b/tests/ui/default_constructed_unit_structs.stderr @@ -0,0 +1,34 @@ +error: use of `default` to create a unit struct + --> $DIR/default_constructed_unit_structs.rs:13:13 + | +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: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: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: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:108:23 + | +LL | let _ = UnitStruct::default(); + | ^^^^^^^^^^^ help: remove this call to `default` + +error: aborting due to 5 previous errors + diff --git a/tests/ui/from_over_into.fixed b/tests/ui/from_over_into.fixed index fc6d937060dc..d18f93875658 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 fe1ebee35f16..de8ff0b06bdc 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 990b905c1b15..6039f86fe670 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 4623aeeb0eb2..20138a29fd1f 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 d7d76dd96233..bf697b01a42f 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 090729b9c3d5..6257f802dd80 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