wip

Author: nyurik

clippy_lints/src/default_constructed_unit_structs.rs

@@ -46,30 +46,23 @@ declare_clippy_lint! {
 }
 declare_lint_pass!(DefaultConstructedUnitStructs => [DEFAULT_CONSTRUCTED_UNIT_STRUCTS]);
 
-fn is_alias(ty: hir::Ty<'_>) -> bool {
-    if let hir::TyKind::Path(ref qpath) = ty.kind {
-        is_ty_alias(qpath)
-    } else {
-        false
-    }
-}
-
 impl LateLintPass<'_> for DefaultConstructedUnitStructs {
     fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
         if let ExprKind::Call(fn_expr, &[]) = expr.kind
             // make sure we have a call to `Default::default`
             && let ExprKind::Path(ref qpath @ hir::QPath::TypeRelative(base, _)) = fn_expr.kind
             // make sure this isn't a type alias:
             // `<Foo as Bar>::Assoc` cannot be used as a constructor
-            && !is_alias(*base)
+            && !matches!(base.kind, hir::TyKind::Path(ref qpath) if is_ty_alias(qpath))
             && let Res::Def(_, def_id) = cx.qpath_res(qpath, fn_expr.hir_id)
             && cx.tcx.is_diagnostic_item(sym::default_fn, def_id)
             // make sure we have a struct with no fields (unit struct)
             && let ty::Adt(def, ..) = cx.typeck_results().expr_ty(expr).kind()
             && def.is_struct()
             && let var @ ty::VariantDef { ctor: Some((hir::def::CtorKind::Const, _)), .. } = def.non_enum_variant()
             && !var.is_field_list_non_exhaustive()
-            && !expr.span.from_expansion() && !qpath.span().from_expansion()
+            && !expr.span.from_expansion()
+            && !qpath.span().from_expansion()
         {
             span_lint_and_sugg(
                 cx,

clippy_lints/src/new_without_default.rs

@@ -1,14 +1,14 @@
 use clippy_utils::diagnostics::span_lint_hir_and_then;
-use clippy_utils::source::snippet;
+use clippy_utils::source::{snippet, trim_span};
 use clippy_utils::sugg::DiagExt;
 use clippy_utils::{is_default_equivalent_call, return_ty};
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_hir::HirIdMap;
 use rustc_lint::{LateContext, LateLintPass, LintContext};
-use rustc_middle::ty::Ty;
+use rustc_middle::ty::{Adt, Ty, VariantDef};
 use rustc_session::impl_lint_pass;
-use rustc_span::sym;
+use rustc_span::{BytePos, Pos as _, Span, sym};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -199,30 +199,13 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault {
                                     return;
                                 }
                                 suggest_new_without_default(cx, item, impl_item, id, self_ty, generics, impl_self_ty);
-                            } else if let hir::ExprKind::Block(block, _) = cx.tcx.hir().body(body_id).value.kind {
+                            } else if let hir::ExprKind::Block(block, _) = cx.tcx.hir().body(body_id).value.kind
+                                && !is_unit_struct(cx, self_ty)
+                            {
                                 // this type has an automatically derived `Default` implementation
                                 // check if `new` and `default` are equivalent
-                                if !check_block_calls_default(cx, block) {
-                                    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,
-                                        DEFAULT_MISMATCHES_NEW,
-                                        id.into(),
-                                        impl_item.span,
-                                        format!(
-                                            "you should consider delegating to the auto-derived `Default` for `{self_type_snip}`"
-                                        ),
-                                        |diag| {
-                                            diag.suggest_prepend_item(
-                                                cx,
-                                                block.span,
-                                                "try using this",
-                                                &"Self::default()",
-                                                Applicability::MaybeIncorrect,
-                                            );
-                                        },
-                                    );
+                                if let Some(span) = check_block_calls_default(cx, block) {
+                                    suggest_default_mismatch_new(cx, span, id, block, self_ty, impl_self_ty);
                                 }
                             }
                         }
@@ -233,36 +216,56 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault {
     }
 }
 
+// Check if Self is a unit struct, and avoid any kind of suggestions
+// FIXME: this was copied from DefaultConstructedUnitStructs,
+//        and should be refactored into a common function
+fn is_unit_struct(_cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
+    if let Adt(def, ..) = ty.kind()
+        && def.is_struct()
+        && let var @ VariantDef {
+            ctor: Some((hir::def::CtorKind::Const, _)),
+            ..
+        } = def.non_enum_variant()
+        && !var.is_field_list_non_exhaustive()
+    {
+        true
+    } else {
+        false
+    }
+}
+
 /// Check if a block contains one of these:
 /// - Empty block with an expr (e.g., `{ Self::default() }`)
 /// - One statement (e.g., `{ return Self::default(); }`)
-fn check_block_calls_default(cx: &LateContext<'_>, block: &hir::Block<'_>) -> bool {
+fn check_block_calls_default(cx: &LateContext<'_>, block: &hir::Block<'_>) -> Option<Span> {
     if let Some(expr) = block.expr
         && block.stmts.is_empty()
     {
         // Check the block's trailing expression (e.g., `Self::default()`)
-        check_expr_call_default(cx, expr)
+        if check_expr_call_default(cx, expr) {
+            return None;
+        }
     } else if let [hir::Stmt { kind, .. }] = block.stmts
         && let hir::StmtKind::Expr(expr) | hir::StmtKind::Semi(expr) = kind
+        && let hir::ExprKind::Ret(Some(ret_expr)) = expr.kind
     {
-        // Check if the single statement is a return or expr
-        match expr.kind {
-            // Case 1: `return Self::default();`
-            hir::ExprKind::Ret(Some(ret_expr)) => check_expr_call_default(cx, ret_expr),
-            // Case 2: `Self::default()` (possibly inside a block)
-            hir::ExprKind::Block(block, _) => check_block_calls_default(cx, block),
-            // Case 3: Direct call (e.g., `Self::default()` as the entire body)
-            _ => check_expr_call_default(cx, expr),
+        if check_expr_call_default(cx, ret_expr) {
+            return None;
         }
-    } else {
-        false
     }
+
+    // trim first and last character, and trim spaces
+    let mut span = block.span;
+    span = span.with_lo(span.lo() + BytePos::from_usize(1));
+    span = span.with_hi(span.hi() - BytePos::from_usize(1));
+    span = trim_span(cx.sess().source_map(), span);
+
+    Some(span)
 }
 
 /// Check for `Self::default()` call syntax or equivalent
 fn check_expr_call_default(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
-    if let hir::ExprKind::Call(callee, args) = expr.kind
-        && args.is_empty()
+    if let hir::ExprKind::Call(callee, &[]) = expr.kind
         // FIXME: does this include `Self { }` style calls, which is equivalent,
         //        but not the same as `Self::default()`?
         // FIXME: what should the whole_call_expr (3rd arg) be?
@@ -274,6 +277,36 @@ fn check_expr_call_default(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
     }
 }
 
+fn suggest_default_mismatch_new<'tcx>(
+    cx: &LateContext<'tcx>,
+    span: Span,
+    id: rustc_hir::OwnerId,
+    block: &rustc_hir::Block<'_>,
+    self_ty: Ty<'tcx>,
+    impl_self_ty: &&rustc_hir::Ty<'_>,
+) {
+    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,
+        DEFAULT_MISMATCHES_NEW,
+        id.into(),
+        block.span,
+        format!("you should consider delegating to the auto-derived `Default` for `{self_type_snip}`"),
+        |diag| {
+            // This would replace any comments, and we could work around the first comment,
+            // but in case of a block of code with multiple statements and comment lines,
+            // we can't do much.  For now, we always mark this as a MaybeIncorrect suggestion.
+            diag.span_suggestion(
+                span,
+                "try using this",
+                &"Self::default()",
+                Applicability::MaybeIncorrect,
+            );
+        },
+    );
+}
+
 fn suggest_new_without_default<'tcx>(
     cx: &LateContext<'tcx>,
     item: &hir::Item<'_>,

tests/ui/default_constructed_unit_structs.stderr

@@ -1,3 +1,8 @@
+block.expr = {
+        //should lint
+        Self::default()
+        //~^ default_constructed_unit_structs
+    }
 error: use of `default` to create a unit struct
   --> tests/ui/default_constructed_unit_structs.rs:11:13
    |
@@ -7,12 +12,66 @@ LL |         Self::default()
    = note: `-D clippy::default-constructed-unit-structs` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::default_constructed_unit_structs)]`
 
+block.expr = {
+        // should not lint
+        Self(Default::default())
+    }
+error: you should consider delegating to the auto-derived `Default` for `TupleStruct`
+  --> tests/ui/default_constructed_unit_structs.rs:20:22
+   |
+LL |        fn new() -> Self {
+   |  _______________________^
+LL | |/         // should not lint
+LL | ||         Self(Default::default())
+   | ||________________________________- help: try using this: `Self::default()`
+LL | |      }
+   | |______^
+   |
+   = note: `-D clippy::default-mismatches-new` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::default_mismatches_new)]`
+
+block.expr = {
+        // should lint
+        Self {
+            inner: PhantomData::default(),
+            //~^ default_constructed_unit_structs
+        }
+    }
+error: you should consider delegating to the auto-derived `Default` for `NormalStruct`
+  --> tests/ui/default_constructed_unit_structs.rs:51:22
+   |
+LL |        fn new() -> Self {
+   |  _______________________^
+LL | |/         // should lint
+LL | ||         Self {
+LL | ||             inner: PhantomData::default(),
+LL | ||
+LL | ||         }
+   | ||_________- help: try using this: `Self::default()`
+LL | |      }
+   | |______^
+
 error: use of `default` to create a unit struct
   --> tests/ui/default_constructed_unit_structs.rs:54:31
    |
 LL |             inner: PhantomData::default(),
    |                               ^^^^^^^^^^^ help: remove this call to `default`
 
+block.expr = {
+        // should not lint
+        Self { t: T::default() }
+    }
+error: you should consider delegating to the auto-derived `Default` for `GenericStruct<T>`
+  --> tests/ui/default_constructed_unit_structs.rs:73:22
+   |
+LL |        fn new() -> Self {
+   |  _______________________^
+LL | |/         // should not lint
+LL | ||         Self { t: T::default() }
+   | ||________________________________- help: try using this: `Self::default()`
+LL | |      }
+   | |______^
+
 error: use of `default` to create a unit struct
   --> tests/ui/default_constructed_unit_structs.rs:128:33
    |
@@ -37,5 +96,5 @@ error: use of `default` to create a unit struct
 LL |     let _ = UnitStruct::default();
    |                       ^^^^^^^^^^^ help: remove this call to `default`
 
-error: aborting due to 6 previous errors
+error: aborting due to 9 previous errors