-use crate::utils::{match_type, paths, span_lint_and_help};
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::ty::is_type_diagnostic_item;
+use clippy_utils::SpanlessEq;
use if_chain::if_chain;
-use rustc_hir::{Expr, ExprKind, MatchSource, StmtKind};
+use rustc_hir::intravisit::{self as visit, NestedVisitorMap, Visitor};
+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};
declare_clippy_lint! {
/// **Why is this bad?** The Mutex lock remains held for the whole
/// `if let ... else` block and deadlocks.
///
- /// **Known problems:** This lint does not generate an auto-applicable suggestion.
+ /// **Known problems:** None.
///
/// **Example:**
///
declare_lint_pass!(IfLetMutex => [IF_LET_MUTEX]);
-impl LateLintPass<'_, '_> for IfLetMutex {
- fn check_expr(&mut self, cx: &LateContext<'_, '_>, ex: &'_ Expr<'_>) {
- if_chain! {
- if let ExprKind::Match(ref op, ref arms, MatchSource::IfLetDesugar {
+impl<'tcx> LateLintPass<'tcx> for IfLetMutex {
+ fn check_expr(&mut self, cx: &LateContext<'tcx>, ex: &'tcx Expr<'tcx>) {
+ 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(
+ op,
+ arms,
+ MatchSource::IfLetDesugar {
contains_else_clause: true,
- }) = ex.kind; // if let ... {} else {}
- if let ExprKind::MethodCall(_, _, ref args) = op.kind;
- let ty = cx.tables.expr_ty(&args[0]);
- if match_type(cx, ty, &paths::MUTEX); // make sure receiver is Mutex
- if method_chain_names(op, 10).iter().any(|s| s == "lock"); // and lock is called
+ },
+ ) = ex.kind
+ {
+ op_visit.visit_expr(op);
+ if op_visit.mutex_lock_called {
+ for arm in arms {
+ arm_visit.visit_arm(arm);
+ }
- if arms.iter().any(|arm| if_chain! {
- if let ExprKind::Block(ref block, _l) = arm.body.kind;
- if block.stmts.iter().any(|stmt| match stmt.kind {
- StmtKind::Local(l) => if_chain! {
- if let Some(ex) = l.init;
- if let ExprKind::MethodCall(_, _, _) = op.kind;
- if method_chain_names(ex, 10).iter().any(|s| s == "lock"); // and lock is called
- then {
- match_type_method_chain(cx, ex, 5)
- } else {
- false
- }
- },
- StmtKind::Expr(e) => if_chain! {
- if let ExprKind::MethodCall(_, _, _) = e.kind;
- if method_chain_names(e, 10).iter().any(|s| s == "lock"); // and lock is called
- then {
- match_type_method_chain(cx, ex, 5)
- } else {
- false
- }
- },
- StmtKind::Semi(e) => if_chain! {
- if let ExprKind::MethodCall(_, _, _) = e.kind;
- if method_chain_names(e, 10).iter().any(|s| s == "lock"); // and lock is called
- then {
- match_type_method_chain(cx, ex, 5)
- } else {
- false
- }
- },
- _ => {
- false
- },
- });
- then {
- true
- } else {
- false
+ if arm_visit.mutex_lock_called && arm_visit.same_mutex(cx, op_visit.found_mutex.unwrap()) {
+ span_lint_and_help(
+ cx,
+ IF_LET_MUTEX,
+ ex.span,
+ "calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock",
+ None,
+ "move the lock call outside of the `if let ...` expression",
+ );
}
- });
- then {
- span_lint_and_help(
- cx,
- IF_LET_MUTEX,
- ex.span,
- "calling `Mutex::lock` inside the scope of another `Mutex::lock` causes a deadlock",
- "move the lock call outside of the `if let ...` expression",
- );
}
}
}
}
-/// Return the names of `max_depth` number of methods called in the chain.
-fn method_chain_names<'tcx>(expr: &'tcx Expr<'tcx>, max_depth: usize) -> Vec<String> {
- let mut method_names = Vec::with_capacity(max_depth);
- let mut current = expr;
- for _ in 0..max_depth {
- if let ExprKind::MethodCall(path, _, args) = ¤t.kind {
- if args.iter().any(|e| e.span.from_expansion()) {
- break;
- }
- method_names.push(path.ident.to_string());
- current = &args[0];
- } else {
- break;
+/// Checks if `Mutex::lock` is called in the `if let _ = expr.
+pub struct OppVisitor<'a, 'tcx> {
+ mutex_lock_called: bool,
+ found_mutex: Option<&'tcx Expr<'tcx>>,
+ cx: &'a LateContext<'tcx>,
+}
+
+impl<'tcx> Visitor<'tcx> for OppVisitor<'_, 'tcx> {
+ type Map = Map<'tcx>;
+
+ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
+ if let Some(mutex) = is_mutex_lock_call(self.cx, expr) {
+ self.found_mutex = Some(mutex);
+ self.mutex_lock_called = true;
+ return;
}
+ visit::walk_expr(self, expr);
+ }
+
+ fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
+ NestedVisitorMap::None
}
- method_names
}
-/// Check that lock is called on a `Mutex`.
-fn match_type_method_chain<'tcx>(cx: &LateContext<'_, '_>, expr: &'tcx Expr<'tcx>, max_depth: usize) -> bool {
- let mut current = expr;
- for _ in 0..max_depth {
- if let ExprKind::MethodCall(_, _, args) = ¤t.kind {
- let ty = cx.tables.expr_ty(&args[0]);
- if match_type(cx, ty, &paths::MUTEX) {
- return true;
- }
- current = &args[0];
+/// Checks if `Mutex::lock` is called in any of the branches.
+pub struct ArmVisitor<'a, 'tcx> {
+ mutex_lock_called: bool,
+ found_mutex: Option<&'tcx Expr<'tcx>>,
+ cx: &'a LateContext<'tcx>,
+}
+
+impl<'tcx> Visitor<'tcx> for ArmVisitor<'_, 'tcx> {
+ type Map = Map<'tcx>;
+
+ fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
+ if let Some(mutex) = is_mutex_lock_call(self.cx, expr) {
+ self.found_mutex = Some(mutex);
+ self.mutex_lock_called = true;
+ return;
+ }
+ visit::walk_expr(self, expr);
+ }
+
+ 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 {
+ self.found_mutex
+ .map_or(false, |arm_mutex| SpanlessEq::new(cx).eq_expr(op_mutex, arm_mutex))
+ }
+}
+
+fn is_mutex_lock_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
+ if_chain! {
+ if let ExprKind::MethodCall(path, _span, args, _) = &expr.kind;
+ if path.ident.as_str() == "lock";
+ let ty = cx.typeck_results().expr_ty(&args[0]);
+ if is_type_diagnostic_item(cx, ty, sym!(mutex_type));
+ then {
+ Some(&args[0])
+ } else {
+ None
}
}
- false
}