Small cleanup of option_if_let_else and additional tests.

rust-lang · Aug 26, 2021 · 3e5dcba · 3e5dcba
1 parent 8b3ca9a
commit 3e5dcba
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 17 deletions.
diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs
@@ -174,9 +174,9 @@ fn detect_option_if_let_else<'tcx>(
                     ExprKind::Field(e, _) | ExprKind::AddrOf(_, _, e) => Some(e),
                     _ => None,
                 });
-                if let ExprKind::Path(QPath::Resolved(None, Path { res: Res::Local(l), .. })) = e.kind {
-                    match some_captures.get(l)
-                        .or_else(|| (method_sugg == "map_or_else").then(|| ()).and_then(|_| none_captures.get(l)))
+                if let ExprKind::Path(QPath::Resolved(None, Path { res: Res::Local(local_id), .. })) = e.kind {
+                    match some_captures.get(local_id)
+                        .or_else(|| (method_sugg == "map_or_else").then(|| ()).and_then(|_| none_captures.get(local_id)))
                     {
                         Some(CaptureKind::Value | CaptureKind::Ref(Mutability::Mut)) => return None,
                         Some(CaptureKind::Ref(Mutability::Not)) if as_mut => return None,

diff --git a/tests/ui/option_if_let_else.fixed b/tests/ui/option_if_let_else.fixed
@@ -1,3 +1,4 @@
+// edition:2018
 // run-rustfix
 #![warn(clippy::option_if_let_else)]
 #![allow(clippy::redundant_closure)]
@@ -128,4 +129,23 @@ fn main() {
         let _s = s;
         10
     };
+
+    let mut s = Some(String::new());
+    // Don't lint, `Some` branch mutably borrows `s`, but else branch also borrows  `s`
+    let _ = if let Some(x) = &mut s {
+        x.push_str("test");
+        x.len()
+    } else {
+        let _s = &s;
+        10
+    };
+
+    async fn _f1(x: u32) -> u32 {
+        x
+    }
+
+    async fn _f2() {
+        // Don't lint. `await` can't be moved into a closure.
+        let _ = if let Some(x) = Some(0) { _f1(x).await } else { 0 };
+    }
 }
diff --git a/tests/ui/option_if_let_else.rs b/tests/ui/option_if_let_else.rs
@@ -1,3 +1,4 @@
+// edition:2018
 // run-rustfix
 #![warn(clippy::option_if_let_else)]
 #![allow(clippy::redundant_closure)]
@@ -153,4 +154,23 @@ fn main() {
         let _s = s;
         10
     };
+
+    let mut s = Some(String::new());
+    // Don't lint, `Some` branch mutably borrows `s`, but else branch also borrows  `s`
+    let _ = if let Some(x) = &mut s {
+        x.push_str("test");
+        x.len()
+    } else {
+        let _s = &s;
+        10
+    };
+
+    async fn _f1(x: u32) -> u32 {
+        x
+    }
+
+    async fn _f2() {
+        // Don't lint. `await` can't be moved into a closure.
+        let _ = if let Some(x) = Some(0) { _f1(x).await } else { 0 };
+    }
 }
diff --git a/tests/ui/option_if_let_else.stderr b/tests/ui/option_if_let_else.stderr
@@ -1,5 +1,5 @@
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:7:5
+  --> $DIR/option_if_let_else.rs:8:5
    |
 LL | /     if let Some(x) = string {
 LL | |         (true, x)
@@ -11,19 +11,19 @@ LL | |     }
    = note: `-D clippy::option-if-let-else` implied by `-D warnings`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:25:13
+  --> $DIR/option_if_let_else.rs:26:13
    |
 LL |     let _ = if let Some(s) = *string { s.len() } else { 0 };
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `string.map_or(0, |s| s.len())`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:26:13
+  --> $DIR/option_if_let_else.rs:27:13
    |
 LL |     let _ = if let Some(s) = &num { s } else { &0 };
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `num.as_ref().map_or(&0, |s| s)`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:27:13
+  --> $DIR/option_if_let_else.rs:28:13
    |
 LL |       let _ = if let Some(s) = &mut num {
    |  _____________^
@@ -43,13 +43,13 @@ LL ~     });
    |
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:33:13
+  --> $DIR/option_if_let_else.rs:34:13
    |
 LL |     let _ = if let Some(ref s) = num { s } else { &0 };
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `num.as_ref().map_or(&0, |s| s)`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:34:13
+  --> $DIR/option_if_let_else.rs:35:13
    |
 LL |       let _ = if let Some(mut s) = num {
    |  _____________^
@@ -69,7 +69,7 @@ LL ~     });
    |
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:40:13
+  --> $DIR/option_if_let_else.rs:41:13
    |
 LL |       let _ = if let Some(ref mut s) = num {
    |  _____________^
@@ -89,7 +89,7 @@ LL ~     });
    |
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:49:5
+  --> $DIR/option_if_let_else.rs:50:5
    |
 LL | /     if let Some(x) = arg {
 LL | |         let y = x * x;
@@ -108,7 +108,7 @@ LL +     })
    |
 
 error: use Option::map_or_else instead of an if let/else
-  --> $DIR/option_if_let_else.rs:62:13
+  --> $DIR/option_if_let_else.rs:63:13
    |
 LL |       let _ = if let Some(x) = arg {
    |  _____________^
@@ -120,7 +120,7 @@ LL | |     };
    | |_____^ help: try: `arg.map_or_else(|| side_effect(), |x| x)`
 
 error: use Option::map_or_else instead of an if let/else
-  --> $DIR/option_if_let_else.rs:71:13
+  --> $DIR/option_if_let_else.rs:72:13
    |
 LL |       let _ = if let Some(x) = arg {
    |  _____________^
@@ -143,13 +143,13 @@ LL ~     }, |x| x * x * x * x);
    |
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:100:13
+  --> $DIR/option_if_let_else.rs:101:13
    |
 LL |     let _ = if let Some(x) = optional { x + 2 } else { 5 };
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:109:13
+  --> $DIR/option_if_let_else.rs:110:13
    |
 LL |       let _ = if let Some(x) = Some(0) {
    |  _____________^
@@ -171,13 +171,13 @@ LL ~         });
    |
 
 error: use Option::map_or_else instead of an if let/else
-  --> $DIR/option_if_let_else.rs:137:13
+  --> $DIR/option_if_let_else.rs:138:13
    |
 LL |     let _ = if let Some(x) = Some(0) { s.len() + x } else { s.len() };
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(0).map_or_else(|| s.len(), |x| s.len() + x)`
 
 error: use Option::map_or instead of an if let/else
-  --> $DIR/option_if_let_else.rs:141:13
+  --> $DIR/option_if_let_else.rs:142:13
    |
 LL |       let _ = if let Some(x) = Some(0) {
    |  _____________^