Auto merge of rust-lang#7316 - lengyijun:rc_mutex, r=llogiq

Add new lint: `rc_mutex`

changelog: Add new lint `rc_mutex`.

It lints on `Rc<Mutex<T>>`.

`Rc<Mutex<T>>` should be corrected to `Rc<RefCell<T>>`

Author: bors

CHANGELOG.md

@@ -2744,6 +2744,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

clippy_lints/src/lib.rs

@@ -944,6 +944,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,
@@ -1042,6 +1043,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),

clippy_lints/src/types/mod.rs

@@ -3,6 +3,7 @@ mod box_vec;
 mod linked_list;
 mod option_option;
 mod rc_buffer;
+mod rc_mutex;
 mod redundant_allocation;
 mod type_complexity;
 mod utils;
@@ -250,12 +251,41 @@ declare_clippy_lint! {
     "usage of very complex types that might be better factored into `type` definitions"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for `Rc<Mutex<T>>`.
+    ///
+    /// **Why is this bad?** `Rc` is used in single thread and `Mutex` is used in multi thread.
+    /// Consider using `Rc<RefCell<T>>` in single thread or `Arc<Mutex<T>>` in multi thread.
+    ///
+    /// **Known problems:** Sometimes combining generic types can lead to the requirement that a
+    /// type use Rc in conjunction with Mutex. We must consider those cases false positives, but
+    /// alas they are quite hard to rule out. Luckily they are also rare.
+    ///
+    /// **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 `Rc<Mutex<T>>`"
+}
+
 pub struct Types {
     vec_box_size_threshold: u64,
     type_complexity_threshold: u64,
 }
 
-impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, TYPE_COMPLEXITY]);
+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) {
@@ -375,6 +405,7 @@ impl Types {
                     triggered |= vec_box::check(cx, hir_ty, qpath, def_id, self.vec_box_size_threshold);
                     triggered |= option_option::check(cx, hir_ty, qpath, def_id);
                     triggered |= linked_list::check(cx, hir_ty, def_id);
+                    triggered |= rc_mutex::check(cx, hir_ty, qpath, def_id);
 
                     if triggered {
                         return;

clippy_lints/src/types/rc_mutex.rs

@@ -0,0 +1,27 @@
+use clippy_utils::diagnostics::span_lint;
+use clippy_utils::is_ty_param_diagnostic_item;
+use if_chain::if_chain;
+use rustc_hir::{self as hir, def_id::DefId, QPath};
+use rustc_lint::LateContext;
+use rustc_span::symbol::sym;
+
+use super::RC_MUTEX;
+
+pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, qpath: &QPath<'_>, def_id: DefId) -> bool {
+    if_chain! {
+        if cx.tcx.is_diagnostic_item(sym::Rc, def_id) ;
+        if let Some(_) = is_ty_param_diagnostic_item(cx, qpath, sym!(mutex_type)) ;
+
+        then{
+            span_lint(
+                cx,
+                RC_MUTEX,
+                hir_ty.span,
+                "found `Rc<Mutex<_>>`. Consider using `Rc<RefCell<_>>` or `Arc<Mutex<_>>` instead",
+            );
+            return true;
+        }
+    }
+
+    false
+}

tests/ui/rc_mutex.rs

@@ -0,0 +1,34 @@
+#![warn(clippy::rc_mutex)]
+#![allow(clippy::blacklisted_name)]
+
+use std::rc::Rc;
+use std::sync::Mutex;
+
+pub struct MyStruct {
+    foo: Rc<Mutex<i32>>,
+}
+
+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() {
+    test1(Rc::new(Mutex::new(1)));
+    test2(Rc::new(Mutex::new(MyEnum::One)));
+    test3(Rc::new(Mutex::new(SubT { foo: 1 })));
+
+    let _my_struct = MyStruct {
+        foo: Rc::new(Mutex::new(1)),
+    };
+}

tests/ui/rc_mutex.stderr

@@ -0,0 +1,28 @@
+error: found `Rc<Mutex<_>>`. Consider using `Rc<RefCell<_>>` or `Arc<Mutex<_>>` instead
+  --> $DIR/rc_mutex.rs:8:10
+   |
+LL |     foo: Rc<Mutex<i32>>,
+   |          ^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::rc-mutex` implied by `-D warnings`
+
+error: found `Rc<Mutex<_>>`. Consider using `Rc<RefCell<_>>` or `Arc<Mutex<_>>` instead
+  --> $DIR/rc_mutex.rs:20:22
+   |
+LL | pub fn test1<T>(foo: Rc<Mutex<T>>) {}
+   |                      ^^^^^^^^^^^^
+
+error: found `Rc<Mutex<_>>`. Consider using `Rc<RefCell<_>>` or `Arc<Mutex<_>>` instead
+  --> $DIR/rc_mutex.rs:22:19
+   |
+LL | pub fn test2(foo: Rc<Mutex<MyEnum>>) {}
+   |                   ^^^^^^^^^^^^^^^^^
+
+error: found `Rc<Mutex<_>>`. Consider using `Rc<RefCell<_>>` or `Arc<Mutex<_>>` instead
+  --> $DIR/rc_mutex.rs:24:19
+   |
+LL | pub fn test3(foo: Rc<Mutex<SubT<usize>>>) {}
+   |                   ^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 4 previous errors
+