-use crate::utils::{
- match_type, method_calls, method_chain_args, paths, snippet, snippet_with_applicability, span_lint_and_sugg,
-};
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::higher;
+use clippy_utils::ty::is_type_diagnostic_item;
+use clippy_utils::SpanlessEq;
use if_chain::if_chain;
-use rustc::ty;
-use rustc_errors::Applicability;
-use rustc_hir::{print, Expr, ExprKind, MatchSource, PatKind, QPath, StmtKind};
+use rustc_hir::intravisit::{self as visit, Visitor};
+use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::sym;
declare_clippy_lint! {
- /// **What it does:** Checks for `Mutex::lock` calls in `if let` expression
+ /// ### What it does
+ /// Checks for `Mutex::lock` calls in `if let` expression
/// with lock calls in any of the else blocks.
///
- /// **Why is this bad?** The Mutex lock remains held for the whole
+ /// ### Why is this bad?
+ /// The Mutex lock remains held for the whole
/// `if let ... else` block and deadlocks.
///
- /// **Known problems:** None.
- ///
- /// **Example:**
- ///
- /// ```rust
- /// # use std::sync::Mutex;
- /// let mutex = Mutex::new(10);
+ /// ### Example
+ /// ```rust,ignore
/// if let Ok(thing) = mutex.lock() {
/// do_thing();
/// } else {
/// mutex.lock();
/// }
/// ```
+ /// Should be written
+ /// ```rust,ignore
+ /// let locked = mutex.lock();
+ /// if let Ok(thing) = locked {
+ /// do_thing(thing);
+ /// } else {
+ /// use_locked(locked);
+ /// }
+ /// ```
+ #[clippy::version = "1.45.0"]
pub IF_LET_MUTEX,
correctness,
- "locking a `Mutex` in an `if let` block can cause deadlock"
+ "locking a `Mutex` in an `if let` block can cause deadlocks"
}
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 {
- 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 let ty::Adt(_, subst) = ty.kind;
- 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
+impl<'tcx> LateLintPass<'tcx> for IfLetMutex {
+ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'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 Some(higher::IfLet {
+ let_expr,
+ if_then,
+ if_else: Some(if_else),
+ ..
+ }) = higher::IfLet::hir(cx, expr)
+ {
+ op_visit.visit_expr(let_expr);
+ if op_visit.mutex_lock_called {
+ arm_visit.visit_expr(if_then);
+ arm_visit.visit_expr(if_else);
- let mut suggestion = String::from(&format!("if let _ = {} {{\n", snippet(cx, op.span, "_")));
-
- 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(_, _, ref args) = op.kind;
- if method_chain_names(ex, 10).iter().any(|s| s == "lock"); // and lock is called
- then {
- let ty = cx.tables.expr_ty(&args[0]);
- // // make sure receiver is Result<MutexGuard<...>>
- match_type(cx, ty, &paths::RESULT)
- } else {
- suggestion.push_str(&format!(" {}\n", snippet(cx, l.span, "_")));
- false
- }
- },
- StmtKind::Expr(e) => if_chain! {
- if let ExprKind::MethodCall(_, _, ref args) = e.kind;
- if method_chain_names(e, 10).iter().any(|s| s == "lock"); // and lock is called
- then {
- let ty = cx.tables.expr_ty(&args[0]);
- // // make sure receiver is Result<MutexGuard<...>>
- match_type(cx, ty, &paths::RESULT)
- } else {
- suggestion.push_str(&format!(" {}\n", snippet(cx, e.span, "_")));
- false
- }
- },
- StmtKind::Semi(e) => if_chain! {
- if let ExprKind::MethodCall(_, _, ref args) = e.kind;
- if method_chain_names(e, 10).iter().any(|s| s == "lock"); // and lock is called
- then {
- let ty = cx.tables.expr_ty(&args[0]);
- // // make sure receiver is Result<MutexGuard<...>>
- match_type(cx, ty, &paths::RESULT)
- } else {
- suggestion.push_str(&format!(" {}\n", snippet(cx, e.span, "_")));
- false
- }
- },
- _ => { suggestion.push_str(&format!(" {}\n", snippet(cx, stmt.span, "_"))); false },
- });
- then {
- true
- } else {
- suggestion.push_str(&format!("else {}\n", snippet(cx, arm.span, "_")));
- 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,
+ expr.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 {
- println!("{}", suggestion);
- span_lint_and_sugg(
- cx,
- IF_LET_MUTEX,
- ex.span,
- "using a `Mutex` in inner scope of `.lock` call",
- "try",
- format!("{:?}", "hello"),
- Applicability::MachineApplicable,
- );
}
}
}
}
-fn method_chain_names<'tcx>(expr: &'tcx Expr<'tcx>, max_depth: usize) -> Vec<String> {
- let mut method_names = Vec::with_capacity(max_depth);
+/// 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>,
+}
- let mut current = expr;
- for _ in 0..max_depth {
- if let ExprKind::MethodCall(path, span, args) = ¤t.kind {
- if args.iter().any(|e| e.span.from_expansion()) {
- break;
- }
- method_names.push(path.ident.to_string());
- println!("{:?}", method_names);
- current = &args[0];
- } else {
- break;
+impl<'tcx> Visitor<'tcx> for OppVisitor<'_, '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);
}
+}
+
+/// 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>,
+}
- method_names
+impl<'tcx> Visitor<'tcx> for ArmVisitor<'_, '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);
+ }
+}
+
+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, [self_arg, ..], _) = &expr.kind;
+ if path.ident.as_str() == "lock";
+ let ty = cx.typeck_results().expr_ty(self_arg);
+ if is_type_diagnostic_item(cx, ty, sym::Mutex);
+ then {
+ Some(self_arg)
+ } else {
+ None
+ }
+ }
}
projects / rust.git / blobdiff