diff --git a/src/bit_mask.rs b/src/bit_mask.rs index d84da654eb45..352826dffaea 100644 --- a/src/bit_mask.rs +++ b/src/bit_mask.rs @@ -1,22 +1,3 @@ -//! Checks for incompatible bit masks in comparisons, e.g. `x & 1 == 2`. This cannot work because the bit that makes up -//! the value two was zeroed out by the bit-and with 1. So the formula for detecting if an expression of the type -//! `_ m c` (where `` is one of {`&`, '|'} and `` is one of {`!=`, `>=`, `>` ,`!=`, `>=`, -//! `>`}) can be determined from the following table: -//! -//! |Comparison |Bit-Op|Example |is always|Formula | -//! |------------|------|------------|---------|----------------------| -//! |`==` or `!=`| `&` |`x & 2 == 3`|`false` |`c & m != c` | -//! |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` | -//! |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` | -//! |`==` or `!=`| `|` |`x | 1 == 0`|`false` |`c | m != c` | -//! |`<` or `>=`| `|` |`x | 1 < 1` |`false` |`m >= c` | -//! |`<=` or `>` | `|` |`x | 1 > 0` |`true` |`m > c` | -//! -//! *TODO*: There is the open question if things like `x | 1 > 1` should be caught by this lint, because it is basically -//! an obfuscated version of `x > 1`. -//! -//! This lint is **deny** by default - use rustc::plugin::Registry; use rustc::lint::*; use rustc::middle::const_eval::lookup_const_by_id; @@ -29,15 +10,37 @@ use syntax::codemap::Span; declare_lint! { pub BAD_BIT_MASK, Deny, - "Deny the use of incompatible bit masks in comparisons, e.g. '(a & 1) == 2'" + "Deny the use of incompatible bit masks in comparisons, e.g. \ + '(a & 1) == 2'" } declare_lint! { pub INEFFECTIVE_BIT_MASK, Warn, - "Warn on the use of an ineffective bit mask in comparisons, e.g. '(a & 1) > 2'" + "Warn on the use of an ineffective bit mask in comparisons, e.g. \ + '(a & 1) > 2'" } +/// Checks for incompatible bit masks in comparisons, e.g. `x & 1 == 2`. +/// This cannot work because the bit that makes up the value two was +/// zeroed out by the bit-and with 1. So the formula for detecting if an +/// expression of the type `_ m c` (where `` +/// is one of {`&`, '|'} and `` is one of {`!=`, `>=`, `>` , +/// `!=`, `>=`, `>`}) can be determined from the following table: +/// +/// |Comparison |Bit-Op|Example |is always|Formula | +/// |------------|------|------------|---------|----------------------| +/// |`==` or `!=`| `&` |`x & 2 == 3`|`false` |`c & m != c` | +/// |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` | +/// |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` | +/// |`==` or `!=`| `|` |`x | 1 == 0`|`false` |`c | m != c` | +/// |`<` or `>=`| `|` |`x | 1 < 1` |`false` |`m >= c` | +/// |`<=` or `>` | `|` |`x | 1 > 0` |`true` |`m > c` | +/// +/// This lint is **deny** by default +/// +/// There is also a lint that warns on ineffective masks that is *warn* +/// by default #[derive(Copy,Clone)] pub struct BitMask; @@ -49,11 +52,12 @@ impl LintPass for BitMask { fn check_expr(&mut self, cx: &Context, e: &Expr) { if let ExprBinary(ref cmp, ref left, ref right) = e.node { if is_comparison_binop(cmp.node) { - fetch_int_literal(cx, right).map(|cmp_opt| - check_compare(cx, left, cmp.node, cmp_opt, &e.span)) - .unwrap_or_else(|| fetch_int_literal(cx, left).map(|cmp_val| - check_compare(cx, right, invert_cmp(cmp.node), cmp_val, - &e.span)).unwrap_or(())) + fetch_int_literal(cx, right).map_or_else(|| + fetch_int_literal(cx, left).map_or((), |cmp_val| + check_compare(cx, right, invert_cmp(cmp.node), + cmp_val, &e.span)), + |cmp_opt| check_compare(cx, left, cmp.node, cmp_opt, + &e.span)) } } } @@ -77,11 +81,9 @@ fn check_compare(cx: &Context, bit_op: &Expr, cmp_op: BinOp_, cmp_value: u64, sp &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; } - if let Some(mask_value) = fetch_int_literal(cx, right) { - check_bit_mask(cx, op.node, cmp_op, mask_value, cmp_value, span); - } else if let Some(mask_value) = fetch_int_literal(cx, left) { - check_bit_mask(cx, op.node, cmp_op, mask_value, cmp_value, span); - } + fetch_int_literal(cx, right).or_else(|| fetch_int_literal( + cx, left)).map_or((), |mask| check_bit_mask(cx, op.node, + cmp_op, mask, cmp_value, span)) }, _ => () } diff --git a/src/eq_op.rs b/src/eq_op.rs index 0223820447d2..94a49e748e20 100644 --- a/src/eq_op.rs +++ b/src/eq_op.rs @@ -33,18 +33,20 @@ fn is_exp_equal(left : &Expr, right : &Expr) -> bool { match (&left.node, &right.node) { (&ExprBinary(ref lop, ref ll, ref lr), &ExprBinary(ref rop, ref rl, ref rr)) => - lop.node == rop.node && is_exp_equal(ll, rl) && is_exp_equal(lr, rr), - (&ExprBox(ref lpl, ref lboxedpl), &ExprBox(ref rpl, ref rboxedpl)) => + lop.node == rop.node && + is_exp_equal(ll, rl) && is_exp_equal(lr, rr), + (&ExprBox(ref lpl, ref lbox), &ExprBox(ref rpl, ref rbox)) => both(lpl, rpl, |l, r| is_exp_equal(l, r)) && - is_exp_equal(lboxedpl, rboxedpl), - (&ExprCall(ref lcallee, ref largs), &ExprCall(ref rcallee, ref rargs)) => - is_exp_equal(lcallee, rcallee) && is_exps_equal(largs, rargs), - (&ExprCast(ref lcast, ref lty), &ExprCast(ref rcast, ref rty)) => - is_ty_equal(lty, rty) && is_exp_equal(lcast, rcast), + is_exp_equal(lbox, rbox), + (&ExprCall(ref lcallee, ref largs), + &ExprCall(ref rcallee, ref rargs)) => is_exp_equal(lcallee, + rcallee) && is_exps_equal(largs, rargs), + (&ExprCast(ref lc, ref lty), &ExprCast(ref rc, ref rty)) => + is_ty_equal(lty, rty) && is_exp_equal(lc, rc), (&ExprField(ref lfexp, ref lfident), &ExprField(ref rfexp, ref rfident)) => lfident.node == rfident.node && is_exp_equal(lfexp, rfexp), - (&ExprLit(ref llit), &ExprLit(ref rlit)) => llit.node == rlit.node, + (&ExprLit(ref l), &ExprLit(ref r)) => l.node == r.node, (&ExprMethodCall(ref lident, ref lcty, ref lmargs), &ExprMethodCall(ref rident, ref rcty, ref rmargs)) => lident.node == rident.node && is_tys_equal(lcty, rcty) && @@ -55,10 +57,11 @@ fn is_exp_equal(left : &Expr, right : &Expr) -> bool { &ExprPath(ref rqself, ref rsubpath)) => both(lqself, rqself, |l, r| is_qself_equal(l, r)) && is_path_equal(lsubpath, rsubpath), - (&ExprTup(ref ltup), &ExprTup(ref rtup)) => is_exps_equal(ltup, rtup), - (&ExprUnary(lunop, ref lparam), &ExprUnary(runop, ref rparam)) => - lunop == runop && is_exp_equal(lparam, rparam), - (&ExprVec(ref lvec), &ExprVec(ref rvec)) => is_exps_equal(lvec, rvec), + (&ExprTup(ref ltup), &ExprTup(ref rtup)) => + is_exps_equal(ltup, rtup), + (&ExprUnary(lunop, ref l), &ExprUnary(runop, ref r)) => + lunop == runop && is_exp_equal(l, r), + (&ExprVec(ref l), &ExprVec(ref r)) => is_exps_equal(l, r), _ => false } } @@ -83,18 +86,17 @@ fn is_ty_equal(left : &Ty, right : &Ty) -> bool { is_ty_equal(lfvty, rfvty) && is_exp_equal(lfvexp, rfvexp), (&TyPtr(ref lmut), &TyPtr(ref rmut)) => is_mut_ty_equal(lmut, rmut), (&TyRptr(ref ltime, ref lrmut), &TyRptr(ref rtime, ref rrmut)) => - both(ltime, rtime, is_lifetime_equal) && is_mut_ty_equal(lrmut, rrmut), + both(ltime, rtime, is_lifetime_equal) && + is_mut_ty_equal(lrmut, rrmut), (&TyBareFn(ref lbare), &TyBareFn(ref rbare)) => is_bare_fn_ty_equal(lbare, rbare), (&TyTup(ref ltup), &TyTup(ref rtup)) => is_tys_equal(ltup, rtup), - (&TyPath(Option::None, ref lpath), &TyPath(Option::None, ref rpath)) => - is_path_equal(lpath, rpath), - (&TyPath(Option::Some(ref lqself), ref lsubpath), - &TyPath(Option::Some(ref rqself), ref rsubpath)) => - is_qself_equal(lqself, rqself) && is_path_equal(lsubpath, rsubpath), + (&TyPath(ref lq, ref lpath), &TyPath(ref rq, ref rpath)) => + both(lq, rq, is_qself_equal) && is_path_equal(lpath, rpath), (&TyObjectSum(ref lsumty, ref lobounds), &TyObjectSum(ref rsumty, ref robounds)) => - is_ty_equal(lsumty, rsumty) && is_param_bounds_equal(lobounds, robounds), + is_ty_equal(lsumty, rsumty) && + is_param_bounds_equal(lobounds, robounds), (&TyPolyTraitRef(ref ltbounds), &TyPolyTraitRef(ref rtbounds)) => is_param_bounds_equal(ltbounds, rtbounds), (&TyParen(ref lty), &TyParen(ref rty)) => is_ty_equal(lty, rty), @@ -104,7 +106,8 @@ fn is_ty_equal(left : &Ty, right : &Ty) -> bool { } } -fn is_param_bound_equal(left : &TyParamBound, right : &TyParamBound) -> bool { +fn is_param_bound_equal(left : &TyParamBound, right : &TyParamBound) + -> bool { match(left, right) { (&TraitTyParamBound(ref lpoly, ref lmod), &TraitTyParamBound(ref rpoly, ref rmod)) => @@ -115,12 +118,14 @@ fn is_param_bound_equal(left : &TyParamBound, right : &TyParamBound) -> bool { } } -fn is_poly_traitref_equal(left : &PolyTraitRef, right : &PolyTraitRef) -> bool { - is_lifetimedefs_equal(&left.bound_lifetimes, &right.bound_lifetimes) && - is_path_equal(&left.trait_ref.path, &right.trait_ref.path) +fn is_poly_traitref_equal(left : &PolyTraitRef, right : &PolyTraitRef) + -> bool { + is_lifetimedefs_equal(&left.bound_lifetimes, &right.bound_lifetimes) + && is_path_equal(&left.trait_ref.path, &right.trait_ref.path) } -fn is_param_bounds_equal(left : &TyParamBounds, right : &TyParamBounds) -> bool { +fn is_param_bounds_equal(left : &TyParamBounds, right : &TyParamBounds) + -> bool { over(left, right, is_param_bound_equal) } @@ -135,20 +140,23 @@ fn is_bare_fn_ty_equal(left : &BareFnTy, right : &BareFnTy) -> bool { } fn is_fndecl_equal(left : &P, right : &P) -> bool { - left.variadic == right.variadic && is_args_equal(&left.inputs, &right.inputs) && + left.variadic == right.variadic && + is_args_equal(&left.inputs, &right.inputs) && is_fnret_ty_equal(&left.output, &right.output) } -fn is_fnret_ty_equal(left : &FunctionRetTy, right : &FunctionRetTy) -> bool { +fn is_fnret_ty_equal(left : &FunctionRetTy, right : &FunctionRetTy) + -> bool { match (left, right) { - (&NoReturn(_), &NoReturn(_)) | (&DefaultReturn(_), &DefaultReturn(_)) => true, + (&NoReturn(_), &NoReturn(_)) | + (&DefaultReturn(_), &DefaultReturn(_)) => true, (&Return(ref lty), &Return(ref rty)) => is_ty_equal(lty, rty), _ => false } } -fn is_arg_equal(left : &Arg, right : &Arg) -> bool { - is_ty_equal(&left.ty, &right.ty) && is_pat_equal(&left.pat, &right.pat) +fn is_arg_equal(l: &Arg, r : &Arg) -> bool { + is_ty_equal(&l.ty, &r.ty) && is_pat_equal(&l.pat, &r.pat) } fn is_args_equal(left : &[Arg], right : &[Arg]) -> bool { @@ -165,17 +173,16 @@ fn is_pat_equal(left : &Pat, right : &Pat) -> bool { &PatIdent(ref rmode, ref rident, Option::Some(ref rpat))) => lmode == rmode && is_ident_equal(&lident.node, &rident.node) && is_pat_equal(lpat, rpat), - (&PatEnum(ref lpath, Option::None), &PatEnum(ref rpath, Option::None)) => - is_path_equal(lpath, rpath), - (&PatEnum(ref lpath, Option::Some(ref lenum)), - &PatEnum(ref rpath, Option::Some(ref renum))) => - is_path_equal(lpath, rpath) && is_pats_equal(lenum, renum), + (&PatEnum(ref lpath, ref lenum), &PatEnum(ref rpath, ref renum)) => + is_path_equal(lpath, rpath) && both(lenum, renum, |l, r| + is_pats_equal(l, r)), (&PatStruct(ref lpath, ref lfieldpat, lbool), &PatStruct(ref rpath, ref rfieldpat, rbool)) => lbool == rbool && is_path_equal(lpath, rpath) && is_spanned_fieldpats_equal(lfieldpat, rfieldpat), (&PatTup(ref ltup), &PatTup(ref rtup)) => is_pats_equal(ltup, rtup), - (&PatBox(ref lboxed), &PatBox(ref rboxed)) => is_pat_equal(lboxed, rboxed), + (&PatBox(ref lboxed), &PatBox(ref rboxed)) => + is_pat_equal(lboxed, rboxed), (&PatRegion(ref lpat, ref lmut), &PatRegion(ref rpat, ref rmut)) => is_pat_equal(lpat, rpat) && lmut == rmut, (&PatLit(ref llit), &PatLit(ref rlit)) => is_exp_equal(llit, rlit), @@ -212,12 +219,14 @@ fn is_pats_equal(left : &[P], right : &[P]) -> bool { over(left, right, |l, r| is_pat_equal(l, r)) } -fn is_lifetimedef_equal(left : &LifetimeDef, right : &LifetimeDef) -> bool { +fn is_lifetimedef_equal(left : &LifetimeDef, right : &LifetimeDef) + -> bool { is_lifetime_equal(&left.lifetime, &right.lifetime) && over(&left.bounds, &right.bounds, is_lifetime_equal) } -fn is_lifetimedefs_equal(left : &[LifetimeDef], right : &[LifetimeDef]) -> bool { +fn is_lifetimedefs_equal(left : &[LifetimeDef], right : &[LifetimeDef]) + -> bool { over(left, right, is_lifetimedef_equal) } @@ -231,19 +240,20 @@ fn is_tys_equal(left : &[P], right : &[P]) -> bool { fn over(left: &[X], right: &[X], mut eq_fn: F) -> bool where F: FnMut(&X, &X) -> bool { - left.len() == right.len() && left.iter().zip(right).all(|(x, y)| eq_fn(x, y)) + left.len() == right.len() && left.iter().zip(right).all(|(x, y)| + eq_fn(x, y)) } fn both(l: &Option, r: &Option, mut eq_fn : F) -> bool where F: FnMut(&X, &X) -> bool { - l.as_ref().map(|x| r.as_ref().map(|y| eq_fn(x, y)).unwrap_or(false)) - .unwrap_or_else(|| r.is_none()) + l.as_ref().map_or_else(|| r.is_none(), |x| r.as_ref().map_or(false, + |y| eq_fn(x, y))) } fn is_cmp_or_bit(op : &BinOp) -> bool { match op.node { - BiEq | BiLt | BiLe | BiGt | BiGe | BiNe | BiAnd | BiOr | BiBitXor | - BiBitAnd | BiBitOr => true, + BiEq | BiLt | BiLe | BiGt | BiGe | BiNe | BiAnd | BiOr | + BiBitXor | BiBitAnd | BiBitOr => true, _ => false } } diff --git a/src/identity_op.rs b/src/identity_op.rs index ec4495539d2a..25697199dc39 100644 --- a/src/identity_op.rs +++ b/src/identity_op.rs @@ -17,66 +17,64 @@ impl LintPass for IdentityOp { fn get_lints(&self) -> LintArray { lint_array!(IDENTITY_OP) } - + fn check_expr(&mut self, cx: &Context, e: &Expr) { - if let ExprBinary(ref cmp, ref left, ref right) = e.node { - match cmp.node { - BiAdd | BiBitOr | BiBitXor => { - check(cx, left, 0, e.span, right.span); - check(cx, right, 0, e.span, left.span); - }, - BiShl | BiShr | BiSub => - check(cx, right, 0, e.span, left.span), - BiMul => { - check(cx, left, 1, e.span, right.span); - check(cx, right, 1, e.span, left.span); - }, - BiDiv => - check(cx, right, 1, e.span, left.span), - BiBitAnd => { - check(cx, left, -1, e.span, right.span); - check(cx, right, -1, e.span, left.span); - }, - _ => () - } - } + if let ExprBinary(ref cmp, ref left, ref right) = e.node { + match cmp.node { + BiAdd | BiBitOr | BiBitXor => { + check(cx, left, 0, e.span, right.span); + check(cx, right, 0, e.span, left.span); + }, + BiShl | BiShr | BiSub => + check(cx, right, 0, e.span, left.span), + BiMul => { + check(cx, left, 1, e.span, right.span); + check(cx, right, 1, e.span, left.span); + }, + BiDiv => + check(cx, right, 1, e.span, left.span), + BiBitAnd => { + check(cx, left, -1, e.span, right.span); + check(cx, right, -1, e.span, left.span); + }, + _ => () + } + } } } fn check(cx: &Context, e: &Expr, m: i8, span: Span, arg: Span) { - if have_lit(cx, e, m) { - let map = cx.sess().codemap(); - cx.span_lint(IDENTITY_OP, span, &format!( - "The operation is ineffective. Consider reducing it to '{}'", - &*map.span_to_snippet(arg).unwrap_or("..".to_string()))); - } + if have_lit(cx, e, m) { + let map = cx.sess().codemap(); + cx.span_lint(IDENTITY_OP, span, &format!( + "The operation is ineffective. Consider reducing it to '{}'", + &*map.span_to_snippet(arg).unwrap_or("..".to_string()))); + } } fn have_lit(cx: &Context, e : &Expr, m: i8) -> bool { - match &e.node { - &ExprUnary(UnNeg, ref litexp) => have_lit(cx, litexp, -m), - &ExprLit(ref lit) => { - match (&lit.node, m) { - (&LitInt(0, _), 0) => true, - (&LitInt(1, SignedIntLit(_, Plus)), 1) => true, - (&LitInt(1, UnsuffixedIntLit(Plus)), 1) => true, - (&LitInt(1, SignedIntLit(_, Minus)), -1) => true, - (&LitInt(1, UnsuffixedIntLit(Minus)), -1) => true, - _ => false - } - }, - &ExprParen(ref p) => have_lit(cx, p, m), - &ExprPath(_, _) => { - match cx.tcx.def_map.borrow().get(&e.id) { - Some(&PathResolution { base_def: DefConst(def_id), ..}) => - match lookup_const_by_id(cx.tcx, def_id, Option::None) { - Some(l) => have_lit(cx, l, m), - None => false - }, - _ => false - } + match &e.node { + &ExprUnary(UnNeg, ref litexp) => have_lit(cx, litexp, -m), + &ExprLit(ref lit) => { + match (&lit.node, m) { + (&LitInt(0, _), 0) => true, + (&LitInt(1, SignedIntLit(_, Plus)), 1) => true, + (&LitInt(1, UnsuffixedIntLit(Plus)), 1) => true, + (&LitInt(1, SignedIntLit(_, Minus)), -1) => true, + (&LitInt(1, UnsuffixedIntLit(Minus)), -1) => true, + _ => false } - _ => false - } + }, + &ExprParen(ref p) => have_lit(cx, p, m), + &ExprPath(_, _) => { + match cx.tcx.def_map.borrow().get(&e.id) { + Some(&PathResolution { base_def: DefConst(id), ..}) => + lookup_const_by_id(cx.tcx, id, Option::None) + .map_or(false, |l| have_lit(cx, l, m)), + _ => false + } + }, + _ => false + } } diff --git a/src/mut_mut.rs b/src/mut_mut.rs index 569f7b813444..bf8155b5c3c9 100644 --- a/src/mut_mut.rs +++ b/src/mut_mut.rs @@ -23,7 +23,7 @@ impl LintPass for MutMut { } } - unwrap_addr(expr).map(|e| { + unwrap_addr(expr).map_or((), |e| { unwrap_addr(e).map(|_| { cx.span_lint(MUT_MUT, expr.span, "Generally you want to avoid &mut &mut _ if possible.") @@ -35,13 +35,12 @@ impl LintPass for MutMut { Consider reborrowing") } }) - }).unwrap_or(()) + }) } fn check_ty(&mut self, cx: &Context, ty: &Ty) { - unwrap_mut(ty).and_then(unwrap_mut).map(|_| cx.span_lint(MUT_MUT, - ty.span, "Generally you want to avoid &mut &mut _ if possible.")). - unwrap_or(()) + unwrap_mut(ty).and_then(unwrap_mut).map_or((), |_| cx.span_lint(MUT_MUT, + ty.span, "Generally you want to avoid &mut &mut _ if possible.")) } } diff --git a/src/ptr_arg.rs b/src/ptr_arg.rs index f8f056e79281..86b87a942be1 100644 --- a/src/ptr_arg.rs +++ b/src/ptr_arg.rs @@ -47,23 +47,22 @@ impl LintPass for PtrArg { fn check_fn(cx: &Context, decl: &FnDecl) { for arg in &decl.inputs { - let ty = &arg.ty; - match ty.node { - TyPtr(ref pty) => check_ptr_subtype(cx, ty.span, &pty.ty), - TyRptr(_, ref rpty) => check_ptr_subtype(cx, ty.span, &rpty.ty), + match &arg.ty.node { + &TyPtr(ref p) | &TyRptr(_, ref p) => + check_ptr_subtype(cx, arg.ty.span, &p.ty), _ => () } } } fn check_ptr_subtype(cx: &Context, span: Span, ty: &Ty) { - match_ty_unwrap(ty, &["Vec"]).map(|_| { - cx.span_lint(PTR_ARG, span, "Writing '&Vec<_>' instead of '&[_]' \ - involves one more reference and cannot be used with non-vec-based \ - slices. Consider changing the type to &[...]") - }).unwrap_or_else(|| match_ty_unwrap(ty, &["String"]).map(|_| { + match_ty_unwrap(ty, &["Vec"]).map_or_else(|| match_ty_unwrap(ty, + &["String"]).map_or((), |_| { cx.span_lint(PTR_ARG, span, "Writing '&String' instead of '&str' involves a new Object \ where a slices will do. Consider changing the type to &str") - }).unwrap_or(())); + }), |_| cx.span_lint(PTR_ARG, span, "Writing '&Vec<_>' instead of \ + '&[_]' involves one more reference and cannot be used with \ + non-vec-based slices. Consider changing the type to &[...]") + ) }