correct lint

7 files changed, 127 insertions, 10 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 41af8e190dd..4fb2ce8dfc8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2611,6 +2611,7 @@ Released 2018-09-13
 [`range_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_step_by_zero
 [`range_zip_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_zip_with_len
 [`rc_buffer`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_buffer
+[`rc_mutex`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex
 [`redundant_allocation`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_allocation
 [`redundant_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone
 [`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index e7dd3952b3a..2b999da15ca 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -931,6 +931,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         types::LINKEDLIST,
         types::OPTION_OPTION,
         types::RC_BUFFER,
+        types::RC_MUTEX,
         types::REDUNDANT_ALLOCATION,
         types::TYPE_COMPLEXITY,
         types::VEC_BOX,
@@ -1027,6 +1028,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(strings::STRING_TO_STRING),
         LintId::of(strings::STR_TO_STRING),
         LintId::of(types::RC_BUFFER),
+        LintId::of(types::RC_MUTEX),
         LintId::of(unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS),
         LintId::of(unwrap_in_result::UNWRAP_IN_RESULT),
         LintId::of(verbose_file_reads::VERBOSE_FILE_READS),
diff --git a/clippy_lints/src/types/mod.rs b/clippy_lints/src/types/mod.rs
index acf00825b7a..07dec2de827 100644
--- a/clippy_lints/src/types/mod.rs
+++ b/clippy_lints/src/types/mod.rs
@@ -3,11 +3,11 @@ mod box_vec;
 mod linked_list;
 mod option_option;
 mod rc_buffer;
+mod rc_mutex;
 mod redundant_allocation;
 mod type_complexity;
 mod utils;
 mod vec_box;
-mod rc_mutex;
 
 use rustc_hir as hir;
 use rustc_hir::intravisit::FnKind;
@@ -252,10 +252,42 @@ declare_clippy_lint! {
 }
 
 declare_clippy_lint! {
-    /// TODO
+    /// **What it does:** Checks for `Rc<Mutex<T>>`.
+    ///
+    /// **Why is this bad?** `Rc<Mutex<T>>` may introduce a deadlock in single thread. Consider
+    /// using `Rc<RefCell<T>>` instead.
+    /// ```rust
+    /// fn main() {
+    ///     use std::rc::Rc;
+    ///     use std::sync::Mutex;
+    ///
+    ///     let a: Rc<Mutex<i32>> = Rc::new(Mutex::new(1));
+    ///     let a_clone = a.clone();
+    ///     let mut data = a.lock().unwrap();
+    ///     println!("{:?}", *a_clone.lock().unwrap());
+    ///     *data = 10;
+    ///  }
+    /// ```
+    ///
+    /// **Known problems:** `Rc<RefCell<T>>` may panic in runtime.
+    ///
+    /// **Example:**
+    /// ```rust,ignore
+    /// use std::rc::Rc;
+    /// use std::sync::Mutex;
+    /// fn foo(interned: Rc<Mutex<i32>>) { ... }
+    /// ```
+    ///
+    /// Better:
+    ///
+    /// ```rust,ignore
+    /// use std::rc::Rc;
+    /// use std::cell::RefCell
+    /// fn foo(interned: Rc<RefCell<i32>>) { ... }
+    /// ```
     pub RC_MUTEX,
     restriction,
-    "usage of Mutex inside Rc"
+    "usage of `Rc<Mutex<T>>`"
 }
 
 pub struct Types {
@@ -263,7 +295,7 @@ pub struct Types {
     type_complexity_threshold: u64,
 }
 
-impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, TYPE_COMPLEXITY,RC_MUTEX]);
+impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, RC_MUTEX, TYPE_COMPLEXITY]);
 
 impl<'tcx> LateLintPass<'tcx> for Types {
     fn check_fn(&mut self, cx: &LateContext<'_>, _: FnKind<'_>, decl: &FnDecl<'_>, _: &Body<'_>, _: Span, id: HirId) {
diff --git a/clippy_lints/src/types/rc_mutex.rs b/clippy_lints/src/types/rc_mutex.rs
index b53b55fd01c..122df01d153 100644
--- a/clippy_lints/src/types/rc_mutex.rs
+++ b/clippy_lints/src/types/rc_mutex.rs
@@ -1,20 +1,24 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::{ get_qpath_generic_tys,is_ty_param_diagnostic_item};
 use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::{get_qpath_generic_tys, is_ty_param_diagnostic_item};
+use if_chain::if_chain;
 use rustc_errors::Applicability;
-use rustc_hir::{self as hir, def_id::DefId, QPath};
+use rustc_hir::{self as hir, def_id::DefId, QPath, TyKind};
 use rustc_lint::LateContext;
 use rustc_span::symbol::sym;
-// use rustc_middle::ty::Adt;
 
 use super::RC_MUTEX;
 
 pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, qpath: &QPath<'_>, def_id: DefId) -> bool {
-    if cx.tcx.is_diagnostic_item(sym::Rc, def_id) {
-        if let Some(ty) = is_ty_param_diagnostic_item(cx, qpath, sym!(mutex_type)) {
+    if_chain! {
+        if cx.tcx.is_diagnostic_item(sym::Rc, def_id) ;
+        if let Some(ty) = is_ty_param_diagnostic_item(cx, qpath, sym!(mutex_type)) ;
+        if let TyKind::Path(ref qpath_inner)=ty.kind;
+
+        then{
             let mut applicability = Applicability::MachineApplicable;
 
-            let inner_span = match get_qpath_generic_tys(qpath).skip(1).next() {
+            let inner_span = match get_qpath_generic_tys(qpath_inner).next() {
                 Some(ty) => ty.span,
                 None => return false,
             };
diff --git a/tests/ui/rc_mutex.fixed b/tests/ui/rc_mutex.fixed
new file mode 100644
index 00000000000..b36b1d0914b
--- /dev/null
+++ b/tests/ui/rc_mutex.fixed
@@ -0,0 +1,28 @@
+// run-rustfix
+#![warn(clippy::rc_mutex)]
+#![allow(unused_imports)]
+#![allow(clippy::boxed_local, clippy::needless_pass_by_value)]
+#![allow(clippy::blacklisted_name, unused_variables, dead_code)]
+
+use std::cell::RefCell;
+use std::rc::Rc;
+use std::sync::Mutex;
+
+pub struct MyStruct {}
+
+pub struct SubT<T> {
+    foo: T,
+}
+
+pub enum MyEnum {
+    One,
+    Two,
+}
+
+pub fn test1<T>(foo: Rc<RefCell<T>>) {}
+
+pub fn test2(foo: Rc<RefCell<MyEnum>>) {}
+
+pub fn test3(foo: Rc<RefCell<SubT<usize>>>) {}
+
+fn main() {}
diff --git a/tests/ui/rc_mutex.rs b/tests/ui/rc_mutex.rs
new file mode 100644
index 00000000000..e6ec4549de9
--- /dev/null
+++ b/tests/ui/rc_mutex.rs
@@ -0,0 +1,28 @@
+// run-rustfix
+#![warn(clippy::rc_mutex)]
+#![allow(unused_imports)]
+#![allow(clippy::boxed_local, clippy::needless_pass_by_value)]
+#![allow(clippy::blacklisted_name, unused_variables, dead_code)]
+
+use std::cell::RefCell;
+use std::rc::Rc;
+use std::sync::Mutex;
+
+pub struct MyStruct {}
+
+pub struct SubT<T> {
+    foo: T,
+}
+
+pub enum MyEnum {
+    One,
+    Two,
+}
+
+pub fn test1<T>(foo: Rc<Mutex<T>>) {}
+
+pub fn test2(foo: Rc<Mutex<MyEnum>>) {}
+
+pub fn test3(foo: Rc<Mutex<SubT<usize>>>) {}
+
+fn main() {}
diff --git a/tests/ui/rc_mutex.stderr b/tests/ui/rc_mutex.stderr
new file mode 100644
index 00000000000..ad0340dcf55
--- /dev/null
+++ b/tests/ui/rc_mutex.stderr
@@ -0,0 +1,22 @@
+error: you seem to be trying to use `Rc<Mutex<T>>`. Consider using `Rc<RefCell<T>>`
+  --> $DIR/rc_mutex.rs:22:22
+   |
+LL | pub fn test1<T>(foo: Rc<Mutex<T>>) {}
+   |                      ^^^^^^^^^^^^ help: try: `Rc<RefCell<T>>`
+   |
+   = note: `-D clippy::rc-mutex` implied by `-D warnings`
+
+error: you seem to be trying to use `Rc<Mutex<T>>`. Consider using `Rc<RefCell<T>>`
+  --> $DIR/rc_mutex.rs:24:19
+   |
+LL | pub fn test2(foo: Rc<Mutex<MyEnum>>) {}
+   |                   ^^^^^^^^^^^^^^^^^ help: try: `Rc<RefCell<MyEnum>>`
+
+error: you seem to be trying to use `Rc<Mutex<T>>`. Consider using `Rc<RefCell<T>>`
+  --> $DIR/rc_mutex.rs:26:19
+   |
+LL | pub fn test3(foo: Rc<Mutex<SubT<usize>>>) {}
+   |                   ^^^^^^^^^^^^^^^^^^^^^^ help: try: `Rc<RefCell<SubT<usize>>>`
+
+error: aborting due to 3 previous errors
+