about summary refs log tree commit diff
diff options
context:
space:
mode:
-rw-r--r--clippy_lints/src/types.rs22
-rw-r--r--tests/ui/let_unit.fixed63
-rw-r--r--tests/ui/let_unit.rs17
-rw-r--r--tests/ui/let_unit.stderr35
4 files changed, 120 insertions, 17 deletions
diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs
index 654674de06a..7faf1911775 100644
--- a/clippy_lints/src/types.rs
+++ b/clippy_lints/src/types.rs
@@ -25,7 +25,7 @@ use crate::utils::paths;
 use crate::utils::{
     clip, comparisons, differing_macro_contexts, higher, in_constant, in_macro_or_desugar, int_bits, last_path_segment,
     match_def_path, match_path, multispan_sugg, same_tys, sext, snippet, snippet_opt, snippet_with_applicability,
-    span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, unsext,
+    snippet_with_macro_callsite, span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, unsext,
 };
 
 declare_clippy_lint! {
@@ -467,15 +467,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetUnitValue {
                 if higher::is_from_for_desugar(local) {
                     return;
                 }
-                span_lint(
-                    cx,
-                    LET_UNIT_VALUE,
-                    stmt.span,
-                    &format!(
-                        "this let-binding has unit value. Consider omitting `let {} =`",
-                        snippet(cx, local.pat.span, "..")
-                    ),
-                );
+                span_lint_and_then(cx, LET_UNIT_VALUE, stmt.span, "this let-binding has unit value", |db| {
+                    if let Some(expr) = &local.init {
+                        let snip = snippet_with_macro_callsite(cx, expr.span, "()");
+                        db.span_suggestion(
+                            stmt.span,
+                            "omit the `let` binding",
+                            format!("{};", snip),
+                            Applicability::MachineApplicable, // snippet
+                        );
+                    }
+                });
             }
         }
     }
diff --git a/tests/ui/let_unit.fixed b/tests/ui/let_unit.fixed
new file mode 100644
index 00000000000..f398edc23cb
--- /dev/null
+++ b/tests/ui/let_unit.fixed
@@ -0,0 +1,63 @@
+// run-rustfix
+
+#![warn(clippy::let_unit_value)]
+#![allow(clippy::no_effect)]
+#![allow(unused_variables)]
+
+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 {
+        ();
+    }
+
+    consume_units_with_for_loop(); // should be fine as well
+
+    multiline_sugg();
+
+    let_and_return!(()) // should be fine
+}
+
+// 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
diff --git a/tests/ui/let_unit.rs b/tests/ui/let_unit.rs
index dc17c98f6a8..af5b1fb2ac7 100644
--- a/tests/ui/let_unit.rs
+++ b/tests/ui/let_unit.rs
@@ -1,4 +1,7 @@
+// run-rustfix
+
 #![warn(clippy::let_unit_value)]
+#![allow(clippy::no_effect)]
 #![allow(unused_variables)]
 
 macro_rules! let_and_return {
@@ -17,6 +20,8 @@ fn main() {
 
     consume_units_with_for_loop(); // should be fine as well
 
+    multiline_sugg();
+
     let_and_return!(()) // should be fine
 }
 
@@ -42,5 +47,17 @@ fn consume_units_with_for_loop() {
     assert_eq!(count, 1);
 }
 
+fn multiline_sugg() {
+    let v: Vec<u8> = vec![2];
+
+    let _ = v
+        .into_iter()
+        .map(|i| i * 2)
+        .filter(|i| i % 2 == 0)
+        .map(|_| ())
+        .next()
+        .unwrap();
+}
+
 #[derive(Copy, Clone)]
 pub struct ContainsUnit(()); // should be fine
diff --git a/tests/ui/let_unit.stderr b/tests/ui/let_unit.stderr
index e1773a40225..7130fcd870e 100644
--- a/tests/ui/let_unit.stderr
+++ b/tests/ui/let_unit.stderr
@@ -1,16 +1,37 @@
-error: this let-binding has unit value. Consider omitting `let _x =`
-  --> $DIR/let_unit.rs:11:5
+error: this let-binding has unit value
+  --> $DIR/let_unit.rs:14:5
    |
 LL |     let _x = println!("x");
-   |     ^^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `println!("x");`
    |
    = note: `-D clippy::let-unit-value` implied by `-D warnings`
 
-error: this let-binding has unit value. Consider omitting `let _a =`
-  --> $DIR/let_unit.rs:15:9
+error: this let-binding has unit value
+  --> $DIR/let_unit.rs:18:9
    |
 LL |         let _a = ();
-   |         ^^^^^^^^^^^^
+   |         ^^^^^^^^^^^^ help: omit the `let` binding: `();`
 
-error: aborting due to 2 previous errors
+error: this let-binding has unit value
+  --> $DIR/let_unit.rs:53:5
+   |
+LL | /     let _ = v
+LL | |         .into_iter()
+LL | |         .map(|i| i * 2)
+LL | |         .filter(|i| i % 2 == 0)
+LL | |         .map(|_| ())
+LL | |         .next()
+LL | |         .unwrap();
+   | |__________________^
+help: omit the `let` binding
+   |
+LL |     v
+LL |         .into_iter()
+LL |         .map(|i| i * 2)
+LL |         .filter(|i| i % 2 == 0)
+LL |         .map(|_| ())
+LL |         .next()
+ ...
+
+error: aborting due to 3 previous errors