about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-01-05 16:13:38 +0000
committerbors <bors@rust-lang.org>2024-01-05 16:13:38 +0000
commit394f63fe94bb3d991f64644d247a2f7e334d7e4f (patch)
treeecb5b5fb235e574ed57fa2a47a65ad209de98e73
parent9ee4d9ea9a6179eef7c0ad2dd8b55270b3b5d00a (diff)
parentfd9d330839c5ae8ddfa3438b0db422515b06ad36 (diff)
downloadrust-394f63fe94bb3d991f64644d247a2f7e334d7e4f.tar.gz
rust-394f63fe94bb3d991f64644d247a2f7e334d7e4f.zip
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
-rw-r--r--clippy_lints/src/unit_types/let_unit_value.rs21
-rw-r--r--tests/ui/crashes/ice-8821.fixed10
-rw-r--r--tests/ui/crashes/ice-8821.rs2
-rw-r--r--tests/ui/crashes/ice-8821.stderr11
-rw-r--r--tests/ui/let_unit.fixed50
-rw-r--r--tests/ui/let_unit.rs48
-rw-r--r--tests/ui/let_unit.stderr56
7 files changed, 71 insertions, 127 deletions
diff --git a/clippy_lints/src/unit_types/let_unit_value.rs b/clippy_lints/src/unit_types/let_unit_value.rs
index ef67f4b04b5..ed0958197f8 100644
--- 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
deleted file mode 100644
index a25bb46f9ff..00000000000
--- a/tests/ui/crashes/ice-8821.fixed
+++ /dev/null
@@ -1,10 +0,0 @@
-#![warn(clippy::let_unit_value)]
-
-fn f() {}
-static FN: fn() = f;
-
-fn main() {
-    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.rs b/tests/ui/crashes/ice-8821.rs
index 082f7c92646..fb87b79aeed 100644
--- 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
deleted file mode 100644
index 94ebb20918e..00000000000
--- a/tests/ui/crashes/ice-8821.stderr
+++ /dev/null
@@ -1,11 +0,0 @@
-error: this let-binding has unit value
-  --> $DIR/ice-8821.rs:7:5
-   |
-LL |     let _: () = FN();
-   |     ^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `FN();`
-   |
-   = note: `-D clippy::let-unit-value` implied by `-D warnings`
-   = help: to override `-D warnings` add `#[allow(clippy::let_unit_value)]`
-
-error: aborting due to 1 previous error
-
diff --git a/tests/ui/let_unit.fixed b/tests/ui/let_unit.fixed
index f98ce9d50a9..4d41b5e5e50 100644
--- 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
index 6d942ca8908..daa660be25e 100644
--- 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
index de106f50e0e..00a3c439ba0 100644
--- 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