squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Merge pull request #66 from Manishearth/refactoring

Browse source 
another refactoring…
This commit is contained in:
llogiq 2015-05-23 12:43:24 +02:00
parent 450c0fb40c b51ca1c3db
commit 62f0fe0cc0
5 changed files with 148 additions and 140 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

64
src/bit_mask.rs
View file

@ -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
//! `_ <bit_op> m <cmp_op> c` (where `<bit_op>` is one of {`&`, '|'} and `<cmp_op>` 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 `_ <bit_op> m <cmp_op> c` (where `<bit_op>`
/// is one of {`&`, '|'} and `<cmp_op>` 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))
},
_ => ()
}

94
src/eq_op.rs
View file

@ -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<FnDecl>, right : &P<FnDecl>) -> 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<Pat>], right : &[P<Pat>]) -> 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<Ty>], right : &[P<Ty>]) -> bool {
fn over<X, F>(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<X, F>(l: &Option<X>, r: &Option<X>, 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
}
}

102
src/identity_op.rs
View file

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

9
src/mut_mut.rs
View file

@ -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."))
}
}

19
src/ptr_arg.rs
View file

@ -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 &[...]")
)
}