nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2025-01-02 02:46:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #216 from birkenfeld/match_pass

Browse Source 
new lint: using &Ref patterns instead of matching on *expr (fixes #187)
This commit is contained in:
llogiq 2015-08-22 19:06:48 +02:00
commit 5a5b1ba96b
12 changed files with 155 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
README.md
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_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
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
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 }`

8
src/approx_const.rs
View File

@ -37,10 +37,10 @@ impl LintPass for ApproxConstant {
}
fn check_lit(cx: &Context, lit: &Lit, span: Span) {
match &lit.node {
&LitFloat(ref str, TyF32) => check_known_consts(cx, span, str, "f32"),
&LitFloat(ref str, TyF64) => check_known_consts(cx, span, str, "f64"),
&LitFloatUnsuffixed(ref str) => check_known_consts(cx, span, str, "f{32, 64}"),
match lit.node {
LitFloat(ref str, TyF32) => check_known_consts(cx, span, str, "f32"),
LitFloat(ref str, TyF64) => check_known_consts(cx, span, str, "f64"),
LitFloatUnsuffixed(ref str) => check_known_consts(cx, span, str, "f{32, 64}"),
_ => ()
}
}

12
src/bit_mask.rs
View File

@ -82,9 +82,9 @@ fn invert_cmp(cmp : BinOp_) -> BinOp_ {
fn check_compare(cx: &Context, bit_op: &Expr, cmp_op: BinOp_, cmp_value: u64, span: &Span) {
match &bit_op.node {
&ExprParen(ref subexp) => check_compare(cx, subexp, cmp_op, cmp_value, span),
&ExprBinary(ref op, ref left, ref right) => {
match bit_op.node {
ExprParen(ref subexp) => check_compare(cx, subexp, cmp_op, cmp_value, span),
ExprBinary(ref op, ref left, ref right) => {
if op.node != BiBitAnd && op.node != BiBitOr { return; }
fetch_int_literal(cx, right).or_else(|| fetch_int_literal(
cx, left)).map_or((), |mask| check_bit_mask(cx, op.node,
@ -182,13 +182,13 @@ fn check_ineffective_gt(cx: &Context, span: Span, m: u64, c: u64, op: &str) {
}
fn fetch_int_literal(cx: &Context, lit : &Expr) -> Option<u64> {
match &lit.node {
&ExprLit(ref lit_ptr) => {
match lit.node {
ExprLit(ref lit_ptr) => {
if let &LitInt(value, _) = &lit_ptr.node {
Option::Some(value) //TODO: Handle sign
} else { Option::None }
},
&ExprPath(_, _) => {
ExprPath(_, _) => {
// Important to let the borrow expire before the const lookup to avoid double
// borrowing.
let def_map = cx.tcx.def_map.borrow();

6
src/eta_reduction.rs
View File

@ -18,9 +18,9 @@ impl LintPass for EtaPass {
}
fn check_expr(&mut self, cx: &Context, expr: &Expr) {
match &expr.node {
&ExprCall(_, ref args) |
&ExprMethodCall(_, _, ref args) => {
match expr.node {
ExprCall(_, ref args) |
ExprMethodCall(_, _, ref args) => {
for arg in args {
check_closure(cx, &*arg)
}

8
src/len_zero.rs
View File

@ -22,10 +22,10 @@ impl LintPass for LenZero {
}
fn check_item(&mut self, cx: &Context, item: &Item) {
match &item.node {
&ItemTrait(_, _, _, ref trait_items) =>
match item.node {
ItemTrait(_, _, _, ref trait_items) =>
check_trait_items(cx, item, trait_items),
&ItemImpl(_, _, _, None, _, ref impl_items) => // only non-trait
ItemImpl(_, _, _, None, _, ref impl_items) => // only non-trait
check_impl_items(cx, item, impl_items),
_ => ()
}
@ -100,7 +100,7 @@ fn check_cmp(cx: &Context, span: Span, left: &Expr, right: &Expr, op: &str) {
fn check_len_zero(cx: &Context, span: Span, method: &SpannedIdent,
args: &[P<Expr>], lit: &Lit, op: &str) {
if let &Spanned{node: LitInt(0, _), ..} = lit {
if let Spanned{node: LitInt(0, _), ..} = *lit {
if method.node.name == "len" && args.len() == 1 &&
has_is_empty(cx, &*args[0]) {
span_lint(cx, LEN_ZERO, span, &format!(

6
src/lib.rs
View File

@ -38,11 +38,11 @@ pub mod returns;
pub mod lifetimes;
pub mod loops;
pub mod ranges;
pub mod matches;
#[plugin_registrar]
pub fn plugin_registrar(reg: &mut Registry) {
reg.register_lint_pass(box types::TypePass as LintPassObject);
reg.register_lint_pass(box misc::MiscPass as LintPassObject);
reg.register_lint_pass(box misc::TopLevelRefPass as LintPassObject);
reg.register_lint_pass(box misc::CmpNan as LintPassObject);
reg.register_lint_pass(box eq_op::EqOp as LintPassObject);
@ -71,6 +71,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
reg.register_lint_pass(box ranges::StepByZero as LintPassObject);
reg.register_lint_pass(box types::CastPass as LintPassObject);
reg.register_lint_pass(box types::TypeComplexityPass as LintPassObject);
reg.register_lint_pass(box matches::MatchPass as LintPassObject);
reg.register_lint_group("clippy", vec![
approx_const::APPROX_CONSTANT,
@ -87,6 +88,8 @@ pub fn plugin_registrar(reg: &mut Registry) {
loops::EXPLICIT_ITER_LOOP,
loops::ITER_NEXT_LOOP,
loops::NEEDLESS_RANGE_LOOP,
matches::MATCH_REF_PATS,
matches::SINGLE_MATCH,
methods::OPTION_UNWRAP_USED,
methods::RESULT_UNWRAP_USED,
methods::STR_TO_STRING,
@ -96,7 +99,6 @@ pub fn plugin_registrar(reg: &mut Registry) {
misc::FLOAT_CMP,
misc::MODULO_ONE,
misc::PRECEDENCE,
misc::SINGLE_MATCH,
misc::TOPLEVEL_REF_ARG,
mut_mut::MUT_MUT,
needless_bool::NEEDLESS_BOOL,

86
src/matches.rs Normal file
View File

@ -0,0 +1,86 @@
use rustc::lint::*;
use syntax::ast;
use syntax::ast::*;
use std::borrow::Cow;
use utils::{snippet, snippet_block, span_lint, span_help_and_lint};
declare_lint!(pub SINGLE_MATCH, Warn,
"a match statement with a single nontrivial arm (i.e, where the other arm \
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)]
pub struct MatchPass;
impl LintPass for MatchPass {
fn get_lints(&self) -> LintArray {
lint_array!(SINGLE_MATCH, MATCH_REF_PATS)
}
fn check_expr(&mut self, cx: &Context, expr: &Expr) {
if let ExprMatch(ref ex, ref arms, ast::MatchSource::Normal) = expr.node {
// check preconditions for SINGLE_MATCH
// only two arms
if arms.len() == 2 &&
// both of the arms have a single pattern and no guard
arms[0].pats.len() == 1 && arms[0].guard.is_none() &&
arms[1].pats.len() == 1 && arms[1].guard.is_none() &&
// and the second pattern is a `_` wildcard: this is not strictly necessary,
// since the exhaustiveness check will ensure the last one is a catch-all,
// but in some cases, an explicit match is preferred to catch situations
// when an enum is extended, so we don't consider these cases
arms[1].pats[0].node == PatWild(PatWildSingle) &&
// finally, we don't want any content in the second arm (unit or empty block)
is_unit_expr(&*arms[1].body)
{
let body_code = snippet_block(cx, arms[0].body.span, "..");
let body_code = if let ExprBlock(_) = arms[0].body.node {
body_code
} else {
Cow::Owned(format!("{{ {} }}", body_code))
};
span_help_and_lint(cx, SINGLE_MATCH, expr.span,
"you seem to be trying to use match for \
destructuring a single pattern. Did you mean to \
use `if let`?",
&*format!("try\nif let {} = {} {}",
snippet(cx, arms[0].pats[0].span, ".."),
snippet(cx, ex.span, ".."),
body_code)
);
}
// check preconditions for MATCH_REF_PATS
if has_only_ref_pats(arms) {
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 \
and the patterns: use `match {} {{ ...`", snippet(cx, inner.span, "..")));
} else {
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, "..")));
}
}
}
}
}
fn is_unit_expr(expr: &Expr) -> bool {
match expr.node {
ExprTup(ref v) if v.is_empty() => true,
ExprBlock(ref b) if b.stmts.is_empty() && b.expr.is_none() => true,
_ => false,
}
}
fn has_only_ref_pats(arms: &[Arm]) -> bool {
arms.iter().flat_map(|a| &a.pats).all(|p| match p.node {
PatRegion(..) => true, // &-patterns
PatWild(..) => true, // an "anything" wildcard is also fine
_ => false,
})
}

69
src/misc.rs
View File

@ -1,75 +1,14 @@
use rustc::lint::*;
use syntax::ptr::P;
use syntax::ast;
use syntax::ast::*;
use syntax::ast_util::{is_comparison_binop, binop_to_string};
use syntax::codemap::{Span, Spanned};
use syntax::visit::FnKind;
use rustc::middle::ty;
use std::borrow::Cow;
use utils::{match_path, snippet, snippet_block, span_lint, span_help_and_lint, walk_ptrs_ty};
use utils::{match_path, snippet, span_lint, walk_ptrs_ty};
use consts::constant;
/// Handles uncategorized lints
/// Currently handles linting of if-let-able matches
#[allow(missing_copy_implementations)]
pub struct MiscPass;
declare_lint!(pub SINGLE_MATCH, Warn,
"a match statement with a single nontrivial arm (i.e, where the other arm \
is `_ => {}`) is used; recommends `if let` instead");
impl LintPass for MiscPass {
fn get_lints(&self) -> LintArray {
lint_array!(SINGLE_MATCH)
}
fn check_expr(&mut self, cx: &Context, expr: &Expr) {
if let ExprMatch(ref ex, ref arms, ast::MatchSource::Normal) = expr.node {
// check preconditions: only two arms
if arms.len() == 2 &&
// both of the arms have a single pattern and no guard
arms[0].pats.len() == 1 && arms[0].guard.is_none() &&
arms[1].pats.len() == 1 && arms[1].guard.is_none() &&
// and the second pattern is a `_` wildcard: this is not strictly necessary,
// since the exhaustiveness check will ensure the last one is a catch-all,
// but in some cases, an explicit match is preferred to catch situations
// when an enum is extended, so we don't consider these cases
arms[1].pats[0].node == PatWild(PatWildSingle) &&
// finally, we don't want any content in the second arm (unit or empty block)
is_unit_expr(&*arms[1].body)
{
let body_code = snippet_block(cx, arms[0].body.span, "..");
let body_code = if let ExprBlock(_) = arms[0].body.node {
body_code
} else {
Cow::Owned(format!("{{ {} }}", body_code))
};
span_help_and_lint(cx, SINGLE_MATCH, expr.span,
"you seem to be trying to use match for \
destructuring a single pattern. Did you mean to \
use `if let`?",
&*format!("try\nif let {} = {} {}",
snippet(cx, arms[0].pats[0].span, ".."),
snippet(cx, ex.span, ".."),
body_code)
);
}
}
}
}
fn is_unit_expr(expr: &Expr) -> bool {
match expr.node {
ExprTup(ref v) if v.is_empty() => true,
ExprBlock(ref b) if b.stmts.is_empty() && b.expr.is_none() => true,
_ => false,
}
}
declare_lint!(pub TOPLEVEL_REF_ARG, Warn,
"a function argument is declared `ref` (i.e. `fn foo(ref x: u8)`, but not \
`fn foo((ref x, ref y): (u8, u8))`)");
@ -236,8 +175,8 @@ impl LintPass for CmpOwned {
}
fn check_to_owned(cx: &Context, expr: &Expr, other_span: Span) {
match &expr.node {
&ExprMethodCall(Spanned{node: ref ident, ..}, _, ref args) => {
match expr.node {
ExprMethodCall(Spanned{node: ref ident, ..}, _, ref args) => {
let name = ident.name;
if name == "to_string" ||
name == "to_owned" && is_str_arg(cx, args) {
@ -247,7 +186,7 @@ fn check_to_owned(cx: &Context, expr: &Expr, other_span: Span) {
snippet(cx, other_span, "..")))
}
},
&ExprCall(ref path, _) => {
ExprCall(ref path, _) => {
if let &ExprPath(None, ref path) = &path.node {
if match_path(path, &["String", "from_str"]) ||
match_path(path, &["String", "from"]) {

6
src/needless_bool.rs
View File

@ -60,9 +60,9 @@ fn fetch_bool_block(block: &Block) -> Option<bool> {
}
fn fetch_bool_expr(expr: &Expr) -> Option<bool> {
match &expr.node {
&ExprBlock(ref block) => fetch_bool_block(block),
&ExprLit(ref lit_ptr) => if let &LitBool(value) = &lit_ptr.node {
match expr.node {
ExprBlock(ref block) => fetch_bool_block(block),
ExprLit(ref lit_ptr) => if let LitBool(value) = lit_ptr.node {
Some(value) } else { None },
_ => None
}

8
src/strings.rs
View File

@ -65,13 +65,13 @@ fn is_string(cx: &Context, e: &Expr) -> bool {
}
fn is_add(cx: &Context, src: &Expr, target: &Expr) -> bool {
match &src.node {
&ExprBinary(Spanned{ node: BiAdd, .. }, ref left, _) =>
match src.node {
ExprBinary(Spanned{ node: BiAdd, .. }, ref left, _) =>
is_exp_equal(cx, target, left),
&ExprBlock(ref block) => block.stmts.is_empty() &&
ExprBlock(ref block) => block.stmts.is_empty() &&
block.expr.as_ref().map_or(false,
|expr| is_add(cx, &*expr, target)),
&ExprParen(ref expr) => is_add(cx, &*expr, target),
ExprParen(ref expr) => is_add(cx, &*expr, target),
_ => false
}
}

16
src/types.rs
View File

@ -116,10 +116,10 @@ declare_lint!(pub CAST_POSSIBLE_TRUNCATION, Allow,
/// Returns the size in bits of an integral type.
/// Will return 0 if the type is not an int or uint variant
fn int_ty_to_nbits(typ: &ty::TyS) -> usize {
let n = match &typ.sty {
&ty::TyInt(i) => 4 << (i as usize),
&ty::TyUint(u) => 4 << (u as usize),
_ => 0
let n = match typ.sty {
ty::TyInt(i) => 4 << (i as usize),
ty::TyUint(u) => 4 << (u as usize),
_ => 0
};
// n == 4 is the usize/isize case
if n == 4 { ::std::usize::BITS } else { n }
@ -139,16 +139,16 @@ impl LintPass for CastPass {
match (cast_from.is_integral(), cast_to.is_integral()) {
(true, false) => {
let from_nbits = int_ty_to_nbits(cast_from);
let to_nbits : usize = match &cast_to.sty {
&ty::TyFloat(ast::TyF32) => 32,
&ty::TyFloat(ast::TyF64) => 64,
let to_nbits : usize = match cast_to.sty {
ty::TyFloat(ast::TyF32) => 32,
ty::TyFloat(ast::TyF64) => 64,
_ => 0
};
if from_nbits != 0 {
if from_nbits >= to_nbits {
span_lint(cx, CAST_PRECISION_LOSS, expr.span,
&format!("converting from {0} to {1}, which causes a loss of precision \
({0} is {2} bits wide, but {1}'s mantissa is only {3} bits wide)",
({0} is {2} bits wide, but {1}'s mantissa is only {3} bits wide)",
cast_from, cast_to, from_nbits, if to_nbits == 64 {52} else {23} ));
}
}

29
tests/compile-fail/match_if_let.rs → tests/compile-fail/matches.rs
View File

@ -2,8 +2,9 @@
#![plugin(clippy)]
#![deny(clippy)]
#![allow(unused)]
fn main(){
fn single_match(){
let x = Some(1u8);
match x { //~ ERROR you seem to be trying to use match
//~^ HELP try
@ -36,3 +37,29 @@ fn main(){
_ => 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"),
}
// special case: using & both in expr and pats
let w = Some(0);
match &w { //~ERROR you don't need to add `&` to both
&Some(v) => println!("{:?}", v),
&None => println!("none"),
}
}
fn main() {
}