From 9f641a1009e92431b4a49a835c4e80c51ce44c88 Mon Sep 17 00:00:00 2001 From: mcarton Date: Wed, 13 Jan 2016 01:19:27 +0100 Subject: [PATCH] Add known enums to SINGLE_MATCH --- src/matches.rs | 53 ++++++++++++++++++++++++++++++++--- src/utils.rs | 1 + tests/compile-fail/matches.rs | 49 ++++++++++++++++++++++++++++---- 3 files changed, 94 insertions(+), 9 deletions(-) diff --git a/src/matches.rs b/src/matches.rs index 5e207093c15..217c4abf45b 100644 --- a/src/matches.rs +++ b/src/matches.rs @@ -8,7 +8,8 @@ use std::cmp::Ordering; use syntax::ast::Lit_::LitBool; use syntax::codemap::Span; -use utils::{snippet, span_lint, span_note_and_lint, span_help_and_lint, in_external_macro, expr_block}; +use utils::{COW_PATH, OPTION_PATH, RESULT_PATH}; +use utils::{match_type, snippet, span_lint, span_note_and_lint, span_help_and_lint, in_external_macro, expr_block}; /// **What it does:** This lint checks for matches with a single arm where an `if let` will usually suffice. It is `Warn` by default. /// @@ -109,9 +110,20 @@ impl LateLintPass for MatchPass { } fn check_single_match(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) { - if arms.len() == 2 && arms[0].pats.len() == 1 && arms[0].guard.is_none() && arms[1].pats.len() == 1 && - arms[1].guard.is_none() && arms[1].pats[0].node == PatWild && is_unit_expr(&arms[1].body) && - (cx.tcx.expr_ty(ex).sty != ty::TyBool || cx.current_level(MATCH_BOOL) == Allow) { + if arms.len() == 2 && + arms[0].pats.len() == 1 && arms[0].guard.is_none() && + arms[1].pats.len() == 1 && arms[1].guard.is_none() && + is_unit_expr(&arms[1].body) { + let ty = cx.tcx.expr_ty(ex); + if ty.sty != ty::TyBool || cx.current_level(MATCH_BOOL) == Allow { + check_single_match_single_pattern(cx, ex, arms, expr); + check_single_match_opt_like(cx, ex, arms, expr, ty); + } + } +} + +fn check_single_match_single_pattern(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) { + if arms[1].pats[0].node == PatWild { span_help_and_lint(cx, SINGLE_MATCH, expr.span, @@ -124,6 +136,39 @@ fn check_single_match(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) { } } +fn check_single_match_opt_like(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr, ty: ty::Ty) { + // list of candidate Enums we know will never get any more membre + let candidates = &[ + (&COW_PATH, "Borrowed"), + (&COW_PATH, "Cow::Borrowed"), + (&COW_PATH, "Cow::Owned"), + (&COW_PATH, "Owned"), + (&OPTION_PATH, "None"), + (&RESULT_PATH, "Err"), + (&RESULT_PATH, "Ok"), + ]; + + let path = match arms[1].pats[0].node { + PatEnum(ref path, _) => path.to_string(), + PatIdent(BindByValue(MutImmutable), ident, None) => ident.node.to_string(), + _ => return + }; + + for &(ty_path, pat_path) in candidates { + if &path == pat_path && match_type(cx, ty, ty_path) { + 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`", + &format!("try\nif let {} = {} {}", + snippet(cx, arms[0].pats[0].span, ".."), + snippet(cx, ex.span, ".."), + expr_block(cx, &arms[0].body, None, ".."))); + } + } +} + fn check_match_bool(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) { // type of expression == bool if cx.tcx.expr_ty(ex).sty == ty::TyBool { diff --git a/src/utils.rs b/src/utils.rs index 74a15927ad1..77e63fe4458 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -29,6 +29,7 @@ pub const OPEN_OPTIONS_PATH: [&'static str; 3] = ["std", "fs", "OpenOptions"]; pub const MUTEX_PATH: [&'static str; 4] = ["std", "sync", "mutex", "Mutex"]; pub const CLONE_PATH: [&'static str; 2] = ["Clone", "clone"]; pub const BEGIN_UNWIND: [&'static str; 3] = ["std", "rt", "begin_unwind"]; +pub const COW_PATH: [&'static str; 3] = ["collections", "borrow", "Cow"]; /// Produce a nested chain of if-lets and ifs from the patterns: /// diff --git a/tests/compile-fail/matches.rs b/tests/compile-fail/matches.rs index b569f9566ef..c58e62419c6 100644 --- a/tests/compile-fail/matches.rs +++ b/tests/compile-fail/matches.rs @@ -4,8 +4,14 @@ #![deny(clippy)] #![allow(unused)] +use std::borrow::Cow; + +enum Foo { Bar, Baz(u8) } +use Foo::*; + fn single_match(){ let x = Some(1u8); + match x { //~ ERROR you seem to be trying to use match //~^ HELP try Some(y) => { @@ -13,11 +19,7 @@ fn single_match(){ } _ => () } - // Not linted - match x { - Some(y) => println!("{:?}", y), - None => () - } + let z = (1u8,1u8); match z { //~ ERROR you seem to be trying to use match //~^ HELP try @@ -38,6 +40,43 @@ fn single_match(){ } } +fn single_match_know_enum() { + let x = Some(1u8); + let y : Result<_, i8> = Ok(1i8); + + match x { //~ ERROR you seem to be trying to use match + //~^ HELP try + Some(y) => println!("{:?}", y), + None => () + } + + match y { //~ ERROR you seem to be trying to use match + //~^ HELP try + Ok(y) => println!("{:?}", y), + Err(..) => () + } + + let c = Cow::Borrowed(""); + + match c { //~ ERROR you seem to be trying to use match + //~^ HELP try + Cow::Borrowed(..) => println!("42"), + Cow::Owned(..) => (), + } + + let z = Foo::Bar; + // no warning + match z { + Bar => println!("42"), + Baz(_) => (), + } + + match z { + Baz(_) => println!("42"), + Bar => (), + } +} + fn match_bool() { let test: bool = true;