new lint: using &Ref patterns instead of matching on *expr (fixes #187)

This commit is contained in:
Georg Brandl 2015-08-21 19:49:00 +02:00
parent 5403e82681
commit 017dac2301
4 changed files with 51 additions and 4 deletions

View File

@ -29,6 +29,7 @@ len_zero | warn | checking `.len() == 0` or `.len() > 0` (or
let_and_return | warn | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a function let_and_return | warn | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a function
let_unit_value | warn | creating a let binding to a value of unit type, which usually can't be used afterwards let_unit_value | warn | creating a let binding to a value of unit type, which usually can't be used afterwards
linkedlist | warn | usage of LinkedList, usually a vector is faster, or a more specialized data structure like a RingBuf linkedlist | warn | usage of LinkedList, usually a vector is faster, or a more specialized data structure like a RingBuf
match_ref_pats | warn | a match has all arms prefixed with `&`; the match expression can be dereferenced instead
modulo_one | warn | taking a number modulo 1, which always returns 0 modulo_one | warn | taking a number modulo 1, which always returns 0
mut_mut | warn | usage of double-mut refs, e.g. `&mut &mut ...` (either copy'n'paste error, or shows a fundamental misunderstanding of references) mut_mut | warn | usage of double-mut refs, e.g. `&mut &mut ...` (either copy'n'paste error, or shows a fundamental misunderstanding of references)
needless_bool | warn | if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }` needless_bool | warn | if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }`

View File

@ -88,6 +88,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
loops::EXPLICIT_ITER_LOOP, loops::EXPLICIT_ITER_LOOP,
loops::ITER_NEXT_LOOP, loops::ITER_NEXT_LOOP,
loops::NEEDLESS_RANGE_LOOP, loops::NEEDLESS_RANGE_LOOP,
matches::MATCH_REF_PATS,
matches::SINGLE_MATCH, matches::SINGLE_MATCH,
methods::OPTION_UNWRAP_USED, methods::OPTION_UNWRAP_USED,
methods::RESULT_UNWRAP_USED, methods::RESULT_UNWRAP_USED,

View File

@ -3,23 +3,27 @@ use syntax::ast;
use syntax::ast::*; use syntax::ast::*;
use std::borrow::Cow; use std::borrow::Cow;
use utils::{snippet, snippet_block, span_help_and_lint}; use utils::{snippet, snippet_block, span_lint, span_help_and_lint};
declare_lint!(pub SINGLE_MATCH, Warn, declare_lint!(pub SINGLE_MATCH, Warn,
"a match statement with a single nontrivial arm (i.e, where the other arm \ "a match statement with a single nontrivial arm (i.e, where the other arm \
is `_ => {}`) is used; recommends `if let` instead"); is `_ => {}`) is used; recommends `if let` instead");
declare_lint!(pub MATCH_REF_PATS, Warn,
"a match has all arms prefixed with `&`; the match expression can be \
dereferenced instead");
#[allow(missing_copy_implementations)] #[allow(missing_copy_implementations)]
pub struct MatchPass; pub struct MatchPass;
impl LintPass for MatchPass { impl LintPass for MatchPass {
fn get_lints(&self) -> LintArray { fn get_lints(&self) -> LintArray {
lint_array!(SINGLE_MATCH) lint_array!(SINGLE_MATCH, MATCH_REF_PATS)
} }
fn check_expr(&mut self, cx: &Context, expr: &Expr) { fn check_expr(&mut self, cx: &Context, expr: &Expr) {
if let ExprMatch(ref ex, ref arms, ast::MatchSource::Normal) = expr.node { if let ExprMatch(ref ex, ref arms, ast::MatchSource::Normal) = expr.node {
// check preconditions: only two arms // check preconditions for SINGLE_MATCH
// only two arms
if arms.len() == 2 && if arms.len() == 2 &&
// both of the arms have a single pattern and no guard // both of the arms have a single pattern and no guard
arms[0].pats.len() == 1 && arms[0].guard.is_none() && arms[0].pats.len() == 1 && arms[0].guard.is_none() &&
@ -48,6 +52,13 @@ impl LintPass for MatchPass {
body_code) body_code)
); );
} }
// check preconditions for MATCH_REF_PATS
if has_only_ref_pats(arms) {
span_lint(cx, MATCH_REF_PATS, expr.span, &format!(
"instead of prefixing all patterns with `&`, you can dereference the \
expression to match: `match *{} {{ ...`", snippet(cx, ex.span, "..")));
}
} }
} }
} }
@ -59,3 +70,16 @@ fn is_unit_expr(expr: &Expr) -> bool {
_ => false, _ => false,
} }
} }
fn has_only_ref_pats(arms: &[Arm]) -> bool {
for arm in arms {
for pat in &arm.pats {
match pat.node {
PatRegion(..) => (), // &-patterns
PatWild(..) => (), // an "anything" wildcard is also fine
_ => return false,
}
}
}
true
}

View File

@ -2,8 +2,9 @@
#![plugin(clippy)] #![plugin(clippy)]
#![deny(clippy)] #![deny(clippy)]
#![allow(unused)]
fn main(){ fn single_match(){
let x = Some(1u8); let x = Some(1u8);
match x { //~ ERROR you seem to be trying to use match match x { //~ ERROR you seem to be trying to use match
//~^ HELP try //~^ HELP try
@ -36,3 +37,23 @@ fn main(){
_ => println!("nope"), _ => println!("nope"),
} }
} }
fn ref_pats() {
let ref v = Some(0);
match v { //~ERROR instead of prefixing all patterns with `&`
&Some(v) => println!("{:?}", v),
&None => println!("none"),
}
match v { // this doesn't trigger, we have a different pattern
&Some(v) => println!("some"),
other => println!("other"),
}
let ref tup = (1, 2);
match tup { //~ERROR instead of prefixing all patterns with `&`
&(v, 1) => println!("{}", v),
_ => println!("none"),
}
}
fn main() {
}