fix: unit_arg suggests wrongly for Default::default

Author: profetia

clippy_lints/src/default.rs

@@ -1,10 +1,9 @@
 use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_sugg};
 use clippy_utils::source::snippet_with_context;
 use clippy_utils::ty::{has_drop, is_copy};
-use clippy_utils::{contains_name, get_parent_expr, in_automatically_derived, is_from_proc_macro};
+use clippy_utils::{contains_name, get_parent_expr, in_automatically_derived, is_expr_default, is_from_proc_macro};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::Applicability;
-use rustc_hir::def::Res;
 use rustc_hir::{Block, Expr, ExprKind, PatKind, QPath, Stmt, StmtKind, StructTailExpr};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty;
@@ -129,7 +128,7 @@ impl<'tcx> LateLintPass<'tcx> for Default {
                 // only take bindings to identifiers
                 && let PatKind::Binding(_, binding_id, ident, _) = local.pat.kind
                 // only when assigning `... = Default::default()`
-                && is_expr_default(expr, cx)
+                && is_expr_default(cx, expr)
                 && let binding_type = cx.typeck_results().node_type(binding_id)
                 && let ty::Adt(adt, args) = *binding_type.kind()
                 && adt.is_struct()
@@ -251,19 +250,6 @@ impl<'tcx> LateLintPass<'tcx> for Default {
     }
 }
 
-/// Checks if the given expression is the `default` method belonging to the `Default` trait.
-fn is_expr_default<'tcx>(expr: &'tcx Expr<'tcx>, cx: &LateContext<'tcx>) -> bool {
-    if let ExprKind::Call(fn_expr, []) = &expr.kind
-        && let ExprKind::Path(qpath) = &fn_expr.kind
-        && let Res::Def(_, def_id) = cx.qpath_res(qpath, fn_expr.hir_id)
-    {
-        // right hand side of assignment is `Default::default`
-        cx.tcx.is_diagnostic_item(sym::default_fn, def_id)
-    } else {
-        false
-    }
-}
-
 /// Returns the reassigned field and the assigning expression (right-hand side of assign).
 fn field_reassigned_by_stmt<'tcx>(this: &Stmt<'tcx>, binding_name: Symbol) -> Option<(Ident, &'tcx Expr<'tcx>)> {
     if let StmtKind::Semi(later_expr) = this.kind

clippy_lints/src/unit_types/unit_arg.rs

@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::is_from_proc_macro;
 use clippy_utils::source::{SourceText, SpanRangeExt, indent_of, reindent_multiline};
+use clippy_utils::{is_expr_default, is_from_proc_macro};
 use rustc_errors::Applicability;
 use rustc_hir::{Block, Expr, ExprKind, MatchSource, Node, StmtKind};
 use rustc_lint::LateContext;
@@ -59,7 +59,7 @@ fn is_questionmark_desugar_marked_call(expr: &Expr<'_>) -> bool {
     }
 }
 
-fn lint_unit_args(cx: &LateContext<'_>, expr: &Expr<'_>, args_to_recover: &[&Expr<'_>]) {
+fn lint_unit_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, args_to_recover: &[&'tcx Expr<'tcx>]) {
     let mut applicability = Applicability::MachineApplicable;
     let (singular, plural) = if args_to_recover.len() > 1 {
         ("", "s")
@@ -102,9 +102,11 @@ fn lint_unit_args(cx: &LateContext<'_>, expr: &Expr<'_>, args_to_recover: &[&Exp
                 .iter()
                 .filter_map(|arg| arg.span.get_source_text(cx))
                 .collect();
-            let arg_snippets_without_empty_blocks: Vec<_> = args_to_recover
+
+            // If the argument is an empty block or `Default::default()`, we can replace it with `()`.
+            let arg_snippets_without_redundant_exprs: Vec<_> = args_to_recover
                 .iter()
-                .filter(|arg| !is_empty_block(arg))
+                .filter(|arg| !is_empty_block(arg) && !is_expr_default(cx, arg))
                 .filter_map(|arg| arg.span.get_source_text(cx))
                 .collect();
 
@@ -114,10 +116,10 @@ fn lint_unit_args(cx: &LateContext<'_>, expr: &Expr<'_>, args_to_recover: &[&Exp
                     expr,
                     &call_snippet,
                     &arg_snippets,
-                    &arg_snippets_without_empty_blocks,
+                    &arg_snippets_without_redundant_exprs,
                 );
 
-                if arg_snippets_without_empty_blocks.is_empty() {
+                if arg_snippets_without_redundant_exprs.is_empty() {
                     db.multipart_suggestion(
                         format!("use {singular}unit literal{plural} instead"),
                         args_to_recover
@@ -127,7 +129,7 @@ fn lint_unit_args(cx: &LateContext<'_>, expr: &Expr<'_>, args_to_recover: &[&Exp
                         applicability,
                     );
                 } else {
-                    let plural = arg_snippets_without_empty_blocks.len() > 1;
+                    let plural = arg_snippets_without_redundant_exprs.len() > 1;
                     let empty_or_s = if plural { "s" } else { "" };
                     let it_or_them = if plural { "them" } else { "it" };
                     db.span_suggestion(

clippy_utils/src/lib.rs

@@ -3471,3 +3471,16 @@ pub fn desugar_await<'tcx>(expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
         None
     }
 }
+
+/// Checks if the given expression is the `default` method belonging to the `Default` trait.
+pub fn is_expr_default<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
+    if let ExprKind::Call(fn_expr, []) = &expr.kind
+        && let ExprKind::Path(qpath) = &fn_expr.kind
+        && let Res::Def(_, def_id) = cx.qpath_res(qpath, fn_expr.hir_id)
+    {
+        // right hand side of assignment is `Default::default`
+        cx.tcx.is_diagnostic_item(sym::default_fn, def_id)
+    } else {
+        false
+    }
+}

tests/ui/unit_arg_empty_blocks.fixed renamed to tests/ui/unit_arg_fixable.fixed

@@ -32,3 +32,9 @@ fn taking_three_units(a: (), b: (), c: ()) {}
 fn main() {
     bad();
 }
+
+fn issue14857() {
+    let fn_take_unit = |_: ()| {};
+    fn_take_unit(());
+    //~^ unit_arg
+}

tests/ui/unit_arg_empty_blocks.rs renamed to tests/ui/unit_arg_fixable.rs

@@ -29,3 +29,9 @@ fn taking_three_units(a: (), b: (), c: ()) {}
 fn main() {
     bad();
 }
+
+fn issue14857() {
+    let fn_take_unit = |_: ()| {};
+    fn_take_unit(Default::default());
+    //~^ unit_arg
+}

tests/ui/unit_arg_empty_blocks.stderr renamed to tests/ui/unit_arg_fixable.stderr

@@ -1,5 +1,5 @@
 error: passing a unit value to a function
-  --> tests/ui/unit_arg_empty_blocks.rs:16:5
+  --> tests/ui/unit_arg_fixable.rs:16:5
    |
 LL |     foo({});
    |     ^^^^--^
@@ -10,15 +10,15 @@ LL |     foo({});
    = help: to override `-D warnings` add `#[allow(clippy::unit_arg)]`
 
 error: passing a unit value to a function
-  --> tests/ui/unit_arg_empty_blocks.rs:18:5
+  --> tests/ui/unit_arg_fixable.rs:18:5
    |
 LL |     foo3({}, 2, 2);
    |     ^^^^^--^^^^^^^
    |          |
    |          help: use a unit literal instead: `()`
 
 error: passing unit values to a function
-  --> tests/ui/unit_arg_empty_blocks.rs:20:5
+  --> tests/ui/unit_arg_fixable.rs:20:5
    |
 LL |     taking_two_units({}, foo(0));
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -30,7 +30,7 @@ LL ~     taking_two_units((), ());
    |
 
 error: passing unit values to a function
-  --> tests/ui/unit_arg_empty_blocks.rs:22:5
+  --> tests/ui/unit_arg_fixable.rs:22:5
    |
 LL |     taking_three_units({}, foo(0), foo(1));
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -42,5 +42,13 @@ LL +     foo(1);
 LL ~     taking_three_units((), (), ());
    |
 
-error: aborting due to 4 previous errors
+error: passing a unit value to a function
+  --> tests/ui/unit_arg_fixable.rs:35:5
+   |
+LL |     fn_take_unit(Default::default());
+   |     ^^^^^^^^^^^^^------------------^
+   |                  |
+   |                  help: use a unit literal instead: `()`
+
+error: aborting due to 5 previous errors