Merged #365

README.md

@@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code.
 [Jump to usage instructions](#usage)
 
 ##Lints
-There are 62 lints included in this crate:
+There are 64 lints included in this crate:
 
 name                                                                                                   | default | meaning
 -------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -38,6 +38,8 @@ name
 [min_max](https://github.com/Manishearth/rust-clippy/wiki#min_max)                                     | warn    | `min(_, max(_, _))` (or vice versa) with bounds clamping the result to a constant
 [modulo_one](https://github.com/Manishearth/rust-clippy/wiki#modulo_one)                               | warn    | taking a number modulo 1, which always returns 0
 [mut_mut](https://github.com/Manishearth/rust-clippy/wiki#mut_mut)                                     | allow   | usage of double-mut refs, e.g. `&mut &mut ...` (either copy'n'paste error, or shows a fundamental misunderstanding of references)
+[mutex_atomic](https://github.com/Manishearth/rust-clippy/wiki#mutex_atomic)                           | warn    | using a Mutex where an atomic value could be used instead
+[mutex_integer](https://github.com/Manishearth/rust-clippy/wiki#mutex_integer)                         | allow   | using a Mutex for an integer type
 [needless_bool](https://github.com/Manishearth/rust-clippy/wiki#needless_bool)                         | warn    | if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }`
 [needless_lifetimes](https://github.com/Manishearth/rust-clippy/wiki#needless_lifetimes)               | warn    | using explicit lifetimes for references in function arguments when elision rules would allow omitting them
 [needless_range_loop](https://github.com/Manishearth/rust-clippy/wiki#needless_range_loop)             | warn    | for-looping over a range of indices where an iterator over items would do

src/lib.rs

@@ -47,6 +47,7 @@ pub mod loops;
 pub mod ranges;
 pub mod matches;
 pub mod precedence;
+pub mod mutex_atomic;
 pub mod zero_div_zero;
 pub mod open_options;
 
@@ -92,12 +93,14 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_late_lint_pass(box minmax::MinMaxPass);
     reg.register_late_lint_pass(box open_options::NonSensicalOpenOptions);
     reg.register_late_lint_pass(box zero_div_zero::ZeroDivZeroPass);
+    reg.register_late_lint_pass(box mutex_atomic::MutexAtomic);
 
     reg.register_lint_group("clippy_pedantic", vec![
         methods::OPTION_UNWRAP_USED,
         methods::RESULT_UNWRAP_USED,
         methods::WRONG_PUB_SELF_CONVENTION,
         mut_mut::MUT_MUT,
+        mutex_atomic::MUTEX_INTEGER,
         ptr_arg::PTR_ARG,
         shadow::SHADOW_REUSE,
         shadow::SHADOW_SAME,
@@ -146,6 +149,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         misc::REDUNDANT_PATTERN,
         misc::TOPLEVEL_REF_ARG,
         mut_reference::UNNECESSARY_MUT_PASSED,
+        mutex_atomic::MUTEX_ATOMIC,
         needless_bool::NEEDLESS_BOOL,
         open_options::NONSENSICAL_OPEN_OPTIONS,
         precedence::PRECEDENCE,

src/mutex_atomic.rs

@@ -0,0 +1,67 @@
+//! Checks for uses of Mutex where an atomic value could be used
+//!
+//! This lint is **warn** by default
+
+use rustc::lint::{LintPass, LintArray, LateLintPass, LateContext};
+use rustc_front::hir::Expr;
+
+use syntax::ast;
+use rustc::middle::ty;
+use rustc::middle::subst::ParamSpace;
+
+use utils::{span_lint, MUTEX_PATH, match_type};
+
+declare_lint! {
+    pub MUTEX_ATOMIC,
+    Warn,
+    "using a Mutex where an atomic value could be used instead"
+}
+
+declare_lint! {
+    pub MUTEX_INTEGER,
+    Allow,
+    "using a Mutex for an integer type"
+}
+
+impl LintPass for MutexAtomic {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(MUTEX_ATOMIC, MUTEX_INTEGER)
+    }
+}
+
+pub struct MutexAtomic;
+
+impl LateLintPass for MutexAtomic {
+    fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
+        let ty = cx.tcx.expr_ty(expr);
+        if let &ty::TyStruct(_, subst) = &ty.sty {
+            if match_type(cx, ty, &MUTEX_PATH) {
+                let mutex_param = &subst.types.get(ParamSpace::TypeSpace, 0).sty;
+                if let Some(atomic_name) = get_atomic_name(mutex_param) {
+                    let msg = format!("Consider using an {} instead of a \
+                                       Mutex here. If you just want the \
+                                       locking behaviour and not the internal \
+                                       type, consider using Mutex<()>.",
+                                      atomic_name);
+                    match *mutex_param {
+                        ty::TyUint(t) if t != ast::TyUs =>
+                            span_lint(cx, MUTEX_INTEGER, expr.span, &msg),
+                        ty::TyInt(t) if t != ast::TyIs =>
+                            span_lint(cx, MUTEX_INTEGER, expr.span, &msg),
+                        _ => span_lint(cx, MUTEX_ATOMIC, expr.span, &msg)
+                    }
+                }
+            }
+        }
+    }
+}
+
+fn get_atomic_name(ty: &ty::TypeVariants) -> Option<(&'static str)> {
+    match *ty {
+        ty::TyBool => Some("AtomicBool"),
+        ty::TyUint(_) => Some("AtomicUsize"),
+        ty::TyInt(_) => Some("AtomicIsize"),
+        ty::TyRawPtr(_) => Some("AtomicPtr"),
+        _ => None
+    }
+}

src/utils.rs

@@ -10,12 +10,13 @@ use syntax::ast::Lit_::*;
 use syntax::ast;
 
 // module DefPaths for certain structs/enums we check for
-pub const OPTION_PATH:       [&'static str; 3] = ["core", "option", "Option"];
-pub const RESULT_PATH:       [&'static str; 3] = ["core", "result", "Result"];
-pub const STRING_PATH:       [&'static str; 3] = ["collections", "string", "String"];
-pub const VEC_PATH:          [&'static str; 3] = ["collections", "vec", "Vec"];
-pub const LL_PATH:           [&'static str; 3] = ["collections", "linked_list", "LinkedList"];
+pub const OPTION_PATH: [&'static str; 3] = ["core", "option", "Option"];
+pub const RESULT_PATH: [&'static str; 3] = ["core", "result", "Result"];
+pub const STRING_PATH: [&'static str; 3] = ["collections", "string", "String"];
+pub const VEC_PATH:    [&'static str; 3] = ["collections", "vec", "Vec"];
+pub const LL_PATH:     [&'static str; 3] = ["collections", "linked_list", "LinkedList"];
 pub const OPEN_OPTIONS_PATH: [&'static str; 3] = ["std", "fs", "OpenOptions"];
+pub const MUTEX_PATH:  [&'static str; 4] = ["std", "sync", "mutex", "Mutex"];
 
 /// Produce a nested chain of if-lets and ifs from the patterns:
 ///
@@ -78,8 +79,8 @@ pub fn in_external_macro<T: LintContext>(cx: &T, span: Span) -> bool {
         // no ExpnInfo = no macro
         opt_info.map_or(false, |info| {
             if let ExpnFormat::MacroAttribute(..) = info.callee.format {
-                // these are all plugins
-                return true;
+                    // these are all plugins
+                    return true;
             }
             // no span for the callee = external macro
             info.callee.span.map_or(true, |span| {
@@ -322,3 +323,49 @@ pub fn is_integer_literal(expr: &Expr, value: u64) -> bool
     }
     false
 }
+
+/// Produce a nested chain of if-lets and ifs from the patterns:
+///
+///     if_let_chain! {
+///         [
+///             Some(y) = x,
+///             y.len() == 2,
+///             Some(z) = y,
+///         ],
+///         {
+///             block
+///         }
+///     }
+///
+/// becomes
+///
+///     if let Some(y) = x {
+///         if y.len() == 2 {
+///             if let Some(z) = y {
+///                 block
+///             }
+///         }
+///     }
+#[macro_export]
+macro_rules! if_let_chain {
+    ([let $pat:pat = $expr:expr, $($tt:tt)+], $block:block) => {
+        if let $pat = $expr {
+           if_let_chain!{ [$($tt)+], $block }
+        }
+    };
+    ([let $pat:pat = $expr:expr], $block:block) => {
+        if let $pat = $expr {
+           $block
+        }
+    };
+    ([$expr:expr, $($tt:tt)+], $block:block) => {
+        if $expr {
+           if_let_chain!{ [$($tt)+], $block }
+        }
+    };
+    ([$expr:expr], $block:block) => {
+        if $expr {
+           $block
+        }
+    };
+}

tests/compile-fail/mutex_atomic.rs

@@ -0,0 +1,18 @@
+#![feature(plugin)]
+
+#![plugin(clippy)]
+#![deny(clippy)]
+#![deny(mutex_integer)]
+
+fn main() {
+    use std::sync::Mutex;
+    Mutex::new(true); //~ERROR Consider using an AtomicBool instead of a Mutex here.
+    Mutex::new(5usize); //~ERROR Consider using an AtomicUsize instead of a Mutex here.
+    Mutex::new(9isize); //~ERROR Consider using an AtomicIsize instead of a Mutex here.
+    let mut x = 4u32;
+    Mutex::new(&x as *const u32); //~ERROR Consider using an AtomicPtr instead of a Mutex here.
+    Mutex::new(&mut x as *mut u32); //~ERROR Consider using an AtomicPtr instead of a Mutex here.
+    Mutex::new(0u32); //~ERROR Consider using an AtomicUsize instead of a Mutex here.
+    Mutex::new(0i32); //~ERROR Consider using an AtomicIsize instead of a Mutex here.
+    Mutex::new(0f32); // there are no float atomics, so this should not lint
+}