Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-24 05:33:27 +00:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #547 from mcarton/single_match

Browse source 
Improve the single_match lint
This commit is contained in:
Manish Goregaokar 2016-01-13 20:50:14 +05:30
commit e24730cb84
3 changed files with 131 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

137
src/matches.rs
View file

@ -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_lint_and_then, 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,25 +110,73 @@ 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) {
span_help_and_lint(cx,
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_lint_and_then(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, "..")));
"you seem to be trying to use match for destructuring a single pattern. \
Consider using `if let`", |db| {
db.span_suggestion(expr.span, "try this",
format!("if let {} = {} {}",
snippet(cx, arms[0].pats[0].span, ".."),
snippet(cx, ex.span, ".."),
expr_block(cx, &arms[0].body, None, "..")));
});
}
}
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_lint_and_then(cx,
SINGLE_MATCH,
expr.span,
"you seem to be trying to use match for destructuring a single pattern. \
Consider using `if let`", |db| {
db.span_suggestion(expr.span, "try this",
format!("if 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 {
if arms.len() == 2 && arms[0].pats.len() == 1 {
let sugg = if arms.len() == 2 && arms[0].pats.len() == 1 {
// no guards
let exprs = if let PatLit(ref arm_bool) = arms[0].pats[0].node {
if let ExprLit(ref lit) = arm_bool.node {
@ -142,56 +191,42 @@ fn check_match_bool(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) {
} else {
None
};
if let Some((ref true_expr, ref false_expr)) = exprs {
if !is_unit_expr(true_expr) {
if !is_unit_expr(false_expr) {
span_help_and_lint(cx,
MATCH_BOOL,
expr.span,
"you seem to be trying to match on a boolean expression. Consider using \
an if..else block:",
&format!("try\nif {} {} else {}",
snippet(cx, ex.span, "b"),
expr_block(cx, true_expr, None, ".."),
expr_block(cx, false_expr, None, "..")));
Some(format!("if {} {} else {}",
snippet(cx, ex.span, "b"),
expr_block(cx, true_expr, None, ".."),
expr_block(cx, false_expr, None, "..")))
} else {
span_help_and_lint(cx,
MATCH_BOOL,
expr.span,
"you seem to be trying to match on a boolean expression. Consider using \
an if..else block:",
&format!("try\nif {} {}",
snippet(cx, ex.span, "b"),
expr_block(cx, true_expr, None, "..")));
Some(format!("if {} {}",
snippet(cx, ex.span, "b"),
expr_block(cx, true_expr, None, "..")))
}
} else if !is_unit_expr(false_expr) {
span_help_and_lint(cx,
MATCH_BOOL,
expr.span,
"you seem to be trying to match on a boolean expression. Consider using an \
if..else block:",
&format!("try\nif !{} {}",
snippet(cx, ex.span, "b"),
expr_block(cx, false_expr, None, "..")));
Some(format!("try\nif !{} {}",
snippet(cx, ex.span, "b"),
expr_block(cx, false_expr, None, "..")))
} else {
span_lint(cx,
MATCH_BOOL,
expr.span,
"you seem to be trying to match on a boolean expression. Consider using an if..else \
block");
None
}
} else {
span_lint(cx,
MATCH_BOOL,
expr.span,
"you seem to be trying to match on a boolean expression. Consider using an if..else block");
None
}
} else {
span_lint(cx,
MATCH_BOOL,
expr.span,
"you seem to be trying to match on a boolean expression. Consider using an if..else block");
}
None
};
span_lint_and_then(cx,
MATCH_BOOL,
expr.span,
"you seem to be trying to match on a boolean expression. Consider using \
an if..else block:", move |db| {
if let Some(ref sugg) = sugg {
db.span_suggestion(expr.span, "try this", sugg.clone());
}
});
}
}

1
src/utils.rs
View file

@ -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:
///

49
tests/compile-fail/matches.rs
View file

@ -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;