X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Fif_let_mutex.rs;h=f661f7ede821a6d486a33448eab7c56fd8e106da;hb=4f3b49fffa13518aa6006762c0eb6851c0c0b2d5;hp=34f0b22f65e6812a95f4ec9c0738dd34eee40158;hpb=001a42e632573abca10b2f8272f50a3999846e70;p=rust.git diff --git a/clippy_lints/src/if_let_mutex.rs b/clippy_lints/src/if_let_mutex.rs index 34f0b22f65e..f661f7ede82 100644 --- a/clippy_lints/src/if_let_mutex.rs +++ b/clippy_lints/src/if_let_mutex.rs @@ -1,11 +1,11 @@ -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::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, 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! { @@ -19,117 +19,135 @@ /// /// **Example:** /// - /// ```rust - /// # use std::sync::Mutex; - /// let mutex = Mutex::new(10); + /// ```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); + /// } + /// ``` 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 { +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 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 + }, + ) = ex.kind + { + op_visit.visit_expr(op); + if op_visit.mutex_lock_called { + for arm in arms { + arm_visit.visit_arm(arm); + } - 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> - 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> - 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> - 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, + 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 { - 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 { - 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> { + 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 { + NestedVisitorMap::None + } +} + +/// 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); } - method_names + fn nested_visit_map(&mut self) -> NestedVisitorMap { + 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 + } + } }