diff --git a/README.md b/README.md index 3b231f34a5a..e57ca8d1e02 100644 --- a/README.md +++ b/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 diff --git a/src/lib.rs b/src/lib.rs index ff8d099b3fc..5276ad711c0 100755 --- a/src/lib.rs +++ b/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, diff --git a/src/mutex_atomic.rs b/src/mutex_atomic.rs new file mode 100644 index 00000000000..9c10a062419 --- /dev/null +++ b/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 + } +} diff --git a/src/utils.rs b/src/utils.rs index c84bf3692c4..1ba6029ebe6 100644 --- a/src/utils.rs +++ b/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(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 + } + }; +} diff --git a/tests/compile-fail/mutex_atomic.rs b/tests/compile-fail/mutex_atomic.rs new file mode 100644 index 00000000000..20a34ba5547 --- /dev/null +++ b/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 +}