Auto merge of #10844 - Centri3:let_unit_value, r=Alexendoo

Don't lint `let_unit_value` when `()` is explicit since these are explicitly written (and not the result of a function call or anything else), they should be allowed, as they are both useful in some cases described in #9048 Fixes #9048 changelog: [`let_unit_value`]: Don't lint when `()` is explicit
rust-lang · Jan 5, 2024 · 394f63f · 394f63f
2 parents 9ee4d9e + fd9d330
commit 394f63f
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 127 deletions.
diff --git a/clippy_lints/src/unit_types/let_unit_value.rs b/clippy_lints/src/unit_types/let_unit_value.rs
@@ -13,12 +13,31 @@ use rustc_middle::ty;
 use super::LET_UNIT_VALUE;
 
 pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, local: &'tcx Local<'_>) {
+    // skip `let () = { ... }`
+    if let PatKind::Tuple(fields, ..) = local.pat.kind
+        && fields.is_empty()
+    {
+        return;
+    }
+
     if let Some(init) = local.init
         && !local.pat.span.from_expansion()
         && !in_external_macro(cx.sess(), local.span)
         && !is_from_async_await(local.span)
         && cx.typeck_results().pat_ty(local.pat).is_unit()
     {
+        // skip `let awa = ()`
+        if let ExprKind::Tup([]) = init.kind {
+            return;
+        }
+
+        // skip `let _: () = { ... }`
+        if let Some(ty) = local.ty
+            && let TyKind::Tup([]) = ty.kind
+        {
+            return;
+        }
+
         if (local.ty.map_or(false, |ty| !matches!(ty.kind, TyKind::Infer))
             || matches!(local.pat.kind, PatKind::Tuple([], ddpos) if ddpos.as_opt_usize().is_none()))
             && expr_needs_inferred_result(cx, init)
@@ -34,7 +53,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, local: &'tcx Local<'_>) {
                     |diag| {
                         diag.span_suggestion(
                             local.pat.span,
-                            "use a wild (`_`) binding",
+                            "use a wildcard binding",
                             "_",
                             Applicability::MaybeIncorrect, // snippet
                         );

diff --git a/tests/ui/crashes/ice-8821.fixed b/tests/ui/crashes/ice-8821.fixed
diff --git a/tests/ui/crashes/ice-8821.rs b/tests/ui/crashes/ice-8821.rs
@@ -5,6 +5,4 @@ static FN: fn() = f;
 
 fn main() {
     let _: () = FN();
-    //~^ ERROR: this let-binding has unit value
-    //~| NOTE: `-D clippy::let-unit-value` implied by `-D warnings`
 }
diff --git a/tests/ui/crashes/ice-8821.stderr b/tests/ui/crashes/ice-8821.stderr
diff --git a/tests/ui/let_unit.fixed b/tests/ui/let_unit.fixed
@@ -13,7 +13,14 @@ fn main() {
     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
@@ -23,6 +30,8 @@ fn main() {
     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.
@@ -74,40 +83,29 @@ fn _returns_generic() {
         x.then(|| T::default())
     }
 
-    let _: () = f(); // Ok
-    let _: () = f(); // Lint.
+    let _: () = f();
+    let x: () = f();
 
-    let _: () = f2(0i32); // Ok
-    let _: () = f2(0i32); // Lint.
+    let _: () = f2(0i32);
+    let x: () = f2(0i32);
 
-    f3(()); // Lint
-    f3(()); // Lint
+    let _: () = f3(());
+    let x: () = f3(());
 
-    // Should lint:
-    // fn f4<T>(mut x: Vec<T>) -> T {
-    //    x.pop().unwrap()
-    // }
-    // let _: () = f4(vec![()]);
-    // let x: () = f4(vec![()]);
+    fn f4<T>(mut x: Vec<T>) -> T {
+        x.pop().unwrap()
+    }
+    let _: () = f4(vec![()]);
+    let x: () = f4(vec![()]);
 
-    // Ok
     let _: () = {
         let x = 5;
         f2(x)
     };
 
-    let _: () = if true { f() } else { f2(0) }; // Ok
-    let _: () = if true { f() } else { f2(0) }; // Lint
-
-    // Ok
-    let _: () = match Some(0) {
-        None => f2(1),
-        Some(0) => f(),
-        Some(1) => f2(3),
-        Some(_) => f2('x'),
-    };
+    let _: () = if true { f() } else { f2(0) };
+    let x: () = if true { f() } else { f2(0) };
 
-    // Lint
     match Some(0) {
         None => f2(1),
         Some(0) => f(),
@@ -155,7 +153,7 @@ fn _returns_generic() {
         {
             let _: () = x;
             let _: () = y;
-            z;
+            let _: () = z;
             let _: () = x1;
             let _: () = x2;
             let _: () = opt;

diff --git a/tests/ui/let_unit.rs b/tests/ui/let_unit.rs
@@ -13,7 +13,14 @@ fn main() {
     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
@@ -23,6 +30,8 @@ fn main() {
     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.
@@ -74,41 +83,30 @@ fn _returns_generic() {
         x.then(|| T::default())
     }
 
-    let _: () = f(); // Ok
-    let x: () = f(); // Lint.
+    let _: () = f();
+    let x: () = f();
 
-    let _: () = f2(0i32); // Ok
-    let x: () = f2(0i32); // Lint.
+    let _: () = f2(0i32);
+    let x: () = f2(0i32);
 
-    let _: () = f3(()); // Lint
-    let x: () = f3(()); // Lint
+    let _: () = f3(());
+    let x: () = f3(());
 
-    // Should lint:
-    // fn f4<T>(mut x: Vec<T>) -> T {
-    //    x.pop().unwrap()
-    // }
-    // let _: () = f4(vec![()]);
-    // let x: () = f4(vec![()]);
+    fn f4<T>(mut x: Vec<T>) -> T {
+        x.pop().unwrap()
+    }
+    let _: () = f4(vec![()]);
+    let x: () = f4(vec![()]);
 
-    // Ok
     let _: () = {
         let x = 5;
         f2(x)
     };
 
-    let _: () = if true { f() } else { f2(0) }; // Ok
-    let x: () = if true { f() } else { f2(0) }; // Lint
-
-    // Ok
-    let _: () = match Some(0) {
-        None => f2(1),
-        Some(0) => f(),
-        Some(1) => f2(3),
-        Some(_) => f2('x'),
-    };
+    let _: () = if true { f() } else { f2(0) };
+    let x: () = if true { f() } else { f2(0) };
 
-    // Lint
-    let _: () = match Some(0) {
+    let x = match Some(0) {
         None => f2(1),
         Some(0) => f(),
         Some(1) => f2(3),

diff --git a/tests/ui/let_unit.stderr b/tests/ui/let_unit.stderr
@@ -8,13 +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
-  --> $DIR/let_unit.rs:16:9
-   |
-LL |         let _a = ();
-   |         ^^^^^^^^^^^^ help: omit the `let` binding: `();`
-
-error: this let-binding has unit value
-  --> $DIR/let_unit.rs:51:5
+  --> $DIR/let_unit.rs:60:5
    |
 LL | /     let _ = v
 LL | |         .into_iter()
@@ -37,45 +31,9 @@ LL +         .unwrap();
    |
 
 error: this let-binding has unit value
-  --> $DIR/let_unit.rs:78:5
-   |
-LL |     let x: () = f(); // Lint.
-   |     ^^^^-^^^^^^^^^^^
-   |         |
-   |         help: use a wild (`_`) binding: `_`
-
-error: this let-binding has unit value
-  --> $DIR/let_unit.rs:81:5
+  --> $DIR/let_unit.rs:109:5
    |
-LL |     let x: () = f2(0i32); // Lint.
-   |     ^^^^-^^^^^^^^^^^^^^^^
-   |         |
-   |         help: use a wild (`_`) binding: `_`
-
-error: this let-binding has unit value
-  --> $DIR/let_unit.rs:83:5
-   |
-LL |     let _: () = f3(()); // Lint
-   |     ^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `f3(());`
-
-error: this let-binding has unit value
-  --> $DIR/let_unit.rs:84:5
-   |
-LL |     let x: () = f3(()); // Lint
-   |     ^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `f3(());`
-
-error: this let-binding has unit value
-  --> $DIR/let_unit.rs:100:5
-   |
-LL |     let x: () = if true { f() } else { f2(0) }; // Lint
-   |     ^^^^-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |         |
-   |         help: use a wild (`_`) binding: `_`
-
-error: this let-binding has unit value
-  --> $DIR/let_unit.rs:111:5
-   |
-LL | /     let _: () = match Some(0) {
+LL | /     let x = match Some(0) {
 LL | |         None => f2(1),
 LL | |         Some(0) => f(),
 LL | |         Some(1) => f2(3),
@@ -93,11 +51,5 @@ LL +         Some(_) => (),
 LL +     };
    |
 
-error: this let-binding has unit value
-  --> $DIR/let_unit.rs:158:13
-   |
-LL |             let _: () = z;
-   |             ^^^^^^^^^^^^^^ help: omit the `let` binding: `z;`
-
-error: aborting due to 10 previous errors
+error: aborting due to 3 previous errors