Lint more cases in unit_arg

Author: profetia

clippy_lints/src/unit_types/unit_arg.rs

@@ -1,6 +1,9 @@
+use std::iter;
+
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::{SpanRangeExt, indent_of, reindent_multiline};
 use clippy_utils::sugg::Sugg;
+use clippy_utils::ty::expr_type_is_certain;
 use clippy_utils::{is_expr_default, is_from_proc_macro};
 use rustc_errors::Applicability;
 use rustc_hir::{Block, Expr, ExprKind, MatchSource, Node, StmtKind};
@@ -111,18 +114,10 @@ fn lint_unit_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, args_to_
             let arg_snippets_without_redundant_exprs: Vec<_> = args_to_recover
                 .iter()
                 .filter(|arg| !is_expr_default_nested(cx, arg) && (arg.span.from_expansion() || !is_empty_block(arg)))
-                .filter_map(|arg| get_expr_snippet(cx, arg))
+                .filter_map(|arg| get_expr_snippet_with_type_certainty(cx, arg))
                 .collect();
 
             if let Some(call_snippet) = expr.span.get_source_text(cx) {
-                let sugg = fmt_stmts_and_call(
-                    cx,
-                    expr,
-                    &call_snippet,
-                    &arg_snippets,
-                    &arg_snippets_without_redundant_exprs,
-                );
-
                 if arg_snippets_without_redundant_exprs.is_empty()
                     && let suggestions = args_to_recover
                         .iter()
@@ -138,6 +133,13 @@ fn lint_unit_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, args_to_
                     );
                 } else {
                     let plural = arg_snippets_without_redundant_exprs.len() > 1;
+                    let sugg = fmt_stmts_and_call(
+                        cx,
+                        expr,
+                        &call_snippet,
+                        arg_snippets,
+                        arg_snippets_without_redundant_exprs,
+                    );
                     let empty_or_s = if plural { "s" } else { "" };
                     let it_or_them = if plural { "them" } else { "it" };
                     db.span_suggestion(
@@ -160,6 +162,20 @@ fn is_expr_default_nested<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>)
         if block.expr.is_some() && is_expr_default_nested(cx, block.expr.unwrap()))
 }
 
+enum MaybeTypeUncertain<'tcx> {
+    Certain(Sugg<'tcx>),
+    Uncertain(Sugg<'tcx>),
+}
+
+impl From<MaybeTypeUncertain<'_>> for String {
+    fn from(value: MaybeTypeUncertain<'_>) -> Self {
+        match value {
+            MaybeTypeUncertain::Certain(sugg) => sugg.to_string(),
+            MaybeTypeUncertain::Uncertain(sugg) => format!("let _: () = {sugg}"),
+        }
+    }
+}
+
 fn get_expr_snippet<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<Sugg<'tcx>> {
     let mut app = Applicability::MachineApplicable;
     let snip = Sugg::hir_with_context(cx, expr, SyntaxContext::root(), "..", &mut app);
@@ -170,6 +186,25 @@ fn get_expr_snippet<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Opt
     Some(snip)
 }
 
+fn get_expr_snippet_with_type_certainty<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'tcx>,
+) -> Option<MaybeTypeUncertain<'tcx>> {
+    get_expr_snippet(cx, expr).map(|snip| {
+        // If the type of the expression is certain, we can use it directly.
+        // Otherwise, we wrap it in a `let _: () = ...` to ensure the type is correct.
+        if !expr_type_is_certain(cx, expr) && !is_block_with_no_expr(expr) {
+            MaybeTypeUncertain::Uncertain(snip)
+        } else {
+            MaybeTypeUncertain::Certain(snip)
+        }
+    })
+}
+
+fn is_block_with_no_expr(expr: &Expr<'_>) -> bool {
+    matches!(expr.kind, ExprKind::Block(Block { expr: None, .. }, _))
+}
+
 fn is_empty_block(expr: &Expr<'_>) -> bool {
     matches!(
         expr.kind,
@@ -188,23 +223,20 @@ fn fmt_stmts_and_call(
     cx: &LateContext<'_>,
     call_expr: &Expr<'_>,
     call_snippet: &str,
-    args_snippets: &[Sugg<'_>],
-    non_empty_block_args_snippets: &[Sugg<'_>],
+    args_snippets: Vec<Sugg<'_>>,
+    non_empty_block_args_snippets: Vec<MaybeTypeUncertain<'_>>,
 ) -> String {
     let call_expr_indent = indent_of(cx, call_expr.span).unwrap_or(0);
-    let call_snippet_with_replacements = args_snippets.iter().fold(call_snippet.to_owned(), |acc, arg| {
+    let call_snippet_with_replacements = args_snippets.into_iter().fold(call_snippet.to_owned(), |acc, arg| {
         acc.replacen(&arg.to_string(), "()", 1)
     });
 
-    let mut stmts_and_call = non_empty_block_args_snippets
-        .iter()
-        .map(ToString::to_string)
-        .collect::<Vec<_>>();
-    stmts_and_call.push(call_snippet_with_replacements);
-    stmts_and_call = stmts_and_call
+    let stmts_and_call = non_empty_block_args_snippets
         .into_iter()
+        .map(Into::into)
+        .chain(iter::once(call_snippet_with_replacements))
         .map(|v| reindent_multiline(&v, true, Some(call_expr_indent)))
-        .collect();
+        .collect::<Vec<_>>();
 
     let mut stmts_and_call_snippet = stmts_and_call.join(&format!("{}{}", ";\n", " ".repeat(call_expr_indent)));
     // expr is not in a block statement or result expression position, wrap in a block

tests/ui/unit_arg_fixable.fixed

@@ -67,4 +67,12 @@ fn issue14857() {
     mac!(empty_block);
     fn_take_unit(());
     //~^ unit_arg
+
+    fn def<T: Default>() -> T {
+        Default::default()
+    }
+
+    let _: () = def();
+    fn_take_unit(());
+    //~^ unit_arg
 }

tests/ui/unit_arg_fixable.rs

@@ -61,4 +61,11 @@ fn issue14857() {
     //~^ unit_arg
     fn_take_unit(mac!(empty_block));
     //~^ unit_arg
+
+    fn def<T: Default>() -> T {
+        Default::default()
+    }
+
+    fn_take_unit(def());
+    //~^ unit_arg
 }

tests/ui/unit_arg_fixable.stderr

@@ -94,5 +94,17 @@ LL ~     mac!(empty_block);
 LL ~     fn_take_unit(());
    |
 
-error: aborting due to 9 previous errors
+error: passing a unit value to a function
+  --> tests/ui/unit_arg_fixable.rs:69:5
+   |
+LL |     fn_take_unit(def());
+   |     ^^^^^^^^^^^^^^^^^^^
+   |
+help: move the expression in front of the call and replace it with the unit literal `()`
+   |
+LL ~     let _: () = def();
+LL ~     fn_take_unit(());
+   |
+
+error: aborting due to 10 previous errors