factor ifs into function, add differing mutex test

3 files changed, 34 insertions, 12 deletions

diff --git a/clippy_lints/src/if_let_mutex.rs b/clippy_lints/src/if_let_mutex.rs
index 95b9c2823fe..b2ece37fdb0 100644
--- a/clippy_lints/src/if_let_mutex.rs
+++ b/clippy_lints/src/if_let_mutex.rs
@@ -93,12 +93,9 @@ impl<'tcx, 'l> Visitor<'tcx> for OppVisitor<'tcx, 'l> {
 
     fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
         if_chain! {
-            if let ExprKind::MethodCall(path, _span, args) = &expr.kind;
-            if path.ident.to_string() == "lock";
-            let ty = self.cx.tables.expr_ty(&args[0]);
-            if match_type(self.cx, ty, &paths::MUTEX);
+            if let Some(mutex) = is_mutex_lock_call(self.cx, expr);
             then {
-                self.found_mutex = Some(&args[0]);
+                self.found_mutex = Some(mutex);
                 self.mutex_lock_called = true;
                 return;
             }
@@ -123,12 +120,9 @@ impl<'tcx, 'l> Visitor<'tcx> for ArmVisitor<'tcx, 'l> {
 
     fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
         if_chain! {
-            if let ExprKind::MethodCall(path, _span, args) = &expr.kind;
-            if path.ident.to_string() == "lock";
-            let ty = self.cx.tables.expr_ty(&args[0]);
-            if match_type(self.cx, ty, &paths::MUTEX);
+            if let Some(mutex) = is_mutex_lock_call(self.cx, expr);
             then {
-                self.found_mutex = Some(&args[0]);
+                self.found_mutex = Some(mutex);
                 self.mutex_lock_called = true;
                 return;
             }
@@ -150,3 +144,17 @@ impl<'tcx, 'l> ArmVisitor<'tcx, 'l> {
         }
     }
 }
+
+fn is_mutex_lock_call<'a>(cx: &LateContext<'a, '_>, expr: &'a Expr<'_>) -> Option<&'a Expr<'a>> {
+    if_chain! {
+        if let ExprKind::MethodCall(path, _span, args) = &expr.kind;
+        if path.ident.to_string() == "lock";
+        let ty = cx.tables.expr_ty(&args[0]);
+        if match_type(cx, ty, &paths::MUTEX);
+        then {
+            Some(&args[0])
+        } else {
+            None
+        }
+    }
+}
diff --git a/doc/adding_lints.md b/doc/adding_lints.md
index 0013165d533..9ad1315c175 100644
--- a/doc/adding_lints.md
+++ b/doc/adding_lints.md
@@ -101,8 +101,8 @@ Once we are satisfied with the output, we need to run
 Please note that, we should run `TESTNAME=foo_functions cargo uitest`
 every time before running `tests/ui/update-all-references.sh`.
 Running `TESTNAME=foo_functions cargo uitest` should pass then. When we commit
-our lint, we need to commit the generated `.stderr` files, too. In general you
-should only commit changed files by `tests/ui/update-all-references.sh` for the
+our lint, we need to commit the generated `.stderr` files, too. In general, you
+should only commit files changed by `tests/ui/update-all-references.sh` for the
 specific lint you are creating/editing.
 
 ## Rustfix tests
diff --git a/tests/ui/if_let_mutex.rs b/tests/ui/if_let_mutex.rs
index d59ec83679a..58feae422a3 100644
--- a/tests/ui/if_let_mutex.rs
+++ b/tests/ui/if_let_mutex.rs
@@ -15,6 +15,8 @@ fn if_let() {
     };
 }
 
+// This is the most common case as the above case is pretty
+// contrived.
 fn if_let_option() {
     let m = Mutex::new(Some(0_u8));
     if let Some(locked) = m.lock().unwrap().deref() {
@@ -25,4 +27,16 @@ fn if_let_option() {
     };
 }
 
+// When mutexs are different don't warn
+fn if_let_different_mutex() {
+    let m = Mutex::new(Some(0_u8));
+    let other = Mutex::new(None::<u8>);
+    if let Some(locked) = m.lock().unwrap().deref() {
+        do_stuff(locked);
+    } else {
+        let lock = other.lock().unwrap();
+        do_stuff(lock);
+    };
+}
+
 fn main() {}