let_unit_value: Use multipart suggestions

clippy_lints/src/unit_types/let_unit_value.rs

@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::snippet_with_context;
-use clippy_utils::visitors::{for_each_local_assignment, for_each_value_source, is_local_used};
+use clippy_utils::visitors::{for_each_local_assignment, for_each_value_source};
 use core::ops::ControlFlow;
 use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
@@ -71,25 +71,38 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, local: &'tcx LetStmt<'_>) {
                 local.span,
                 "this let-binding has unit value",
                 |diag| {
+                    let mut suggestions = Vec::new();
+
+                    // Suggest omitting the `let` binding
                     if let Some(expr) = &local.init {
                         let mut app = Applicability::MachineApplicable;
                         let snip = snippet_with_context(cx, expr.span, local.span.ctxt(), "()", &mut app).0;
-                        diag.span_suggestion(local.span, "omit the `let` binding", format!("{snip};"), app);
+                        suggestions.push((local.span, format!("{snip};")));
                     }
 
-                    if let PatKind::Binding(_, binding_hir_id, ident, ..) = local.pat.kind
+                    // If this is a binding pattern, we need to add suggestions to remove any usages
+                    // of the variable
+                    if let PatKind::Binding(_, binding_hir_id, ..) = local.pat.kind
                         && let Some(body_id) = cx.enclosing_body.as_ref()
-                        && let body = cx.tcx.hir().body(*body_id)
-                        && is_local_used(cx, body, binding_hir_id)
                     {
-                        let identifier = ident.as_str();
+                        let body = cx.tcx.hir().body(*body_id);
+
+                        // Collect variable usages
                         let mut visitor = UnitVariableCollector::new(binding_hir_id);
                         walk_body(&mut visitor, body);
-                        visitor.spans.into_iter().for_each(|span| {
-                            let msg =
-                                format!("variable `{identifier}` of type `()` can be replaced with explicit `()`");
-                            diag.span_suggestion(span, msg, "()", Applicability::MachineApplicable);
-                        });
+
+                        // Add suggestions for replacing variable usages
+                        suggestions.extend(visitor.spans.into_iter().map(|span| (span, "()".to_string())));
+                    }
+
+                    // Emit appropriate diagnostic based on whether there are usages of the let binding
+                    if !suggestions.is_empty() {
+                        let message = if suggestions.len() == 1 {
+                            "omit the `let` binding"
+                        } else {
+                            "omit the `let` binding and replace variable usages with `()`"
+                        };
+                        diag.multipart_suggestion(message, suggestions, Applicability::MachineApplicable);
                     }
                 },
             );

tests/ui/let_unit.fixed

@@ -0,0 +1,196 @@
+#![warn(clippy::let_unit_value)]
+#![allow(unused, clippy::no_effect, clippy::needless_late_init, path_statements)]
+
+macro_rules! let_and_return {
+    ($n:expr) => {{
+        let ret = $n;
+    }};
+}
+
+fn main() {
+    println!("x");
+    let _y = 1; // this is fine
+    let _z = ((), 1); // this as well
+    if true {
+        // do not lint this, since () is explicit
+        let _a = ();
+        let () = dummy();
+        let () = ();
+        () = dummy();
+        () = ();
+        let _a: () = ();
+        let _a: () = dummy();
+    }
+
+    consume_units_with_for_loop(); // should be fine as well
+
+    multiline_sugg();
+
+    let_and_return!(()) // should be fine
+}
+
+fn dummy() {}
+
+// Related to issue #1964
+fn consume_units_with_for_loop() {
+    // `for_let_unit` lint should not be triggered by consuming them using for loop.
+    let v = vec![(), (), ()];
+    let mut count = 0;
+    for _ in v {
+        count += 1;
+    }
+    assert_eq!(count, 3);
+
+    // Same for consuming from some other Iterator<Item = ()>.
+    let (tx, rx) = ::std::sync::mpsc::channel();
+    tx.send(()).unwrap();
+    drop(tx);
+
+    count = 0;
+    for _ in rx.iter() {
+        count += 1;
+    }
+    assert_eq!(count, 1);
+}
+
+fn multiline_sugg() {
+    let v: Vec<u8> = vec![2];
+
+    v
+        .into_iter()
+        .map(|i| i * 2)
+        .filter(|i| i % 2 == 0)
+        .map(|_| ())
+        .next()
+        .unwrap();
+}
+
+#[derive(Copy, Clone)]
+pub struct ContainsUnit(()); // should be fine
+
+fn _returns_generic() {
+    fn f<T>() -> T {
+        unimplemented!()
+    }
+    fn f2<T, U>(_: T) -> U {
+        unimplemented!()
+    }
+    fn f3<T>(x: T) -> T {
+        x
+    }
+    fn f5<T: Default>(x: bool) -> Option<T> {
+        x.then(|| T::default())
+    }
+
+    let _: () = f();
+    let x: () = f();
+
+    let _: () = f2(0i32);
+    let x: () = f2(0i32);
+
+    let _: () = f3(());
+    let x: () = f3(());
+
+    fn f4<T>(mut x: Vec<T>) -> T {
+        x.pop().unwrap()
+    }
+    let _: () = f4(vec![()]);
+    let x: () = f4(vec![()]);
+
+    let _: () = {
+        let x = 5;
+        f2(x)
+    };
+
+    let _: () = if true { f() } else { f2(0) };
+    let x: () = if true { f() } else { f2(0) };
+
+    match Some(0) {
+        None => f2(1),
+        Some(0) => f(),
+        Some(1) => f2(3),
+        Some(_) => (),
+    };
+
+    let _: () = f5(true).unwrap();
+
+    #[allow(clippy::let_unit_value)]
+    {
+        let x = f();
+        let y;
+        let z;
+        match 0 {
+            0 => {
+                y = f();
+                z = f();
+            },
+            1 => {
+                println!("test");
+                y = f();
+                z = f3(());
+            },
+            _ => panic!(),
+        }
+
+        let x1;
+        let x2;
+        if true {
+            x1 = f();
+            x2 = x1;
+        } else {
+            x2 = f();
+            x1 = x2;
+        }
+
+        let opt;
+        match f5(true) {
+            Some(x) => opt = x,
+            None => panic!(),
+        };
+
+        #[warn(clippy::let_unit_value)]
+        {
+            let _: () = x;
+            let _: () = y;
+            let _: () = z;
+            let _: () = x1;
+            let _: () = x2;
+            let _: () = opt;
+        }
+    }
+
+    let () = f();
+}
+
+fn attributes() {
+    fn f() {}
+
+    #[allow(clippy::let_unit_value)]
+    let _ = f();
+    #[expect(clippy::let_unit_value)]
+    let _ = f();
+}
+
+async fn issue10433() {
+    let _pending: () = std::future::pending().await;
+}
+
+pub async fn issue11502(a: ()) {}
+
+pub fn issue12594() {
+    fn returns_unit() {}
+
+    fn returns_result<T>(res: T) -> Result<T, ()> {
+        Ok(res)
+    }
+
+    fn actual_test() {
+        // create first a unit value'd value
+        returns_unit();
+        returns_result(()).unwrap();
+        returns_result(()).unwrap();
+        // make sure we replace only the first variable
+        let res = 1;
+        returns_result(res).unwrap();
+    }
+}

tests/ui/let_unit.rs

@@ -1,8 +1,6 @@
 #![warn(clippy::let_unit_value)]
 #![allow(unused, clippy::no_effect, clippy::needless_late_init, path_statements)]
 
-//@no-rustfix: need to change the suggestion to a multipart suggestion
-
 macro_rules! let_and_return {
     ($n:expr) => {{
         let ret = $n;

tests/ui/let_unit.stderr

@@ -1,5 +1,5 @@
 error: this let-binding has unit value
-  --> tests/ui/let_unit.rs:13:5
+  --> tests/ui/let_unit.rs:11:5
    |
 LL |     let _x = println!("x");
    |     ^^^^^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `println!("x");`
@@ -8,7 +8,7 @@ LL |     let _x = println!("x");
    = help: to override `-D warnings` add `#[allow(clippy::let_unit_value)]`
 
 error: this let-binding has unit value
-  --> tests/ui/let_unit.rs:61:5
+  --> tests/ui/let_unit.rs:59:5
    |
 LL | /     let _ = v
 LL | |         .into_iter()
@@ -31,7 +31,7 @@ LL +         .unwrap();
    |
 
 error: this let-binding has unit value
-  --> tests/ui/let_unit.rs:110:5
+  --> tests/ui/let_unit.rs:108:5
    |
 LL | /     let x = match Some(0) {
 LL | |         None => f2(1),
@@ -52,23 +52,17 @@ LL +     };
    |
 
 error: this let-binding has unit value
-  --> tests/ui/let_unit.rs:191:9
+  --> tests/ui/let_unit.rs:189:9
    |
 LL |         let res = returns_unit();
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
-help: omit the `let` binding
+help: omit the `let` binding and replace variable usages with `()`
    |
-LL |         returns_unit();
+LL ~         returns_unit();
+LL ~         returns_result(()).unwrap();
+LL ~         returns_result(()).unwrap();
    |
-help: variable `res` of type `()` can be replaced with explicit `()`
-   |
-LL |         returns_result(()).unwrap();
-   |                        ~~
-help: variable `res` of type `()` can be replaced with explicit `()`
-   |
-LL |         returns_result(()).unwrap();
-   |                        ~~
 
 error: aborting due to 4 previous errors