test for mutex eq, add another test case

3 files changed, 51 insertions, 29 deletions

diff --git a/clippy_lints/src/if_let_mutex.rs b/clippy_lints/src/if_let_mutex.rs
index b98f190eb36..21749f8ad09 100644
--- a/clippy_lints/src/if_let_mutex.rs
+++ b/clippy_lints/src/if_let_mutex.rs
@@ -1,7 +1,7 @@
+use crate::utils::{match_type, paths, span_lint_and_help, SpanlessEq};
 use if_chain::if_chain;
-use crate::utils::{match_type, paths, span_lint_and_help};
 use rustc_hir::intravisit::{self as visit, NestedVisitorMap, Visitor};
-use rustc_hir::{Arm, Expr, ExprKind, MatchSource, StmtKind};
+use rustc_hir::{Expr, ExprKind, MatchSource};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::hir::map::Map;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -44,10 +44,12 @@ impl LateLintPass<'_, '_> for IfLetMutex {
     fn check_expr(&mut self, cx: &LateContext<'_, '_>, ex: &'_ Expr<'_>) {
         let mut arm_visit = ArmVisitor {
             mutex_lock_called: false,
+            found_mutex: None,
             cx,
         };
         let mut op_visit = OppVisitor {
             mutex_lock_called: false,
+            found_mutex: None,
             cx,
         };
         if let ExprKind::Match(
@@ -64,7 +66,7 @@ impl LateLintPass<'_, '_> for IfLetMutex {
                     arm_visit.visit_arm(arm);
                 }
 
-                if arm_visit.mutex_lock_called {
+                if arm_visit.mutex_lock_called && arm_visit.same_mutex(cx, op_visit.found_mutex.unwrap()) {
                     span_lint_and_help(
                         cx,
                         IF_LET_MUTEX,
@@ -80,8 +82,9 @@ impl LateLintPass<'_, '_> for IfLetMutex {
 
 /// Checks if `Mutex::lock` is called in the `if let _ = expr.
 pub struct OppVisitor<'tcx, 'l> {
-    pub mutex_lock_called: bool,
-    pub cx: &'tcx LateContext<'tcx, 'l>,
+    mutex_lock_called: bool,
+    found_mutex: Option<&'tcx Expr<'tcx>>,
+    cx: &'tcx LateContext<'tcx, 'l>,
 }
 
 impl<'tcx, 'l> Visitor<'tcx> for OppVisitor<'tcx, 'l> {
@@ -94,6 +97,7 @@ impl<'tcx, 'l> Visitor<'tcx> for OppVisitor<'tcx, 'l> {
             let ty = self.cx.tables.expr_ty(&args[0]);
             if match_type(self.cx, ty, &paths::MUTEX);
             then {
+                self.found_mutex = Some(&args[0]);
                 self.mutex_lock_called = true;
                 return;
             }
@@ -108,20 +112,22 @@ impl<'tcx, 'l> Visitor<'tcx> for OppVisitor<'tcx, 'l> {
 
 /// Checks if `Mutex::lock` is called in any of the branches.
 pub struct ArmVisitor<'tcx, 'l> {
-    pub mutex_lock_called: bool,
-    pub cx: &'tcx LateContext<'tcx, 'l>,
+    mutex_lock_called: bool,
+    found_mutex: Option<&'tcx Expr<'tcx>>,
+    cx: &'tcx LateContext<'tcx, 'l>,
 }
 
 impl<'tcx, 'l> Visitor<'tcx> for ArmVisitor<'tcx, 'l> {
     type Map = Map<'tcx>;
 
-    fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
+    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);
             then {
+                self.found_mutex = Some(&args[0]);
                 self.mutex_lock_called = true;
                 return;
             }
@@ -129,25 +135,17 @@ impl<'tcx, 'l> Visitor<'tcx> for ArmVisitor<'tcx, 'l> {
         visit::walk_expr(self, expr);
     }
 
-    fn visit_arm(&mut self, arm: &'tcx Arm<'_>) {
-        if let ExprKind::Block(ref block, _l) = arm.body.kind {
-            for stmt in block.stmts {
-                match stmt.kind {
-                    StmtKind::Local(loc) => {
-                        if let Some(expr) = loc.init {
-                            self.visit_expr(expr)
-                        }
-                    },
-                    StmtKind::Expr(expr) | StmtKind::Semi(expr) => self.visit_expr(expr),
-                    // we don't care about `Item`
-                    _ => {},
-                }
-            }
-        };
-        visit::walk_arm(self, arm);
-    }
-
     fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
         NestedVisitorMap::None
     }
 }
+
+impl<'tcx, 'l> ArmVisitor<'tcx, 'l> {
+    fn same_mutex(&self, cx: &LateContext<'_, '_>, op_mutex: &Expr<'_>) -> bool {
+        if let Some(arm_mutex) = self.found_mutex {
+            SpanlessEq::new(cx).eq_expr(op_mutex, arm_mutex)
+        } else {
+            false
+        }
+    }
+}
diff --git a/tests/ui/if_let_mutex.rs b/tests/ui/if_let_mutex.rs
index 1d7cc756dc5..d59ec83679a 100644
--- a/tests/ui/if_let_mutex.rs
+++ b/tests/ui/if_let_mutex.rs
@@ -1,11 +1,12 @@
 #![warn(clippy::if_let_mutex)]
 
+use std::ops::Deref;
 use std::sync::Mutex;
 
 fn do_stuff<T>(_: T) {}
 
 fn if_let() {
-    let m = Mutex::new(1u8);
+    let m = Mutex::new(1_u8);
     if let Err(locked) = m.lock() {
         do_stuff(locked);
     } else {
@@ -14,4 +15,14 @@ fn if_let() {
     };
 }
 
+fn if_let_option() {
+    let m = Mutex::new(Some(0_u8));
+    if let Some(locked) = m.lock().unwrap().deref() {
+        do_stuff(locked);
+    } else {
+        let lock = m.lock().unwrap();
+        do_stuff(lock);
+    };
+}
+
 fn main() {}
diff --git a/tests/ui/if_let_mutex.stderr b/tests/ui/if_let_mutex.stderr
index 84c19ed03d5..0c24d650c27 100644
--- a/tests/ui/if_let_mutex.stderr
+++ b/tests/ui/if_let_mutex.stderr
@@ -1,5 +1,5 @@
 error: calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock
-  --> $DIR/if_let_mutex.rs:9:5
+  --> $DIR/if_let_mutex.rs:10:5
    |
 LL | /     if let Err(locked) = m.lock() {
 LL | |         do_stuff(locked);
@@ -12,5 +12,18 @@ LL | |     };
    = note: `-D clippy::if-let-mutex` implied by `-D warnings`
    = help: move the lock call outside of the `if let ...` expression
 
-error: aborting due to previous error
+error: calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock
+  --> $DIR/if_let_mutex.rs:20:5
+   |
+LL | /     if let Some(locked) = m.lock().unwrap().deref() {
+LL | |         do_stuff(locked);
+LL | |     } else {
+LL | |         let lock = m.lock().unwrap();
+LL | |         do_stuff(lock);
+LL | |     };
+   | |_____^
+   |
+   = help: move the lock call outside of the `if let ...` expression
+
+error: aborting due to 2 previous errors