Auto merge of #10716 - Icxolu:unitstruct_default_construction, r=Manishearth

Fixes #10609: 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: [`default_constructed_unit_structs`]: detects construction of unit structs using `default`

fixes #10609

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

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,

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<T> {
+    ///     _marker: PhantomData<T>
+    /// }
+    ///
+    /// let _: S<i32> = S {
+    ///     _marker: PhantomData::default()
+    /// };
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// # use std::marker::PhantomData;
+    /// struct S<T> {
+    ///     _marker: PhantomData<T>
+    /// }
+    ///
+    /// let _: S<i32> = 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,
+                )
+            }
+        );
+    }
+}

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`
 }
 

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;

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;

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<String> = Box::new(Default::default());
    |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()`
@@ -7,91 +7,91 @@ LL |     let _string: Box<String> = 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::<u8>::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::<u8>::new());
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<Vec<u8>>::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::<ImplementsDefault>::default()`
 
 error: `Box::new(_)` of default value
-  --> $DIR/box_default.rs:26:18
+  --> $DIR/box_default.rs:27:18
    |
 LL |     let _impl2 = Box::new(<ImplementsDefault as Default>::default());
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<ImplementsDefault>::default()`
 
 error: `Box::new(_)` of default value
-  --> $DIR/box_default.rs:27:42
+  --> $DIR/box_default.rs:28:42
    |
 LL |     let _impl3: Box<ImplementsDefault> = 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::<String>::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::<String>::default()`
 
 error: `Box::new(_)` of default value
-  --> $DIR/box_default.rs:31:46
+  --> $DIR/box_default.rs:32:46
    |
 LL |     let _vec2: Box<Vec<ImplementsDefault>> = 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<Vec<bool>> = 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::<Vec<bool>>::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::<bool>::default()`
 
 error: `Box::new(_)` of default value
-  --> $DIR/box_default.rs:57:28
+  --> $DIR/box_default.rs:58:28
    |
 LL |     let _: Box<dyn Read> = Box::new(ImplementsDefault::default());
    |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<ImplementsDefault>::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::<WeirdPathed>::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::<Foo>::default()`

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<usize>,
+}
+
+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: T,
+}
+
+impl<T: Default> GenericStruct<T> {
+    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::<usize>;
+    let _: PhantomData<i32> = 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 _ = <FakeDefault as Default>::default();
+}

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<usize>,
+}
+
+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: T,
+}
+
+impl<T: Default> GenericStruct<T> {
+    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::<usize>::default();
+    let _: PhantomData<i32> = 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 _ = <FakeDefault as Default>::default();
+}

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::<usize>::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<i32> = 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
+

tests/ui/from_over_into.fixed

@@ -32,7 +32,7 @@ struct SelfKeywords;
 
 impl From<X> for SelfKeywords {
     fn from(val: X) -> Self {
-        let _ = X::default();
+        let _ = X;
         let _ = X::FOO;
         let _: X = val;
 

tests/ui/from_over_into.rs

@@ -32,7 +32,7 @@ struct SelfKeywords;
 
 impl Into<SelfKeywords> for X {
     fn into(self) -> SelfKeywords {
-        let _ = Self::default();
+        let _ = Self;
         let _ = Self::FOO;
         let _: Self = self;
 

tests/ui/from_over_into.stderr

@@ -35,7 +35,7 @@ help: replace the `Into` implementation with `From<X>`
    |
 LL ~ impl From<X> for SelfKeywords {
 LL ~     fn from(val: X) -> Self {
-LL ~         let _ = X::default();
+LL ~         let _ = X;
 LL ~         let _ = X::FOO;
 LL ~         let _: X = val;
    |

tests/ui/use_self_trait.fixed

@@ -33,7 +33,7 @@ impl SelfTrait for Bad {
     fn nested(_p1: Box<Self>, _p2: (&u8, &Self)) {}
 
     fn vals(_: Self) -> Self {
-        Self::default()
+        Self
     }
 }
 
@@ -70,7 +70,7 @@ impl SelfTrait for Good {
     fn nested(_p1: Box<Self>, _p2: (&u8, &Self)) {}
 
     fn vals(_: Self) -> Self {
-        Self::default()
+        Self
     }
 }
 

tests/ui/use_self_trait.rs

@@ -33,7 +33,7 @@ impl SelfTrait for Bad {
     fn nested(_p1: Box<Bad>, _p2: (&u8, &Bad)) {}
 
     fn vals(_: Bad) -> Bad {
-        Bad::default()
+        Bad
     }
 }
 
@@ -70,7 +70,7 @@ impl SelfTrait for Good {
     fn nested(_p1: Box<Self>, _p2: (&u8, &Self)) {}
 
     fn vals(_: Self) -> Self {
-        Self::default()
+        Self
     }
 }
 

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