diff --git a/README.md b/README.md index e50407eaaaa..990db2e5460 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 66 lints included in this crate: +There are 67 lints included in this crate: name | default | meaning -------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -34,6 +34,7 @@ name [let_and_return](https://github.com/Manishearth/rust-clippy/wiki#let_and_return) | warn | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block [let_unit_value](https://github.com/Manishearth/rust-clippy/wiki#let_unit_value) | warn | creating a let binding to a value of unit type, which usually can't be used afterwards [linkedlist](https://github.com/Manishearth/rust-clippy/wiki#linkedlist) | warn | usage of LinkedList, usually a vector is faster, or a more specialized data structure like a VecDeque +[match_bool](https://github.com/Manishearth/rust-clippy/wiki#match_bool) | warn | a match on boolean expression; recommends `if..else` block instead [match_ref_pats](https://github.com/Manishearth/rust-clippy/wiki#match_ref_pats) | warn | a match has all arms prefixed with `&`; the match expression can be dereferenced instead [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 diff --git a/src/lib.rs b/src/lib.rs index a3657460fe9..6f06641ef45 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -136,6 +136,7 @@ pub fn plugin_registrar(reg: &mut Registry) { loops::REVERSE_RANGE_LOOP, loops::UNUSED_COLLECT, loops::WHILE_LET_LOOP, + matches::MATCH_BOOL, matches::MATCH_REF_PATS, matches::SINGLE_MATCH, methods::SHOULD_IMPLEMENT_TRAIT, diff --git a/src/matches.rs b/src/matches.rs index e935a6aa6e1..da770c6f484 100644 --- a/src/matches.rs +++ b/src/matches.rs @@ -1,5 +1,6 @@ use rustc::lint::*; use rustc_front::hir::*; +use rustc::middle::ty; use utils::{snippet, span_lint, span_help_and_lint, in_external_macro, expr_block}; @@ -9,19 +10,23 @@ declare_lint!(pub SINGLE_MATCH, Warn, declare_lint!(pub MATCH_REF_PATS, Warn, "a match has all arms prefixed with `&`; the match expression can be \ dereferenced instead"); +declare_lint!(pub MATCH_BOOL, Warn, + "a match on boolean expression; recommends `if..else` block instead"); #[allow(missing_copy_implementations)] pub struct MatchPass; impl LintPass for MatchPass { fn get_lints(&self) -> LintArray { - lint_array!(SINGLE_MATCH, MATCH_REF_PATS) + lint_array!(SINGLE_MATCH, MATCH_REF_PATS, MATCH_BOOL) } } impl LateLintPass for MatchPass { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { if let ExprMatch(ref ex, ref arms, MatchSource::Normal) = expr.node { + if in_external_macro(cx, expr.span) { return; } + // check preconditions for SINGLE_MATCH // only two arms if arms.len() == 2 && @@ -36,7 +41,6 @@ impl LateLintPass for MatchPass { // finally, we don't want any content in the second arm (unit or empty block) is_unit_expr(&arms[1].body) { - if in_external_macro(cx, expr.span) {return;} span_help_and_lint(cx, SINGLE_MATCH, expr.span, "you seem to be trying to use match for destructuring a \ single pattern. Consider using `if let`", @@ -48,7 +52,6 @@ impl LateLintPass for MatchPass { // check preconditions for MATCH_REF_PATS if has_only_ref_pats(arms) { - if in_external_macro(cx, expr.span) { return; } if let ExprAddrOf(Mutability::MutImmutable, ref inner) = ex.node { span_lint(cx, MATCH_REF_PATS, expr.span, &format!( "you don't need to add `&` to both the expression to match \ @@ -59,6 +62,15 @@ impl LateLintPass for MatchPass { expression to match: `match *{} {{ ...`", snippet(cx, ex.span, ".."))); } } + + // check preconditions for MATCH_BOOL + // type of expression == bool + if cx.tcx.expr_ty(ex).sty == ty::TyBool { + + span_lint(cx, MATCH_BOOL, expr.span, + "you seem to be trying to match on a boolean expression. \ + Consider using an if..else block"); + } } } } diff --git a/tests/compile-fail/matches.rs b/tests/compile-fail/matches.rs index f25a5fa3fa4..1cdf813bc00 100755 --- a/tests/compile-fail/matches.rs +++ b/tests/compile-fail/matches.rs @@ -38,6 +38,28 @@ fn single_match(){ } } +fn match_bool() { + let test: bool = true; + + match test { //~ ERROR you seem to be trying to match on a boolean expression + true => (), + false => (), + }; + + let option = 1; + match option == 1 { //~ ERROR you seem to be trying to match on a boolean expression + true => (), + false => (), + }; + + // Not linted + match option { + 1 ... 10 => (), + 10 ... 20 => (), + _ => (), + }; +} + fn ref_pats() { { let v = &Some(0);