Merge pull request #1967 from koivunej/issue-1964

Fix let_unit_value with for loop iterating over units

clippy_lints/src/utils/higher.rs

@@ -114,6 +114,13 @@ pub fn range(expr: &hir::Expr) -> Option<Range> {
 
 /// Checks if a `let` decl is from a `for` loop desugaring.
 pub fn is_from_for_desugar(decl: &hir::Decl) -> bool {
+    // This will detect plain for-loops without an actual variable binding:
+    //
+    // ```
+    // for x in some_vec {
+    //   // do stuff
+    // }
+    // ```
     if_let_chain! {[
         let hir::DeclLocal(ref loc) = decl.node,
         let Some(ref expr) = loc.init,
@@ -121,6 +128,22 @@ pub fn is_from_for_desugar(decl: &hir::Decl) -> bool {
     ], {
         return true;
     }}
+
+    // This detects a variable binding in for loop to avoid `let_unit_value`
+    // lint (see issue #1964).
+    //
+    // ```
+    // for _ in vec![()] {
+    //   // anything
+    // }
+    // ```
+    if_let_chain! {[
+        let hir::DeclLocal(ref loc) = decl.node,
+        let hir::LocalSource::ForLoopDesugar = loc.source,
+    ], {
+        return true;
+    }}
+
     false
 }
 

tests/ui/let_unit.rs

@@ -18,8 +18,32 @@ fn main() {
         let _a = ();
     }
 
+    consume_units_with_for_loop(); // should be fine as well
+
     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);
+}
+
 #[derive(Copy, Clone)]
 pub struct ContainsUnit(()); // should be fine